Remove redundant array construction calls

partiql · Aug 21, 2024 · 75f3f35 · 75f3f35
1 parent 5d5920b
commit 75f3f35
Show file tree

Hide file tree

Showing 7 changed files with 52 additions and 43 deletions.
diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/helpers/RecordUtility.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/helpers/RecordUtility.kt
@@ -1,17 +1,18 @@
 package org.partiql.eval.internal.helpers
 
-import org.partiql.eval.internal.Record
 import org.partiql.eval.value.Datum
 
 internal object RecordUtility {
     /**
-     * Converts the [Record] into an array of Datum, while coercing any missing values into null values.
+     * Coerces missing values into null values. Currently used when the [Datum.comparator] is used in a TreeSet/TreeMap
+     * (treats null and missing as the same value) and we need to deterministically return a value. Here we use coerce
+     * to null to follow the PartiQL spec's grouping function.
      */
-    fun Record.toDatumArrayCoerceMissing(): Array<Datum> = Array(this.values.size) {
-        val d = this@toDatumArrayCoerceMissing.values[it]
-        when (d.isMissing) {
-            true -> Datum.nullValue()
-            else -> d
+    fun Array<Datum>.coerceMissing() {
+        for (i in indices) {
+            if (this[i].isMissing) {
+                this[i] = Datum.nullValue()
+            }
         }
     }
 }
diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelExceptAll.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelExceptAll.kt
@@ -2,7 +2,7 @@ package org.partiql.eval.internal.operator.rel
 
 import org.partiql.eval.internal.Environment
 import org.partiql.eval.internal.Record
-import org.partiql.eval.internal.helpers.RecordUtility.toDatumArrayCoerceMissing
+import org.partiql.eval.internal.helpers.RecordUtility.coerceMissing
 import org.partiql.eval.internal.operator.Operator
 import org.partiql.eval.value.Datum
 import java.util.TreeMap
@@ -27,13 +27,13 @@ internal class RelExceptAll(
             seed()
         }
         for (row in lhs) {
-            val partiqlRow = row.toDatumArrayCoerceMissing()
-            val remaining = seen[partiqlRow] ?: 0
+            row.values.coerceMissing()
+            val remaining = seen[row.values] ?: 0
             if (remaining > 0) {
-                seen[partiqlRow] = remaining - 1
+                seen[row.values] = remaining - 1
                 continue
             }
-            return Record.of(*partiqlRow)
+            return Record(row.values)
         }
         return null
     }
@@ -50,9 +50,9 @@ internal class RelExceptAll(
     private fun seed() {
         init = true
         for (row in rhs) {
-            val partiqlRow = row.toDatumArrayCoerceMissing()
-            val n = seen[partiqlRow] ?: 0
-            seen[partiqlRow] = n + 1
+            row.values.coerceMissing()
+            val n = seen[row.values] ?: 0
+            seen[row.values] = n + 1
         }
     }
 }
diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelExceptDistinct.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelExceptDistinct.kt
@@ -2,7 +2,7 @@ package org.partiql.eval.internal.operator.rel
 
 import org.partiql.eval.internal.Environment
 import org.partiql.eval.internal.Record
-import org.partiql.eval.internal.helpers.RecordUtility.toDatumArrayCoerceMissing
+import org.partiql.eval.internal.helpers.RecordUtility.coerceMissing
 import org.partiql.eval.internal.operator.Operator
 import java.util.TreeSet
 
@@ -31,9 +31,9 @@ internal class RelExceptDistinct(
             seed()
         }
         for (row in lhs) {
-            val partiqlRow = row.toDatumArrayCoerceMissing()
-            if (!seen.contains(partiqlRow)) {
-                return Record.of(*partiqlRow)
+            row.values.coerceMissing()
+            if (!seen.contains(row.values)) {
+                return Record(row.values)
             }
         }
         return null
@@ -51,8 +51,8 @@ internal class RelExceptDistinct(
     private fun seed() {
         init = true
         for (row in rhs) {
-            val partiqlRow = row.toDatumArrayCoerceMissing()
-            seen.add(partiqlRow)
+            row.values.coerceMissing()
+            seen.add(row.values)
         }
     }
 }
diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelIntersectAll.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelIntersectAll.kt
@@ -2,7 +2,7 @@ package org.partiql.eval.internal.operator.rel
 
 import org.partiql.eval.internal.Environment
 import org.partiql.eval.internal.Record
-import org.partiql.eval.internal.helpers.RecordUtility.toDatumArrayCoerceMissing
+import org.partiql.eval.internal.helpers.RecordUtility.coerceMissing
 import org.partiql.eval.internal.operator.Operator
 import org.partiql.eval.value.Datum
 import java.util.TreeMap
@@ -27,11 +27,11 @@ internal class RelIntersectAll(
             seed()
         }
         for (row in rhs) {
-            val partiqlRow = row.toDatumArrayCoerceMissing()
-            val remaining = seen[partiqlRow] ?: 0
+            row.values.coerceMissing()
+            val remaining = seen[row.values] ?: 0
             if (remaining > 0) {
-                seen[partiqlRow] = remaining - 1
-                return Record.of(*partiqlRow)
+                seen[row.values] = remaining - 1
+                return Record(row.values)
             }
         }
         return null
@@ -49,9 +49,9 @@ internal class RelIntersectAll(
     private fun seed() {
         init = true
         for (row in lhs) {
-            val partiqlRow = row.toDatumArrayCoerceMissing()
-            val alreadySeen = seen[partiqlRow] ?: 0
-            seen[partiqlRow] = alreadySeen + 1
+            row.values.coerceMissing()
+            val alreadySeen = seen[row.values] ?: 0
+            seen[row.values] = alreadySeen + 1
         }
     }
 }
diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelIntersectDistinct.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelIntersectDistinct.kt
@@ -2,7 +2,7 @@ package org.partiql.eval.internal.operator.rel
 
 import org.partiql.eval.internal.Environment
 import org.partiql.eval.internal.Record
-import org.partiql.eval.internal.helpers.RecordUtility.toDatumArrayCoerceMissing
+import org.partiql.eval.internal.helpers.RecordUtility.coerceMissing
 import org.partiql.eval.internal.operator.Operator
 import java.util.TreeSet
 
@@ -26,9 +26,9 @@ internal class RelIntersectDistinct(
             seed()
         }
         for (row in rhs) {
-            val partiqlRow = row.toDatumArrayCoerceMissing()
-            if (seen.remove(partiqlRow)) {
-                return Record.of(*partiqlRow)
+            row.values.coerceMissing()
+            if (seen.remove(row.values)) {
+                return Record(row.values)
             }
         }
         return null
@@ -46,8 +46,8 @@ internal class RelIntersectDistinct(
     private fun seed() {
         init = true
         for (row in lhs) {
-            val partiqlRow = row.toDatumArrayCoerceMissing()
-            seen.add(partiqlRow)
+            row.values.coerceMissing()
+            seen.add(row.values)
         }
     }
 }
diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelUnionAll.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelUnionAll.kt
@@ -2,7 +2,7 @@ package org.partiql.eval.internal.operator.rel
 
 import org.partiql.eval.internal.Environment
 import org.partiql.eval.internal.Record
-import org.partiql.eval.internal.helpers.RecordUtility.toDatumArrayCoerceMissing
+import org.partiql.eval.internal.helpers.RecordUtility.coerceMissing
 import org.partiql.eval.internal.operator.Operator
 
 internal class RelUnionAll(
@@ -21,8 +21,16 @@ internal class RelUnionAll(
 
     override fun next(): Record {
         return when (lhs.hasNext()) {
-            true -> Record.of(*lhs.next().toDatumArrayCoerceMissing())
-            false -> Record.of(*rhs.next().toDatumArrayCoerceMissing())
+            true -> {
+                val record = lhs.next()
+                record.values.coerceMissing()
+                record
+            }
+            false -> {
+                val record = rhs.next()
+                record.values.coerceMissing()
+                record
+            }
         }
     }
 

diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelUnionDistinct.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelUnionDistinct.kt
@@ -3,7 +3,7 @@ package org.partiql.eval.internal.operator.rel
 import org.partiql.eval.internal.Environment
 import org.partiql.eval.internal.Record
 import org.partiql.eval.internal.helpers.IteratorChain
-import org.partiql.eval.internal.helpers.RecordUtility.toDatumArrayCoerceMissing
+import org.partiql.eval.internal.helpers.RecordUtility.coerceMissing
 import org.partiql.eval.internal.operator.Operator
 import java.util.TreeSet
 
@@ -25,10 +25,10 @@ internal class RelUnionDistinct(
 
     override fun peek(): Record? {
         for (record in input) {
-            val partiqlRow = record.toDatumArrayCoerceMissing()
-            if (!seen.contains(partiqlRow)) {
-                seen.add(partiqlRow)
-                return Record.of(*partiqlRow)
+            record.values.coerceMissing()
+            if (!seen.contains(record.values)) {
+                seen.add(record.values)
+                return Record(record.values)
             }
         }
         return null