Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support '.' and '-' as characters in named parameters #2481

Merged
merged 1 commit into from Sep 1, 2023

Conversation

hgschmie
Copy link
Contributor

@hgschmie hgschmie commented Sep 1, 2023

Fixes #2471

@sonarcloud
Copy link

sonarcloud bot commented Sep 1, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@hgschmie hgschmie merged commit 9cba817 into jdbi:master Sep 1, 2023
37 checks passed
@hgschmie hgschmie deleted the issue-2471 branch September 1, 2023 18:48
@gokristian
Copy link

Hey! I think this PR just destroyed reading input parameter json (jdbi version 3.41.1):

:data->>'field'

resulting in an error Missing named parameter 'data-' in binding:.... Wrapping the param helps though:

(:data)->>'field'

Is it intentional?

@stevenschlansker
Copy link
Member

Thank you for reporting this regression. @hgschmie , do we need to revert this change?

@stevenschlansker
Copy link
Member

Or, maybe it would be sufficient to allow - only when it is followed by more alphabetic characters, so that -> and -3 are unambiguously operators not names.

hgschmie added a commit to hgschmie/jdbi that referenced this pull request Oct 1, 2023
A regression was highlighted that the change in 3.41.2 swallowed `-`
characters at the end of named parameters and break expressions such
as `:foo->>'stuff'`.

- Rework the grammar to not support `-` as the last character.
- Align the HashStatementLexer to the ColonStatementLexer
- Add tests

This addresses jdbi#2481 (comment)
@hgschmie
Copy link
Contributor Author

hgschmie commented Oct 1, 2023

@gokristian #2499 should fix this. I plan to release a 3.41.3 around this regression.

hgschmie added a commit to hgschmie/jdbi that referenced this pull request Oct 1, 2023
A regression was highlighted that the change in 3.41.2 swallowed `-`
characters at the end of named parameters and break expressions such
as `:foo->>'stuff'`.

- Rework the grammar to not support `-` as the last character.
- Align the HashStatementLexer to the ColonStatementLexer
- Add tests

This addresses jdbi#2481 (comment)
@hgschmie
Copy link
Contributor Author

hgschmie commented Oct 1, 2023

I changed the grammar to match the old version and not accept - at the end. To be able to differentiate between "next character is alphanumeric or not", this requires lookahead which can probably be done by the lexer but I don't know exactly how. :-)

hgschmie added a commit that referenced this pull request Oct 2, 2023
@hgschmie
Copy link
Contributor Author

hgschmie commented Oct 3, 2023

@gokristian again, thank you for reporting the regression. Please try with 3.41.3, this fixes the problem.

@gokristian
Copy link

@hgschmie No problem, thank you for fixing it. 3.41.3 works great with param json field accessor!

hgschmie added a commit to hgschmie/jdbi that referenced this pull request Oct 24, 2023
This hopefully addresses all the issues raised in jdbi#2481, jdbi#2499 and finally jdbi#2510.

The case of the trailing '-' is not solvable without a trailing
delimiter. There is not enough information to decide whether
`:foo-bar` is "identifier 'foo-bar'" or "identifier 'foo' minus bar".

This PR drops the '-' again as a valid character in a parameter or
binding (sorry if you added it; but then again this was only added in
3.41.1). The dot (`.`) is still supported as a valid character in a
binding or parameter name. It always was for the parameters but not
for a binding (this caused the original jdbi#2481 issue).
hgschmie added a commit to hgschmie/jdbi that referenced this pull request Nov 19, 2023
This hopefully addresses all the issues raised in jdbi#2481, jdbi#2499 and finally jdbi#2510.

The case of the trailing '-' is not solvable without a trailing
delimiter. There is not enough information to decide whether
`:foo-bar` is "identifier 'foo-bar'" or "identifier 'foo' minus bar".

This PR drops the '-' again as a valid character in a parameter or
binding (sorry if you added it; but then again this was only added in
3.41.1). The dot (`.`) is still supported as a valid character in a
binding or parameter name. It always was for the parameters but not
for a binding (this caused the original jdbi#2481 issue).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Can't bind list named parameter if the alias contains a dot
3 participants