Fixes structs to handle non-text struct fields. (#450)

* Fixes structs to handle non-text struct fields. * Adds Semantic error. * Addresses comments. * Minor cleanup
partiql · Oct 7, 2021 · 0a82f30 · 0a82f30
1 parent 28701e2
commit 0a82f30
Show file tree

Hide file tree

Showing 5 changed files with 117 additions and 2 deletions.
diff --git a/lang/src/org/partiql/lang/errors/ErrorCode.kt b/lang/src/org/partiql/lang/errors/ErrorCode.kt
@@ -641,14 +641,22 @@ enum class ErrorCode(private val category: ErrorCategory,
             "escape char = ${errorContext?.get(Property.LIKE_ESCAPE)?.stringValue() ?: "none given"}"
     },
 
-    EVALUATOR_NON_INT_LIMIT_VALUE (
+    EVALUATOR_NON_INT_LIMIT_VALUE(
         ErrorCategory.EVALUATOR,
         LOCATION + setOf(Property.ACTUAL_TYPE),
         "") {
         override fun getErrorMessage(errorContext: PropertyValueMap?): String =
             "LIMIT value must be an integer but found ${errorContext.getProperty(Property.ACTUAL_TYPE)}}"
     },
 
+    EVALUATOR_NON_TEXT_STRUCT_FIELD_KEY(
+        ErrorCategory.EVALUATOR,
+        LOCATION + setOf(Property.ACTUAL_TYPE),
+        "") {
+        override fun getErrorMessage(errorContext: PropertyValueMap?): String =
+            "Struct field key should be text but found ${errorContext.getProperty(Property.ACTUAL_TYPE)}}."
+    },
+
     EVALUATOR_NEGATIVE_LIMIT(
         ErrorCategory.EVALUATOR,
         LOCATION,
@@ -664,6 +672,14 @@ enum class ErrorCode(private val category: ErrorCategory,
         LOCATION,
         "% by zero"),
 
+    SEMANTIC_NON_TEXT_STRUCT_FIELD_KEY(
+        ErrorCategory.SEMANTIC,
+        LOCATION + setOf(Property.ACTUAL_TYPE),
+        "") {
+        override fun getErrorMessage(errorContext: PropertyValueMap?): String =
+            "Struct field key should be text but found ${errorContext.getProperty(Property.ACTUAL_TYPE)}}."
+    },
+
     SEMANTIC_ILLEGAL_GLOBAL_VARIABLE_ACCESS(
         ErrorCategory.SEMANTIC,
         LOCATION + setOf(Property.BINDING_NAME),

diff --git a/lang/src/org/partiql/lang/eval/EvaluatingCompiler.kt b/lang/src/org/partiql/lang/eval/EvaluatingCompiler.kt
@@ -871,7 +871,20 @@ internal class EvaluatingCompiler(
         }
 
         return thunkFactory.thunkEnv(metas) { env ->
-            val seq = fieldThunks.map { it.valueThunk(env).namedValue(it.nameThunk(env)) }.asSequence()
+            val seq = fieldThunks.map {
+                val nameValue = it.nameThunk(env)
+                if (!nameValue.type.isText) {
+                    // Evaluation time error where variable reference might be evaluated to non-text struct field.
+                    err("Found struct field key to be of type ${nameValue.type}",
+                        ErrorCode.EVALUATOR_NON_TEXT_STRUCT_FIELD_KEY,
+                        errorContextFrom(metas.sourceLocationMeta).also { pvm ->
+                            pvm[Property.ACTUAL_TYPE] = nameValue.type.toString()
+                        },
+                        internal = false
+                    )
+                }
+                it.valueThunk(env).namedValue(nameValue)
+            }.asSequence()
             createStructExprValue(seq, StructOrdering.ORDERED)
         }
     }

diff --git a/lang/src/org/partiql/lang/eval/visitors/PartiqlAstSanityValidator.kt b/lang/src/org/partiql/lang/eval/visitors/PartiqlAstSanityValidator.kt
@@ -17,6 +17,7 @@ package org.partiql.lang.eval.visitors
 
 import com.amazon.ionelement.api.IntElement
 import com.amazon.ionelement.api.IntElementSize
+import com.amazon.ionelement.api.TextElement
 import org.partiql.lang.ast.IsCountStarMeta
 import org.partiql.lang.ast.passes.SemanticException
 import org.partiql.lang.domains.PartiqlAst
@@ -98,4 +99,22 @@ object PartiqlAstSanityValidator : PartiqlAst.Visitor() {
                                     PropertyValueMap().addSourceLocation(metas))
         }
     }
+
+    override fun visitExprStruct(node: PartiqlAst.Expr.Struct) {
+        node.fields.forEach { field ->
+            if (field.first is PartiqlAst.Expr.Missing || (field.first is PartiqlAst.Expr.Lit && field.first.value !is TextElement)) {
+                val type = when (field.first) {
+                    is PartiqlAst.Expr.Lit -> field.first.value.type.toString()
+                    else -> "MISSING"
+                }
+                throw SemanticException(
+                    "Found struct field to be of type $type",
+                    ErrorCode.SEMANTIC_NON_TEXT_STRUCT_FIELD_KEY,
+                    PropertyValueMap().addSourceLocation(field.first.metas).also { pvm ->
+                        pvm[Property.ACTUAL_TYPE] = type
+                    }
+                )
+            }
+        }
+    }
 }
