eliaperantoni commented on issue #14433: URL: https://github.com/apache/datafusion/issues/14433#issuecomment-2647246100
Hey @alan910127, sorry for the delay, thank you so much for taking on this ticket! I suggest you look at `datafusion/sql/tests/cases/diagnostic.rs` and create a test case there, that should be enough to validate that you're producing a good `Diagnostic` 😊. You can see some good examples there. We try to make our own "viewer" implementation not influence the design of `Diagnostic` in DataFusion, which should instead be generic. The type of `Diagnostic` should guide you towards a correct instance. i.e. it's either an error or a warning (not the case of this ticket) with a string message and (ideally) a `Span`. Plus zero or more notes and helps that also have a string message and (possibly) a `Span`. A note is to give more contextual information, an help is to directly propose a fix to the user. As for the `Span`, you usually want to call `Expr::spans` to get source information about an expression. This currently only works if it's an `Expr::Column` so you should change the method a bit so it works for `Expr::Negative` and `Expr::Not`. Currently we only store spans inside of `datafusion::common::Column`. If your implementation cannot work just with that, you might want to copy over more spans from the AST tree to the logical nodes. You'd do that in `SqlToRel`, looks like `SqlToRel::parse_sql_unary_op` is the one you're looking for. But bear in mind that the parser itself is not yet perfect in this regard and might not always have the data you need. e.g. I wouldn't be surprised if it had no clue about the `Span` for a negation operator. In any case, I think it's perfectly fine if in this PR you don't get it 100% right. e.g. I think it's okay if nested unary expressions only highlight the column name (e.g. `SELECT -+c -- only c is highlighted`). We'll improve `Expr::spans` as the parser improves. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
