Support relation visitor to visit the Option field

[goldmedal]

close #1554

If the field is a Option and add #[with = "visit_xxx"] to the field, the generated code
will try to access the field only if it is Some:

#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
pub struct ShowStatementIn {
    pub clause: ShowStatementInClause,
    pub parent_type: Option<ShowStatementInParentType>,
    #[cfg_attr(feature = "visitor", visit(with = "visit_relation"))]
    pub parent_name: Option<ObjectName>,
}

This will generate

impl sqlparser::ast::Visit for ShowStatementIn {
    fn visit<V: sqlparser::ast::Visitor>(
        &self,
        visitor: &mut V,
    ) -> ::std::ops::ControlFlow<V::Break> {
        sqlparser::ast::Visit::visit(&self.clause, visitor)?;
        sqlparser::ast::Visit::visit(&self.parent_type, visitor)?;
        if let Some(value) = &self.parent_name {
            visitor.pre_visit_relation(value)?;
            sqlparser::ast::Visit::visit(value, visitor)?;
            visitor.post_visit_relation(value)?;
        }
        ::std::ops::ControlFlow::Continue(())
    }
}

impl sqlparser::ast::VisitMut for ShowStatementIn {
    fn visit<V: sqlparser::ast::VisitorMut>(
        &mut self,
        visitor: &mut V,
    ) -> ::std::ops::ControlFlow<V::Break> {
        sqlparser::ast::VisitMut::visit(&mut self.clause, visitor)?;
        sqlparser::ast::VisitMut::visit(&mut self.parent_type, visitor)?;
        if let Some(value) = &mut self.parent_name {
            visitor.pre_visit_relation(value)?;
            sqlparser::ast::VisitMut::visit(value, visitor)?;
            visitor.post_visit_relation(value)?;
        }
        ::std::ops::ControlFlow::Continue(())
    }
}

[alamb]

Thank you @goldmedal . I think there were some other fields that have some wrapping structure simply because this feature was missing

I think we will also have to release a new version of sqlparser derive (which we don't always do) as part of this release given the change to the proc macro; I left a note to remind us here

• Release sqlparser-rs version 0.53.0 / sqlparser_derive 0.3.0 #1517

I will also test this with DataFusion as well and report back

[alamb]

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

datafusion-sqlparser-rs/src/ast/dml.rs

Lines 580 to 581 in 4c629e8

	/// Multi tables delete are supported in mysql
	pub tables: Vec<ObjectName>,

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

[goldmedal]

Filed #1568 to track it.

[alamb]

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)

[goldmedal]

Agreed, I think we won't have another struct called Option in this project 🤔. Or we can match the full path with std::option::Option or core::option::Option if required.

[iffyio]

Thanks @goldmedal!

[alamb]

I double checked with apache/datafusion#13546 and this works great. Thank you again @goldmedal and @iffyio