morningman opened a new pull request, #70118:
URL: https://github.com/apache/doris/pull/70118
### What problem does this PR solve?
Issue Number: None
Related PR: #69749
Problem Summary:
**In short.** `SHOW CREATE ROUTINE LOAD` prints the statement a job is
recreated from, but a job recreated from it was not the same job: eight
properties that CREATE accepts were never printed, a value with a quote or a
backslash made the statement unparseable, `TEMPORARY` partitions turned into
formal ones, and the comment was dropped. This PR makes the statement carry all
of them.
**Background.** Users recreate a routine load job from `SHOW CREATE ROUTINE
LOAD` to rename it, to move it to another database, or to start it over after
`STOP`; the statement prints the next Kafka offsets to consume for exactly that
purpose. #69749 made the statement carry the `compute_group` a job declares,
and found the rest of the list incomplete: `RoutineLoadJob.getShowCreateInfo()`
writes the properties by hand and covered 17 of the 25 keys in
`CreateRoutineLoadInfo.PROPERTIES_SET`.
**The problem, and what it cost.** Measured on master `1aa505cb0be` with a
unit test that creates jobs from SQL, recreates each one from its `SHOW CREATE`
output and compares every property:
| Job | Recreated from its SHOW CREATE output |
|---|---|
| CSV job with `enclose` `"`, `escape` `\`, `empty_field_as_null`,
`send_batch_parallelism` = 3, `load_to_single_tablet`, `workload_group` | all
six back to their defaults; quoted fields get split at the separators inside
the quotes |
| `unique_key_update_mode` = `UPDATE_FLEXIBLE_COLUMNS` with
`partial_update_new_key_behavior` = `ERROR` | an `UPSERT` job with `APPEND` |
| partial columns with `partial_update_new_key_behavior` = `ERROR` |
`APPEND`: rows with new keys get inserted instead of failing the load |
| JSON job with `jsonpaths` such as `["$.k1","$.v1"]` | the statement does
not parse (`mismatched input '$'`) |
| Kafka property whose value holds a `"`, such as a JAAS config | the
statement does not parse |
| `TEMPORARY PARTITION(tp1)` | printed as `PARTITION(tp1)`: `Partition tp1
does not exist` |
| `COMMENT '...'` | gone |
The jobs themselves keep running as declared; only the statement is wrong.
The cost is a recreated job that silently loads differently (CSV parsing,
partial update semantics, single-tablet loading, workload group, parallelism),
or a statement that cannot be executed at all.
**How this PR fixes it.**
- `getShowCreateInfo()` prints the eight missing properties wherever leaving
one out would give the recreated job a different value:
`unique_key_update_mode` when it is not `UPSERT` (`partial_columns` alone
cannot express `UPDATE_FLEXIBLE_COLUMNS`), `partial_update_new_key_behavior`
when it is `ERROR` and a partial update mode is on (CREATE rejects the policy
otherwise), `enclose` and `escape` when set, `empty_field_as_null` and
`load_to_single_tablet` when true, and `workload_group` when set.
`send_batch_parallelism` is always printed, as the timezone is, because CREATE
otherwise takes it from the session variable of whoever recreates the job. The
values are the ones the job runs with.
- Keys and values are quoted with `SqlLiteralUtils.quoteStringLiteral`,
which follows the session's `NO_BACKSLASH_ESCAPES` mode. A value without a
quote or a backslash prints exactly as before; the column separator is left as
it is, since it is kept as typed and already reads back.
- `TEMPORARY PARTITION(...)` keeps `TEMPORARY`, and the job's comment is
printed as `COMMENT "..."`.
- `CREATE ROUTINE LOAD` reads its `COMMENT` with
`SqlLiteralUtils.parseStringLiteral`, as table, view and column comments are
read, so that a printed comment with a quote reads back unchanged. It used to
strip the outer quotes only, so `COMMENT 'it''s'` was stored as `it''s`.
For a job that uses none of these, the output gains a single line,
`"send_batch_parallelism" = "<the job's value>"`.
**Results.**
| Job | Before | After |
|---|---|---|
| CSV job above | 6 properties and the comment lost | recreated with the
same definition |
| flexible partial update with `ERROR` | `UPSERT` / `APPEND` | same
definition |
| JSON job with `jsonpaths` | statement does not parse | same definition |
| Kafka property with `"` and `\` | statement does not parse | same
definition |
| temporary partition | `Partition tp1 does not exist` | same definition |
| `NO_BACKSLASH_ESCAPES` session, `enclose` `"`, `escape` `\` | both lost |
same definition |
The same test also found that a job's `max_filter_ratio`, `exec_mem_limit`,
`send_batch_parallelism`, `load_to_single_tablet`, `enclose` and `escape` fall
back to their defaults when a follower replays the job or the FE restarts, and
that `ALTER ROUTINE LOAD` on `enclose` / `escape` does not reach the running
job. That is a separate defect and will be fixed in its own PR; since this PR
prints the values the job runs with, the statement follows that fix. branch-4.1
and branch-4.2 have the same gaps as master.
### Release note
SHOW CREATE ROUTINE LOAD now prints send_batch_parallelism,
load_to_single_tablet, workload_group, unique_key_update_mode,
partial_update_new_key_behavior, enclose, escape, empty_field_as_null,
TEMPORARY partitions and the job comment, and quotes property values, so that
the statement it prints can be executed again. CREATE ROUTINE LOAD now reads a
doubled quote in its COMMENT as a single quote.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
New `RoutineLoadShowCreateRoundTripTest` (8 cases on a mocked FE: the
CSV job above, flexible and fixed partial updates, defaults that must not be
printed, `WITH MERGE`, a temporary partition, a quoted Kafka property, a
`NO_BACKSLASH_ESCAPES` session) recreates each job from its statement, compares
every property and requires the recreated job to print the same statement.
`compute_group` can only be declared in cloud mode and stays covered by
`RoutineLoadJobTest#testGetShowCreateInfoCarriesDeclaredComputeGroup`; a new
`RoutineLoadJobTest` case pins the printed format. 11 routine load test
classes, 108 cases, pass. With the two production files reverted to master, 6
of the 8 round trip cases and the 3 exact-output cases fail; the other 2 round
trip cases guard that defaults stay out of the statement. Each part of the fix
was also removed on its own, and each removal fails its own case.
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change -->
The output of SHOW CREATE ROUTINE LOAD, as in the release note; jobs
that use none of the new properties only gain the `send_batch_parallelism`
line. CREATE ROUTINE LOAD reads a doubled quote in COMMENT as one quote.
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
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]
