Re: [PR] Support relation visitor to visit the `Option` field [datafusion-sqlparser-rs]

["alamb (via GitHub)" <gi...@apache.org>]

alamb commented on code in PR #1556:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1556#discussion_r1862005838


##########
src/ast/mod.rs:
##########
@@ -7653,6 +7653,7 @@ impl fmt::Display for ShowStatementInParentType {
 pub struct ShowStatementIn {
     pub clause: ShowStatementInClause,
     pub parent_type: Option<ShowStatementInParentType>,
+    #[cfg_attr(feature = "visitor", visit(with = "visit_relation"))]

Review Comment:
   I took a look through the code, and it appears there are oehter fields that hold relation names that are not visited
   
   For example:
   https://github.com/alamb/sqlparser-rs/blob/a8432b57db6b8efc635e4903d712e76f7adb6e6d/src/ast/ddl.rs#L712-L713
   
   https://github.com/apache/datafusion-sqlparser-rs/blob/4c629e8520b68eb289b34882aa326d80a6f8e022/src/ast/dml.rs#L580-L581
   
   https://github.com/apache/datafusion-sqlparser-rs/blob/3c8fd748043188957d2dfcadb4bfcfb0e1f70c82/src/ast/mod.rs#L2432-L2431
   
   https://github.com/apache/datafusion-sqlparser-rs/blob/3c8fd748043188957d2dfcadb4bfcfb0e1f70c82/src/ast/mod.rs#L2495-L2494
   
   https://github.com/apache/datafusion-sqlparser-rs/blob/3c8fd748043188957d2dfcadb4bfcfb0e1f70c82/src/ast/mod.rs#L2666-L2665
   
   Maybe we can file a ticket to annotate these ones too



##########
derive/src/lib.rs:
##########
@@ -256,3 +265,16 @@ fn visit_children(
         Data::Union(_) => unimplemented!(),
     }
 }
+
+fn is_option(ty: &Type) -> bool {
+    if let Type::Path(TypePath { path: Path { segments, .. }, .. }) = ty {
+        if let Some(segment) = segments.last() {
+            if segment.ident == "Option" {

Review Comment:
   it seems in theory this would match anything called `Option` (even if it wasn't `std::Option`) but I think that seems ok to me (I don't think we'll have anything else realistically)



-- 
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-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org