feat(utilities): select more than one file extension in cloud incremental sources - #19672
feat(utilities): select more than one file extension in cloud incremental sources#19672rahil-c wants to merge 2 commits into
Conversation
…ntal sources hoodie.streamer.source.cloud.data.select.file.extension was interpolated into a single SQL predicate, so a comma separated value produced `s3.object.key like '%json,jsonl'`: a literal match on the joined string that selects nothing, with no error to explain it. A comma separated value now renders one predicate per extension, combined with OR inside parentheses so it composes with the size and path filters. A single value renders exactly the predicate it did before, so existing pipelines are unaffected, as is the long standing fallback to the data file format when the config is unset. generateFilter had no test coverage. Six cases were added to the existing suite, covering the single and multiple forms, blank and empty entries, the format fallback, the GCS column names, and composition with the path prefix filter.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #19672 +/- ##
============================================
+ Coverage 77.83% 77.86% +0.02%
- Complexity 33140 33195 +55
============================================
Files 2527 2532 +5
Lines 139942 140066 +124
Branches 16849 16908 +59
============================================
+ Hits 108926 109064 +138
+ Misses 23396 23381 -15
- Partials 7620 7621 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
The Azure failure here looks unrelated to this change. The failing phase is This PR only touches Reviewed the change itself as part of a pass over the stack and found nothing to raise: |
| .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, " |
There was a problem hiding this comment.
is this true did you check to see this matches behavior of "hoodie.streamer.source.hoodieincr.file.format"?
There was a problem hiding this 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-agent
left a comment
hudi-agent
left a comment
There was a problem hiding this comment.
Thanks for working on this! The PR makes hoodie.streamer.source.cloud.data.select.file.extension accept a comma-separated list, rendering one OR'd like predicate per extension so mixed-extension buckets can be ingested in a single sync. The SQL precedence handling and the single-value/format-fallback backwards compatibility look correct and are now well covered by tests. One edge-case around all-blank list entries is worth double-checking in the inline comment. Please take a look, and this should be ready for a Hudi committer or PMC member to take it from here.
| .filter(extension -> !extension.isEmpty()) | ||
| .map(extension -> String.format("%s like '%%%s'", objectKey, extension)) | ||
| .collect(Collectors.toList()); | ||
| if (predicates.isEmpty()) { |
There was a problem hiding this 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?
| .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, " |
There was a problem hiding this 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.
4 participants
Describe the issue this Pull Request addresses
closes #19671
hoodie.streamer.source.cloud.data.select.file.extensionis interpolated into a single SQL predicate, so a bucket holding more than one file type cannot be ingested by one streamer. Setting a list looks like it should work and fails quietly:select.file.extension=json,jsonlgeneratess3.object.key like '%json,jsonl', a literal match on the joined string that selects nothing, and the job then commits nothing without reporting an error.Summary and Changelog
A comma separated value now renders one
likepredicate per extension, OR'd inside parentheses so it composes correctly with the size and relative-path predicates. A single value renders exactly the predicate it did before, and the fallback to the data file format when the config is unset is unchanged.generateFilterhad no test coverage, so six cases were added to the existing suite: single and multiple extensions, blank and empty list entries, the format fallback, the GCS column names, and composition with the path prefix filter.Impact
Additive and backwards compatible. Existing single-extension pipelines produce a byte-identical filter, and the config keeps its current default. Users with mixed-extension buckets can now select them in one pipeline instead of running one per extension.
Risk Level
low. The change is confined to rendering one config value into a filter string, and the previously untested single-value and format-fallback behaviours are now pinned by tests. Verified by running the new cases against the unmodified code first, where the four multiple-extension cases fail as expected (
expected: ...like '%pdf' or ...like '%docx'butwas: ...like '%pdf,docx') while the two backwards-compatibility cases pass both before and after.TestCloudObjectsSelectorCommon(27),TestS3EventsHoodieIncrSource(16),TestGcsEventsHoodieIncrSource(11),TestCloudObjectsSelector(50) andTestDeprecatedCloudIngestionConfigs(3) all pass, with checkstyle clean.Documentation Update
The config documentation on
CLOUD_DATAFILE_EXTENSIONis updated in this PR and the reference tables generate from it, so no separate website change is needed.Contributor's checklist