github-actions[bot] commented on code in PR #68027:
URL: https://github.com/apache/doris/pull/68027#discussion_r4017196995
##########
fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveTextProperties.java:
##########
@@ -160,19 +160,22 @@ private static void extractTextSerDeProps(Map<String,
String> sdParams,
}
private static void extractCsvSerDeProps(Map<String, String> params,
- Map<String, String> result) {
+ Map<String, String> tableParams, Map<String, String> result) {
+ // Trino stores CSV settings in table parameters. Honor Hive's
table-over-SerDe precedence
+ // so valid CSV files are not silently split with the default
delimiter and quote characters.
result.put(ScanNodePropertyKeys.TEXT_COLUMN_SEPARATOR,
- getParamOrDefault(params, SEPARATOR_CHAR, ","));
- result.put(ScanNodePropertyKeys.TEXT_LINE_DELIMITER,
getLineDelimiter(params));
- String quoteChar = getParamOrDefault(params, QUOTE_CHAR, "\"");
+ getParamOrDefault(params, tableParams, SEPARATOR_CHAR, ","));
+ result.put(ScanNodePropertyKeys.TEXT_LINE_DELIMITER,
+ getParamOrDefault(params, tableParams, LINE_DELIM,
DEFAULT_LINE_DELIM));
Review Comment:
[P1] Keep table-level `line.delim` from changing record boundaries
The stated Trino layout has only separator/quote/escape in table parameters.
Hive OpenCSVSerde never reads `line.delim`, and Hadoop TextInputFormat's actual
record-delimiter setting is `textinputformat.record.delimiter`; however, this
now forwards any unrelated table `line.delim` to BE, where both CSV readers
split records on it. A newline-delimited table carrying `TBLPROPERTIES
('line.delim'='|')` will therefore return different rows in Doris than Hive.
Please keep this lookup on the previous SerDe-only path (or the newline
default), apply table precedence only to the three supported CSV character
keys, and add a negative record-boundary case.
##########
fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveTextProperties.java:
##########
@@ -160,19 +160,22 @@ private static void extractTextSerDeProps(Map<String,
String> sdParams,
}
private static void extractCsvSerDeProps(Map<String, String> params,
- Map<String, String> result) {
+ Map<String, String> tableParams, Map<String, String> result) {
+ // Trino stores CSV settings in table parameters. Honor Hive's
table-over-SerDe precedence
+ // so valid CSV files are not silently split with the default
delimiter and quote characters.
result.put(ScanNodePropertyKeys.TEXT_COLUMN_SEPARATOR,
- getParamOrDefault(params, SEPARATOR_CHAR, ","));
- result.put(ScanNodePropertyKeys.TEXT_LINE_DELIMITER,
getLineDelimiter(params));
- String quoteChar = getParamOrDefault(params, QUOTE_CHAR, "\"");
+ getParamOrDefault(params, tableParams, SEPARATOR_CHAR, ","));
Review Comment:
[P1] Normalize table CSV values before forwarding them
Hive OpenCSVSerde consumes the first Java character of each of these three
properties and fails on an empty value, but this new table-level path forwards
the raw strings: `separatorChar='||'` becomes a two-byte BE delimiter, empty
quote/escape values are silently omitted downstream, and a one-character
non-ASCII quote/escape such as `é` is reduced by `getBytes()[0]` to one byte in
thrift `i8`. Those cases therefore still return wrong rows or silently differ
from the source reader. Please normalize non-empty values to the first
character, reject empty values, and either reject non-byte-representable
quote/escape characters or extend the FE/BE contract; add coverage after thrift
construction rather than only at the property-map layer.
--
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]
