diff --git a/CHANGELOG.md b/CHANGELOG.md index a56635cce4..28803d07d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,8 +28,10 @@ Thank you to all who have contributed! ## [Unreleased] ### Added +- Added constrained decimal as valid parameter type to functions that take in numeric parameters. ### Changed +- Function resolution logic: Now the function resolver would match all possible candidate(based on if the argument can be coerced to the Signature parameter type). If there are multiple match it will first attempt to pick the one requires the least cast, then pick the function with the highest precedence. ### Deprecated diff --git a/partiql-planner/src/main/kotlin/org/partiql/planner/internal/typer/FnResolver.kt b/partiql-planner/src/main/kotlin/org/partiql/planner/internal/typer/FnResolver.kt index dbfa6084f9..0cf04f2c59 100644 --- a/partiql-planner/src/main/kotlin/org/partiql/planner/internal/typer/FnResolver.kt +++ b/partiql-planner/src/main/kotlin/org/partiql/planner/internal/typer/FnResolver.kt @@ -5,6 +5,7 @@ import org.partiql.planner.internal.ir.Agg import org.partiql.planner.internal.ir.Fn import org.partiql.planner.internal.ir.Identifier import org.partiql.planner.internal.ir.Rex +import org.partiql.planner.internal.typer.FnResolver.Companion.compareTo import org.partiql.types.AnyOfType import org.partiql.types.NullType import org.partiql.types.StaticType @@ -247,15 +248,28 @@ internal class FnResolver(private val header: Header) { /** * Functions are sorted by precedence (which is not rigorously defined/specified at the moment). + * + * This function first attempts to find all possible match for given args + * If there are multiple matches, then + * - return the matches that requires the lowest number of coercion + * - This is to match edges like concat(symbol, symbol) which should return symbol + * but because string has higher precedence, + * we also would have concat(cast(symbol as string), cast(symbol as string)) + * added to the map first. + * - return the matches which has the highest argument precedence. */ private fun match(signatures: List, args: Args): Match? { + val candidates = mutableListOf>() for (signature in signatures) { val mapping = match(signature, args) if (mapping != null) { - return Match(signature, mapping) + candidates.add(Match(signature, mapping)) } } - return null + + // Sorted By is stable, we don't have to resort based on parameter type precedence + candidates.sortBy { it.mapping.filterNotNull().size } + return candidates.firstOrNull() } /** @@ -288,14 +302,7 @@ internal class FnResolver(private val header: Header) { } } } - // if all elements requires casting, then no match - // because there must be another function definition that requires no casting - return if (mapping.isEmpty() || mapping.contains(null)) { - // we made a match - mapping - } else { - null - } + return mapping } /** diff --git a/partiql-planner/src/main/kotlin/org/partiql/planner/internal/typer/TypeLattice.kt b/partiql-planner/src/main/kotlin/org/partiql/planner/internal/typer/TypeLattice.kt index 767dd91922..a6ade5f7da 100644 --- a/partiql-planner/src/main/kotlin/org/partiql/planner/internal/typer/TypeLattice.kt +++ b/partiql-planner/src/main/kotlin/org/partiql/planner/internal/typer/TypeLattice.kt @@ -86,6 +86,7 @@ internal class TypeLattice private constructor( INT32, INT64, INT, + DECIMAL, DECIMAL_ARBITRARY, FLOAT32, FLOAT64, diff --git a/partiql-planner/src/main/kotlin/org/partiql/planner/internal/typer/TypeUtils.kt b/partiql-planner/src/main/kotlin/org/partiql/planner/internal/typer/TypeUtils.kt index 24ecb54c56..bccd22c451 100644 --- a/partiql-planner/src/main/kotlin/org/partiql/planner/internal/typer/TypeUtils.kt +++ b/partiql-planner/src/main/kotlin/org/partiql/planner/internal/typer/TypeUtils.kt @@ -121,9 +121,15 @@ private fun StaticType.asRuntimeType(): PartiQLValueType = when (this) { is ListType -> PartiQLValueType.LIST is SexpType -> PartiQLValueType.SEXP is DateType -> PartiQLValueType.DATE - is DecimalType -> when (this.precisionScaleConstraint) { - is DecimalType.PrecisionScaleConstraint.Constrained -> PartiQLValueType.DECIMAL - DecimalType.PrecisionScaleConstraint.Unconstrained -> PartiQLValueType.DECIMAL_ARBITRARY + // TODO: Run time decimal type does not model precision scale constraint yet + // despite that we match to Decimal vs Decimal_ARBITRARY (PVT) here + // but when mapping it back to Static Type, (i.e, mapping function return type to Value Type) + // we can only map to Unconstrained decimal (Static Type) + is DecimalType -> { + when (this.precisionScaleConstraint) { + is DecimalType.PrecisionScaleConstraint.Constrained -> PartiQLValueType.DECIMAL + DecimalType.PrecisionScaleConstraint.Unconstrained -> PartiQLValueType.DECIMAL_ARBITRARY + } } is FloatType -> PartiQLValueType.FLOAT64 is GraphType -> error("Graph type missing from runtime types")