Skip to content

Commit 890d2b7

Browse files
committed
Improve decimal arithmetic operation type inference
Update the type inference rules to provide more reasonable results when performing arithmetic operations between decimal values. The logic is modeled after MSSQL's behavior described in: https://learn.microsoft.com/en-us/sql/t-sql/data-types/precision-scale-and-length-transact-sql?view=sql-server-ver16 The legacy behavior can be restored via the deprecated.legacy-arithmetic-decimal-operators configuration option.
1 parent e16a172 commit 890d2b7

File tree

24 files changed

+3384
-3269
lines changed

24 files changed

+3384
-3269
lines changed

core/trino-main/src/main/java/io/trino/FeaturesConfig.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ public class FeaturesConfig
123123

124124
private boolean faultTolerantExecutionExchangeEncryptionEnabled = true;
125125

126+
private boolean legacyArithmeticDecimalOperators;
127+
126128
public enum DataIntegrityVerification
127129
{
128130
NONE,
@@ -523,4 +525,16 @@ public void applyFaultTolerantExecutionDefaults()
523525
{
524526
exchangeCompressionCodec = LZ4;
525527
}
528+
529+
public boolean isLegacyArithmeticDecimalOperators()
530+
{
531+
return legacyArithmeticDecimalOperators;
532+
}
533+
534+
@Config("deprecated.legacy-arithmetic-decimal-operators")
535+
public FeaturesConfig setLegacyArithmeticDecimalOperators(boolean value)
536+
{
537+
this.legacyArithmeticDecimalOperators = value;
538+
return this;
539+
}
526540
}

core/trino-main/src/main/java/io/trino/metadata/SystemFunctionBundle.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,10 @@
339339
import static io.trino.type.DecimalOperators.DECIMAL_MODULUS_OPERATOR;
340340
import static io.trino.type.DecimalOperators.DECIMAL_MULTIPLY_OPERATOR;
341341
import static io.trino.type.DecimalOperators.DECIMAL_SUBTRACT_OPERATOR;
342+
import static io.trino.type.DecimalOperators.LEGACY_DECIMAL_ADD_OPERATOR;
343+
import static io.trino.type.DecimalOperators.LEGACY_DECIMAL_DIVIDE_OPERATOR;
344+
import static io.trino.type.DecimalOperators.LEGACY_DECIMAL_MULTIPLY_OPERATOR;
345+
import static io.trino.type.DecimalOperators.LEGACY_DECIMAL_SUBTRACT_OPERATOR;
342346
import static io.trino.type.DecimalSaturatedFloorCasts.BIGINT_TO_DECIMAL_SATURATED_FLOOR_CAST;
343347
import static io.trino.type.DecimalSaturatedFloorCasts.DECIMAL_TO_BIGINT_SATURATED_FLOOR_CAST;
344348
import static io.trino.type.DecimalSaturatedFloorCasts.DECIMAL_TO_DECIMAL_SATURATED_FLOOR_CAST;
@@ -540,7 +544,11 @@ public static FunctionBundle create(FeaturesConfig featuresConfig, TypeOperators
540544
.functions(DECIMAL_TO_VARCHAR_CAST, DECIMAL_TO_INTEGER_CAST, DECIMAL_TO_BIGINT_CAST, DECIMAL_TO_DOUBLE_CAST, DECIMAL_TO_REAL_CAST, DECIMAL_TO_BOOLEAN_CAST, DECIMAL_TO_TINYINT_CAST, DECIMAL_TO_SMALLINT_CAST)
541545
.functions(VARCHAR_TO_DECIMAL_CAST, INTEGER_TO_DECIMAL_CAST, BIGINT_TO_DECIMAL_CAST, DOUBLE_TO_DECIMAL_CAST, REAL_TO_DECIMAL_CAST, BOOLEAN_TO_DECIMAL_CAST, TINYINT_TO_DECIMAL_CAST, SMALLINT_TO_DECIMAL_CAST)
542546
.functions(JSON_TO_DECIMAL_CAST, DECIMAL_TO_JSON_CAST)
543-
.functions(DECIMAL_ADD_OPERATOR, DECIMAL_SUBTRACT_OPERATOR, DECIMAL_MULTIPLY_OPERATOR, DECIMAL_DIVIDE_OPERATOR, DECIMAL_MODULUS_OPERATOR)
547+
.functions(featuresConfig.isLegacyArithmeticDecimalOperators() ? LEGACY_DECIMAL_ADD_OPERATOR : DECIMAL_ADD_OPERATOR)
548+
.functions(featuresConfig.isLegacyArithmeticDecimalOperators() ? LEGACY_DECIMAL_SUBTRACT_OPERATOR : DECIMAL_SUBTRACT_OPERATOR)
549+
.functions(featuresConfig.isLegacyArithmeticDecimalOperators() ? LEGACY_DECIMAL_MULTIPLY_OPERATOR : DECIMAL_MULTIPLY_OPERATOR)
550+
.functions(featuresConfig.isLegacyArithmeticDecimalOperators() ? LEGACY_DECIMAL_DIVIDE_OPERATOR : DECIMAL_DIVIDE_OPERATOR)
551+
.functions(DECIMAL_MODULUS_OPERATOR)
544552
.function(DECIMAL_TO_DECIMAL_SATURATED_FLOOR_CAST)
545553
.functions(DECIMAL_TO_BIGINT_SATURATED_FLOOR_CAST, BIGINT_TO_DECIMAL_SATURATED_FLOOR_CAST)
546554
.functions(DECIMAL_TO_INTEGER_SATURATED_FLOOR_CAST, INTEGER_TO_DECIMAL_SATURATED_FLOOR_CAST)

core/trino-main/src/main/java/io/trino/type/DecimalOperators.java

Lines changed: 157 additions & 74 deletions
Large diffs are not rendered by default.

core/trino-main/src/test/java/io/trino/sql/analyzer/TestAnalyzer.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2954,7 +2954,7 @@ public void testWithRecursiveUncoercibleTypes()
29542954
" )" +
29552955
" SELECT * from t")
29562956
.hasErrorCode(TYPE_MISMATCH)
2957-
.hasMessage("line 1:82: recursion step relation output type (decimal(2,1)) is not coercible to recursion base relation output type (decimal(1,0)) at column 1");
2957+
.hasMessage("line 1:82: recursion step relation output type (decimal(3,1)) is not coercible to recursion base relation output type (decimal(1,0)) at column 1");
29582958

