Skip to content

Commit

Permalink
Another round of lexer fixes
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
hgschmie committed Oct 24, 2023
1 parent 1489eef commit f774d5f
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ fragment COLON: {_input.LA(2) != ':'}? ':';
fragment DOUBLE_COLON: {_input.LA(2) == ':'}? '::';
fragment QUESTION: {_input.LA(2) != '?'}? '?';
fragment DOUBLE_QUESTION: {_input.LA(2) == '?'}? '??';
fragment NAME_END: JAVA_LETTER | [0-9] | '.' | '?.';
fragment NAME: NAME_END | '-';
fragment NAME: JAVA_LETTER | [0-9] | '.' | '?.';

/* Lovingly lifted from https://github.com/antlr/grammars-v4/blob/master/java/JavaLexer.g4 */
fragment JAVA_LETTER : [a-zA-Z$_] | ~[\u0000-\u007F\uD800-\uDBFF] | [\uD800-\uDBFF] [\uDC00-\uDFFF];
Expand All @@ -33,7 +32,7 @@ QUOTED_TEXT: QUOTE (ESCAPE_QUOTE | ~'\'')* QUOTE;
DOUBLE_QUOTED_TEXT: DOUBLE_QUOTE (~'"')+ DOUBLE_QUOTE;
ESCAPED_TEXT : ESCAPE . ;

NAMED_PARAM: COLON (NAME* NAME_END);
NAMED_PARAM: COLON (NAME)+;
POSITIONAL_PARAM: QUESTION;

LITERAL: DOUBLE_COLON | DOUBLE_QUESTION | .;
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ fragment ESCAPE_QUOTE: ESCAPE QUOTE ;
fragment DOUBLE_QUOTE: '"' ;
fragment LT: '<' ;
fragment GT: '>' ;
fragment NAME: JAVA_LETTER | [0-9] | '.' | '-' ;
fragment NAME: JAVA_LETTER | [0-9] | '.';

/* Lovingly lifted from https://github.com/antlr/grammars-v4/blob/master/java/java/JavaLexer.g4 */
fragment JAVA_LETTER : [a-zA-Z$_] | ~[\u0000-\u007F\uD800-\uDBFF] | [\uD800-\uDBFF] [\uDC00-\uDFFF];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ fragment DOUBLE_QUOTE: '"';
fragment HASH: '#';
fragment QUESTION: {_input.LA(2) != '?'}? '?';
fragment DOUBLE_QUESTION: {_input.LA(2) == '?'}? '??';
fragment NAME_END: JAVA_LETTER | [0-9] | '.' | '?.';
fragment NAME: NAME_END | '-';
fragment NAME: JAVA_LETTER | [0-9] | '.' | '?.';

/* Lovingly lifted from https://github.com/antlr/grammars-v4/blob/master/java/JavaLexer.g4 */
fragment JAVA_LETTER : [a-zA-Z$_] | ~[\u0000-\u007F\uD800-\uDBFF] | [\uD800-\uDBFF] [\uDC00-\uDFFF];
Expand All @@ -32,7 +31,7 @@ QUOTED_TEXT: QUOTE (ESCAPE_QUOTE | ~'\'')* QUOTE;
DOUBLE_QUOTED_TEXT: DOUBLE_QUOTE (~'"')+ DOUBLE_QUOTE;
ESCAPED_TEXT : ESCAPE . ;

NAMED_PARAM: HASH (NAME* NAME_END);
NAMED_PARAM: HASH (NAME)+;
POSITIONAL_PARAM: QUESTION;

