github-actions[bot] commented on code in PR #67921:
URL: https://github.com/apache/doris/pull/67921#discussion_r4004062604
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/lance/LanceTypeConverter.java:
##########
@@ -89,7 +84,8 @@ private static Type toDorisType(Field field, boolean
allowNull) {
ArrowType arrowType = field.getType();
switch (arrowType.getTypeID()) {
case Null:
- return allowNull ? Type.NULL : Type.UNSUPPORTED;
+ // Required Null leaves cannot use the nullable SerDe that
handles Arrow NA buffers.
+ return field.isNullable() ? Type.NULL : Type.UNSUPPORTED;
Review Comment:
[P1] Preserve the upgrade fence through file()\n\nThis change makes nullable
nested Null roots queryable and marks them as requiring the current BE reader,
but the generic `file(uri=..., format="lance", ...S3 properties...)` path loses
that marker: `FileTableValuedFunction` delegates the schema and Lance scan
metadata while inheriting its own empty `lanceCurrentReaderColumns`, and
`TVFScanNode` consults that wrapper from `FunctionGenTable`. It can therefore
select a smooth-upgrade source BE that lacks the new Arrow NA materialization
path and can crash on this column. Please delegate
`requiresCurrentLanceReader()` through the wrapper and cover this route in the
upgrade test.
##########
be/src/core/data_type/data_type.h:
##########
@@ -170,7 +170,9 @@ class IDataType : private boost::noncopyable {
auto node = ptype->add_types();
node->set_type(TTypeNodeType::SCALAR);
auto scalar_type = node->mutable_scalar_type();
- scalar_type->set_type(doris::to_thrift(get_primitive_type()));
+ // NULL uses UInt8 internally; preserve its logical type even inside
complex schemas.
+ scalar_type->set_type(is_null_literal() ? TPrimitiveType::NULL_TYPE
Review Comment:
[P1] Keep local Lance execution on the schema backend\n\nThis change allows
a current BE to return nested logical NULL types during Local Lance schema
discovery, but the generic `file(file_path=..., format="lance",
shared_storage="true")` wrapper loses Local execution-backend affinity.
`FunctionGenTable` retains `FileTableValuedFunction`, so `TVFScanNode` misses
its `instanceof LocalTableValuedFunction` branch and can schedule the dataset
on an old smooth-upgrade source BE that cannot materialize the new nested Arrow
NA shape. Forwarding the S3 capability check does not cover Local because its
safety invariant is backend affinity; please expose and delegate the effective
execution backend (or unwrap the delegate) and add a generic-file Local upgrade
test.
--
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]
