[fix](fe) Keep MAP key/value decimal slots independent in default deimal v3 precision promotion - #67154
[fix](fe) Keep MAP key/value decimal slots independent in default deimal v3 precision promotion#67154starocean999 wants to merge 2 commits into
Conversation
…imal v3 precision promotion
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
/review |
There was a problem hiding this comment.
The fixed-argument MAP path now preserves independent DecimalV3 leaves, and the added unit/regression expectations are deterministic and internally consistent. However, the shared promotion helper still introduces three blocking correctness failures: linked Any/Follow slots can diverge across Decimal storage widths; field can retain different representations for its fixed operand and repeated search operands; and complex vararg signatures still collapse independent nested leaves into one global type. The two inline comments cover those three root causes.
Checkpoint summary: the four-file scope is focused, but the generic helper affects many built-ins and extension functions. Parallel Follow consumers and scalar/complex vararg paths were traced through FE cast insertion and BE dispatch. NULL/empty inputs and all Decimal32/64/128/256 boundaries were checked. There are no new concurrency, lifecycle, static-initialization, configuration-definition, transaction, persistence, data-write, wire/protocol, rolling-upgrade, observability, or status-propagation surfaces; the bounded signature-tree traversal has no actionable performance concern. Test coverage is useful but omits all three blocking triggers. No build or test command was run because this runner's review instructions prohibit them. No additional reviewer focus was supplied. The three-round review converged with no unresolved or undeduplicated candidates.
…ts aligned in default decimal v3 precision promotion The previous per-slot promotion kept MAP key/value independent but broke other scenarios (field's fixed first operand + vararg tail comparison) and still merged linked Any/Follow slots (e.g. element_at's MAP key and lookup). Redesign defaultDecimalV3PrecisionPromotion to be group based: - Non-MAP decimal slots (scalar, ARRAY item, vararg scalar) keep the original behavior of using the single wider type across all decimal slots, so field and other scalar/array functions are not affected. - MAP key/value leaves are independent groups keyed by their resolved leaf type: key and value never merge, while the corresponding leaves across different (or repeated/vararg) MAP arguments aggregate; a NULL MAP argument falls back to its own group's wider type. - A top-level scalar slot with a concrete resolved type that matches a MAP key/value leaf (Any/Follow linked, e.g. element_at/map_contains_key lookup) is promoted together with that leaf, so the BE compares columns of the same concrete decimal class across the DECIMAL32/64 boundary and for NULL. Tests: updated ComputeSignatureHelperTest (non-MAP back to wider type, NULL MAP uses group wider) + new tests for vararg MAP, field vararg, element_at linked slots and NULL lookup; regression suite extended with storage-width boundary, NULL lookup and field cases.
|
/review |
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Nereids' default DECIMALV3 precision promotion
(
ComputeSignatureHelper.defaultDecimalV3PrecisionPromotion) recursively collects everyDECIMALV3 found in the argument types, computes a single global "wider" type, and then
replaces ALL decimal slots with that one type via
TypeCoercionUtils.replaceDecimalV3WithTarget.For
MAP<K, V>where K and V are independent type variables with differentprecision/scale, merging them into one wider type breaks correctness in two ways
(they share the same type-derivation path):
MAP_KEYS(and other map consumers) silently turns legal keys into NULL.Repro: a
MAP<DECIMAL(76,0), DECIMAL(76,18)>built from a 76-digit integral keyand
0.125000000000000000. Widening the key slot to scale 18 overflows the bigintegral key, so
MAP_KEYSreturns[null]instead of the key.UNNEST(MAP)(rewritten toEXPLODE_MAP) fails type analysis. The promoted MAPargument type no longer matches the original input MAP type.
Fix: promote each decimal slot independently instead of merging them. Each slot
(MAP key/value, ARRAY item, scalar) keeps its own precision/scale derived from its
own argument type; the wider type is only used for NULL-typed slots and the
placeholder return type. Vararg slots keep the previous behavior to avoid changing
vararg decimal functions (e.g.
field).None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)