github-actions[bot] commented on code in PR #63088:
URL: https://github.com/apache/doris/pull/63088#discussion_r3256070720
##########
be/src/core/data_type/data_type_array.cpp:
##########
@@ -48,7 +48,13 @@ namespace ErrorCodes {
extern const int NUMBER_OF_ARGUMENTS_DOESNT_MATCH;
}
-DataTypeArray::DataTypeArray(const DataTypePtr& nested_) : nested {nested_} {}
+DataTypeArray::DataTypeArray(const DataTypePtr& nested_) {
+ DataTypePtr nullable_nested = make_nullable(nested_);
Review Comment:
Wrapping every array element type in `Nullable` changes the BE block/native
serialization shape for arrays: `DataTypeArray::serialize()` and
`get_uncompressed_serialized_bytes()` now delegate to a `DataTypeNullable`,
which adds a null map before the nested element payload. The PR updates
`all_types_single_row.native`, which confirms the binary representation
changes, but there is no `be_exec_version` branch or backward-compatible
deserialize path. In a rolling upgrade or when reading native blocks/files
produced by the old non-null nested representation, old/new BEs will disagree
about the array payload layout and misread the stream. Please gate the new
nullable-nested encoding by `be_exec_version` or add compatibility decoding for
the old layout before changing the serialized format.
##########
fe/fe-type/src/main/java/org/apache/doris/catalog/ArrayType.java:
##########
@@ -48,20 +55,31 @@ public ArrayType() {
}
public ArrayType(Type itemType) {
- this(itemType, true);
+ this.itemType = itemType;
+ this.containsNull = true;
}
+ /**
+ * @deprecated Array elements are always nullable. Use {@link
#ArrayType(Type)} instead.
+ * This constructor is retained only for call-site
compatibility during transition.
+ */
+ @Deprecated
public ArrayType(Type itemType, boolean containsNull) {
this.itemType = itemType;
- this.containsNull = containsNull;
+ // Ignore the parameter — always true
+ this.containsNull = true;
}
public Type getItemType() {
return itemType;
}
+ /**
+ * Always returns {@code true}. Array elements are always nullable in
Doris.
+ */
public boolean getContainsNull() {
- return containsNull;
Review Comment:
Making `getContainsNull()` always return true silently changes catalog type
semantics that external schema conversion still relies on. Iceberg conversion
uses this flag to emit optional vs required list elements, Paimon copies it
into the element type, and Iceberg alter validation compares required/optional
list elements against `newArrayType.getContainsNull()`. After this change, any
existing or constructed `ARRAY<... NOT NULL>` is reported as nullable and can
be converted/validated as optional, changing external table schema contracts.
If Doris execution requires nullable array elements, external schema paths need
an explicit rejection or separate preservation of external element nullability
instead of collapsing the flag globally.
--
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]
