Fix regression for jdbi#2481

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 · Oct 1, 2023 · 316ef21 · 316ef21
1 parent df5e9e8
commit 316ef21
Show file tree

Hide file tree

Showing 5 changed files with 120 additions and 4 deletions.
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
@@ -1,5 +1,7 @@
 # Unreleased
 
+  - Fix regression introduced by #2481 where `-` at the end of named parameters get swallowed. (#2499, thanks @gokristian for reporting).
+
 # 3.41.2
 
   - Deprecate the `otjPostgres` support in jdbi-testing. This will be undeprecated if they ship a version that provides an automatic module name for JPMS, otherwise it will be removed when Jdbi ships with full JPMS support.

diff --git a/core/src/main/antlr4/org/jdbi/v3/core/internal/lexer/ColonStatementLexer.g4 b/core/src/main/antlr4/org/jdbi/v3/core/internal/lexer/ColonStatementLexer.g4
@@ -22,7 +22,8 @@ fragment COLON: {_input.LA(2) != ':'}? ':';
 fragment DOUBLE_COLON: {_input.LA(2) == ':'}? '::';
 fragment QUESTION: {_input.LA(2) != '?'}? '?';
 fragment DOUBLE_QUESTION: {_input.LA(2) == '?'}? '??';
-fragment NAME: JAVA_LETTER | [0-9] | '.' | '?.' | '-';
+fragment NAME_END: JAVA_LETTER | [0-9] | '.' | '?.';
+fragment NAME: NAME_END | '-';
 
 /* 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];
@@ -32,7 +33,7 @@ QUOTED_TEXT: QUOTE (ESCAPE_QUOTE | ~'\'')* QUOTE;
 DOUBLE_QUOTED_TEXT: DOUBLE_QUOTE (~'"')+ DOUBLE_QUOTE;
 ESCAPED_TEXT : ESCAPE . ;
 
-NAMED_PARAM: COLON (NAME)+;
+NAMED_PARAM: COLON (NAME* NAME_END);
 POSITIONAL_PARAM: QUESTION;
 
 LITERAL: DOUBLE_COLON | DOUBLE_QUESTION | .;
diff --git a/core/src/main/antlr4/org/jdbi/v3/core/internal/lexer/HashStatementLexer.g4 b/core/src/main/antlr4/org/jdbi/v3/core/internal/lexer/HashStatementLexer.g4
@@ -21,7 +21,8 @@ fragment DOUBLE_QUOTE: '"';
 fragment HASH: '#';
 fragment QUESTION: {_input.LA(2) != '?'}? '?';
 fragment DOUBLE_QUESTION: {_input.LA(2) == '?'}? '??';
-fragment NAME: JAVA_LETTER | [0-9] | '.' | '?.';
+fragment NAME_END: JAVA_LETTER | [0-9] | '.' | '?.';
+fragment NAME: NAME_END | '-';
 
 /* 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];
@@ -31,7 +32,7 @@ QUOTED_TEXT: QUOTE (ESCAPE_QUOTE | ~'\'')* QUOTE;
 DOUBLE_QUOTED_TEXT: DOUBLE_QUOTE (~'"')+ DOUBLE_QUOTE;
 ESCAPED_TEXT : ESCAPE . ;
 
-NAMED_PARAM: HASH (NAME)+;
+NAMED_PARAM: HASH (NAME* NAME_END);
 POSITIONAL_PARAM: QUESTION;
 
 LITERAL: DOUBLE_QUESTION | .;
diff --git a/core/src/test/java/org/jdbi/v3/core/statement/TestColonPrefixSqlParser.java b/core/src/test/java/org/jdbi/v3/core/statement/TestColonPrefixSqlParser.java
@@ -202,4 +202,60 @@ public void testEmojiParameterNames() {
                 .append(")")
                 .build());
     }
+
+    @Test
+    public void test2481DashRegression() {
+        assertThat(parser.parse("select :data->>'field'", ctx))
+                .isEqualTo(ParsedSql.builder()
+                        .append("select ")
+                        .appendNamedParameter("data")
+                        .append("->>'field'")
+                        .build());
+    }
+
+    // grammar always allowed name ending with .
+    @Test
+    public void test2481DotRegression() {
+        assertThat(parser.parse("select :data.", ctx))
+                .isEqualTo(ParsedSql.builder()
+                        .append("select ")
+                        .appendNamedParameter("data.")
+                        .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))
+                .isEqualTo(ParsedSql.builder()
+                        .append("select ")
+                        .appendNamedParameter(".data")
+                        .build());
+    }
+
+    @Test
+    public void test2481DashName() {
+        assertThat(parser.parse("select :data-foo-bar", ctx))
+                .isEqualTo(ParsedSql.builder()
+                        .append("select ")
+                        .appendNamedParameter("data-foo-bar")
+                        .build());
+    }
+
+    @Test
+    public void test2481DotName() {
+        assertThat(parser.parse("select :data.foo.bar", ctx))
+                .isEqualTo(ParsedSql.builder()
+                        .append("select ")
+                        .appendNamedParameter("data.foo.bar")
+                        .build());
+    }
 }
diff --git a/core/src/test/java/org/jdbi/v3/core/statement/TestHashPrefixSqlParser.java b/core/src/test/java/org/jdbi/v3/core/statement/TestHashPrefixSqlParser.java
@@ -147,4 +147,60 @@ public void testEmojiParameterNames() {
                 .append(")")
                 .build());
     }
+
+    @Test
+    public void test2481DashRegression() {
+        assertThat(parser.parse("select #data->>'field'", ctx))
+                .isEqualTo(ParsedSql.builder()
+                        .append("select ")
+                        .appendNamedParameter("data")
+                        .append("->>'field'")
+                        .build());
+    }
+
+    // grammar always allowed name ending with .
+    @Test
+    public void test2481DotRegression() {
+        assertThat(parser.parse("select #data.", ctx))
+                .isEqualTo(ParsedSql.builder()
+                        .append("select ")
+                        .appendNamedParameter("data.")
+                        .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))
+                .isEqualTo(ParsedSql.builder()
+                        .append("select ")
+                        .appendNamedParameter(".data")
+                        .build());
+    }
+
+    @Test
+    public void test2481DashName() {
+        assertThat(parser.parse("select #data-foo-bar", ctx))
+                .isEqualTo(ParsedSql.builder()
+                        .append("select ")
+                        .appendNamedParameter("data-foo-bar")
+                        .build());
+    }
+
+    @Test
+    public void test2481DotName() {
+        assertThat(parser.parse("select #data.foo.bar", ctx))
+                .isEqualTo(ParsedSql.builder()
+                        .append("select ")
+                        .appendNamedParameter("data.foo.bar")
+                        .build());
+    }
 }