29592959
assertFails("WITH RECURSIVE t(n) AS (" +
29602960
" SELECT * FROM (VALUES('a'), ('b')) AS t(n)" +

core/trino-main/src/test/java/io/trino/sql/analyzer/TestFeaturesConfig.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ public void testDefaults()
6666
.setHideInaccessibleColumns(false)
6767
.setForceSpillingJoin(false)
6868
.setColumnarFilterEvaluationEnabled(true)
69+
.setLegacyArithmeticDecimalOperators(false)
6970
.setFaultTolerantExecutionExchangeEncryptionEnabled(true));
7071
}
7172

@@ -100,6 +101,7 @@ public void testExplicitPropertyMappings()
100101
.put("hide-inaccessible-columns", "true")
101102
.put("force-spilling-join-operator", "true")
102103
.put("experimental.columnar-filter-evaluation.enabled", "false")
104+
.put("deprecated.legacy-arithmetic-decimal-operators", "true")
103105
.put("fault-tolerant-execution-exchange-encryption-enabled", "false")
104106
.buildOrThrow();
105107

@@ -131,6 +133,7 @@ public void testExplicitPropertyMappings()
131133
.setHideInaccessibleColumns(true)
132134
.setForceSpillingJoin(true)
133135
.setColumnarFilterEvaluationEnabled(false)
136+
.setLegacyArithmeticDecimalOperators(true)
134137
.setFaultTolerantExecutionExchangeEncryptionEnabled(false);
135138
assertFullMapping(properties, expected);
136139
}

core/trino-main/src/test/java/io/trino/sql/planner/iterative/rule/TestInlineProjections.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ public void testInlineEffectivelyLiteral()
111111
p.project(
112112
Assignments.builder()
113113
// Use the literal-like expression multiple times. Single-use expression may be inlined regardless of whether it's a literal
114-
.put(p.symbol("decimal_multiplication", createDecimalType(16, 8)), new Call(MULTIPLY_DECIMAL_8_4, ImmutableList.of(new Reference(createDecimalType(8, 4), "decimal_literal"), new Reference(createDecimalType(8, 4), "decimal_literal"))))
114+
.put(p.symbol("decimal_multiplication", createDecimalType(17, 8)), new Call(MULTIPLY_DECIMAL_8_4, ImmutableList.of(new Reference(createDecimalType(8, 4), "decimal_literal"), new Reference(createDecimalType(8, 4), "decimal_literal"))))
115115
.put(p.symbol("decimal_addition", createDecimalType(9, 4)), new Call(ADD_DECIMAL_8_4, ImmutableList.of(new Reference(createDecimalType(8, 4), "decimal_literal"), new Reference(createDecimalType(8, 4), "decimal_literal"))))
116116
.build(),
117117
p.project(Assignments.builder()

0 commit comments

Comments
 (0)