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))]pubstructShowStatementIn{pubclause:ShowStatementInClause,pubparent_type:Option<ShowStatementInParentType>,#[cfg_attr(feature = "visitor", visit(with = "visit_relation"))]pubparent_name:Option<ObjectName>,}

This will generate

impl sqlparser::ast::VisitforShowStatementIn{fnvisit<V: sqlparser::ast::Visitor>(&self,visitor:&mutV,) -> ::std::ops::ControlFlow<V::Break>{
sqlparser::ast::Visit::visit(&self.clause, visitor)?;
sqlparser::ast::Visit::visit(&self.parent_type, visitor)?;ifletSome(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::VisitMutforShowStatementIn{fnvisit<V: sqlparser::ast::VisitorMut>(&mutself,visitor:&mutV,) -> ::std::ops::ControlFlow<V::Break>{
sqlparser::ast::VisitMut::visit(&mutself.clause, visitor)?;
sqlparser::ast::VisitMut::visit(&mutself.parent_type, visitor)?;ifletSome(value) = &mutself.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

• #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
	pubtables: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