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


##########
be/src/format/arrow/arrow_row_batch.cpp:
##########
@@ -97,17 +97,27 @@ Status convert_to_arrow_type(const DataTypePtr& origin_type,
     case TYPE_DATEV2:
         *result = std::make_shared<arrow::Date32Type>();
         break;
-    // TODO: maybe need to distinguish TYPE_DATETIME and TYPE_TIMESTAMPTZ
     case TYPE_TIMESTAMPTZ:
-    case TYPE_DATETIMEV2:
+    case TYPE_DATETIMEV2: {
+        arrow::TimeUnit::type time_unit;
         if (type->get_scale() > 3) {
-            *result = 
std::make_shared<arrow::TimestampType>(arrow::TimeUnit::MICRO, timezone);
+            time_unit = arrow::TimeUnit::MICRO;
         } else if (type->get_scale() > 0) {
-            *result = 
std::make_shared<arrow::TimestampType>(arrow::TimeUnit::MILLI, timezone);
+            time_unit = arrow::TimeUnit::MILLI;
         } else {
-            *result = 
std::make_shared<arrow::TimestampType>(arrow::TimeUnit::SECOND, timezone);
+            time_unit = arrow::TimeUnit::SECOND;
+        }
+        // Doris DATETIMEV2 represents a wall-clock value without a timezone. 
Arrow Flight
+        // exposes it as a timezone-naive timestamp so clients do not 
interpret it as an instant.
+        // This option only changes the DATETIMEV2 output schema. TIMESTAMPTZ 
remains timezone-aware,
+        // and Arrow-to-Doris conversions are unaffected.

Review Comment:
   [P2] Update the Flight SQL table-schema path as well
   
   `CommandGetTables(includeSchema=true)` still builds DATETIMEV2 with the UTC 
timezone in `FlightSqlSchemaHelper`, then serializes that aware type into 
`table_schema`. After this branch makes SELECT return an empty-timezone 
timestamp, schema discovery reports `TIMESTAMP_WITH_TIMEZONE` semantics for the 
same column that a query reports as `TIMESTAMP`. Please make the FE GetTables 
mapping emit a naive DATETIMEV2 timestamp too (while keeping TIMESTAMPTZ aware) 
and add a metadata-vs-SELECT schema test.



##########
be/src/exec/operator/result_sink_operator.cpp:
##########
@@ -57,7 +57,8 @@ Status ResultSinkLocalState::init(RuntimeState* state, 
LocalSinkStateInfo& info)
         std::shared_ptr<arrow::Schema> arrow_schema;
         if (p._sink_type == TResultSinkType::ARROW_FLIGHT_PROTOCOL) {
             
RETURN_IF_ERROR(get_arrow_schema_from_expr_ctxs(_output_vexpr_ctxs, 
&arrow_schema,
-                                                            
state->timezone()));
+                                                            state->timezone(),
+                                                            
/*datetime_naive=*/true));

Review Comment:
   [P1] Preserve schema equality during rolling BE upgrades
   
   Parallel result sinks are enabled by default, and FE fetches the schema from 
every top-fragment result BE and requires exact `Schema.equals`. During a 
rolling upgrade an old BE still returns DATETIMEV2 as `timestamp(unit, 
session_timezone)`, while a BE with this change returns `timestamp(unit)`; a 
query whose endpoints span both versions therefore fails `getFlightInfo` with a 
schema mismatch before any rows can be fetched. Please negotiate this 
capability at query level (using the aware common denominator until all 
selected BEs support naive DATETIMEV2, or otherwise constrain endpoints to one 
capability) and cover a mixed-version multi-endpoint query.



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