hudi-agent commented on code in PR #19672:
URL: https://github.com/apache/hudi/pull/19672#discussion_r3834695475
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/config/CloudSourceConfig.java:
##########
@@ -131,7 +131,9 @@ public class CloudSourceConfig extends HoodieConfig {
.noDefaultValue()
.withAlternatives(DELTA_STREAMER_CONFIG_PREFIX +
"source.cloud.data.select.file.extension")
.markAdvanced()
- .withDocumentation("Only match files with this extension. By default,
this is the same as hoodie.streamer.source.hoodieincr.file.format");
+ .withDocumentation("Only match files with this extension. Accepts a
comma separated list, "
Review Comment:
🤖 Traced it — the doc is close but slightly imprecise. When
`CLOUD_DATAFILE_EXTENSION` is unset, `CloudObjectsSelectorCommon` falls back to
`fileFormat = CloudDataFetcher.getFileFormat(props)` (line 306).
`getFileFormat` returns `DATAFILE_FORMAT` (`...cloud.data.datafile.format`)
when that's set, and only falls back to `SOURCE_FILE_FORMAT`
(`hoodie.streamer.source.hoodieincr.file.format`, default `parquet`) when it
isn't. So the extension default really tracks the effective data file format,
which equals `hoodieincr.file.format` only when `cloud.data.datafile.format` is
also unset. Might be worth wording the doc as "defaults to the resolved data
file format" to capture that.
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/sources/helpers/CloudObjectsSelectorCommon.java:
##########
@@ -302,13 +302,33 @@ public static String generateFilter(Type type,
filter.append(SPACE_DELIMTER).append(String.format("and %s not like
'%%%s%%'", objectKey, ignoreRelativePathSubStr.get()));
}
- // Match files with a given extension, or use the fileFormat as the
default.
+ // Match files with any of the given extensions, or use the fileFormat as
the default.
getPropVal(props, CLOUD_DATAFILE_EXTENSION).or(() -> Option.of(fileFormat))
- .map(val -> filter.append(SPACE_DELIMTER).append(String.format("and %s
like '%%%s'", objectKey, val)));
+ .map(extensions -> filter.append(extensionClause(objectKey,
extensions)));
return filter.toString();
}
+ /**
+ * Renders the file extension predicate. A comma separated value matches any
one of the
+ * extensions, so a prefix holding more than one file type can be selected
in a single sync;
+ * a single value renders exactly the predicate it always did. Empty when no
usable extension
+ * is configured, which leaves the filter unchanged.
+ */
+ private static String extensionClause(String objectKey, String extensions) {
+ List<String> predicates = Arrays.stream(extensions.split(","))
+ .map(String::trim)
+ .filter(extension -> !extension.isEmpty())
+ .map(extension -> String.format("%s like '%%%s'", objectKey,
extension))
+ .collect(Collectors.toList());
+ if (predicates.isEmpty()) {
Review Comment:
🤖 When the config is set but every entry trims to empty (e.g. `" , "` or
`,`), `extensionClause` returns "" so no extension predicate is emitted at all
— and because `getPropVal` already returned present, the fallback to the data
file format is skipped too. The net effect is that objects of every extension
get selected. Is that the intended behavior for a fully-blank list, or would
falling back to the default format be safer here so a misconfiguration doesn't
silently ingest non-data files?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
