Use identifiable structs for AST variant fields

[cmoog]

When consuming the generated AST for further processing, one needs to match against and deconstruct ast::Statement instances. Currently, the fields of this enum's variants are defined inline... which makes it impossible for downstream code to represent the nested structures as types. For example, consider this structure:

https://github.com/ballista-compute/sqlparser-rs/blob/35ef0eee3857c59827b2587bc813218a00dce264/src/ast/mod.rs#L490-L497

I propose we migrate to the following pattern.

pub struct AnalyzeStatement {
  table_name: ObjectName, 
  partitions: Option<Vec<Expr>>, 
  for_columns: bool, 
  ....
}
pub enum Statement { 
  Analyze(AnalyzeStatement),
  ...
}

Obviously this is a massive breaking change, but that seems OK given the 0.9.x crates.io version.

Thoughts?

[alamb]

Is the primary usecase of this proposal so that downstream code can re-use AnalyzeStatement directly?

[cmoog]

Is the primary usecase of this proposal so that downstream code can re-use AnalyzeStatement directly?

Yes, exactly.

[tvallotton]

I totally agree with this proposal. This would simplify my code considerably. I've got structs that are complete translations of some enum variants.

[tvallotton]

I forked the project and made those changes if you want to see how it would look.

[alamb]

I don't feel qualified to approve / accept this level of change without some additional buy in from @nickolay / @andygrove / @Dandandan . #311 (comment) seems like a good usecase to me but it is a pretty major change for all users of sqlparser-rs.

[nickolay]

#40 could be marked as a duplicate of this issue as well. As mentioned there, I didn't think creating a separate struct for every single variant (even those holding one or two fields) of every enum was worth the breakage, but extracting complex variants like Statement::Analyze is something we have done before.

In #315 (comment) @Dandandan suggested doing this for all variants of the Statement enum, which seems a sensible suggestion too, as most statements exceed the complexity threshold anyway.

[alamb]

Your guidance makes a lot of sense to me @nickolay -- thank you

[alamb]

Sadly I don't have the time to help drive projects such as this. Unless one of the other maintainers wants to do so, PRs like this are not likely to be merged. I am sorry I don't have the time and I am sorry if your time was wasted.

As the current state of affairs might not have been clear in the project's README. I have added #416 to try and help