rangareddy opened a new pull request, #19974:
URL: https://github.com/apache/hudi/pull/19974
### Describe the issue this Pull Request addresses
Closes #19862.
Every `show_*` procedure that accepts both `limit` and `filter` truncated
its rows to `limit` first and evaluated `filter` on the truncated list, so
`limit => 10, filter => ...` returned the matching subset of the first 10 rows
rather than the first 10 matching rows. With a selective filter the result is
empty whenever the matches sit past the cut-off, and since every one of these
procedures applies a default `limit` (10, 20 or 100), a plain `filter => ...`
call with no explicit limit was affected too.
Introduced alongside the generic filter option in #13736 / #13790.
### Summary and Changelog
`BaseProcedure` gains `applyFilterAndLimit(results, filter, schema, limit)`,
which filters and then truncates, plus a `hasFilter` helper. The ten affected
procedures now call it. They are not all the same shape, and the difference is
the substance of the change:
**Truncating an already-materialised list.** `ShowTablePropertiesProcedure`,
`ShowFileSystemViewProcedure`, `ShowHoodieLogFileMetadataProcedure` and
`ShowMetadataTableFilesProcedure` just dropped the early `stream().limit(...)`;
the rows were already in memory, so nothing about the work done changes.
**Bounding at the source.** `ShowBootstrapMappingProcedure` and
`ShowFsPathDetailProcedure` used `df.orderBy(...).limit(n).collect()`, and
`ShowInvalidParquetProcedure` used `rdd.take(n)`. The bound now moves after the
filter. To keep the unfiltered path exactly as it was, the early bound is only
skipped when a filter is actually present.
**Bounding real work.** In `ShowCleansProcedure`, `ShowTimelineProcedure`
and `ShowHoodieLogFileRecordsProcedure`, `limit` is not display truncation: it
caps how many clean instants get their metadata read and how many records are
pulled out of log files. Filtering first without care would make those
unbounded, turning a correctness fix into a performance regression. These use
`scanLimit = if (hasFilter(filter)) Int.MaxValue else limit`, so a call with no
filter does exactly the work it did before, and only the filtered path reads
further. `limit` is never used to size an allocation on those paths, so
`Int.MaxValue` is only ever a `take` bound or a loop comparison.
`ShowTimelineProcedure` needs one extra condition. `getTimelineEntries`
deliberately ignores `limit` when both `startTime` and `endTime` are given
("Apply limit only if time range is not fully specified"), so reapplying the
bound unconditionally would have truncated a fully specified range to the
default of 20. The reapplied bound honours that case.
The six other procedures that call `applyFilter`
(`ShowBootstrapPartitionsProcedure`, `ShowColumnStatsOverlapProcedure`,
`ShowFileStatusProcedure`, `ShowMetadataTableColumnStatsProcedure`,
`ShowMetadataTablePartitionsProcedure`, `ShowMetadataTableStatsProcedure`) take
no `limit` at all and are untouched, so the list of ten in the issue is
complete.
### Verification
Two regression tests, both confirmed to fail without the corresponding fix.
`TestShowCleansProcedures`: three cleans are created, then the **oldest** is
requested with `limit => 1`. Cleans come back newest first, so the match sits
past a limit of 1.
```
Test show_cleans applies the filter before the limit *** FAILED ***
Array() had length 0 instead of expected length 1
limit must bound the rows the filter matched, not the rows the filter was
shown;
got 0 rows for clean_time = 20260916090635697
```
`TestShowTimelineTableProcedure` Test Case 13, which runs across all four
existing variants (V1/V2 x COW/MOR): a fully specified time range with `limit
=> 1` must not be truncated. Reintroducing the unconditional bound fails it
everywhere:
```
Test show_timeline with various parameters - V2 MOR *** FAILED ***
had length 1 instead of expected length 39
Test 13: a fully specified range must not be truncated by limit, expected
39 got 1
Test show_timeline with various parameters - V2 COW *** FAILED ***
had length 1 instead of expected length 34
```
Full procedure package on Spark 3.5 / Scala 2.12:
```
mvn test -Punit-tests -Dspark3.5 -Dscala-2.12 -pl
hudi-spark-datasource/hudi-spark \
-DwildcardSuites=org.apache.spark.sql.hudi.procedure
-> Suites: completed 50, aborted 0
-> Tests: succeeded 263, failed 0, canceled 0, ignored 2, pending 0
mvn scalastyle:check checkstyle:check -pl hudi-spark-datasource/hudi-spark
-> Found 0 errors
```
### Impact
`limit => n, filter => ...` now returns the first n matching rows instead of
the matches among the first n rows. Calls that pass no filter are unaffected,
including the work they do: the early bound is only lifted when a filter is
present.
One behaviour change worth calling out: with a filter,
`show_logfile_records` reads all log records rather than stopping at `limit`,
and `show_cleans` reads the metadata of every clean rather than the newest
`limit`. That is required for the filter to see the rows it is supposed to
match. These are CLI and debugging procedures, and the unfiltered path, which
is the common one, is unchanged.
### Risk Level
low: the change is confined to how `limit` and `filter` compose inside these
procedures. No API, config, or table format change, and no engine or table-type
specific behaviour is involved.
### Documentation Update
none: this restores the documented meaning of `limit` combined with `filter`
rather than changing it.
### Contributor's checklist
- [x] Read through [contributor's
guide](https://hudi.apache.org/contribute/how-to-contribute)
- [x] Enough context is provided in the sections above
- [x] Adequate tests were added if applicable
- [x] CI passes on my PR
--
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]
