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


##########
fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/scan/ScanNodePropertyKeys.java:
##########
@@ -132,6 +132,9 @@ public final class ScanNodePropertyKeys {
     /** Quote character enclosing a field; a single character. */
     public static final String TEXT_ENCLOSE = TEXT_PROPERTY_PREFIX + "enclose";
 
+    /** {@code "true"} for Hive OpenCSV field states and physical record 
boundaries. */
+    public static final String TEXT_HIVE_OPEN_CSV = TEXT_PROPERTY_PREFIX + 
"hive_open_csv";

Review Comment:
   [P1] Version the connector SPI property addition
   
   This is a new engine-read member of the shared connector SPI, but the 
connector API and its pinned test remain at 8.0 even though this repository 
defines every SPI-surface addition as a major change. Because this `String` 
constant is compile-time-inlined, a new API-8 Hive plugin still loads on an old 
API-8 FE, emits the literal key, and the old FE silently ignores it; the 
reverse pairing never emits it. Both paths fall back to generic CSV semantics 
instead of OpenCSV. Please bump the connector API major/update its pin and 
extend the frozen surface/baseline to include these public engine-read property 
keys so this protocol change cannot bypass the version gate.



##########
gensrc/thrift/PlanNodes.thrift:
##########
@@ -313,6 +313,9 @@ struct TFileAttributes {
     // org.openx.data.jsonserde.JsonSerDe
     13: optional bool openx_json_ignore_malformed = false;
 
+    // Hive OpenCSVSerde has different field states and physical record 
boundaries from load CSV.
+    14: optional bool hive_open_csv = false;

Review Comment:
   [P1] Fence this semantic flag from old backends
   
   An older BE legally ignores unknown optional field 14, so it sees the 
default `false` and runs the generic CSV reader even though the new FE promised 
OpenCSVSerde semantics. That fallback is observably different for records 
already covered by this PR (for example custom quotes/leading whitespace), yet 
this change neither advances/checks a minimum BE execution version nor marks 
the Hive scan with `REQUIRED_CURRENT_BACKEND_SEMANTICS`. During a rolling 
upgrade, placement can therefore change query results instead of failing. 
Please gate OpenCSV scans on a BE version that implements this field (including 
the smooth-upgrade-source fence) and add an absent-field/mixed-version 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]

Reply via email to