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()
This 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
This 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]
