From 3be4a5ed279741b3c20c86d9dd061441f34f8a90 Mon Sep 17 00:00:00 2001 From: Alan Cai Date: Thu, 11 Jan 2024 15:58:00 -0800 Subject: [PATCH] Address RelExclude comments; slight refactor of subsumption + testing; add comments --- .../eval/internal/operator/rel/RelExclude.kt | 83 +++++++++---------- partiql-planner/build.gradle.kts | 1 - .../planner/internal/exclude/Subsumption.kt | 27 +++--- .../planner/internal/typer/PlanTyper.kt | 2 +- .../planner/internal/typer/TypeUtils.kt | 15 ++-- .../internal/exclude/SubsumptionTest.kt | 22 +++-- 6 files changed, 82 insertions(+), 68 deletions(-) diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelExclude.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelExclude.kt index 755aadb510..e92476bb43 100644 --- a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelExclude.kt +++ b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelExclude.kt @@ -31,13 +31,8 @@ internal class RelExclude( } override fun next(): Record? { - while (true) { - val row = input.next() ?: return null - val newRecord = exclusions.fold(row) { curRecord, expr -> - excludeOnRecord(curRecord, expr) - } - return newRecord - } + val record = input.next() ?: return null + return exclusions.fold(record) { rec, path -> exclude(rec, path) } } override fun close() { @@ -45,14 +40,14 @@ internal class RelExclude( } @OptIn(PartiQLValueExperimental::class) - private fun excludeOnRecord( + private fun exclude( record: Record, path: Rel.Op.Exclude.Path ): Record { val values = record.values val value = values.getOrNull(path.root.ref) val newValues = if (value != null) { - values[path.root.ref] = excludeOnPartiQLValue(value, path.steps) + values[path.root.ref] = exclude(value, path.steps) values } else { values @@ -61,57 +56,57 @@ internal class RelExclude( } @OptIn(PartiQLValueExperimental::class) - private fun excludeOnStructValue( + private fun exclude( structValue: StructValue<*>, exclusions: List ): PartiQLValue { - val attrSymbolsToRemove = mutableSetOf() - val attrKeysToRemove = mutableSetOf() + val structSymbolsToRemove = mutableSetOf() + val structKeysToRemove = mutableSetOf() // keys stored as lowercase strings val branches = mutableMapOf>() exclusions.forEach { exclusion -> when (exclusion.substeps.isEmpty()) { true -> { when (val leafType = exclusion.type) { is Rel.Op.Exclude.Type.StructWildcard -> { - // tuple wildcard at current level. return empty struct + // struct wildcard at current level. return empty struct return structValue() } - is Rel.Op.Exclude.Type.StructSymbol -> attrSymbolsToRemove.add(leafType.symbol) - is Rel.Op.Exclude.Type.StructKey -> attrKeysToRemove.add(leafType.key.lowercase()) - else -> { /* non-coll step; do nothing */ } + is Rel.Op.Exclude.Type.StructSymbol -> structSymbolsToRemove.add(leafType.symbol) + is Rel.Op.Exclude.Type.StructKey -> structKeysToRemove.add(leafType.key.lowercase()) + else -> { /* coll step; do nothing */ } } } false -> { when (exclusion.type) { is Rel.Op.Exclude.Type.StructWildcard, is Rel.Op.Exclude.Type.StructSymbol, is Rel.Op.Exclude.Type.StructKey -> branches[exclusion.type] = exclusion.substeps - else -> { /* non-coll step; do nothing */ } + else -> { /* coll step; do nothing */ } } } } } val finalStruct = structValue.entries.mapNotNull { structField -> - if (attrSymbolsToRemove.contains(structField.first) || attrKeysToRemove.contains(structField.first.lowercase())) { + if (structSymbolsToRemove.contains(structField.first) || structKeysToRemove.contains(structField.first.lowercase())) { // struct attr is to be removed at current level null } else { // deeper level exclusions val name = structField.first var value = structField.second - // apply case-sensitive tuple attr exclusions at deeper levels - val structFieldCaseSensitiveKey = relOpExcludeTypeStructKey(name) - branches[structFieldCaseSensitiveKey]?.let { - value = excludeOnPartiQLValue(value, it) + // apply struct key exclusions at deeper levels + val structKey = relOpExcludeTypeStructKey(name) + branches[structKey]?.let { + value = exclude(value, it) } - // apply case-insensitive tuple attr exclusions at deeper levels - val structFieldCaseInsensitiveKey = relOpExcludeTypeStructSymbol(name) - branches[structFieldCaseInsensitiveKey]?.let { - value = excludeOnPartiQLValue(value, it) + // apply struct symbol exclusions at deeper levels + val structSymbol = relOpExcludeTypeStructSymbol(name) + branches[structSymbol]?.let { + value = exclude(value, it) } - // apply tuple wildcard exclusions at deeper levels - val tupleWildcardKey = relOpExcludeTypeStructWildcard() - branches[tupleWildcardKey]?.let { - value = excludeOnPartiQLValue(value, it) + // apply struct wildcard exclusions at deeper levels + val structWildcard = relOpExcludeTypeStructWildcard() + branches[structWildcard]?.let { + value = exclude(value, it) } Pair(name, value) } @@ -134,7 +129,7 @@ internal class RelExclude( } @OptIn(PartiQLValueExperimental::class) - private fun excludeOnCollValue( + private fun exclude( coll: CollectionValue<*>, type: PartiQLValueType, exclusions: List @@ -152,14 +147,14 @@ internal class RelExclude( is Rel.Op.Exclude.Type.CollIndex -> { indexesToRemove.add(leafType.index) } - else -> { /* non-coll step; do nothing */ } + else -> { /* struct step; do nothing */ } } } false -> { when (exclusion.type) { is Rel.Op.Exclude.Type.CollWildcard, is Rel.Op.Exclude.Type.CollIndex -> branches[exclusion.type] = exclusion.substeps - else -> { /* non-coll step; do nothing */ } + else -> { /* struct step; do nothing */ } } } } @@ -173,15 +168,15 @@ internal class RelExclude( var value = element if (coll is ListValue || coll is SexpValue) { // apply collection index exclusions at deeper levels for lists and sexps - val elementKey = relOpExcludeTypeCollIndex(index) - branches[elementKey]?.let { - value = excludeOnPartiQLValue(element, it) + val collIndex = relOpExcludeTypeCollIndex(index) + branches[collIndex]?.let { + value = exclude(element, it) } } // apply collection wildcard exclusions at deeper levels for lists, bags, and sexps - val collectionWildcardKey = relOpExcludeTypeCollWildcard() - branches[collectionWildcardKey]?.let { - value = excludeOnPartiQLValue(value, it) + val collWildcard = relOpExcludeTypeCollWildcard() + branches[collWildcard]?.let { + value = exclude(value, it) } value } @@ -190,12 +185,12 @@ internal class RelExclude( } @OptIn(PartiQLValueExperimental::class) - private fun excludeOnPartiQLValue(initialPartiQLValue: PartiQLValue, exclusions: List): PartiQLValue { + private fun exclude(initialPartiQLValue: PartiQLValue, exclusions: List): PartiQLValue { return when (initialPartiQLValue) { - is StructValue<*> -> excludeOnStructValue(initialPartiQLValue, exclusions) - is BagValue<*> -> excludeOnCollValue(initialPartiQLValue, PartiQLValueType.BAG, exclusions) - is ListValue<*> -> excludeOnCollValue(initialPartiQLValue, PartiQLValueType.LIST, exclusions) - is SexpValue<*> -> excludeOnCollValue(initialPartiQLValue, PartiQLValueType.SEXP, exclusions) + is StructValue<*> -> exclude(initialPartiQLValue, exclusions) + is BagValue<*> -> exclude(initialPartiQLValue, PartiQLValueType.BAG, exclusions) + is ListValue<*> -> exclude(initialPartiQLValue, PartiQLValueType.LIST, exclusions) + is SexpValue<*> -> exclude(initialPartiQLValue, PartiQLValueType.SEXP, exclusions) else -> { initialPartiQLValue } diff --git a/partiql-planner/build.gradle.kts b/partiql-planner/build.gradle.kts index 9ca5031cee..aca57c6517 100644 --- a/partiql-planner/build.gradle.kts +++ b/partiql-planner/build.gradle.kts @@ -34,7 +34,6 @@ dependencies { testImplementation(project(":partiql-parser")) testImplementation(project(":plugins:partiql-local")) testImplementation(project(":plugins:partiql-memory")) - testImplementation(testFixtures(project(":partiql-lang"))) // TODO replace usage of `partiql-lang` test fixture // Test Fixtures testFixturesImplementation(project(":partiql-spi")) } diff --git a/partiql-planner/src/main/kotlin/org/partiql/planner/internal/exclude/Subsumption.kt b/partiql-planner/src/main/kotlin/org/partiql/planner/internal/exclude/Subsumption.kt index e071133dc8..48f074c8d9 100644 --- a/partiql-planner/src/main/kotlin/org/partiql/planner/internal/exclude/Subsumption.kt +++ b/partiql-planner/src/main/kotlin/org/partiql/planner/internal/exclude/Subsumption.kt @@ -7,6 +7,8 @@ import org.partiql.planner.internal.ir.relOpExcludeTypeStructSymbol import org.partiql.planner.internal.ir.relOpExcludeTypeStructWildcard /** + * An internal data structure of exclude steps that makes it a bit easier to remove any redundant exclude steps. + * * Store as a map of exclude type to its substeps; previous representation as a set of `Rel.Op.Exclude.Step` didn't * work since retrieval of element in set required looping through the whole list. */ @@ -27,12 +29,14 @@ internal class ExcludeRepr(val steps: Map) { } } + internal fun isLeaf(): Boolean = steps.isEmpty() + internal fun removeRedundantSteps(): ExcludeRepr { val newSubsteps = mutableMapOf() val collIndexSteps = mutableMapOf() val structSymbolSteps = mutableMapOf() val structKeySteps = mutableMapOf() - // Group the steps by their type and add wildcards to `substeps` + // Group the steps by their type and first add wildcards to `substeps` steps.forEach { (type, substeps) -> when (type) { is Rel.Op.Exclude.Type.StructWildcard -> { @@ -48,7 +52,7 @@ internal class ExcludeRepr(val steps: Map) { is Rel.Op.Exclude.Type.StructKey -> structKeySteps[type] = substeps.removeRedundantSteps() } } - // Next add non-wildcard steps + // Next add non-wildcard steps and check if they are subsumed by previously added steps stepsSubsume(newSubsteps, collIndexSteps)?.let { newSubsteps.putAll(it) } @@ -62,6 +66,7 @@ internal class ExcludeRepr(val steps: Map) { } } +// null indicates all the steps on rhs are subsumed by the lhs private fun stepsSubsume(lhs: Map, rhs: Map?): Map? { if (rhs == null) { return null @@ -72,7 +77,7 @@ private fun stepsSubsume(lhs: Map, rhs: Map { val lhsCollWildcard = lhs[relOpExcludeTypeCollWildcard()] if (lhsCollWildcard != null) { - newSubsteps = if (lhsCollWildcard.steps.isEmpty()) { + newSubsteps = if (lhsCollWildcard.isLeaf()) { // coll wildcard leaf in lhs null } else { @@ -83,7 +88,7 @@ private fun stepsSubsume(lhs: Map, rhs: Map { val lhsCollWildcard = lhs[relOpExcludeTypeCollWildcard()] if (lhsCollWildcard != null) { - newSubsteps = if (lhsCollWildcard.steps.isEmpty()) { + newSubsteps = if (lhsCollWildcard.isLeaf()) { // coll wildcard leaf in lhs null } else { @@ -92,7 +97,7 @@ private fun stepsSubsume(lhs: Map, rhs: Map, rhs: Map { val lhsStructWildcard = lhs[relOpExcludeTypeStructWildcard()] if (lhsStructWildcard != null) { - newSubsteps = if (lhsStructWildcard.steps.isEmpty()) { + newSubsteps = if (lhsStructWildcard.isLeaf()) { // struct wildcard leaf in lhs null } else { @@ -114,7 +119,7 @@ private fun stepsSubsume(lhs: Map, rhs: Map { val lhsStructWildcard = lhs[relOpExcludeTypeStructWildcard()] if (lhsStructWildcard != null) { - newSubsteps = if (lhsStructWildcard.steps.isEmpty()) { + newSubsteps = if (lhsStructWildcard.isLeaf()) { // struct wildcard leaf in lhs null } else { @@ -123,7 +128,7 @@ private fun stepsSubsume(lhs: Map, rhs: Map, rhs: Map { val lhsStructWildcard = lhs[relOpExcludeTypeStructWildcard()] if (lhsStructWildcard != null) { - newSubsteps = if (lhsStructWildcard.steps.isEmpty()) { + newSubsteps = if (lhsStructWildcard.isLeaf()) { // struct wildcard leaf in lhs null } else { @@ -144,7 +149,7 @@ private fun stepsSubsume(lhs: Map, rhs: Map, rhs: Map, lastStepOptional: Boolean = true): StaticType { +internal fun StaticType.exclude(steps: List, lastStepOptional: Boolean = false): StaticType { val type = this return steps.fold(type) { acc, step -> when (acc) { @@ -166,7 +171,7 @@ internal fun StaticType.exclude(steps: List, lastStepOption * @param lastStepOptional * @return */ -internal fun StructType.exclude(step: Rel.Op.Exclude.Step, lastStepOptional: Boolean = true): StaticType { +internal fun StructType.exclude(step: Rel.Op.Exclude.Step, lastStepOptional: Boolean = false): StaticType { val type = step.type val substeps = step.substeps val output = fields.mapNotNull { field -> @@ -211,13 +216,13 @@ internal fun StructType.exclude(step: Rel.Op.Exclude.Step, lastStepOptional: Boo * @param lastStepOptional * @return */ -internal fun CollectionType.exclude(step: Rel.Op.Exclude.Step, lastStepOptional: Boolean = true): StaticType { +internal fun CollectionType.exclude(step: Rel.Op.Exclude.Step, lastStepOptional: Boolean = false): StaticType { var e = this.elementType val substeps = step.substeps when (step.type) { is Rel.Op.Exclude.Type.CollIndex -> { if (substeps.isNotEmpty()) { - e = e.exclude(substeps, true) + e = e.exclude(substeps, lastStepOptional = true) } } is Rel.Op.Exclude.Type.CollWildcard -> { diff --git a/partiql-planner/src/test/kotlin/org/partiql/planner/internal/exclude/SubsumptionTest.kt b/partiql-planner/src/test/kotlin/org/partiql/planner/internal/exclude/SubsumptionTest.kt index bfd95f6794..5ccbd19202 100644 --- a/partiql-planner/src/test/kotlin/org/partiql/planner/internal/exclude/SubsumptionTest.kt +++ b/partiql-planner/src/test/kotlin/org/partiql/planner/internal/exclude/SubsumptionTest.kt @@ -1,8 +1,12 @@ package org.partiql.planner.internal.exclude +import org.junit.jupiter.api.extension.ExtensionContext +import org.junit.jupiter.api.parallel.Execution +import org.junit.jupiter.api.parallel.ExecutionMode import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.Arguments +import org.junit.jupiter.params.provider.ArgumentsProvider import org.junit.jupiter.params.provider.ArgumentsSource -import org.partiql.lang.util.ArgumentsProviderBase import org.partiql.parser.PartiQLParser import org.partiql.plan.Rel import org.partiql.plan.Rex @@ -16,6 +20,7 @@ import org.partiql.plan.relOpExcludeTypeStructSymbol import org.partiql.plan.relOpExcludeTypeStructWildcard import org.partiql.plan.rexOpVar import org.partiql.planner.PartiQLPlanner +import java.util.stream.Stream import kotlin.test.assertEquals class SubsumptionTest { @@ -37,14 +42,19 @@ class SubsumptionTest { assertEquals(tc.expectedExcludeExprs, excludeClause) } - internal data class SubsumptionTC(val excludeExprStr: String, val expectedExcludeExprs: List) + data class SubsumptionTC(val excludeExprStr: String, val expectedExcludeExprs: List) @ParameterizedTest @ArgumentsSource(ExcludeSubsumptionTests::class) - internal fun subsumptionTests(tc: SubsumptionTC) = testExcludeExprSubsumption(tc) + @Execution(ExecutionMode.CONCURRENT) + fun subsumptionTests(tc: SubsumptionTC) = testExcludeExprSubsumption(tc) - internal class ExcludeSubsumptionTests : ArgumentsProviderBase() { - override fun getParameters(): List = listOf( + internal class ExcludeSubsumptionTests : ArgumentsProvider { + override fun provideArguments(context: ExtensionContext?): Stream { + return parameters.map { Arguments.of(it) }.stream() + } + + private val parameters = listOf( SubsumptionTC( "s.a, t.a", // different roots listOf( @@ -126,7 +136,7 @@ class SubsumptionTest { ) ), SubsumptionTC( - """ -- duplicates subsumed + """-- duplicates subsumed t.a, t.a, t.b, t.b, t.b, t.c.*, t.c.*,