diff --git a/lang/test/org/partiql/lang/eval/EvaluatingCompilerExceptionsTest.kt b/lang/test/org/partiql/lang/eval/EvaluatingCompilerExceptionsTest.kt
@@ -242,4 +242,16 @@ class EvaluatingCompilerExceptionsTest : EvaluatorTestBase() {
                 Property.COLUMN_NUMBER to 11L,
                 Property.BINDING_NAME to "leading")
         )
+
+    @Test
+    fun variableReferenceToIntAsNonTextStructField() =
+        checkInputThrowingEvaluationException(
+            "SELECT {a : 2} FROM {'a' : 1}",
+            ErrorCode.EVALUATOR_NON_TEXT_STRUCT_FIELD_KEY,
+            mapOf(
+                Property.LINE_NUMBER to 1L,
+                Property.COLUMN_NUMBER to 8L,
+                Property.ACTUAL_TYPE to "INT"
+            )
+        )
 }
diff --git a/lang/test/org/partiql/lang/eval/visitors/PartiqlAstSanityValidatorTests.kt b/lang/test/org/partiql/lang/eval/visitors/PartiqlAstSanityValidatorTests.kt
@@ -14,6 +14,7 @@
 
 package org.partiql.lang.eval.visitors
 
+import com.amazon.ionelement.api.ionTimestamp
 import com.amazon.ionelement.api.toIonElement
 import org.junit.Test
 import org.partiql.lang.TestBase
@@ -22,6 +23,8 @@ import org.partiql.lang.errors.ErrorCode
 
 class PartiqlAstSanityValidatorTests : TestBase() {
     private fun litInt(value: Int) = PartiqlAst.build { lit(ion.newInt(value).toIonElement()) }
+    private val litNull = PartiqlAst.build { lit(ion.newNull().toIonElement()) }
+    private val missingExpr = PartiqlAst.build { missing() }
 
     @Test
     fun groupPartial() {
@@ -233,4 +236,56 @@ class PartiqlAstSanityValidatorTests : TestBase() {
             )
         }
     }
+
+    @Test
+    fun intAsNonTextStructKey() {
+        assertThrowsSqlException(ErrorCode.SEMANTIC_NON_TEXT_STRUCT_FIELD_KEY) {
+            PartiqlAstSanityValidator.validate(
+                PartiqlAst.build {
+                    query(
+                        struct(exprPair(litInt(1), litInt(2)))
+                    )
+                }
+            )
+        }
+    }
+
+    @Test
+    fun timestampAsNonTextStructKey() {
+        assertThrowsSqlException(ErrorCode.SEMANTIC_NON_TEXT_STRUCT_FIELD_KEY) {
+            PartiqlAstSanityValidator.validate(
+                PartiqlAst.build {
+                    query(
+                        struct(exprPair(lit(ionTimestamp("2007-02-23T12:14:33.079-08:00")), litInt(2)))
+                    )
+                }
+            )
+        }
+    }
+
+    @Test
+    fun nullAsNonTextStructKey() {
+        assertThrowsSqlException(ErrorCode.SEMANTIC_NON_TEXT_STRUCT_FIELD_KEY) {
+            PartiqlAstSanityValidator.validate(
+                PartiqlAst.build {
+                    query(
+                        struct(exprPair(litNull, litInt(2)))
+                    )
+                }
+            )
+        }
+    }
+
+    @Test
+    fun missingAsNonTextStructKey() {
+        assertThrowsSqlException(ErrorCode.SEMANTIC_NON_TEXT_STRUCT_FIELD_KEY) {
+            PartiqlAstSanityValidator.validate(
+                PartiqlAst.build {
+                    query(
+                        struct(exprPair(missingExpr, litInt(2)))
+                    )
+                }
+            )
+        }
+    }
 }