github-actions[bot] commented on code in PR #64027:
URL: https://github.com/apache/doris/pull/64027#discussion_r3366860822
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ElementAt.java:
##########
@@ -85,19 +91,65 @@ public <R, C> R accept(ExpressionVisitor<R, C> visitor, C
context) {
}
@Override
- public List<FunctionSignature> getSignatures() {
- if (child(0).getDataType() instanceof StructType) {
- return new StructElement(child(0), child(1)).getSignatures();
+ public boolean foldable() {
+ // A map may legitimately contain a null key, so element_at(map, NULL)
must be evaluated by
+ // the BE (which looks up the null key) rather than being folded to
NULL on the FE through
+ // PropagateNullLiteral. Marking it non-foldable here skips that fold
for the map null-key
+ // case only; a null container (element_at(NULL, ...)) or a null
array/struct index still
+ // folds to NULL as before.
+ if (child(0).getDataType() instanceof MapType && child(1) instanceof
NullLiteral) {
+ return false;
}
- return SIGNATURES;
+ return true;
}
@Override
- public Expression rewriteWhenAnalyze() {
+ public void checkLegalityBeforeTypeCoercion() {
+ // Struct field access (the former struct_element) only accepts a
constant int/string index,
+ // since the selected field — and therefore the result type — must be
known at analysis time.
if (child(0).getDataType() instanceof StructType) {
- return new StructElement(child(0), child(1));
+ Expression field = getArgument(1);
+ if (!(field instanceof StringLikeLiteral || field instanceof
IntegerLikeLiteral)) {
+ throw new AnalysisException("element_at over a struct only
allows a constant int or"
+ + " string second parameter: " + this.toSql());
+ }
+ }
+ }
+
+ @Override
+ public List<FunctionSignature> getSignatures() {
+ DataType arg0Type = child(0).getDataType();
+ if (arg0Type instanceof NullType) {
+ return ImmutableList.of(
+
FunctionSignature.ret(NullType.INSTANCE).args(NullType.INSTANCE,
child(1).getDataType()));
+ }
+ if (arg0Type instanceof StructType) {
+ return
ImmutableList.of(FunctionSignature.ret(structFieldType((StructType) arg0Type))
+ .args(arg0Type, child(1).getDataType()));
+ }
+ return SIGNATURES;
+ }
+
+ // Resolve the type of the struct field selected by the constant
int/string index.
+ private DataType structFieldType(StructType structType) {
+ Expression field = getArgument(1);
+ if (field instanceof IntegerLikeLiteral) {
+ int offset = ((IntegerLikeLiteral) field).getIntValue();
Review Comment:
This truncates wider integer literals before doing the bounds check and
selecting the return type. For example, `element_at(named_struct('a', 1),
4294967297)` reaches this path as an `IntegerLikeLiteral`; `getIntValue()`
wraps it to `1`, so FE infers field 1 instead of rejecting the out-of-range
index. BE resolves the same struct index from the constant column as an
`int64_t` in `get_struct_element_index()` and rejects the real value, so the
plan can pass FE analysis with the wrong return type and then fail in BE
return-type/runtime validation. Please validate using the non-truncated integer
value (and add a regression for an index larger than `Integer.MAX_VALUE`)
before indexing into `structType.getFields()`.
--
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]
