Skip to content

Commit

Permalink
Addresses PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
johnedquinn committed Aug 2, 2024
1 parent 91ac374 commit d1a0506
Show file tree
Hide file tree
Showing 12 changed files with 309 additions and 262 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ internal class Compiler(
error("Dynamic call candidate had different name than others; found $fnName but expected $name")
}
}
ExprCallDynamic.Candidate(fn)
fn
}
return ExprCallDynamic(name, candidates, args)
}
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,11 @@ import org.partiql.value.PartiQLValueExperimental
@OptIn(PartiQLValueExperimental::class)
internal class ExprCallDynamic(
private val name: String,
private val candidates: Array<Candidate>,
candidateFns: Array<Fn>,
private val args: Array<Operator.Expr>
) : Operator.Expr {

private val candidates = Array(candidateFns.size) { Candidate(candidateFns[it]) }
private val paramIndices: IntRange = args.indices
private val paramTypes: List<List<PType>> = this.candidates.map { candidate -> candidate.fn.signature.parameters.map { it.type } }
private val paramFamilies: List<List<CoercionFamily>> = this.candidates.map { candidate -> candidate.fn.signature.parameters.map { family(it.type.kind) } }
Expand All @@ -46,11 +47,11 @@ internal class ExprCallDynamic(
val actualTypes = actualArgs.map { it.type }
val cached = cachedMatches[actualTypes]
if (cached != null) {
return candidates[cached].eval(actualArgs, env)
return candidates[cached].eval(actualArgs)
}
val candidateIndex = match(actualTypes) ?: throw TypeCheckException("Could not find function $name with types: $actualTypes.")
cachedMatches[actualTypes] = candidateIndex
return candidates[candidateIndex].eval(actualArgs, env)
return candidates[candidateIndex].eval(actualArgs)
}

/**
Expand Down Expand Up @@ -89,7 +90,7 @@ internal class ExprCallDynamic(
* TODO: [UNKNOWN] should likely be removed in the future. However, it is needed due to literal nulls and missings.
* TODO: [DYNAMIC] should likely be removed in the future. This is currently only kept to map function signatures.
*/
enum class CoercionFamily {
private enum class CoercionFamily {
NUMBER,
STRING,
BINARY,
Expand All @@ -103,7 +104,7 @@ internal class ExprCallDynamic(
DYNAMIC
}

companion object {
private companion object {
/**
* Gets the coercion family for the given [PType.Kind].
*
Expand Down Expand Up @@ -154,7 +155,7 @@ internal class ExprCallDynamic(
*
* @see ExprCallDynamic
*/
data class Candidate(
private class Candidate(
val fn: Fn,
) {

Expand All @@ -163,7 +164,7 @@ internal class ExprCallDynamic(
*/
private val nil = { Datum.nullValue(fn.signature.returns) }

fun eval(originalArgs: Array<Datum>, env: Environment): Datum {
fun eval(originalArgs: Array<Datum>): Datum {
val args = originalArgs.mapIndexed { i, arg ->
if (arg.isNull && fn.signature.isNullCall) {
return nil.invoke()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package org.partiql.eval.internal.operator.rex

import org.junit.jupiter.api.Test

class CastTableTest {
@Test
fun printCastTable() {
println(CastTable)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class ExprCallDynamicTest {
fun assert() {
val expr = ExprCallDynamic(
name = "example_function",
candidates = candidates,
candidateFns = candidates,
args = arrayOf(ExprLiteral(lhs), ExprLiteral(rhs)),
)
val result = expr.eval(Environment.empty).check(PartiQLValueType.INT32)
Expand All @@ -64,21 +64,19 @@ class ExprCallDynamicTest {
)

@OptIn(PartiQLValueExperimental::class)
internal val candidates = params.mapIndexed { index, it ->
ExprCallDynamic.Candidate(
fn = object : Fn {
override val signature: FnSignature = FnSignature(
name = "example_function",
returns = PartiQLValueType.INT32,
parameters = listOf(
FnParameter("first", type = it.first),
FnParameter("second", type = it.second),
)
internal val candidates: Array<Fn> = params.mapIndexed { index, it ->
object : Fn {
override val signature: FnSignature = FnSignature(
name = "example_function",
returns = PartiQLValueType.INT32,
parameters = listOf(
FnParameter("first", type = it.first),
FnParameter("second", type = it.second),
)
)

override fun invoke(args: Array<PartiQLValue>): PartiQLValue = int32Value(index).toPartiQLValue()
}
)
override fun invoke(args: Array<PartiQLValue>): PartiQLValue = int32Value(index).toPartiQLValue()
}
}.toTypedArray()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,15 @@ internal class PlanTransform(
override fun visitRexOpPath(node: Rex.Op.Path, ctx: Unit) =
super.visitRexOpPath(node, ctx) as org.partiql.plan.Rex.Op.Path

/**
* See PartiQL Specification [Section 4.1.1](https://partiql.org/partiql-lang/#sec:schema-in-tuple-path).
* While it talks about pathing into a tuple, it provides guidance on expressions that always return missing:
*
* > In a more important and common case, an PartiQL implementation can utilize the input data schema to prove
* > that a path expression always returns MISSING and thus throw a compile-time error.
*
* This is accomplished via the signaling mode below.
*/
override fun visitRexOpCastUnresolved(node: Rex.Op.Cast.Unresolved, ctx: Unit): PlanNode {
val problem = ProblemGenerator.undefinedCast(node.arg.type, node.target)
return when (signalMode) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ internal object RexConverter {
is Type.Bag -> call("is_bag", arg0)
is Type.List -> call("is_list", arg0)
is Type.Sexp -> call("is_sexp", arg0)
is Type.Tuple -> call("is_tuple", arg0)
is Type.Tuple -> call("is_struct", arg0)
is Type.Struct -> call("is_struct", arg0)
is Type.Any -> call("is_any", arg0)
is Type.Custom -> call("is_custom", arg0)
Expand Down
7 changes: 7 additions & 0 deletions partiql-spi/api/partiql-spi.api
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,9 @@ public final class org/partiql/errors/TypeCheckException : java/lang/RuntimeExce
public abstract interface class org/partiql/eval/value/Datum : java/lang/Iterable {
public static fun bagValue (Ljava/lang/Iterable;)Lorg/partiql/eval/value/Datum;
public static fun boolValue (Z)Lorg/partiql/eval/value/Datum;
public static fun decimal (Ljava/math/BigDecimal;II)Lorg/partiql/eval/value/Datum;
public static fun decimalArbitrary (Ljava/math/BigDecimal;)Lorg/partiql/eval/value/Datum;
public static fun doublePrecision (D)Lorg/partiql/eval/value/Datum;
public fun get (Ljava/lang/String;)Lorg/partiql/eval/value/Datum;
public fun getBigDecimal ()Ljava/math/BigDecimal;
public fun getBigInteger ()Ljava/math/BigInteger;
Expand All @@ -295,6 +298,7 @@ public abstract interface class org/partiql/eval/value/Datum : java/lang/Iterabl
public abstract fun getType ()Lorg/partiql/types/PType;
public static fun int32Value (I)Lorg/partiql/eval/value/Datum;
public static fun int64Value (J)Lorg/partiql/eval/value/Datum;
public static fun intArbitrary (Ljava/math/BigInteger;)Lorg/partiql/eval/value/Datum;
public fun isMissing ()Z
public fun isNull ()Z
public fun iterator ()Ljava/util/Iterator;
Expand All @@ -304,9 +308,12 @@ public abstract interface class org/partiql/eval/value/Datum : java/lang/Iterabl
public static fun nullValue ()Lorg/partiql/eval/value/Datum;
public static fun nullValue (Lorg/partiql/types/PType;)Lorg/partiql/eval/value/Datum;
public static fun of (Lorg/partiql/value/PartiQLValue;)Lorg/partiql/eval/value/Datum;
public static fun real (F)Lorg/partiql/eval/value/Datum;
public static fun sexpValue (Ljava/lang/Iterable;)Lorg/partiql/eval/value/Datum;
public static fun smallInt (S)Lorg/partiql/eval/value/Datum;
public static fun stringValue (Ljava/lang/String;)Lorg/partiql/eval/value/Datum;
public static fun structValue (Ljava/lang/Iterable;)Lorg/partiql/eval/value/Datum;
public static fun tinyInt (B)Lorg/partiql/eval/value/Datum;
public fun toPartiQLValue ()Lorg/partiql/value/PartiQLValue;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,34 +36,4 @@ public Iterator<Datum> iterator() {
public PType getType() {
return _type;
}

@Override
public String toString() {
StringBuilder sb = new StringBuilder();
sb.append(_type);
sb.append("::");
if (_type.getKind() == PType.Kind.LIST) {
sb.append("[");
} else if (_type.getKind() == PType.Kind.BAG) {
sb.append("<<");
} else if (_type.getKind() == PType.Kind.SEXP) {
sb.append("(");
}
Iterator<Datum> iter = _value.iterator();
while (iter.hasNext()) {
Datum child = iter.next();
sb.append(child);
if (iter.hasNext()) {
sb.append(", ");
}
}
if (_type.getKind() == PType.Kind.LIST) {
sb.append("]");
} else if (_type.getKind() == PType.Kind.BAG) {
sb.append(">>");
} else if (_type.getKind() == PType.Kind.SEXP) {
sb.append(")");
}
return sb.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,4 @@ public BigDecimal getBigDecimal() {
public PType getType() {
return _type;
}

@Override
public String toString() {
return _type + "::" + _value;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,6 @@ internal object SqlBuiltins {
Fn_IS_TIME__ANY__BOOL,
Fn_IS_TIMESTAMP__BOOL_INT32_ANY__BOOL,
Fn_IS_TIMESTAMP__ANY__BOOL,
Fn_IS_TUPLE__ANY__BOOL,
Fn_LIKE__STRING_STRING__BOOL,
Fn_LIKE__CLOB_CLOB__BOOL,
Fn_LIKE__SYMBOL_SYMBOL__BOOL,
Expand Down

This file was deleted.

0 comments on commit d1a0506

Please sign in to comment.