github-actions[bot] commented on code in PR #66007:
URL: https://github.com/apache/doris/pull/66007#discussion_r3687510817
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/CreateNamedStruct.java:
##########
@@ -105,8 +105,10 @@ public FunctionSignature customSignature() {
ImmutableList.Builder<StructField> structFields =
ImmutableList.builder();
for (int i = 0; i < arity(); i = i + 2) {
StringLikeLiteral nameLiteral = (StringLikeLiteral) child(i);
+ // A named struct has the same value-nullability contract as
struct(...); keeping
+ // the field nullable here would reject safe casts into
required target fields.
structFields.add(new StructField(nameLiteral.getStringValue(),
- children.get(i + 1).getDataType(), true, ""));
+ children.get(i + 1).getDataType(), children.get(i +
1).nullable(), ""));
Review Comment:
[P2] Keep strict pre-cast Struct children required
This now copies the child expression's nullability, but `Cast.nullable()` is
still mode-blind. With `enable_strict_cast=true`, `named_struct('metric',
CAST(required_string AS INT))` therefore declares `metric` nullable because
STRING-to-INT can return NULL in loose mode, even though strict execution
aborts bad rows and returns a required INT for successful rows. A subsequent
strict cast or INSERT into `STRUCT<metric:INT NOT NULL>` is rejected by
`CheckCast`'s declared-source-nullable guard before its new strict exemption is
reached. Please make scalar-Cast/Struct-child nullability strict-aware (while
keeping `TryCast` nullable), and cover pre-cast children in both `struct` and
`named_struct`.
##########
be/src/format_v2/table_reader.h:
##########
@@ -1274,23 +1308,24 @@ class TableReader {
return Status::OK();
}
- Status _materialize_present_child_mapping_column(const ColumnMapping&
mapping,
- const ColumnPtr&
file_column,
- const size_t rows,
ColumnPtr* column) {
+ Status _materialize_present_child_mapping_column(
+ const ColumnMapping& mapping, const ColumnPtr& file_column, const
size_t rows,
+ ColumnPtr* column, const NullMap* nullable_parent_null_map =
nullptr) {
DORIS_CHECK(column != nullptr);
DORIS_CHECK(mapping.file_type != nullptr);
DORIS_CHECK(mapping.table_type != nullptr);
*column = file_column;
if (!mapping.is_trivial) {
if (!mapping.child_mappings.empty()) {
- RETURN_IF_ERROR(
- _materialize_complex_mapping_column(mapping, *column,
rows, column));
+ RETURN_IF_ERROR(_materialize_complex_mapping_column(mapping,
*column, rows, column,
+
nullable_parent_null_map));
} else {
RETURN_IF_ERROR(_cast_column_to_type(column,
mapping.file_type, mapping.table_type,
mapping.file_column_name));
}
}
- RETURN_IF_ERROR(_align_column_nullability(column, mapping.table_type));
+ RETURN_IF_ERROR(
+ _align_column_nullability(column, mapping.table_type,
nullable_parent_null_map));
Review Comment:
[P1] Project inherited masks for trivial collection children
This inherited mask is still lost when `mapping.is_trivial` and the mapped
child is an ARRAY or MAP. For example, a nullable Struct can be rematerialized
because one sibling changed while an identical collection sibling remains
trivial; this call passes the Struct row mask into `_align_column_nullability`,
whose ARRAY/MAP branches recurse into entries without projecting that
row-coordinate mask through the offsets. A NULL Struct row with a populated
physical collection can then expose a required descendant's hidden NULL
placeholder and fail with `Default expression produced NULL for non-nullable
table column`. Please project the inherited mask at the ARRAY/MAP branches too,
and add a mapper-driven test with a non-trivial Struct parent and a trivial
collection child.
--
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]
