Skip to content

Commit

Permalink
Fixes tests
Browse files Browse the repository at this point in the history 
Updates planning tests

Removes exhaustive attribute from dynamic call
  • Loading branch information
@johnedquinn
johnedquinn committed Jun 4, 2024
1 parent 9ee2e59 commit cc3654a
Show file tree
Hide file tree
Showing 11 changed files with 237 additions and 191 deletions.
119 changes: 67 additions & 52 deletions partiql-eval/src/test/kotlin/org/partiql/eval/internal/PartiQLEngineDefaultTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,47 +414,6 @@ class PartiQLEngineDefaultTest {

@JvmStatic
fun aggregationTestCases() = kotlin.collections.listOf(
SuccessTestCase(
input = """
SELECT
gk_0, SUM(t.c) AS t_c_sum
FROM <<
{ 'b': NULL, 'c': 1 },
{ 'b': MISSING, 'c': 2 },
{ 'b': 1, 'c': 1 },
{ 'b': 1, 'c': 2 },
{ 'b': 2, 'c': NULL },
{ 'b': 2, 'c': 2 },
{ 'b': 3, 'c': MISSING },
{ 'b': 3, 'c': 2 },
{ 'b': 4, 'c': MISSING },
{ 'b': 4, 'c': NULL }
>> AS t GROUP BY t.b AS gk_0;
""".trimIndent(),
expected = org.partiql.value.bagValue(
org.partiql.value.structValue(
"gk_0" to org.partiql.value.int32Value(1),
"t_c_sum" to org.partiql.value.int32Value(3)
),
org.partiql.value.structValue(
"gk_0" to org.partiql.value.int32Value(2),
"t_c_sum" to org.partiql.value.int32Value(2)
),
org.partiql.value.structValue(
"gk_0" to org.partiql.value.int32Value(3),
"t_c_sum" to org.partiql.value.int32Value(2)
),
org.partiql.value.structValue(
"gk_0" to org.partiql.value.int32Value(4),
"t_c_sum" to org.partiql.value.int32Value(null)
),
org.partiql.value.structValue(
"gk_0" to org.partiql.value.nullValue(),
"t_c_sum" to org.partiql.value.int32Value(3)
),
),
mode = org.partiql.eval.PartiQLEngine.Mode.PERMISSIVE
),
SuccessTestCase(
input = """
SELECT VALUE { 'sensor': sensor,
Expand Down Expand Up @@ -900,17 +859,6 @@ class PartiQLEngineDefaultTest {
mode = PartiQLEngine.Mode.STRICT
),
// PartiQL Specification Section 8
SuccessTestCase(
input = "MISSING AND TRUE;",
expected = boolValue(null),
),
// PartiQL Specification Section 8
SuccessTestCase(
input = "MISSING AND TRUE;",
expected = boolValue(null), // TODO: Is this right?
mode = PartiQLEngine.Mode.STRICT
),
// PartiQL Specification Section 8
SuccessTestCase(
input = "NULL IS MISSING;",
expected = boolValue(false),
Expand Down Expand Up @@ -1385,6 +1333,73 @@ class PartiQLEngineDefaultTest {
)
).assert()

@Test
@Disabled(
"""
We currently do not have support for consolidating collections containing MISSING/NULL. The current
result (value) is correct. However, the types are slightly wrong due to the SUM__ANY_ANY being resolved.
"""
)
fun aggregationOnLiteralBagOfStructs() = SuccessTestCase(
input = """
SELECT
gk_0, SUM(t.c) AS t_c_sum
FROM <<
{ 'b': NULL, 'c': 1 },
{ 'b': MISSING, 'c': 2 },
{ 'b': 1, 'c': 1 },
{ 'b': 1, 'c': 2 },
{ 'b': 2, 'c': NULL },
{ 'b': 2, 'c': 2 },
{ 'b': 3, 'c': MISSING },
{ 'b': 3, 'c': 2 },
{ 'b': 4, 'c': MISSING },
{ 'b': 4, 'c': NULL }
>> AS t GROUP BY t.b AS gk_0;
""".trimIndent(),
expected = org.partiql.value.bagValue(
org.partiql.value.structValue(
"gk_0" to org.partiql.value.int32Value(1),
"t_c_sum" to org.partiql.value.int32Value(3)
),
org.partiql.value.structValue(
"gk_0" to org.partiql.value.int32Value(2),
"t_c_sum" to org.partiql.value.int32Value(2)
),
org.partiql.value.structValue(
"gk_0" to org.partiql.value.int32Value(3),
"t_c_sum" to org.partiql.value.int32Value(2)
),
org.partiql.value.structValue(
"gk_0" to org.partiql.value.int32Value(4),
"t_c_sum" to org.partiql.value.int32Value(null)
),
org.partiql.value.structValue(
"gk_0" to org.partiql.value.nullValue(),
"t_c_sum" to org.partiql.value.int32Value(3)
),
),
mode = org.partiql.eval.PartiQLEngine.Mode.PERMISSIVE
).assert()

// PartiQL Specification Section 8
@Test
@Disabled("Currently, .check(<PartiQLValue>) is failing for MISSING. This will be resolved by Datum.")
fun missingAndTruePermissive() =
SuccessTestCase(
input = "MISSING AND TRUE;",
expected = boolValue(null),
).assert()

// PartiQL Specification Section 8
@Test
@Disabled("Currently, .check(<PartiQLValue>) is failing for MISSING. This will be resolved by Datum.")
fun missingAndTrueStrict() = SuccessTestCase(
input = "MISSING AND TRUE;",
expected = boolValue(null), // TODO: Is this right?
mode = PartiQLEngine.Mode.STRICT
).assert()

@Test
@Disabled("Support for ORDER BY needs to be added for this to pass.")
// PartiQL Specification says that SQL's SELECT is coerced, but SELECT VALUE is not.
Expand Down
3 changes: 1 addition & 2 deletions partiql-lang/src/main/kotlin/org/partiql/lang/syntax/impl/PartiQLPigVisitor.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,8 @@ import org.partiql.lang.util.checkThreadInterrupted
import org.partiql.lang.util.error
import org.partiql.lang.util.getPrecisionFromTimeString
import org.partiql.lang.util.unaryMinus
import org.partiql.parser.antlr.PartiQLParser
import org.partiql.parser.antlr.PartiQLParserBaseVisitor
import org.partiql.parser.internal.antlr.PartiQLParser
import org.partiql.parser.internal.antlr.PartiQLParserBaseVisitor
import org.partiql.pig.runtime.SymbolPrimitive
import org.partiql.value.datetime.DateTimeException
import org.partiql.value.datetime.TimeZone
Expand Down
4 changes: 2 additions & 2 deletions partiql-planner/src/main/kotlin/org/partiql/planner/internal/Env.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ internal class Env(private val session: PartiQLPlanner.Session) {
)
}
return ProblemGenerator.missingRex(
rexOpCallDynamic(args, candidates, false),
rexOpCallDynamic(args, candidates),
ProblemGenerator.incompatibleTypesForOp(args.map { it.type }, path.normalized.joinToString("."))
)
}
Expand All @@ -126,7 +126,7 @@ internal class Env(private val session: PartiQLPlanner.Session) {
)
}
// Rewrite as a dynamic call to be typed by PlanTyper
rex(StaticType.ANY, rexOpCallDynamic(args, candidates, match.exhaustive))
rex(StaticType.ANY, rexOpCallDynamic(args, candidates))
}
is FnMatch.Static -> {
// Create an internal typed reference
Expand Down
2 changes: 0 additions & 2 deletions partiql-planner/src/main/kotlin/org/partiql/planner/internal/FnMatch.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,8 @@ internal sealed class FnMatch {
* This represents dynamic dispatch.
*
* @property candidates Ordered list of potentially applicable functions to dispatch dynamically.
* @property exhaustive True if all argument permutations (branches) are matched.
*/
data class Dynamic(
val candidates: List<Static>,
val exhaustive: Boolean,
) : FnMatch()
}
28 changes: 6 additions & 22 deletions partiql-planner/src/main/kotlin/org/partiql/planner/internal/FnResolver.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import org.partiql.planner.internal.ir.Ref
import org.partiql.planner.internal.typer.toRuntimeTypeOrNull
import org.partiql.spi.fn.FnExperimental
import org.partiql.spi.fn.FnSignature
import org.partiql.types.AnyOfType
import org.partiql.types.NullType
import org.partiql.types.StaticType
import org.partiql.value.PartiQLValueExperimental
import org.partiql.value.PartiQLValueType
Expand Down Expand Up @@ -55,15 +53,7 @@ internal object FnResolver {
}

// Match candidates on all argument permutations
var exhaustive = true
val matches = argPermutations.mapNotNull {
val m = match(candidates, it)
if (m == null) {
// we had a branch whose arguments did not match a static call
exhaustive = false
}
m
}
val matches = argPermutations.mapNotNull { match(candidates, it) }

// Order based on original candidate function ordering
val orderedUniqueMatches = matches.toSet().toList()
Expand All @@ -73,10 +63,10 @@ internal object FnResolver {

// Static call iff only one match for every branch
val n = orderedCandidates.size
return when {
n == 0 -> null
n == 1 && exhaustive -> orderedCandidates.first()
else -> FnMatch.Dynamic(orderedCandidates, exhaustive)
return when (n) {
0 -> null
1 -> orderedCandidates.first()
else -> FnMatch.Dynamic(orderedCandidates)
}
}

Expand Down Expand Up @@ -149,13 +139,7 @@ internal object FnResolver {
}

private fun buildArgumentPermutations(args: List<StaticType>): List<List<StaticType>> {
val flattenedArgs = args.map {
if (it is AnyOfType) {
it.flatten().allTypes.filter { it !is NullType }
} else {
it.flatten().allTypes
}
}
val flattenedArgs = args.map { it.flatten().allTypes }
return buildArgumentPermutations(flattenedArgs, accumulator = emptyList())
}

Expand Down
1 change: 0 additions & 1 deletion partiql-planner/src/main/kotlin/org/partiql/planner/internal/ir/Nodes.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,6 @@ internal data class Rex(
internal data class Dynamic(
@JvmField internal val args: List<Rex>,
@JvmField internal val candidates: List<Candidate>,
@JvmField internal val exhaustive: Boolean,
) : Call() {
public override val children: List<PlanNode> by lazy {
val kids = mutableListOf<PlanNode?>()
Expand Down
42 changes: 32 additions & 10 deletions partiql-planner/src/main/kotlin/org/partiql/planner/internal/typer/PlanTyper.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ import org.partiql.value.MissingValue
import org.partiql.value.PartiQLValueExperimental
import org.partiql.value.TextValue
import org.partiql.value.boolValue
import org.partiql.value.missingValue
import org.partiql.value.stringValue
import kotlin.math.max

Expand Down Expand Up @@ -578,6 +579,14 @@ internal class PlanTyper(private val env: Env) {
}
}

// Check that root is not literal missing
if (root.isLiteralMissing()) {
return ProblemGenerator.missingRex(
rexOpPathIndex(root, key),
ProblemGenerator.expressionAlwaysReturnsMissing()
)
}

// Check that Root was LIST or SEXP by checking accumuated element types
if (elementTypes.isEmpty()) {
return ProblemGenerator.missingRex(
Expand All @@ -588,6 +597,8 @@ internal class PlanTyper(private val env: Env) {
return rex(unionOf(elementTypes), rexOpPathIndex(root, key))
}

private fun Rex.isLiteralMissing(): Boolean = this.op is Rex.Op.Lit && this.op.value.withoutAnnotations() == missingValue()

override fun visitRexOpPathKey(node: Rex.Op.Path.Key, ctx: StaticType?): Rex {
val root = visitRex(node.root, node.root.type)
val key = visitRex(node.key, node.key.type)
Expand All @@ -608,6 +619,14 @@ internal class PlanTyper(private val env: Env) {
)
}

// Check that root is not literal missing
if (root.isLiteralMissing()) {
return ProblemGenerator.missingRex(
rexOpPathKey(root, key),
ProblemGenerator.expressionAlwaysReturnsMissing()
)
}

// Get Element Type
val elementType = root.type.inferListNotNull { type ->
val struct = type as? StructType ?: return@inferListNotNull null
Expand Down Expand Up @@ -645,6 +664,14 @@ internal class PlanTyper(private val env: Env) {
)
}

// Check that root is not literal missing
if (root.isLiteralMissing()) {
return ProblemGenerator.missingRex(
rexOpPathSymbol(root, node.key),
ProblemGenerator.expressionAlwaysReturnsMissing()
)
}

// Get Element Types
val paths = root.type.inferRexListNotNull { type ->
val struct = type as? StructType ?: return@inferRexListNotNull null
Expand All @@ -663,9 +690,11 @@ internal class PlanTyper(private val env: Env) {
val type = when (paths.size) {
// Escape early since no inference could be made
0 -> {
val key = org.partiql.plan.Identifier.Symbol(node.key, org.partiql.plan.Identifier.CaseSensitivity.SENSITIVE)
val inScopeVariables = locals.schema.map { it.name }.toSet()
return ProblemGenerator.missingRex(
rexOpPathSymbol(root, node.key),
ProblemGenerator.expressionAlwaysReturnsMissing("No output types could be gathered.")
ProblemGenerator.undefinedVariable(key, inScopeVariables)
)
}
else -> unionOf(paths.map { it.type }.toSet())
Expand Down Expand Up @@ -740,13 +769,6 @@ internal class PlanTyper(private val env: Env) {
/**
* Typing of a dynamic function call.
*
* isMissable TRUE when the argument permutations may not definitively invoke one of the candidates.
* You can think of [isMissable] as being the same as "not exhaustive". For example, if we have ABS(INT | STRING), then
* this function call [isMissable] because there isn't an `ABS(STRING)` function signature AKA we haven't exhausted
* all the arguments. On the other hand, take an "exhaustive" scenario: ABS(INT | DEC). In this case, [isMissable]
* is false because we have functions for each potential argument AKA we have exhausted the arguments.
*
*
* @param node
* @param ctx
* @return
Expand All @@ -760,8 +782,8 @@ internal class PlanTyper(private val env: Env) {
}.toMutableSet()
if (types.isEmpty()) {
return ProblemGenerator.missingRex(
rexOpCallDynamic(node.args, node.candidates, exhaustive = true), // TODO: Remove exhaustive
ProblemGenerator.expressionAlwaysReturnsMissing("Function is always passed MISSING values.")
rexOpCallDynamic(node.args, node.candidates),
ProblemGenerator.expressionAlwaysReturnsMissing("Function argument is always the missing value.")
)
}
return rex(type = unionOf(types).flatten(), op = node)
Expand Down
1 change: 0 additions & 1 deletion partiql-planner/src/main/resources/partiql_plan_internal.ion
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ rex::{
dynamic::{
args: list::[rex],
candidates: list::[candidate],
exhaustive: bool,
_: [
candidate::{
fn: '.ref.fn',
Expand Down
Loading

0 comments on commit cc3654a

Please sign in to comment.