github-actions[bot] commented on code in PR #68853:
URL: https://github.com/apache/doris/pull/68853#discussion_r4547387358


##########
fe/fe-core/src/main/java/org/apache/doris/qe/runtime/ThriftPlansBuilder.java:
##########
@@ -523,6 +523,7 @@ private static TPipelineFragmentParams 
fragmentToThriftIfAbsent(
 
             // Only used for AI Functions
             params.setAiResources(collectAiResources(connectContext));
+            params.setS3Resources(coordinatorContext.s3Resources);

Review Comment:
   [P2] Forward the S3 snapshot to stream and routine loads
   
   A load mapping such as `file_size=ELEMENT_AT(TO_FILE('res', uri), 'size')` 
is accepted for a BIGINT destination. `NereidsLoadPlanInfoCollector` translates 
it with a local `PlanTranslatorContext`, but drops that context's 
`s3Resources`; `NereidsStreamLoadPlanner` builds fragment parameters directly 
and never calls this setter. `FunctionToFile::open` therefore fails with 
missing query resources on the first load/task, even for NULL URI input. Carry 
the collector snapshot into the direct load parameters. This is a separate 
stream/routine instance from the known broker handoff and 
stale-snapshot-after-rotation threads: no credential change or plan reuse is 
needed.



##########
be/src/core/data_type/data_type_factory.cpp:
##########
@@ -573,7 +587,26 @@ DataTypePtr DataTypeFactory::create_data_type(const 
std::vector<TTypeNode>& type
                 create_data_type(types, idx, node.contains_nulls[0]));
         break;
     }
+    case TTypeNodeType::FILE:
     case TTypeNodeType::STRUCT: {
+        if (node.type == TTypeNodeType::FILE &&
+            (node.struct_fields.size() != DataTypeFile::FIELD_COUNT ||
+             types.size() - static_cast<size_t>(*idx) <= 
DataTypeFile::FIELD_COUNT)) {
+            throw Exception(ErrorCode::INVALID_ARGUMENT, "Invalid FILE type 
descriptor");
+        }
+        if (node.type == TTypeNodeType::FILE) {
+            constexpr TPrimitiveType::type child_types[] = {
+                    TPrimitiveType::STRING, TPrimitiveType::BIGINT, 
TPrimitiveType::BIGINT,

Review Comment:
   [P1] Preserve the FILE child schema during rolling upgrades
   
   `branch-4.1` already supports these FILE ids, but its 
`FileType.canonicalFields()` emits `VARCHAR(65533)` for `uri` and 
`VARCHAR(1024)` for `content_type`/`checksum`. This validation and its protobuf 
counterpart require `STRING`, so a 4.1 FE's `TO_FILE`/`list_files` plan fails 
on an upgraded BE with `FILE requires its canonical scalar children`. Upgrading 
FE first also fails because current `FileType` emits STRING while the 4.1 BE 
requires VARCHAR. Retain a compatible writer schema or capability-gate the 
change, normalize both representations on read, and cover both upgrade orders.



-- 
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]

Reply via email to