Aleksandr Efimov has uploaded this change for review. ( http://gerrit.cloudera.org:8080/24746
Change subject: IMPALA-15300: Narrow integer operands for substring and round ...................................................................... IMPALA-15300: Narrow integer operands for substring and round IMPALA-15297 maps Impala operators onto the Substrait functions they stand for by name. Two of them need more than a name. Substrait's substring takes its position and length as i32, and Impala declares substr with bigint parameters, so the call arrives as SUBSTR(varchar, i64, i64) and matches nothing. Round is the same mismatch the other way: Substrait's round takes the digit count as i32 while Impala types it from the literal, so a two-argument round arrives with an i8. Calcite's own SUBSTRING runs into this here as well, so the width is Substrait's choice rather than an Impala quirk, and Isthmus's own coercion takes the operand types as they come. Narrow those operands to i32 before matching. Isthmus has a hook for supplying different operands but it is package private; the converter itself is public, so the narrowing belongs there, at the point where the two type systems meet, rather than in a pass over the plan. The narrowing is an explicit cast rather than a silent reinterpretation, which Isthmus writes out with THROW_EXCEPTION behaviour. That is stricter than Impala, which returns no rows rather than failing for a position past the i32 range. Substrait has no wider substring to target, so the choice is between saying so in the plan and not exporting these queries at all. A one-argument round stays out. Every Substrait round variant takes a digit count, so there is nothing to match, and carrying it across would mean inventing an operand rather than adjusting one. Testing, in a local cluster on Linux: - SubstraitPlanExporterTest: 17 tests, 0 failures, 0 errors. The added one covers both substr arities, a literal and a column operand, a bigint column, and both spellings of round - CalciteOptimizerTest and CalciteSubstraitCompatibilityTest, neither of which this patch touches: 11 and 4 tests, 0 failures - Dropping the narrowing fails the added test, so it is not vacuous - A probe over the TPC-DS queries carrying Calcite goldens reports 96 of 110 with this patch and 84 without it, the difference being ten substr queries and two round ones - git diff --check Change-Id: I17a08f5f425bdaf038e47a4224c1fd65b0cc70e4 Assisted-by: Claude Opus 5 (Anthropic) --- M java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitOperatorMappings.java M java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitPlanExporterTest.java 2 files changed, 127 insertions(+), 31 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/24746/1 -- To view, visit http://gerrit.cloudera.org:8080/24746 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I17a08f5f425bdaf038e47a4224c1fd65b0cc70e4 Gerrit-Change-Number: 24746 Gerrit-PatchSet: 1 Gerrit-Owner: Aleksandr Efimov <[email protected]>
