englefly opened a new pull request, #67902:
URL: https://github.com/apache/doris/pull/67902
### What problem does this PR solve?
Related PR: #66007
Problem Summary:
An INSERT of an ARRAY/MAP of STRUCT values built from constant expressions
fails whenever the
constructor has to compute a common type for its arguments:
```sql
CREATE TABLE t (id INT, a ARRAY<STRUCT<i: INT, s: VARCHAR(16)>>)
DUPLICATE KEY(id) DISTRIBUTED BY HASH(id) BUCKETS 1 PROPERTIES
('replication_num'='1');
INSERT INTO t VALUES (2, array(struct(100, repeat('x', 2)), struct(200,
repeat('y', 2))));
```
Before this PR the backend aborted on that statement (release builds
returned the error instead):
`Bad cast from type:doris::ColumnStr<unsigned int> to
doris::ColumnNullable`, raised in
`FunctionArrayConstructor::execute_impl` -> `ColumnStruct::insert_from` ->
`ColumnNullable::insert_from`. Inserting a single array element, or two
elements whose types need no
common type (e.g. `struct(100, repeat('x', 2))` and `struct(127, repeat('y',
2))`), already worked.
Root cause: the `array(...)` element type is fixed while its arguments are
still nullable, and the
first argument also needs a widening cast (100 is a TINYINT, the common type
is SMALLINT):
```
array(CAST(struct(100, repeat('x', 2)) AS struct<col1:smallint not null,
col2:varchar(65533)>),
struct(200, repeat('y', 2)))
```
Constant folding then replaces `repeat('x', 2)` with the literal `'xx'`, and
the struct constructor
recomputes its return type from that literal, so the second argument becomes
`struct<col1:smallint not null, col2:varchar(2) not null>` while
`array(...)` still expects `col2`
nullable, and no cast bridges the two. The plan therefore declares an
argument type that its own
signature does not expect: the backend lays out the array element column
from the declared type
(`col2` nullable) but builds that argument column from the drifted type
(`col2` is a plain string
column), and inserting a required child column into a nullable one fails.
The frontend recomputes the signature of the parent from the new children
but never coerces the
arguments again. Several independent rebuild paths exist
(`FoldConstantRuleOnFE` has its own
`rewriteChildren`, plus `DefaultExpressionRewriter.rewriteChildren` and
`ExpressionBottomUpRewriter.rewriteChildren`), which is why the repair has
to live where the parent is
rebuilt instead of only before the plan is handed to the backend.
### What this PR changes
Fix, in three layers:
1. `TypeCoercionUtils.coerceFunctionArguments` returns a function call with
the arguments cast to the
input types its signature expects (per argument, using the existing
implicit cast rules; a call
that already matches is returned unchanged), and is applied by every
generic "rebuild the parent
after rewriting its children" helper:
`FoldConstantRuleOnFE.rewriteChildren` (the constant folding
path that creates the drift), `DefaultExpressionRewriter.rewriteChildren`
and
`ExpressionBottomUpRewriter.rewriteChildren`. The mismatch window is
therefore zero: the rest of the
optimization never sees an expression whose declared type disagrees with
its own arguments.
2. `TypeCoercionUtils.restoreFunctionArgumentTypes` applies that to every
node of an expression and is
called at the boundaries of the expression rewrites
(`ExpressionRuleExecutor.rewrite` and the
`FoldConstantRuleOnFE` entry), so producers that rebuild expressions
outside the helpers above are
covered as well. It also reports the calls that stay inconsistent (a
function whose arguments
cannot be brought back to the expected input types): a warning, and an
`AnalysisException` when
`fe_debug` is enabled, so such a call points at its producer instead of
spreading unnoticed.
3. `ExpressionTranslator.visitScalarFunction` keeps the same repair as a
last resort guard, so a
function call the backend cannot execute cannot reach it from a producer
we have not covered.
The repair cannot be done earlier than the parent's rebuild: the folded
child's type is correct (a
literal is never null, and the backend verifies that the type the frontend
declares for a call matches
the one it infers from its arguments), so the argument has to be cast at the
parent, which is what
happens now.
The backend part of this fix is in the first commit: a nested insert whose
source column layout does
not match the destination no longer aborts the backend process, it reports
`[INTERNAL_ERROR]insert 'String' into 'Nullable(String)'` (see
`ColumnNullable::insert_from` and the two new unit tests).
### Release note
Fixed an error when inserting ARRAY/MAP values whose STRUCT arguments are
built from constant
expressions that need a common type.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
- [x] Unit Test
- [x] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
* new regression suite
`datatype_p0/complex_types/test_array_struct_constructor_nullable`: array with
one element, two elements of different integer types, two rows,
differing varchar widths,
`named_struct`, map of struct and non constant arguments.
* `./run-regression-test.sh --run -d datatype_p0/complex_types`: the new
suite and 7 other suites
pass; `test_pruned_columns` and
`test_light_schema_change_lazy_pruned_struct` fail with a
pre-existing backend error `Invalid access path for column 's': path is
empty`, reproduced with an
unchanged plan shape (no cast inserted by this change).
* backend unit tests:
`ColumnNullableTest.InsertNonNullableColumnReportsError` and
`ColumnStructTest.InsertNonNullableFieldIntoNullableFieldReportsError`
(compile checked against the
current headers, the BE unit test build was not available in the
environment where the fix was
developed).
* manual test: with the `ExpressionTranslator` guard disabled, the failing
statement already produces
a plan containing the missing cast and inserts the expected row, which
proves the repair happens
during the rewrite; the plan of statements that are already consistent
(`100/127`, a single
element) is byte identical to before the change.
- Behavior changed:
- [x] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add doc PR link -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
