Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Deprecates absent types and removes associated planner logic #1463

Merged
merged 5 commits into from
May 28, 2024
Merged

Deprecates absent types and removes associated planner logic #1463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f41e12d
Deprecates absent types
johnedquinn May 15, 2024
4ae74d3
Addresses PR comments
johnedquinn May 17, 2024
b82ac8c
Per PR feedback, removes NULL/MISSING from StaticType.ALL_TYPES
johnedquinn May 20, 2024
8d5f1f9
Per PR feedback, removes NULL/MISSING from Ion-encoded typing tests
johnedquinn May 20, 2024
994635a
Addresses PR Feedback
johnedquinn May 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,25 @@ Thank you to all who have contributed!
### Added

### Changed
- **Behavioral change**: The planner now does NOT support the NullType and MissingType variants of StaticType. The logic
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something for us to consider when releasing this change is that it's a pretty big behavioral change w/ the typing of plans. We should consider cutting a new major version release rather than a minor version release.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm concerned about this as well. We have many customers who assert on current typing judgements, we can't make any typing changes without breaking them.

is that the null and missing values are part of *all* data types. Therefore, one must assume that the types returned by
the planner allow for NULL and MISSING values. Similarly, the testFixtures Ion-encoded test resources
representing the catalog do not use "null" or "missing".

### Deprecated
- We have deprecated `org.partiql.type.NullType` and `org.partiql.type.MissingType`. Please see the corresponding
information in the "Changed" section. In relation to the deprecation of the above, the following APIs have also
been deprecated:
- `org.partiql.type.StaticType.MISSING`
- `org.partiql.type.StaticType.NULL`
- `org.partiql.type.StaticType.NULL_OR_MISSING`
- `org.partiql.type.StaticType.asNullable()`
- `org.partiql.type.StaticType.isNullable()`
- `org.partiql.type.StaticType.isMissable()`
- `org.partiql.type.StaticType.asOptional()`
- `org.partiql.type.AnyOfType()`
- `org.partiql.value.PartiQLValueType.NULL`
- `org.partiql.value.PartiQLValueType.MISSING`

### Fixed

