Skip to content

Design suggestion: change ASTNode variants to contain concrete structs #40

Closed
@khionu

Description

@khionu

As is, the example below can be a necessary evil. Using separate, concrete structs allows the data to be passed as a single value.

fn main() {
    // ...
    match sql_ast {
        SQLSelect { projection, relation, joins, selection, 
                    order_by, group_by, having, limit } =>  {
            select(projection, relation, joins, selection,
                   order_by, group_by, having, limit, &mut out_ts);
        },
        _ => {}
    }
}

fn select(projection: Vec<ASTNode>, relation: Option<Box<ASTNode>>, joins: Vec<Join>,
          selection: Option<Box<ASTNode>>, order_by: Option<Vec<SQLOrderByExpr>>,
          group_by: Option<Vec<ASTNode>>, having: Option<Box<ASTNode>>, limit: Option<Box<ASTNode>>,
          out_ts: &mut TokenStream) {
    // ...
}

Activity

nickolay

nickolay commented on Jan 30, 2019

@nickolay
Contributor

FWIW, I did this to SQLSelect locally in an experiment to make the AST more strongly typed, because it lets me to use the struct both from an ASTNode (for a subquery) and from an SQLStatement (as a top-level statement).

nickolay

nickolay commented on May 7, 2019

@nickolay
Contributor

The branch I mentioned above has been merged, so SQLSelect / SQLQuery are separate structs now. Some ASTNode variants are still inlined though, but I'm not sure it makes sense to mechanically create a separate struct for each variant. Are there any variants where this is still a problem?

alamb

alamb commented on Dec 12, 2021

@alamb
Contributor

Closing as a dupe of #311 as suggested in #311 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @nickolay@alamb@khionu

        Issue actions

          Design suggestion: change ASTNode variants to contain concrete structs · Issue #40 · apache/datafusion-sqlparser-rs