morningman commented on code in PR #70121:
URL: https://github.com/apache/doris/pull/70121#discussion_r4464956922
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/CreateRoutineLoadInfo.java:
##########
@@ -707,6 +720,10 @@ public void checkJobProperties() throws UserException {
}
}
+ checkCsvControlValue(CsvFileFormatProperties.PROP_ENCLOSE,
Review Comment:
Fixed in `3f9e13b4f74`.
Confirmed. Older versions accept U+FFFD as enclose or escape, and a
statement holds it whenever a client that does not send UTF-8 writes a single
non-ASCII character there, since the FE decodes every statement as UTF-8.
`gsonPostProcess` re-parses that statement with `setReplay(true)` and the check
threw from `validate()`, so the first restart after an upgrade cancelled such a
job for good, and during a rolling upgrade a follower replaying it from an
older master cancelled it on its own.
`checkJobProperties` now runs the check only when a statement is admitted,
as the compute group and workload group checks beside it do. The rebuilt job
still takes its enclose and escape from `jobProperties`, where the older
version recorded U+FFFD, so it keeps none, which is what it has run with after
every restart. The same commit widens the check from U+FFFD to every character
that is not ASCII (see the reply to the REQUEST_CHANGES review with F-01 and
F-02), so the gate now covers any such value an older version took.
`RoutineLoadJobReplayTest#testOlderCreateStatementWithNonAsciiValueReplays`
rebuilds jobs whose CREATE statement holds U+FFFD or `é` and whose metadata
holds U+FFFD, as older versions wrote them: they stay NEED_SCHEDULE with no
enclose and no escape. With the check run on the rebuild as well, the case
fails with the job CANCELLED.
##########
fe/fe-core/src/main/java/org/apache/doris/load/routineload/RoutineLoadJob.java:
##########
@@ -2047,8 +2054,24 @@ public void gsonPostProcess() throws IOException {
} else {
partialUpdateNewKeyPolicy =
TPartialUpdateNewRowPolicy.APPEND;
}
+ } else if
(k.equals(CreateRoutineLoadInfo.MAX_FILTER_RATIO_PROPERTY)) {
+ maxFilterRatio = Double.parseDouble(v);
+ } else if (k.equals(CreateRoutineLoadInfo.SEND_BATCH_PARALLELISM))
{
+ sendBatchParallelism = Integer.parseInt(v);
+ } else if (k.equals(CreateRoutineLoadInfo.LOAD_TO_SINGLE_TABLET)) {
+ loadToSingleTablet = Boolean.parseBoolean(v);
+ } else if (k.equals(CsvFileFormatProperties.PROP_ENCLOSE)) {
+ enclose = csvControlByte(k, v, enclose);
Review Comment:
Superseded in `3f9e13b4f74`: CREATE and ALTER ROUTINE LOAD now take only an
ASCII character as enclose or escape, and such a byte survives `new String(new
byte[]{b})`, so CREATE records the byte as it did before. A U+FFFD or any other
non-ASCII value in the metadata can only come from an older version, and the
replay ignores it with a warning. One correction to the reply above: task
planning turns the job's byte into text the same way and parses it again, so a
job with escape `é` ran with 0xEF before any restart as well (a throwaway unit
test planned 0xC3 as 0xEF, and every byte from 0x80 up as 0xFD for enclose and
0xEF for escape), and recording the given value did not change what the CSV
reader used.
--
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]
