morningman commented on PR #70121:
URL: https://github.com/apache/doris/pull/70121#issuecomment-6625518002
F-01: partly, and fixed differently in `3f9e13b4f74`. The conversion in
`NereidsDataDescription` (lines 422-423) predates this PR: since #50882 every
task re-parses the job's enclose and escape from `new String(new byte[]{...})`.
A throwaway unit test measured it on the real paths,
`NereidsDataDescription#analyzeWithoutCheckPriv` and the planner that
`KafkaTaskInfo#rePlan` uses: a task with enclose 0xE9 and escape 0xC3 (what
CREATE takes for `é`) gets 0xFD and 0xEF in its Thrift text parameters, and so
does every byte from 0x80 up, while all 128 ASCII bytes are kept. So on master
a job created with `"escape" = "é"` already runs with escape 0xEF, and after
this PR an ALTER to `é` gives exactly what CREATE with `é` gives. Carrying 0xC3
through planning would not load the data correctly either. Routine load data is
validated as UTF-8 by default, a single byte from 0x80 up is either absent
there or part of a character, and 0xC3 as escape would cut the first byte out
of every `é`
the way 0xEF does to `，`. No such byte works as enclose or escape, and the
documentation of both already asks for a single-byte character. So CREATE and
ALTER ROUTINE LOAD now take only an ASCII character for the two, replacing the
U+FFFD check, and a non-ASCII value that an older version recorded is ignored
with a warning when the job is rebuilt or an ALTER is replayed, as invalid
values from older ALTERs already are. Every byte a job can then hold passes the
conversion in planning unchanged, and stream load, which shares that code, is
not touched. The description said that recording the given value kept 0xEF out
of the CSV reader; it did not, since planning converts the byte again, so
CREATE records the byte as it did before and that part of the description is
rewritten. Cases:
`RoutineLoadJobReplayTest#testCreateRejectsNonAsciiEncloseAndEscape` and
`#testNonAsciiValueAnOlderAlterStoredIsIgnored`, and
`AlterRoutineLoadCommandTest#testValidateRejectsNonAsciiEncloseAndEscape`. No
committed case checks the final Thrift parameters, since no byte from 0x80 up
can reach planning from a job any more; the throwaway test above did.
F-02: confirmed, and fixed in `3f9e13b4f74` as well, though it is older than
this PR and not limited to ALTER: `setMultiTableBaseTaskInfo` never copied
enclose and escape, and the backend's per-table plan request does not carry
them, so a multi-table job did not use the values its CREATE declared either.
The overlay now copies both, as in your diff, so the tasks of a multi-table job
use the values CREATE or ALTER set. Existing multi-table CSV jobs start to
apply their declared enclose and escape once upgraded, which the release note
now says. `NereidsStreamLoadTaskTest#testMultiTableBaseTaskCopiesCsvProperties`
lays a job's `"` and `\` over a per-table request without them, and fails
without either of the two lines.
--
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]
