iffyio commented on code in PR #1849:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1849#discussion_r2088302222
##########
src/ast/query.rs:
##########
@@ -1257,6 +1257,7 @@ pub enum TableFactor {
value: Ident,
name: Ident,
columns: Vec<Ident>,
+ include_nulls: Option<bool>,
Review Comment:
I'm thinking it would be useful to include a description of this parameter -
specifically highlighting that `true` means include nulls and `false` means
exclude nulls (thinking otherwise at least the latter won't be obvious API wise
to a user of the library without reading the code).
Also maybe we can call this property `null_treatment` similar to e.g.
[Function](https://github.com/apache/datafusion-sqlparser-rs/blob/41eb4911df5fc883cbe267b6f5fae8ca2d21ac2f/src/parser/mod.rs#L1203)
vs `include_nulls` which only covers one possible value naming wise?
--
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
