Support ${placeholder} syntax in tokenizer#2239
Conversation
Add support for dollar-brace placeholders (`${name}`, `${1}`, etc.)
in the tokenizer, storing the full `${...}` string in the existing
`Token::Placeholder` / `Value::Placeholder` variants with zero
breaking changes. Unterminated `${name` produces a clear error.
|
@iffyio could we bother your to take a look at this PR please? Thanks! |
| chars.next(); | ||
|
|
||
| // Handle ${placeholder} syntax | ||
| if matches!(chars.peek(), Some('{')) { |
There was a problem hiding this comment.
wondering how this works with the self.dialect.supports_dollar_placeholder() dialects, is it possible that e.g. ${abc}$ is part of a dollar string or other potentially conflicting syntax?
There was a problem hiding this comment.
I could possibly add this as another flag to dialect if that makes sense? I just thought it would be yet another flag, and if we could include it in this flag, but you're right it means that it will parse some dialects as valid even if it's not for that dialect
There was a problem hiding this comment.
I think if the syntax potentially conflicts we might need to ensure that it doesn't, because in this case its rather that ${abc}$ in the example will be parsed incorrectly as something that it is not (a placeholder vs a string)
There was a problem hiding this comment.
So there is some existing behaviour, whereby $placeholder will work currently with postgres and other dialects even though supports_dollar_placeholder() is false — it falls through to the placeholder fallback when the next character after the identifier isn't $.
The ${placeholder} branch is consistent with that existing behavior, and there's no conflict with dollar-quoted strings since { is not a valid tag character
dollar-quoted string tags only allow alphanumeric characters and underscores (e.g., $tag_1$...$tag_1$), so ${abc}$ could never be interpreted as a dollar-quoted string.
|
|
||
| chars.next(); | ||
|
|
||
| // Handle ${placeholder} syntax |
There was a problem hiding this comment.
could we include a reference to the dialect that defines this syntax maybe here?
There was a problem hiding this comment.
This is a tough one, as it's not defined by a specific dialect but used in some of our other integrations around parametrised queries. Namely we're using this with perses
3 participants
Add support for dollar-brace placeholders (
${name},${1}, etc.) in the tokenizer, storing the full${...}string in the existingToken::Placeholder/Value::Placeholdervariants with zero breaking changes. Unterminated${nameproduces a clear error.