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]

Reply via email to