Fix parsing of TRIM specification keywords (BOTH, LEADING, and …

…`TRAILING`) (#326)
partiql · Dec 8, 2020 · a22e529 · a22e529
1 parent a8f59bf
commit a22e529
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 13 deletions.
diff --git a/lang/src/org/partiql/lang/eval/builtins/TrimExprFunction.kt b/lang/src/org/partiql/lang/eval/builtins/TrimExprFunction.kt
@@ -101,7 +101,7 @@ internal class TrimExprFunction(valueFactory: ExprValueFactory) : NullPropagatin
             1    -> Triple(DEFAULT_SPECIFICATION, DEFAULT_TO_REMOVE, args[0].codePoints())
             2    -> {
 
-                if(args[0].type != ExprValueType.STRING){
+                if(!args[0].type.isText){
                     errNoContext("with two arguments trim's first argument must be either the " +
                                  "specification or a 'to remove' string",
                                  internal = false)

diff --git a/lang/src/org/partiql/lang/syntax/LexerConstants.kt b/lang/src/org/partiql/lang/syntax/LexerConstants.kt
@@ -245,9 +245,10 @@ internal val DATE_PART_KEYWORDS: Set<String> = DatePart.values()
     "work",
     "write",
     "zone"
-).union(TRIM_SPECIFICATION_KEYWORDS)
+)
 // Note: DATE_PART_KEYWORDs are not keywords in the traditional sense--they are only keywords within
 // the context of the DATE_ADD, DATE_DIFF and EXTRACT functions, for which [SqlParser] has special support.
+// Similarly, TRIM_SPECIFICATION_KEYWORDS are only keywords within the context of the TRIM function.
 
 /** PartiQL additional keywords. */
 @JvmField internal val SQLPP_KEYWORDS = setOf(

diff --git a/lang/src/org/partiql/lang/syntax/SqlLexer.kt b/lang/src/org/partiql/lang/syntax/SqlLexer.kt
@@ -515,11 +515,6 @@ class SqlLexer(private val ion: IonSystem) : Lexer {
                                         tokenType = FOR
                                         ion.newSymbol(lower)
                                     }
-                                    lower in TRIM_SPECIFICATION_KEYWORDS -> {
-                                        // used to determine the type of trim
-                                        tokenType = TRIM_SPECIFICATION
-                                        ion.newString(lower)
-                                    }
                                     lower in BOOLEAN_KEYWORDS -> {
                                         // literal boolean
                                         tokenType = LITERAL

diff --git a/lang/src/org/partiql/lang/syntax/SqlParser.kt b/lang/src/org/partiql/lang/syntax/SqlParser.kt
@@ -1779,9 +1779,11 @@ class SqlParser(private val ion: IonSystem) : Parser {
             return node.remaining
         }
 
-        val hasSpecification = when(rem.head?.type) {
-            TRIM_SPECIFICATION -> {
-                rem = parseArgument()
+        val maybeTrimSpec = rem.head
+        val hasSpecification = when {
+            maybeTrimSpec?.type == IDENTIFIER && TRIM_SPECIFICATION_KEYWORDS.contains(maybeTrimSpec.text?.toLowerCase()) -> {
+                arguments.add(ParseNode(ATOM, maybeTrimSpec.copy(type = TRIM_SPECIFICATION), listOf(), rem.tail))
+                rem = rem.tail
 
                 true
             }

diff --git a/lang/test/org/partiql/lang/eval/EvaluatingCompilerExceptionsTest.kt b/lang/test/org/partiql/lang/eval/EvaluatingCompilerExceptionsTest.kt
@@ -198,4 +198,48 @@ class EvaluatingCompilerExceptionsTest : EvaluatorTestBase() {
         """SELECT ? FROM <<1>>""",
         ErrorCode.EVALUATOR_UNBOUND_PARAMETER,
         sourceLocationProperties(1, 8) + mapOf(Property.EXPECTED_PARAMETER_ORDINAL to 1, Property.BOUND_PARAMETER_COUNT to 0))
+
+    @Test
+    fun trimSpecKeywordBothNotUsedInTrim() =
+        checkInputThrowingEvaluationException(
+            "SELECT 1 FROM both",
+            ErrorCode.EVALUATOR_BINDING_DOES_NOT_EXIST,
+            mapOf(
+                Property.LINE_NUMBER to 1L,
+                Property.COLUMN_NUMBER to 15L,
+                Property.BINDING_NAME to "both")
+        )
+
+    @Test
+    fun trimSpecKeywordLeadingNotUsedInTrim() =
+        checkInputThrowingEvaluationException(
+            "SELECT 1 FROM leading",
+            ErrorCode.EVALUATOR_BINDING_DOES_NOT_EXIST,
+            mapOf(
+                Property.LINE_NUMBER to 1L,
+                Property.COLUMN_NUMBER to 15L,
+                Property.BINDING_NAME to "leading")
+        )
+
+    @Test
+    fun trimSpecKeywordTrailingNotUsedInTrim() =
+        checkInputThrowingEvaluationException(
+            "SELECT 1 FROM trailing",
+            ErrorCode.EVALUATOR_BINDING_DOES_NOT_EXIST,
+            mapOf(
+                Property.LINE_NUMBER to 1L,
+                Property.COLUMN_NUMBER to 15L,
+                Property.BINDING_NAME to "trailing")
+        )
+
+    @Test
+    fun trimSpecKeywordLeadingUsedAsSecondArgInTrim() =
+        checkInputThrowingEvaluationException(
+            "trim(both leading from 'foo')",
+            ErrorCode.EVALUATOR_BINDING_DOES_NOT_EXIST,
+            mapOf(
+                Property.LINE_NUMBER to 1L,
+                Property.COLUMN_NUMBER to 11L,
+                Property.BINDING_NAME to "leading")
+        )
 }
diff --git a/lang/test/org/partiql/lang/syntax/SqlParserTest.kt b/lang/test/org/partiql/lang/syntax/SqlParserTest.kt
@@ -252,17 +252,17 @@ class SqlParserTest : SqlParserTestBase() {
     @Test
     fun callTrimTwoArgumentsUsingBoth() = assertExpression(
         "trim(both from 'test')",
-        "(call trim (lit \"both\") (lit \"test\"))")
+        "(call trim (lit both) (lit \"test\"))")
 
     @Test
     fun callTrimTwoArgumentsUsingLeading() = assertExpression(
         "trim(leading from 'test')",
-        "(call trim (lit \"leading\") (lit \"test\"))")
+        "(call trim (lit leading) (lit \"test\"))")
 
     @Test
     fun callTrimTwoArgumentsUsingTrailing() = assertExpression(
         "trim(trailing from 'test')",
-        "(call trim (lit \"trailing\") (lit \"test\"))")
+        "(call trim (lit trailing) (lit \"test\"))")
 
     //****************************************
     // Unary operators