[Discussion] Remove use of `dialect_of` macro

[ever0de]

What was the purpose of the current Dialect?
The dialect_of macro makes it difficult to modify your code whenever you add support for Dialect.
And besides the identifier support of Dialect, it seems that it can also support query (Expr).

But... if you have other intentions for macros
Since there is a problem, I would like to discuss it with the maintainers.

[ever0de]

• When implementing the TRIM function function
• Different Dialects support standard and non-standard.

E.g

• in Dialect Trait

    fn is_delimited_identifier_start(&self, ch: char);
    fn is_identifier_start(&self, ch: char) -> bool;
    fn is_identifier_part(&self, ch: char) -> bool;
    ...

    fn is_standard_trim_supported(..) -> bool {
        // e.g. TRIM(LEADING FROM "ABC")
        true
    }

    fn is_non_standard_trim_supported(..) -> bool {
        // e.g. TRIM("ABC", "A");
        true
    }
}

PostgreSQL

MySQL

Sqlite

[ever0de]

There was an issue related to this

• Add conditional compilation to support different SQL dialect #207

After some more thought, how about managing Dialect with features?

[ever0de]

Ast Display should also be considered.
Looks like this should be a different issue.

[nickolay]

The dialect_of!(parser is Dialect1 | Dialect2 | GenericDialect) helper was introduced in #254 to let the parser branch on the dialect's type. An example where such branching is required is when the grammars of various dialects conflict, e.g. #241 (comment)

I do prefer branching on specific dialects in the parser to adding flags (like is_standard_trim_supported / is_non_standard_trim_supported in your example) to the dialect trait and I do not understand what problem you are having with dialect_of!.

[AugustoFKL]

@nickolay can't we have a default implementation for stuff that is generic, i.e., is specified on the ANSII, and specific implementation overwriting them if the child ever needs it?

E.g.,

impl GenericDialect {
   fn parse_limit() {
        #fail
   }
}

impl MySqlDialect {
    fn parse_limit() {
        #specific implementation
    }
}

[nickolay]

You could technically do that, but why?

Current design aims to produce a parser that’s able to parse SQL from common dialects with minimal maintenance effort. Producing “strict” dialect-specific parsers, that reject syntax not supported by the given dialect, would require maintaining a separate parser for each dialect — so it isn’t a goal of this project. One can fork and remove the unneeded code if they need a strict parser.

Note that the “generic” dialect accepts as much dialect specific stuff as possible; not the “lowest common denominator” dialect, and not the ANSI dialect, as you think. So with the approach you suggested, we’d have two copies of all dialect-specific code: one in the dialect-specific trait and another in the generic trait.

With that in mind, and given that all trait implementations must return the same AST type, in most cases branching on the dialect in the shared parser code just seems to make more sense.

[ever0de]

So that's not the goal of this project.

I understood this answer.
Please close the issue when the discussion is over, Thanks!

[AugustoFKL]

@nickolay makes sense, thanks!