alamb commented on PR #1435: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1435#issuecomment-2468945628
Sorry for my silence / inability to find time to review this PR in depth. I am struggling with too many thing TLDR I I think we should: 1. Get the tests passing on this PR and merge it in (if not all AST nodes have spans initially, we can file tickets to add the information over the next few releases) 2. Add some documentation explaining what is happening and how downstream consumers need to change (I think basically by ignoring the newly added fields @iffyio and I spoke about this a bit -- basically in my opinion adding source locations to sqlparser-rs is one of the last major remaining features it is missing. However, it also has the potential to be very disruptive to downstream consumers. It seems to me the requirements for this change are: 1. Any changes required by users of this crate should be mechanical 3. We should clearly document what changes are needed on upgrade (ideally with examples) An example of a mechanical change might be adding ```rust match node { ast::Query { field1, field2, location: _, // add a new line to ignored location } ``` An example of a non mechanical change might be wrapping all AST nodes in some other struct so the structure of the `match` no longer works I looked at this PR and I think it basically satisfies 1 for me so I think we should proceed. Would it be possible to write up something, perhaps in the `README` that explains that we are adding new spans to the code and how we will handle them? -- 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