github-actions[bot] commented on code in PR #64071:
URL: https://github.com/apache/doris/pull/64071#discussion_r3556461079
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AlterRoutineLoadCommand.java:
##########
@@ -313,6 +314,15 @@ private void checkJobProperties() throws UserException {
analyzedJobProperties.put(CsvFileFormatProperties.PROP_EMPTY_FIELD_AS_NULL,
String.valueOf(emptyFieldAsNull));
}
+
+ if
(jobProperties.containsKey(JsonFileFormatProperties.PROP_FILL_MISSING_COLUMNS))
{
Review Comment:
The alter path still lets this JSON-only property through for existing
CSV/default jobs. `validate()` already loads the target job, but
`checkJobProperties()` only checks that `fill_missing_columns` is `true` or
`false` and then puts it into `analyzedJobProperties`; the Kafka/Kinesis
`modifyPropertiesInternal()` paths later merge that map directly into persisted
`jobProperties`. Since routine-load format is not changed by this alter
command, `ALTER ROUTINE LOAD ... PROPERTIES("fill_missing_columns"="true")` on
a CSV job stores a value that the CSV scan path ignores, while the create path
now rejects the same property for non-JSON formats. Please reject this alter
property unless `job.getFormat()` is JSON, and add a default/CSV alter negative
test.
##########
fe/fe-core/src/main/java/org/apache/doris/load/routineload/RoutineLoadJob.java:
##########
@@ -1809,6 +1815,7 @@ public String getShowCreateInfo() {
appendProperties(sb, JsonFileFormatProperties.PROP_NUM_AS_STRING,
isNumAsString(), false);
appendProperties(sb, JsonFileFormatProperties.PROP_FUZZY_PARSE,
isFuzzyParse(), false);
appendProperties(sb, JsonFileFormatProperties.PROP_JSON_ROOT,
getJsonRoot(), false);
+ appendProperties(sb,
JsonFileFormatProperties.PROP_FILL_MISSING_COLUMNS, isFillMissingColumns(),
false);
Review Comment:
`getShowCreateInfo()` now always prints `"fill_missing_columns" = "false"`
even for CSV/default routine-load jobs. That makes the generated statement
non-round-trippable: `getFormat()` defaults to `"csv"`, and
`CreateRoutineLoadInfo.checkJobProperties()` now rejects any
`fill_missing_columns` property unless `format` is JSON. The updated CSV
show-create test also locks in `"format" = "csv"` plus this JSON-only property,
so copying the generated DDL back into `CREATE ROUTINE LOAD` fails. Please emit
this property only for JSON jobs, or otherwise make the create-path guard
accept the generated CSV/default false value and cover the round trip.
--
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]