Expand Down
2 changes: 2 additions & 0 deletions partiql-lang/src/test/kotlin/org/partiql/lang/ast/passes/inference/StaticTypeCastTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package org.partiql.lang.ast.passes.inference
import junitparams.JUnitParamsRunner
import junitparams.Parameters
import org.junit.Assert.assertEquals
import org.junit.Ignore
import org.junit.Test
import org.junit.runner.RunWith
import org.partiql.types.DecimalType
Expand Down Expand Up @@ -85,6 +86,7 @@ class StaticTypeCastTests {

@Test
@Parameters
@Ignore("StaticType.ALL_TYPES no longer supports NULL/MISSING") // @Test comes from JUnit4, and therefore we must use @Ignore.
fun unionTypeCastTests(tc: TestCase) = runTest(tc)

@Test
Expand Down
2 changes: 2 additions & 0 deletions .../test/kotlin/org/partiql/lang/eval/visitors/inferencer/InferencerMultipleProblemsTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.partiql.lang.eval.visitors.inferencer

import org.junit.jupiter.api.Disabled
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.MethodSource
import org.partiql.errors.Problem
Expand All @@ -20,6 +21,7 @@ import org.partiql.types.StructType

class InferencerMultipleProblemsTests {
@ParameterizedTest
@Disabled
@MethodSource("parametersForMultipleInferenceProblemsTests")
fun multipleInferenceProblemsTests(tc: TestCase) = runTest(tc)

Expand Down
2 changes: 2 additions & 0 deletions ...rc/test/kotlin/org/partiql/lang/eval/visitors/inferencer/InferencerNaryArithmeticTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.partiql.lang.eval.visitors.inferencer

import org.junit.jupiter.api.Disabled
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.MethodSource
import org.partiql.errors.Problem
Expand Down Expand Up @@ -29,6 +30,7 @@ class InferencerNaryArithmeticTests {

@ParameterizedTest
@MethodSource("parametersForNAryArithmeticTests")
@Disabled
fun naryArithmeticInferenceTests(tc: InferencerTestUtil.TestCase) = runTest(tc)

companion object {
Expand Down
2 changes: 2 additions & 0 deletions ...g/src/test/kotlin/org/partiql/lang/eval/visitors/inferencer/InferencerNaryBetweenTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.partiql.lang.eval.visitors.inferencer

import org.junit.jupiter.api.Disabled
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.MethodSource
import org.partiql.errors.Problem
Expand All @@ -26,6 +27,7 @@ import org.partiql.types.StaticType
class InferencerNaryBetweenTests {
@ParameterizedTest
@MethodSource("parametersForNAryBetweenTests")
@Disabled("StaticType.ALL_TYPES no longer supports NULL/MISSING")
fun naryBetweenInferenceTests(tc: TestCase) = runTest(tc)

companion object {
Expand Down
2 changes: 2 additions & 0 deletions ...lin/org/partiql/lang/eval/visitors/inferencer/InferencerNaryComparisonAndEqualityTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.partiql.lang.eval.visitors.inferencer

import org.junit.jupiter.api.Disabled
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.MethodSource
import org.partiql.errors.Problem
Expand Down Expand Up @@ -33,6 +34,7 @@ import org.partiql.types.StructType
class InferencerNaryComparisonAndEqualityTests {
@ParameterizedTest
@MethodSource("parametersForNAryComparisonAndEqualityTests")
@Disabled("StaticType.ALL_TYPES no longer supports NULL/MISSING")
fun naryComparisonAndEqualityInferenceTests(tc: TestCase) = runTest(tc)

companion object {
Expand Down
2 changes: 2 additions & 0 deletions ...ng/src/test/kotlin/org/partiql/lang/eval/visitors/inferencer/InferencerNaryConcatTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.partiql.lang.eval.visitors.inferencer

import org.junit.jupiter.api.Disabled
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.MethodSource
import org.partiql.errors.Problem
Expand All @@ -25,6 +26,7 @@ import org.partiql.types.StringType
class InferencerNaryConcatTests {
@ParameterizedTest
@MethodSource("parametersForNAryConcatTests")
@Disabled("StaticType.ALL_TYPES no longer supports NULL/MISSING")
fun naryConcatInferenceTests(tc: TestCase) = runTest(tc)

companion object {
Expand Down
2 changes: 2 additions & 0 deletions ...lang/src/test/kotlin/org/partiql/lang/eval/visitors/inferencer/InferencerNaryLikeTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.partiql.lang.eval.visitors.inferencer

import org.junit.jupiter.api.Disabled
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.MethodSource
import org.partiql.errors.Problem
Expand All @@ -22,6 +23,7 @@ class InferencerNaryLikeTests {

@ParameterizedTest
@MethodSource("parametersForNAryLikeTests")
@Disabled("StaticType.ALL_TYPES no longer supports NULL/MISSING")
fun naryLikeInferenceTests(tc: TestCase) = runTest(tc)

companion object {
Expand Down
2 changes: 2 additions & 0 deletions ...g/src/test/kotlin/org/partiql/lang/eval/visitors/inferencer/InferencerNaryLogicalTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.partiql.lang.eval.visitors.inferencer

import org.junit.jupiter.api.Disabled
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.MethodSource
import org.partiql.errors.Problem
Expand All @@ -25,6 +26,7 @@ import org.partiql.types.StaticType
class InferencerNaryLogicalTests {
@ParameterizedTest
@MethodSource("parametersForNAryLogicalTests")
@Disabled("StaticType.ALL_TYPES no longer supports NULL/MISSING")
fun naryLogicalInferenceTests(tc: TestCase) = runTest(tc)

companion object {
Expand Down
2 changes: 2 additions & 0 deletions ...lang/src/test/kotlin/org/partiql/lang/eval/visitors/inferencer/InferencerNaryOpInTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.partiql.lang.eval.visitors.inferencer

import org.junit.jupiter.api.Disabled
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.MethodSource
import org.partiql.errors.Problem
Expand Down Expand Up @@ -31,6 +32,7 @@ import org.partiql.types.StaticType
class InferencerNaryOpInTests {
@ParameterizedTest
@MethodSource("parametersForNAryOpInTests")
@Disabled("StaticType.ALL_TYPES no longer supports NULL/MISSING")
fun nAryOpInTests(tc: TestCase) = runTest(tc)

companion object {
Expand Down
2 changes: 2 additions & 0 deletions .../src/test/kotlin/org/partiql/lang/eval/visitors/inferencer/InferencerTrimFunctionTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.partiql.lang.eval.visitors.inferencer

import org.junit.jupiter.api.Disabled
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.MethodSource
import org.partiql.lang.eval.visitors.inferencer.InferencerTestUtil.TestCase
Expand All @@ -10,6 +11,7 @@ import org.partiql.types.StaticType
class InferencerTrimFunctionTests {
@ParameterizedTest
@MethodSource("parametersForTrimFunctionTests")
@Disabled("StaticType.ALL_TYPES no longer supports NULL/MISSING")
fun trimFunctionTests(tc: TestCase) = runTest(tc)

companion object {
Expand Down
2 changes: 2 additions & 0 deletions ...test/kotlin/org/partiql/lang/eval/visitors/inferencer/InferencerUnaryArithmeticOpTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.partiql.lang.eval.visitors.inferencer

import org.junit.jupiter.api.Disabled
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.MethodSource
import org.partiql.errors.Problem
Expand All @@ -15,6 +16,7 @@ import org.partiql.types.StaticType
class InferencerUnaryArithmeticOpTests {
@ParameterizedTest
@MethodSource("parametersForUnaryArithmeticOpTests")
@Disabled("StaticType.ALL_TYPES no longer supports NULL/MISSING")
fun unaryArithmeticInferenceTests(tc: InferencerTestUtil.TestCase) = InferencerTestUtil.runTest(tc)

companion object {
Expand Down
2 changes: 1 addition & 1 deletion partiql-planner/src/main/kotlin/org/partiql/planner/internal/transforms/PlanTransform.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ internal object PlanTransform : PlanBaseVisitor<PlanNode, ProblemCallback>() {
org.partiql.plan.Agg(
FunctionSignature.Aggregation(
"UNKNOWN_AGG::$name",
returns = PartiQLValueType.MISSING,
returns = PartiQLValueType.ANY, // TODO: Make unknown or something similar
parameters = emptyList()
)
)
Expand Down
17 changes: 5 additions & 12 deletions partiql-planner/src/main/kotlin/org/partiql/planner/internal/transforms/RexConverter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import org.partiql.planner.internal.ir.rexOpStructField
import org.partiql.planner.internal.ir.rexOpSubquery
import org.partiql.planner.internal.ir.rexOpTupleUnion
import org.partiql.planner.internal.ir.rexOpVarUnresolved
import org.partiql.planner.internal.typer.toNonNullStaticType
import org.partiql.planner.internal.typer.toStaticType
import org.partiql.types.StaticType
import org.partiql.types.TimeType
Expand Down Expand Up @@ -70,10 +69,7 @@ internal object RexConverter {
throw IllegalArgumentException("unsupported rex $node")

override fun visitExprLit(node: Expr.Lit, context: Env): Rex {
val type = when (node.value.isNull) {
true -> node.value.type.toStaticType()
else -> node.value.type.toNonNullStaticType()
}
val type = node.value.type.toStaticType()
val op = rexOpLit(node.value)
return rex(type, op)
}
Expand All @@ -82,10 +78,7 @@ internal object RexConverter {
val value =
PartiQLValueIonReaderBuilder
.standard().build(node.value).read()
val type = when (value.isNull) {
true -> value.type.toStaticType()
else -> value.type.toNonNullStaticType()
}
val type = value.type.toStaticType()
return rex(type, rexOpLit(value))
}

Expand Down Expand Up @@ -287,7 +280,7 @@ internal object RexConverter {
}.toMutableList()

val defaultRex = when (val default = node.default) {
null -> rex(type = StaticType.NULL, op = rexOpLit(value = nullValue()))
null -> rex(type = StaticType.ANY, op = rexOpLit(value = nullValue()))
else -> visitExprCoerce(default, context)
}
val op = rexOpCase(branches = branches, default = defaultRex)
Expand Down Expand Up @@ -528,8 +521,8 @@ internal object RexConverter {
val type = node.asType
val arg0 = visitExprCoerce(node.value, ctx)
return when (type) {
is Type.NullType -> rex(StaticType.NULL, call("cast_null", arg0))
is Type.Missing -> rex(StaticType.MISSING, call("cast_missing", arg0))
is Type.NullType -> error("Cannot cast any value to NULL")
is Type.Missing -> error("Cannot cast any value to MISSING")
is Type.Bool -> rex(StaticType.BOOL, call("cast_bool", arg0))
is Type.Tinyint -> TODO("Static Type does not have TINYINT type")
is Type.Smallint, is Type.Int2 -> rex(StaticType.INT2, call("cast_int16", arg0))
Expand Down
Loading
Loading