iffyio commented on code in PR #1979:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1979#discussion_r2239586088
##########
src/tokenizer.rs:
##########
@@ -1756,6 +1761,30 @@ impl<'a> Tokenizer<'a> {
}
}
+ /// Tokenizes an identifier followed immediately after a colon,
+ /// aka named query parameter, e.g. `:name`. The next char of the
+ /// processed char stream is to be an alphabetic - panics otherwise.
+ fn tokenize_colon_preceeded_placeholder(
Review Comment:
I think that it would likely be ideal to use existing logic like
`tokenize_identifier_or_keyword` or similar. Also heads up in general for the
parser that returning an error when input is invalid etc is preferred over
panicking
##########
src/dialect/mod.rs:
##########
@@ -841,6 +841,12 @@ pub trait Dialect: Debug + Any {
false
}
+ /// Returns true if this dialect allow colon placeholders
+ /// e.g. `SELECT :var` (JPA named parameters)
+ fn supports_colon_placeholder(&self) -> bool {
+ false
+ }
Review Comment:
I think that make sense, it seems that the handling of placeholders isn't
very consistent by the parser as some are done in tokenizer while others later
on. I think it could be good to do a more general hopefully improvement if we
feel that ideally placeholders are managed by the tokenizer, the we can parse
and represent it consistently/accordingly.
Did a quick pass through existing support, and I can imagine something like
the following representation to merge the different representations explicitly,
wdyt?
```rust
enum PlaceholderKind {
/// Example:
/// ```sql
/// SELECT ?, ?foo
/// ```
SingleQuestionMark,
/// Example:
/// ```sql
/// SELECT @foo
/// ```
SingleAt,
/// Example:
/// ```sql
/// SELECT $1, $foo
/// ```
SingleDollar,
/// Example:
/// ```sql
/// SELECT :42, :foo
/// ```
Colon,
/// Example:
/// ```sql
/// SELECT $$, $$abc$$, $abc$, $$$abc$$$
/// ```
DollarEnclosed {
count: usize
}
}
Token::Placeholder {
kind: PlaceholderKind,
value: Option<String>
}
```
It would also remove the need for this code as well
https://github.com/apache/datafusion-sqlparser-rs/blob/15d8bfea62726464a3065765f5511af7fa1548f8/src/parser/mod.rs#L9637-L9646
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
