github-actions[bot] commented on code in PR #68027:
URL: https://github.com/apache/doris/pull/68027#discussion_r4022665237
##########
fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveTextProperties.java:
##########
@@ -160,19 +161,35 @@ private static void extractTextSerDeProps(Map<String,
String> sdParams,
}
private static void extractCsvSerDeProps(Map<String, String> params,
- Map<String, String> result) {
- result.put(ScanNodePropertyKeys.TEXT_COLUMN_SEPARATOR,
- getParamOrDefault(params, SEPARATOR_CHAR, ","));
- result.put(ScanNodePropertyKeys.TEXT_LINE_DELIMITER,
getLineDelimiter(params));
- String quoteChar = getParamOrDefault(params, QUOTE_CHAR, "\"");
+ 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.
+ String separator = getCsvCharacter(params, tableParams,
SEPARATOR_CHAR, ',');
+ String quoteChar = getCsvCharacter(params, tableParams, QUOTE_CHAR,
'"');
+ String escapeChar = getCsvCharacter(params, tableParams, ESCAPE_CHAR,
'"');
Review Comment:
[P2] Handle literal NUL consistently when quote and escape are disabled
The accepted tuple `quoteChar=NUL, escapeChar=NUL` disagrees on
writer-reachable data. For `a\0b,tail`, OpenCSV 2.3 still takes its `c ==
escape` branch for the literal NUL and drops it, returning `['ab', 'tail']`;
its writer emits that NUL unchanged when both features are disabled. Doris
selects plain splitting and skips CSV unescaping when both fields are zero, so
both scanner modes preserve `['a\0b', 'tail']`. This is distinct from the
existing one-feature-disabled threads. Please either reproduce OpenCSV's
literal-NUL behavior or reject the tuple as unsupported, and add a binary
row-level oracle.
##########
fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveTextProperties.java:
##########
@@ -160,19 +161,35 @@ private static void extractTextSerDeProps(Map<String,
String> sdParams,
}
private static void extractCsvSerDeProps(Map<String, String> params,
- Map<String, String> result) {
- result.put(ScanNodePropertyKeys.TEXT_COLUMN_SEPARATOR,
- getParamOrDefault(params, SEPARATOR_CHAR, ","));
- result.put(ScanNodePropertyKeys.TEXT_LINE_DELIMITER,
getLineDelimiter(params));
- String quoteChar = getParamOrDefault(params, QUOTE_CHAR, "\"");
+ 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.
+ String separator = getCsvCharacter(params, tableParams,
SEPARATOR_CHAR, ',');
+ String quoteChar = getCsvCharacter(params, tableParams, QUOTE_CHAR,
'"');
Review Comment:
[P2] Match OpenCSV's embedded-quote state transition
This still diverges after a non-whitespace prefix, independently of the
existing leading-whitespace thread. With table-only `separatorChar='|'`,
`quoteChar='q'`, `escapeChar='e'`, the row `abcqleft|rightq|tail` is parsed by
OpenCSV as `['abcqleft|right', 'tail', NULL]`: the first `q` is retained but
also enters quoted state, so the enclosed separator is data. Doris sees the
initial `a`, moves permanently to `NORMAL`, and both scanner modes split both
separators into `['abcqleft', 'rightq', 'tail']`. A whitespace-only correction
will not cover this branch; the Hive CSV framing path needs OpenCSV's full
field quote transitions, with row-level tests under both scanners.
##########
fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveTextProperties.java:
##########
@@ -160,19 +161,35 @@ private static void extractTextSerDeProps(Map<String,
String> sdParams,
}
private static void extractCsvSerDeProps(Map<String, String> params,
- Map<String, String> result) {
- result.put(ScanNodePropertyKeys.TEXT_COLUMN_SEPARATOR,
- getParamOrDefault(params, SEPARATOR_CHAR, ","));
- result.put(ScanNodePropertyKeys.TEXT_LINE_DELIMITER,
getLineDelimiter(params));
- String quoteChar = getParamOrDefault(params, QUOTE_CHAR, "\"");
+ 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.
+ String separator = getCsvCharacter(params, tableParams,
SEPARATOR_CHAR, ',');
+ String quoteChar = getCsvCharacter(params, tableParams, QUOTE_CHAR,
'"');
Review Comment:
[P2] Preserve Hive's physical record boundaries for custom quotes
With table-only `separatorChar='|'`, `quoteChar='q'`, and `escapeChar='e'`,
consider file bytes `qleft\nrightq|tail\n`. Hive's TextInputFormat gives the
SerDe each physical line separately; OpenCSV reaches EOF with an unmatched
pending quote on each call, so Hive exposes two null-valued rows. Once this
assignment forwards `q`, both Doris scanner modes use
`EncloseCsvLineReaderCtx`, which reads past the first newline until the
second-line `q` and instead returns one row `['left\nright', 'tail']`. This is
separate from the resolved `line.delim` issue because the delimiter stays
newline; activating the quote changes record ownership. Please preserve Hive's
physical-line boundary (or reject this unsupported tuple) and cover the case
with both scanner settings.
--
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]
