Skip to content

Commit

Permalink
Addresses PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
johnedquinn committed May 17, 2024
1 parent f41e12d commit 4ae74d3
Show file tree
Hide file tree
Showing 11 changed files with 38 additions and 100 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ package org.partiql.planner.internal.typer

import org.partiql.planner.internal.ir.Rex
import org.partiql.types.StaticType
import org.partiql.value.MissingValue
import org.partiql.value.NullValue
import org.partiql.value.PartiQLValueExperimental
import org.partiql.value.PartiQLValueType
import org.partiql.value.PartiQLValueType.ANY
Expand Down Expand Up @@ -71,7 +73,7 @@ internal class DynamicTyper {
*/
private fun Rex.isLiteralAbsent(): Boolean {
val op = this.op
return op is Rex.Op.Lit && (op.value.type == PartiQLValueType.MISSING || op.value.type == PartiQLValueType.NULL)
return op is Rex.Op.Lit && (op.value is MissingValue || op.value is NullValue)
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -408,9 +408,9 @@ internal class FnResolver(private val header: Header) {
// This does not imply the ability to CAST; this defines function resolution behavior.
private val precedence: Map<PartiQLValueType, Int> = listOf(
@Suppress("DEPRECATION")
PartiQLValueType.NULL, // TODO: Remove
PartiQLValueType.NULL, // TODO: Remove once functions no longer specify parameter/return types with the NULL type.
@Suppress("DEPRECATION")
PartiQLValueType.MISSING, // TODO: Remove
PartiQLValueType.MISSING, // TODO: Remove once functions no longer specify parameter/return types with the MISSING type.
BOOL,
INT8,
INT16,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ import org.partiql.types.StructType
import org.partiql.types.TupleConstraint
import org.partiql.types.function.FunctionSignature
import org.partiql.value.BoolValue
import org.partiql.value.MissingValue
import org.partiql.value.PartiQLValueExperimental
import org.partiql.value.PartiQLValueType
import org.partiql.value.TextValue
import org.partiql.value.boolValue
import org.partiql.value.stringValue
Expand Down Expand Up @@ -475,8 +475,8 @@ internal class PlanTyper(
}
(type as CollectionType).elementType
}.toSet()
val finalType = unionOf(elementTypes).flatten()
return rex(finalType.swallowAny(), rexOpPathIndex(root, key))
val finalType = unionOf(elementTypes)
return rex(finalType, rexOpPathIndex(root, key))
}

override fun visitRexOpPathKey(node: Rex.Op.Path.Key, ctx: StaticType?): Rex {
Expand Down Expand Up @@ -516,8 +516,7 @@ internal class PlanTyper(
handleAlwaysMissing()
return rex(ANY, rexOpPathKey(root, key))
}
// TODO: SwallowAny should happen by default
return rex(unionOf(elementType).swallowAny(), rexOpPathKey(root, key))
return rex(unionOf(elementType), rexOpPathKey(root, key))
}

override fun visitRexOpPathSymbol(node: Rex.Op.Path.Symbol, ctx: StaticType?): Rex {
Expand Down Expand Up @@ -551,8 +550,7 @@ internal class PlanTyper(
handleAlwaysMissing()
return rex(ANY, Rex.Op.Path.Symbol(root, node.key))
}
// TODO: Flatten() should occur by default
else -> unionOf(paths.map { it.type }.toSet()).flatten()
else -> unionOf(paths.map { it.type }.toSet())
}

// replace step only if all are disambiguated
Expand All @@ -562,19 +560,7 @@ internal class PlanTyper(
true -> firstPathOp
false -> rexOpPathSymbol(root, node.key)
}
return rex(type.swallowAny(), replacementOp)
}

/**
* "Swallows" ANY. If ANY is one of the types in the UNION type, we return ANY. If not, we flatten and return
* the [type].
*/
private fun StaticType.swallowAny(): StaticType {
val flattened = this.flatten()
return when (flattened.allTypes.any { it is AnyType }) {
true -> ANY
false -> flattened
}
return rex(type, replacementOp)
}

private fun rexString(str: String) = rex(STRING, rexOpLit(stringValue(str)))
Expand Down Expand Up @@ -633,13 +619,13 @@ internal class PlanTyper(
// Check literal missing inputs
val argAlwaysMissing = args.any {
val op = it.op as? Rex.Op.Lit ?: return@any false
op.value.type == PartiQLValueType.MISSING
op.value is MissingValue
}
if (argAlwaysMissing) {
// TODO: The V1 branch has support for isMissable and isMissingCall. This codebase, however, does not
// have support for these concepts yet. This specific commit (see Git blame) does not seek to add this
// functionality. Below is a work-around for the lack of "isMissable" and "isMissingCall"
if (match.signature.name !in listOf("is_null", "is_missing", "eq")) {
if (match.signature.name !in listOf("is_null", "is_missing", "eq", "and", "or", "not")) {
handleAlwaysMissing()
}
}
Expand Down Expand Up @@ -1163,9 +1149,7 @@ internal class PlanTyper(
isClosed && isOrdered -> {
struct.fields.firstOrNull { entry -> binding.isEquivalentTo(entry.key) }?.let {
(sensitive(it.key) to it.value)
} ?: run {
return null
}
} ?: return null
}
// 2. Struct is closed
isClosed -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,8 @@ import org.partiql.types.TimestampType
import org.partiql.value.PartiQLValueExperimental
import org.partiql.value.PartiQLValueType

@Suppress("DEPRECATION")
@Deprecated(
message = "This will be removed in a future major-version bump.",
replaceWith = ReplaceWith("ANY")
)
internal fun StaticType.isNullOrMissing(): Boolean = (this is NullType || this is MissingType)

internal fun StaticType.isText(): Boolean = (this is SymbolType || this is StringType)

@Suppress("DEPRECATION")
@Deprecated(
message = "This will be removed in a future major-version bump.",
replaceWith = ReplaceWith("ANY")
)
internal fun StaticType.isUnknown(): Boolean = (this.isNullOrMissing() || this == StaticType.NULL_OR_MISSING)

/**
* Returns whether [this] *may* be of a specific type. AKA: is it the type? Is it a union that holds the type?
*/
Expand All @@ -52,16 +38,16 @@ internal inline fun <reified T> StaticType.mayBeType(): Boolean {
}

/**
* For each type in [this] [StaticType.allTypes], the [block] will be invoked. Non-null outputs to the [block] will be
* returned.
* For each type in [this] type's [StaticType.allTypes], the [block] will be invoked. Non-null outputs of the [block]'s
* invocation will be returned.
*/
internal fun StaticType.inferListNotNull(block: (StaticType) -> StaticType?): List<StaticType> {
return this.flatten().allTypes.mapNotNull { type -> block(type) }
}

/**
* For each type in [this] [StaticType.allTypes], the [block] will be invoked. Non-null outputs to the [block] will be
* returned.
* For each type in [this] type's [StaticType.allTypes], the [block] will be invoked. Non-null outputs of the [block]'s
* invocation will be returned.
*/
internal fun StaticType.inferRexListNotNull(block: (StaticType) -> Rex?): List<Rex> {
return this.flatten().allTypes.mapNotNull { type -> block(type) }
Expand Down Expand Up @@ -195,7 +181,7 @@ internal fun StructType.exclude(steps: List<Rel.Op.Exclude.Step>, lastStepOption
val output = fields.map { field ->
val newField = if (steps.size == 1) {
if (lastStepOptional) {
StructType.Field(field.key, field.value) // TODO: double check this
StructType.Field(field.key, field.value)
} else {
null
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ abstract class PartiQLTyperTestBase {
val actualType = root.type
assert(actualType == StaticType.ANY) {
buildString {
this.appendLine(" expected Type is : MISSING")
this.appendLine(" expected Type is : ANY")
this.appendLine("actual Type is : $actualType")
PlanPrinter.append(this, result.plan)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,7 @@ fun <T> cartesianProduct(a: List<T>, b: List<T>, vararg lists: List<T>): Set<Lis
}.toSet()

val allSupportedType = StaticType.ALL_TYPES.filterNot {
it == StaticType.GRAPH
}.filterNot {
it is NullType
}.filterNot {
it is MissingType
it == StaticType.GRAPH || it is NullType || it is MissingType
}

val allSupportedTypeNotUnknown = allSupportedType.filterNot { it == StaticType.MISSING || it == StaticType.NULL }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,19 +95,6 @@
name: "t_clob_null",
type: "clob",
},
// potentially absent
{
name: "t_null",
type: "any",
},
{
name: "t_missing",
type: "any",
},
{
name: "t_absent",
type: "any",
},
// collections
{
name: "t_bag",
Expand Down
15 changes: 0 additions & 15 deletions partiql-planner/src/testFixtures/resources/inputs/basics/case.sql
Original file line number Diff line number Diff line change
Expand Up @@ -182,21 +182,6 @@ CASE t_item.t_string
WHEN 'a' THEN 'ok!'
END;

--#[case-when-22]
-- type: (null|missing|int32)
CASE t_item.t_string
WHEN 'a' THEN t_item.t_absent
ELSE -1
END;

--#[case-when-23]
-- type: int32
-- false branch is pruned
CASE
WHEN false THEN t_item.t_absent
ELSE -1
END;

-- -----------------------------
-- Heterogeneous Branches
-- -----------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ NULLIF(t_item.t_int64, t_item.t_int32);

--#[nullif-05]
-- type: (int32 | null)
NULLIF(t_item.t_int32, t_item.t_null);
NULLIF(t_item.t_int32, NULL);

--#[nullif-06]
-- type: (null)
NULLIF(t_item.t_null, t_item.t_int32);
NULLIF(NULL, t_item.t_int32);

--#[nullif-07]
-- type: (int32 | null)
Expand Down
31 changes: 14 additions & 17 deletions partiql-types/src/main/kotlin/org/partiql/types/StaticType.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,16 @@ public sealed class StaticType {

/**
* varargs variant, folds [types] into a [Set]
* The usage of LinkedHashSet is to preserve the order of `types` to ensure behavior is consistent in our tests
* The usage of LinkedHashSet is to preserve the order of `types` to ensure behavior is consistent in our tests.
* The returned type is flattened.
*/
@JvmStatic
public fun unionOf(vararg types: StaticType, metas: Map<String, Any> = mapOf()): StaticType =
unionOf(types.toSet(), metas).flatten()
unionOf(types.toSet(), metas)

/**
* Creates a new [StaticType] as a union of the passed [types]. The values typed by the returned type
* are defined as the union of all values typed as [types]
* are defined as the union of all values typed as [types]. The returned type is flattened.
*
* @param types [StaticType] to be unioned.
* @return [StaticType] representing the union of [types]
Expand All @@ -33,12 +34,8 @@ public sealed class StaticType {
@Suppress("DEPRECATION")
public fun unionOf(
types: Set<StaticType>,
metas: Map<String, Any> = mapOf(),
errorOnEmptyTypes: Boolean = false
metas: Map<String, Any> = mapOf()
): StaticType {
if (errorOnEmptyTypes && types.isEmpty()) {
throw IllegalStateException("Cannot make a union of zero types.")
}
return when (types.isEmpty()) {
true -> ANY
false -> AnyOfType(types, metas).flatten()
Expand All @@ -47,7 +44,7 @@ public sealed class StaticType {

/**
* Creates a new [StaticType] as a union of the passed [types]. The values typed by the returned type
* are defined as the union of all values typed as [types]
* are defined as the union of all values typed as [types]. The returned type is flattened.
*
* @param types [StaticType] to be unioned.
* @return [StaticType] representing the union of [types]
Expand Down Expand Up @@ -151,7 +148,8 @@ public sealed class StaticType {
*/
@Suppress("DEPRECATION")
@Deprecated(
message = "This will be removed in a future major-version bump.",
message = "This will be removed in a future major-version bump. All types include the null value. Therefore," +
" this method is redundant.",
replaceWith = ReplaceWith("")
)
public fun asNullable(): StaticType =
Expand All @@ -168,7 +166,8 @@ public sealed class StaticType {
*/
@Suppress("DEPRECATION")
@Deprecated(
message = "This will be removed in a future major-version bump.",
message = "This will be removed in a future major-version bump. All types include the missing value." +
" Therefore, this method is redundant.",
replaceWith = ReplaceWith("")
)
public fun asOptional(): StaticType =
Expand Down Expand Up @@ -214,7 +213,8 @@ public sealed class StaticType {
*/
@Suppress("DEPRECATION")
@Deprecated(
message = "This will be removed in a future major-version bump.",
message = "This will be removed in a future major-version bump. All types are considered nullable. Therefore" +
" this method is redundant.",
replaceWith = ReplaceWith("true")
)
public fun isNullable(): Boolean =
Expand All @@ -231,7 +231,8 @@ public sealed class StaticType {
*/
@Suppress("DEPRECATION")
@Deprecated(
message = "This will be removed in a future major-version bump.",
message = "This will be removed in a future major-version bump. All types are considered missable. Therefore," +
" this method is redundant.",
replaceWith = ReplaceWith("true")
)
public fun isMissable(): Boolean =
Expand All @@ -245,10 +246,6 @@ public sealed class StaticType {
* Type is optional if it is Any, or Missing, or an AnyOfType that contains Any or Missing type
*/
@Suppress("DEPRECATION")
@Deprecated(
message = "This will be removed in a future major-version bump.",
replaceWith = ReplaceWith("true")
)
private fun isOptional(): Boolean =
when (this) {
is AnyType, MissingType -> true // Any includes Missing type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public fun StringElement.toStaticType(): StaticType = when (textValue) {
"list" -> error("`list` is not an atomic type")
"sexp" -> error("`sexp` is not an atomic type")
"struct" -> error("`struct` is not an atomic type")
"null", "missing" -> error("Absent values ($textValue) do not have a corresponding type.")
else -> error("Invalid type `$textValue`")
}

Expand Down

0 comments on commit 4ae74d3

Please sign in to comment.