fix(table): keep every split of a format table under a positive LIMIT - #644
Open
sundapeng wants to merge 1 commit into
Open
fix(table): keep every split of a format table under a positive LIMIT#644sundapeng wants to merge 1 commit into
sundapeng wants to merge 1 commit into
Conversation
A format table has no manifest, so scan planning does not know how many rows a data file holds. apply_limit_pushdown kept only the first `limit` splits anyway, which reads the number of files as if it were a number of rows. An empty data file is enough to break it. An engine that writes one file per task leaves one behind whenever a task matched nothing, so a table of three rows whose first file is empty answers `SELECT id FROM t LIMIT 1` with zero rows, and reports success. Keep every split for a positive limit and let execution stop when it has enough rows. A limit of zero still needs no split. This is what Java does: FormatTableScan.FormatTableScanPlan.splits returns an empty list only for a non-positive limit, with the same reason in a comment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Linked issue: close #643
A format table has no manifest, so scan planning does not know how many rows a data file holds.
FormatTableScan::apply_limit_pushdownkept only the firstlimitsplits anyway, which reads the number of files as if it were a number of rows.An empty data file is enough to break it, and an empty data file is ordinary output: an engine that writes one file per task leaves one behind whenever a task matched nothing. A table of three rows whose first file is empty answers
SELECT id FROM t LIMIT 1with zero rows, and reports success.Brief change log
This is what Java does.
FormatTableScan.FormatTableScanPlan.splitsreturns an empty list only for a non-positive limit, with the same reason in a comment. paimon-rust's own Paimon-table path is already stricter than the format path was:PaimonTableScan::apply_limit_pushdownskips a split whose merged row count is unknown rather than counting it as if it held enough rows.Tests
crates/integrations/datafusion/tests/format_table_statistics.rs:test_format_table_limit_is_not_answered_from_a_file_countwrites an empty file followed by a three-row file and assertsLIMIT 1returns a row. It fails on main withleft: 0, right: 1.API and Format
No API or storage format change.
Documentation
None needed.