Gabriel39 commented on code in PR #68027:
URL: https://github.com/apache/doris/pull/68027#discussion_r4021657308
##########
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:
Fixed in 9c7e82edec. Table-over-SerDe lookup is now limited to
separatorChar, quoteChar, and escapeChar. CSV line.delim retains its previous
SerDe-only override and newline default.
Added property tests for ignored table line.delim values and preserved SerDe
overrides, plus assertions after real scan-node construction and a Thrift round
trip. The Hive regression suite now places pipes in payloads while setting
table line.delim to |, across all four partitioned/unpartitioned layouts. Local
unit/wire tests pass; Hive end-to-end execution remains pending because
HiveServer2 is unavailable.
##########
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:
Fixed in 9c7e82edec with a shared CSV-character resolver. It resolves
precedence first, takes the first Java character as OpenCSVSerde does, rejects
empty values and leading surrogate code units, and rejects non-ASCII
quote/escape characters before the existing i8 fields can truncate them. Valid
UTF-8 separators remain strings. Quote trimming is derived from the normalized
character.
Replaced the incorrect empty-means-disabled test and added boundary tests
for both property sources, numeric-looking values, precedence over invalid
lower-priority values, and Unicode. Five FE Core tests call the real scan-node
attribute builder and assert the deserialized Thrift payload. The Hive
regression suite adds metadata-only multi-character changes and unsupported
quote/escape errors. Empty metadata is tested at the unit boundary because Hive
itself rejects it while validating ALTER TABLE.
Validation: 474 Hive connector tests passed (1,244 with upstream modules);
all five FE Core wire tests and FE Checkstyle passed. The six new
property-boundary tests failed before this follow-up fix. The Hive regression
suite compiles, but its end-to-end assertions could not run without HiveServer2.
--
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]
