github-actions[bot] commented on code in PR #68027:
URL: https://github.com/apache/doris/pull/68027#discussion_r4022063803
##########
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 leading-whitespace quote handling
Forwarding the table-level quote still does not make these rows
Hive-compatible because OpenCSV defaults `ignoreLeadingWhiteSpace` to true,
while the shared Doris line reader recognizes an enclosure only as the literal
first byte of a field. With separator `|`, quote `q`, escape `e`, and record
`x| qa|bq|c`, Hive/OpenCSV discards the two spaces, enters quote state at `q`,
and returns `[x, a|b, c]`. Both Doris readers see the first space, enter NORMAL
state, and split on the `|` inside `qa|bq`, producing four fields. Please
implement the OpenCSV leading-whitespace quote transition for this Hive CSV
path and add a row-level oracle case under both scanner settings.
##########
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,
'"');
+ // Hive treats the writer-default double quote as a sentinel:
newReader selects the constructor
+ // whose effective escape is backslash. Resolve it before validating
the parser's character tuple.
+ if ("\"".equals(escapeChar)) {
+ escapeChar = "\\";
+ }
+ if ("\0".equals(separator)) {
+ throw new DorisConnectorException("Invalid OpenCSVSerde property
'separatorChar': must not be NUL");
+ }
+ // OpenCSV requires distinct active characters; NUL disables
quote/escape and may be shared by both.
Review Comment:
[P2] Cover disabled-quote rows, not only tuple acceptance
This accepts `quoteChar=NUL` with an active escape, but the BE unescape
semantics do not match OpenCSV at the start of an unquoted field. For example,
with separator `,`, quote NUL, and escape `e`, Hive/OpenCSV reads `eeabc,tail`
as `abc,tail`: its parser starts with `inField=false`, so both leading escapes
are discarded. Both Doris readers instead use plain field splitting and
`escape_string_for_csv`, which collapses `ee` to one `e`, returning `eabc`.
Hive's writer itself emits `eeabc` for a value beginning with `e` under this
tuple, so this is reachable without a malformed file. The 343-tuple oracle only
compares acceptance and therefore misses the wrong row. Please align both BE
readers with the OpenCSV field-state behavior (or reject tuples Doris cannot
read equivalently) and add a row-level oracle case. This is distinct from the
earlier validation thread: the tuple is accepted by both sides, but its data is
decoded differently.
##########
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] Preserve quote doubling when escape is disabled
This accepts an active quote with `escapeChar=NUL`, but OpenCSV's
doubled-quote rule is independent of the configured escape while both Doris
value decoders disable all CSV unescaping when the escape byte is zero. With
separator `,`, quote `q`, escape NUL, and record `qaqqbq,tail`, Hive/OpenCSV
collapses the middle `qq` and returns `[aqb, tail]`. Both Doris readers frame
and trim the outer quotes correctly, then skip `escape_string_for_csv`,
returning `[aqqb, tail]`. The tuple oracle only checks empty-record acceptance,
so it misses the wrong value. Please preserve doubled-enclosure decoding with a
NUL escape in both readers (or reject this unsupported tuple) and add a
row-level oracle case under 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]
