github-actions[bot] commented on code in PR #70121:
URL: https://github.com/apache/doris/pull/70121#discussion_r4461711716
##########
fe/fe-core/src/main/java/org/apache/doris/load/routineload/RoutineLoadJob.java:
##########
@@ -2171,6 +2196,51 @@ protected void modifyCommonJobProperties(Map<String,
String> jobProperties) thro
this.jobProperties.put(CreateRoutineLoadInfo.PARTIAL_COLUMNS,
String.valueOf(isPartialUpdate));
this.jobProperties.put(CreateRoutineLoadInfo.UNIQUE_KEY_UPDATE_MODE,
uniqueKeyUpdateMode.name());
}
+
+ // Tasks take enclose and escape from these fields, not from
jobProperties, where the caller puts the
+ // new values
+ if (jobProperties.containsKey(CsvFileFormatProperties.PROP_ENCLOSE)) {
+ this.enclose = csvControlByte(CsvFileFormatProperties.PROP_ENCLOSE,
+ jobProperties.get(CsvFileFormatProperties.PROP_ENCLOSE),
this.enclose);
+ }
+ if (jobProperties.containsKey(CsvFileFormatProperties.PROP_ESCAPE)) {
+ this.escape = csvControlByte(CsvFileFormatProperties.PROP_ESCAPE,
+ jobProperties.get(CsvFileFormatProperties.PROP_ESCAPE),
this.escape);
+ }
+ }
+
+ // The byte an enclose or escape value stands for, as CREATE takes it.
ALTER of an older version did not check
+ // the value, so the metadata can hold one that CREATE rejects. Such a
value never reached the job, and it does
+ // not now: the job keeps the byte it has, where an exception would fail
the replay of the metadata.
+ private byte csvControlByte(String key, String value, byte current) {
+ try {
+ return key.equals(CsvFileFormatProperties.PROP_ENCLOSE)
+ ? CsvFileFormatProperties.parseEnclose(value) :
CsvFileFormatProperties.parseEscape(value);
+ } catch (org.apache.doris.nereids.exceptions.AnalysisException e) {
+ LOG.warn("routine load job {} keeps its {} instead of the invalid
value '{}': {}",
+ id, key, value, e.getMessage());
+ return current;
+ }
+ }
+
+ // What CREATE records for enclose or escape: the value the statement
gave, which the replay parses as CREATE did.
+ // The byte itself does not survive a String: new String(new byte[]{b})
decodes every byte from 0x80 up to U+FFFD.
+ // Without a value the byte is 0, recorded as it always was.
+ private static String csvControlValue(CreateRoutineLoadInfo info, String
key, byte value) {
+ String given = info.getJobProperties().get(key);
+ return Strings.isNullOrEmpty(given) ? new String(new byte[]{value}) :
given;
+ }
+
+ // csvControlByte for a value CREATE or ALTER recorded. CREATE of an older
version recorded the byte, so a byte
+ // from 0x80 up came back as U+FFFD, which would now parse to 0xFD for
enclose and 0xEF for escape. That byte is
+ // lost: the job keeps the default, which is what it has run with after
every restart.
+ private byte recordedCsvControlByte(String key, String value, byte
current) {
+ if (value.equals("\uFFFD")) {
Review Comment:
[P2] Distinguish explicit U+FFFD values from legacy corruption
A literal U+FFFD is accepted by both current parsers: CREATE or ALTER with
`"escape" = "�"` produces byte `0xEF` (and enclose produces `0xFD`), and the
new raw-string writer stores that character intact. This guard nevertheless
treats the current value as old corruption, so CREATE/image replay resets it to
zero; live ALTER and ALTER-log replay use `csvControlByte` and keep the nonzero
byte. CSV parsing therefore changes after restart or checkpoint-based failover.
This is a separate instance from the earlier byte-to-String issue: the string
is preserved correctly here and the new legacy check discards it. Distinguish
new raw records from legacy ones before applying this fallback, and cover
explicit-U+FFFD CREATE/ALTER round trips.
--
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]