LITERAL: DOUBLE_QUESTION | .;
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ void testIssue2471() {
.bindList("values", 3, "abc")
.execute();

for (var specialCharPlaceholderName : new String[] {"xxx.ids", "xxx_ids", "xxx-ids"}) {
for (var specialCharPlaceholderName : new String[] {"xxx.ids", "xxx_ids"}) {

List<Thing> list = handle.createQuery("select id, foo from thing where id in (<" + specialCharPlaceholderName + ">)")
.bindList(specialCharPlaceholderName, 1, 3)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,15 +223,6 @@ public void test2481DotRegression() {
.build());
}

@Test
public void test2481DashStarting() {
assertThat(parser.parse("select :-data", ctx))
.isEqualTo(ParsedSql.builder()
.append("select ")
.appendNamedParameter("-data")
.build());
}

@Test
public void test2481DotStarting() {
assertThat(parser.parse("select :.data", ctx))
Expand All @@ -246,7 +237,8 @@ public void test2481DashName() {
assertThat(parser.parse("select :data-foo-bar", ctx))
.isEqualTo(ParsedSql.builder()
.append("select ")
.appendNamedParameter("data-foo-bar")
.appendNamedParameter("data")
.append("-foo-bar")
.build());
}

Expand All @@ -258,4 +250,22 @@ public void test2481DotName() {
.appendNamedParameter("data.foo.bar")
.build());
}

@Test
public void testQuestionMarkName() {
assertThat(parser.parse("select :data.foo?.bar", ctx))
.isEqualTo(ParsedSql.builder()
.append("select ")
.appendNamedParameter("data.foo?.bar")
.build());
}

@Test
public void testEmojiName() {
assertThat(parser.parse("select :😎", ctx))
.isEqualTo(ParsedSql.builder()
.append("select ")
.appendNamedParameter("😎")
.build());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -168,15 +168,6 @@ public void test2481DotRegression() {
.build());
}

@Test
public void test2481DashStarting() {
assertThat(parser.parse("select #-data", ctx))
.isEqualTo(ParsedSql.builder()
.append("select ")
.appendNamedParameter("-data")
.build());
}

@Test
public void test2481DotStarting() {
assertThat(parser.parse("select #.data", ctx))
Expand All @@ -191,7 +182,8 @@ public void test2481DashName() {
assertThat(parser.parse("select #data-foo-bar", ctx))
.isEqualTo(ParsedSql.builder()
.append("select ")
.appendNamedParameter("data-foo-bar")
.appendNamedParameter("data")
.append("-foo-bar")
.build());
}

Expand All @@ -203,4 +195,22 @@ public void test2481DotName() {
.appendNamedParameter("data.foo.bar")
.build());
}

@Test
public void testQuestionMarkName() {
assertThat(parser.parse("select #data.foo?.bar", ctx))
.isEqualTo(ParsedSql.builder()
.append("select ")
.appendNamedParameter("data.foo?.bar")
.build());
}

@Test
public void testEmojiName() {
assertThat(parser.parse("select #😎", ctx))
.isEqualTo(ParsedSql.builder()
.append("select ")
.appendNamedParameter("😎")
.build());
}
}
4 changes: 2 additions & 2 deletions docs/src/adoc/index.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1339,7 +1339,7 @@ parameters by name:
include::{exampledir}/FiveMinuteTourTest.java[tags=namedParameters]
----

Names can contain alphanumeric characters, dot and dash (`.` and `-`).
A parameter or attribute name can contain any valid Java identifier characters and the dot (`.`).

[NOTE]
By default, Jbdi uses the see the link:{jdbidocs}/core/statement/ColonPrefixSqlParser.html[ColonPrefixSqlParser^] that understands
Expand Down Expand Up @@ -8937,7 +8937,7 @@ handle.setSqlParser(new HashPrefixSqlParser())
.execute();
----

The default parsers recognize any Java identifier and additionally the dot and dash (`.` and `-`) as a valid characters in a parameter or attribute name. Even some strange cases like emoji are allowed, although the Jdbi authors encourage appropriate discretion 🧐.
The default parsers recognize any Java identifier character and the dot (`.`) as a valid characters in a parameter or attribute name. Even some strange cases like emoji are allowed, although the Jdbi authors encourage appropriate discretion 🧐.

[NOTE]
The default parsers try to ignore parameter-like constructions inside of string literals,
Expand Down

0 comments on commit f774d5f

Please sign in to comment.