ThinkRedstone commented on code in PR #2097:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2097#discussion_r2518385028
##########
src/parser/mod.rs:
##########
@@ -51,13 +51,14 @@ mod alter;
pub enum ParserError {
TokenizerError(String),
ParserError(String),
+ SpannedParserError(String, Span),
Review Comment:
I'm not sure why a new error type would be confusing. We already have three,
one of which can't actually be raised by the parser.
We can change `ParserError`, but this would require changing all existing
errors which don't include a span to include an empty span. This would offer no
benefit to a user to the library, and might in fact confuse them- since they
would need to know and remember that for some errors, the span might be empty.
That a field in your error can't actually be relied upon, despite not being
marked as `Option`, I think is much more confusing.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
