Skip to content

Commit d767d08

Browse files
johnedquinnjohnedquinn
committed
Addresses PR feedback
1 parent 234518c commit d767d08

File tree

5 files changed

+44
-65
lines changed

5 files changed

+44
-65
lines changed

‎partiql-planner/src/main/kotlin/org/partiql/planner/Errors.kt

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package org.partiql.planner
33
import org.partiql.errors.ProblemDetails
44
import org.partiql.errors.ProblemSeverity
55
import org.partiql.plan.Identifier
6+
import org.partiql.planner.internal.utils.PlanUtils
67
import org.partiql.types.StaticType
78

89
/**
@@ -31,7 +32,8 @@ public sealed class PlanningProblemDetails(
3132
) : PlanningProblemDetails(
3233
ProblemSeverity.ERROR,
3334
{
34-
"Variable ${pretty(name)} does not exist in the database environment and is not an attribute of the following in-scope variables $inScopeVariables." +
35+
val humanReadableName = PlanUtils.identifierToString(name)
36+
"Variable $humanReadableName does not exist in the database environment and is not an attribute of the following in-scope variables $inScopeVariables." +
3537
quotationHint(isSymbolAndCaseSensitive(name))
3638
}
3739
) {
@@ -75,16 +77,6 @@ public sealed class PlanningProblemDetails(
7577
is Identifier.Symbol -> id.caseSensitivity == Identifier.CaseSensitivity.SENSITIVE
7678
is Identifier.Qualified -> false
7779
}
78-
79-
private fun pretty(id: Identifier): String = when (id) {
80-
is Identifier.Symbol -> pretty(id)
81-
is Identifier.Qualified -> (listOf(id.root) + id.steps).joinToString(".") { pretty(it) }
82-
}
83-
84-
private fun pretty(id: Identifier.Symbol): String = when (id.caseSensitivity) {
85-
Identifier.CaseSensitivity.INSENSITIVE -> id.symbol
86-
Identifier.CaseSensitivity.SENSITIVE -> "\"${id.symbol}\""
87-
}
8880
}
8981
}
9082

@@ -150,7 +142,7 @@ public sealed class PlanningProblemDetails(
150142
})
151143

152144
public data class UnknownAggregateFunction(
153-
val identifier: String,
145+
val identifier: Identifier,
154146
val args: List<StaticType>,
155147
) : PlanningProblemDetails(ProblemSeverity.ERROR, {
156148
val types = args.joinToString { "<${it.toString().lowercase()}>" }

‎partiql-planner/src/main/kotlin/org/partiql/planner/internal/transforms/PlanTransform.kt

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import org.partiql.planner.internal.ir.Rel
1515
import org.partiql.planner.internal.ir.Rex
1616
import org.partiql.planner.internal.ir.Statement
1717
import org.partiql.planner.internal.ir.visitor.PlanBaseVisitor
18+
import org.partiql.planner.internal.utils.PlanUtils
1819
import org.partiql.types.function.FunctionSignature
1920
import org.partiql.value.PartiQLValueExperimental
2021
import org.partiql.value.PartiQLValueType
@@ -351,12 +352,12 @@ internal object PlanTransform : PlanBaseVisitor<PlanNode, ProblemCallback>() {
351352
override fun visitRelOpAggregateCall(node: Rel.Op.Aggregate.Call, ctx: ProblemCallback): org.partiql.plan.Rel.Op.Aggregate.Call {
352353
val agg = when (val agg = node.agg) {
353354
is Agg.Unresolved -> {
354-
val name = agg.identifier.toNormalizedString()
355+
val name = PlanUtils.identifierToString(visitIdentifier(agg.identifier, ctx))
355356
ctx.invoke(
356357
Problem(
357358
UNKNOWN_PROBLEM_LOCATION,
358359
PlanningProblemDetails.UnknownAggregateFunction(
359-
agg.identifier.toString(),
360+
visitIdentifier(agg.identifier, ctx),
360361
node.args.map { it.type }
361362
)
362363
)
@@ -379,25 +380,6 @@ internal object PlanTransform : PlanBaseVisitor<PlanNode, ProblemCallback>() {
379380
)
380381
}
381382

382-
private fun Identifier.toNormalizedString(): String {
383-
return when (this) {
384-
is Identifier.Symbol -> this.toNormalizedString()
385-
is Identifier.Qualified -> {
386-
val toJoin = listOf(this.root) + this.steps
387-
toJoin.joinToString(separator = ".") { ident ->
388-
ident.toNormalizedString()
389-
}
390-
}
391-
}
392-
}
393-
394-
private fun Identifier.Symbol.toNormalizedString(): String {
395-
return when (this.caseSensitivity) {
396-
Identifier.CaseSensitivity.SENSITIVE -> "\"${this.symbol}\""
397-
Identifier.CaseSensitivity.INSENSITIVE -> this.symbol
398-
}
399-
}
400-
401383
override fun visitRelOpExclude(node: Rel.Op.Exclude, ctx: ProblemCallback) = org.partiql.plan.Rel.Op.Exclude(
402384
input = visitRel(node.input, ctx),
403385
items = node.items.map { visitRelOpExcludeItem(it, ctx) },

‎partiql-planner/src/main/kotlin/org/partiql/planner/internal/typer/PlanTyper.kt

Lines changed: 2 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ import org.partiql.planner.internal.ir.rexOpStructField
7070
import org.partiql.planner.internal.ir.rexOpTupleUnion
7171
import org.partiql.planner.internal.ir.statementQuery
7272
import org.partiql.planner.internal.ir.util.PlanRewriter
73+
import org.partiql.planner.internal.transforms.PlanTransform
7374
import org.partiql.spi.BindingCase
7475
import org.partiql.spi.BindingName
7576
import org.partiql.spi.BindingPath
@@ -1266,19 +1267,11 @@ internal class PlanTyper(
12661267
* to each row of T and eliminating null values <--- all NULL values are eliminated as inputs
12671268
*/
12681269
fun resolveAgg(agg: Agg.Unresolved, arguments: List<Rex>): Pair<Rel.Op.Aggregate.Call, StaticType> {
1269-
var missingArg = false
12701270
val args = arguments.map {
12711271
val arg = visitRex(it, it.type)
1272-
if (arg.type is MissingType) missingArg = true
12731272
arg
12741273
}
12751274

1276-
//
1277-
if (missingArg) {
1278-
handleAlwaysMissing()
1279-
return relOpAggregateCall(agg, listOf(rexErr("MISSING"))) to MissingType
1280-
}
1281-
12821275
// Try to match the arguments to functions defined in the catalog
12831276
return when (val match = env.resolveAgg(agg, args)) {
12841277
is FnMatch.Ok -> {
@@ -1404,7 +1397,7 @@ internal class PlanTyper(
14041397
* [PlanningProblemDetails.UndefinedVariable].
14051398
*/
14061399
private fun handleUndefinedVariable(name: Identifier, locals: Set<String>): PlanningProblemDetails.UndefinedVariable {
1407-
val planName = name.toPlan()
1400+
val planName = PlanTransform.visitIdentifier(name, onProblem)
14081401
val details = PlanningProblemDetails.UndefinedVariable(planName, locals)
14091402
onProblem(
14101403
Problem(
@@ -1415,26 +1408,6 @@ internal class PlanTyper(
14151408
return details
14161409
}
14171410

1418-
private fun Identifier.CaseSensitivity.toPlan(): org.partiql.plan.Identifier.CaseSensitivity = when (this) {
1419-
Identifier.CaseSensitivity.SENSITIVE -> org.partiql.plan.Identifier.CaseSensitivity.SENSITIVE
1420-
Identifier.CaseSensitivity.INSENSITIVE -> org.partiql.plan.Identifier.CaseSensitivity.INSENSITIVE
1421-
}
1422-
1423-
private fun Identifier.toPlan(): org.partiql.plan.Identifier = when (this) {
1424-
is Identifier.Symbol -> this.toPlan()
1425-
is Identifier.Qualified -> this.toPlan()
1426-
}
1427-
1428-
private fun Identifier.Symbol.toPlan(): org.partiql.plan.Identifier.Symbol = org.partiql.plan.Identifier.Symbol(
1429-
this.symbol,
1430-
this.caseSensitivity.toPlan()
1431-
)
1432-
1433-
private fun Identifier.Qualified.toPlan(): org.partiql.plan.Identifier.Qualified = org.partiql.plan.Identifier.Qualified(
1434-
this.root.toPlan(),
1435-
this.steps.map { it.toPlan() }
1436-
)
1437-
14381411
private fun handleUnexpectedType(actual: StaticType, expected: Set<StaticType>) {
14391412
onProblem(
14401413
Problem(

‎partiql-planner/src/main/kotlin/org/partiql/planner/internal/typer/TypeEnv.kt

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,15 @@ internal class TypeEnv(public val schema: List<Rel.Binding>) {
168168
return when (val type = this.flatten()) {
169169
is StructType -> type.containsKey(name)
170170
is AnyOfType -> {
171-
val anyKnownToContainKey = type.allTypes.any { it.containsKey(name) == true }
172-
val anyKnownToNotContainKey = type.allTypes.any { it.containsKey(name) == false }
173-
val anyNotKnownToContainKey = type.allTypes.any { it.containsKey(name) == null }
171+
var anyKnownToContainKey = false
172+
var anyKnownToNotContainKey = false
173+
var anyNotKnownToContainKey = false
174+
for (t in type.allTypes) {
175+
val containsKey = t.containsKey(name)
176+
anyKnownToContainKey = anyKnownToContainKey || (containsKey == true)
177+
anyKnownToNotContainKey = anyKnownToNotContainKey || (containsKey == false)
178+
anyNotKnownToContainKey = anyNotKnownToContainKey || (containsKey == null)
179+
}
174180
when {
175181
// There are:
176182
// - No subtypes that are known to not contain the key

‎partiql-planner/src/main/kotlin/org/partiql/planner/internal/utils/PlanUtils.kt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package org.partiql.planner.internal.utils
2+
3+
import org.partiql.plan.Identifier
4+
5+
internal object PlanUtils {
6+
7+
/**
8+
* Transforms an identifier to a human-readable string.
9+
*
10+
* Example output: aCaseInsensitiveCatalog."aCaseSensitiveSchema".aCaseInsensitiveTable
11+
*/
12+
fun identifierToString(node: Identifier): String = when (node) {
13+
is Identifier.Symbol -> identifierSymbolToString(node)
14+
is Identifier.Qualified -> {
15+
val toJoin = listOf(node.root) + node.steps
16+
toJoin.joinToString(separator = ".") { ident ->
17+
identifierSymbolToString(ident)
18+
}
19+
}
20+
}
21+
22+
private fun identifierSymbolToString(node: Identifier.Symbol) = when (node.caseSensitivity) {
23+
Identifier.CaseSensitivity.SENSITIVE -> "\"${node.symbol}\""
24+
Identifier.CaseSensitivity.INSENSITIVE -> node.symbol
25+
}
26+
}

0 commit comments

Comments
 (0)