s5dsn-eqee commented on code in PR #2557:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2557#discussion_r4172859927
##########
src/parser/mod.rs:
##########
@@ -12904,6 +12904,9 @@ impl<'a> Parser<'a> {
let _guard = self.recursion_counter.try_decrease()?;
let dialect = self.dialect;
+ if let Some(custom) = self.maybe_parse(|p|
p.parse_type_with_signed_modifier())? {
Review Comment:
Suggestion on where this lives.
`maybe_parse(|p| p.parse_type_with_signed_modifier())` at the top of
`parse_data_type` is effectively a dialect hook: the dialect gets first chance
to produce a `DataType`, and `maybe_parse` rolls back when it declines. But
it's wired through a boolean flag inside the function, which has two side
effects:
- every data type in every dialect now goes through `maybe_parse`, which
formats an `expected_ref("")` error string and discards it, just to say "not
applicable";
- ~70 lines of SQLite-specific grammar sit in the shared parser instead of
in `sqlite.rs`.
Could we make it a real hook, the same way `parse_prefix` / `parse_infix`
/ `parse_statement` work?
```rust
// Dialect trait
/// Dialect-specific data type parsing. Returning `None` falls back to the
standard parser.
fn parse_data_type(&self, _parser: &mut Parser) -> Option<Result<DataType,
ParserError>> {
None
}
// Parser::parse_data_type
if let Some(result) = self.dialect.parse_data_type(self) {
return result.map(|t| (t, false.into()));
}
SQLiteDialect then implements parse_data_type and owns the signed-modifier
logic, including its own maybe_parse for the rollback. Other dialects pay
nothing, supports_signed_type_modifier goes away, and the SQLite code moves
next to the other SQLite quirks.
With the logic in one place it may also simplify: parse_object_name plus
parse_optional_type_modifiers already produce DataType::Custom, so the dialect
impl could reuse them and only add the +/- handling, instead of the lookahead
pass and the second parse loop.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
