Skip to content

fix(table): compute COUNT(*) for format tables instead of returning 0 - #624

Merged
JingsongLi merged 1 commit into
apache:mainfrom
sundapeng:fix/format-table-count-from-placeholder-row-count
Jul 28, 2026
Merged

fix(table): compute COUNT(*) for format tables instead of returning 0#624
JingsongLi merged 1 commit into
apache:mainfrom
sundapeng:fix/format-table-count-from-placeholder-row-count

Conversation

@sundapeng

@sundapeng sundapeng commented Jul 28, 2026

Copy link
Copy Markdown
Member

Purpose

A format table has no manifest, so scan planning cannot know how many rows a data file holds. format_table_scan recorded that as row_count: 0 — and 0 is a number a reader is entitled to trust. Two consumers trusted it:

  • DataSplit::raw_merged_row_count() returned Some(0), DataFusion took it as Precision::Exact(0), and the aggregate_statistics rule rewrote SELECT COUNT(*) into the constant 0 without opening a file. SELECT * was unaffected because it reads the files, so the same table answered 0 rows and returned rows.
  • ColumnStatsAccumulator::add_file returns early for a file whose row_count is 0, so every file looked empty and the accumulator's initial null_count = 0 went out as Precision::Exact — a statistic derived from files nobody read.

Approach

Make "unknown" expressible instead of overloading a legal value. DataFileMeta::ROW_COUNT_UNKNOWN is -1, with row_count_known() to test it:

  • merged_row_count() returns None as soon as one file is unknown, so no arithmetic over the placeholder ever reaches a caller;
  • row_count() sums only known files and is documented as a lower bound, with row_counts_known() to ask before presenting it as a total;
  • format_table_scan fills in the sentinel.

Nothing here is specific to format tables — any producer that cannot determine a row count can adopt the convention — and a file that genuinely holds zero rows still reports 0 and stays exact.

The column-statistics path is fixed by the same change, but indirectly: -1 no longer matches the row_count == 0 guard, so add_file falls through to value_stats_for_field, finds no statistics, and marks min/max/null invalid. Making that check explicit (row_count_known() before the == 0 early return) would say what is meant rather than relying on the sentinel missing the guard; happy to add it if reviewers prefer.

Tests

source.rs unit tests, on merged_row_count():

case pins
every file unknown None, and row_count() is 0 rather than a total
one unknown among known files None, row_count() is the 10 it does know
unknown with first_row_id set the data-evolution branch does not rescue it
a file that really holds 0 rows stays Some(0) — unknown is a distinct value, not a synonym for empty

crates/integrations/datafusion/tests/format_table_statistics.rs, end to end through SQL:

case pins
two files, five rows COUNT(*) and COUNT(id) both equal what a scan returns; 0 before this change
no data files still exactly 0
one file holding no rows still exactly 0
partitioned, with a partition predicate pruned plans reach partition_statistics() too

cargo test -p paimon --lib source:: stays green.

Notes

  • DataSplit::serialize writes row_count into the v8 wire format as-is, so a consumer that reads splits planned by this crate needs to understand the sentinel. Flagging in case a cross-language path exists that I am not aware of.
  • The partitioned test asserts only predicated queries. An unfiltered scan over a partitioned format table on a file:// warehouse currently finds no splits at all: table_path keeps the file:/// form while listed status paths come back as file:/, so the strip_prefix in partition_row_from_path drops every file. Separate defect, not touched here.

A format table has no manifest, so planning cannot know how many rows a data
file holds. `format_table_scan` recorded that as `row_count: 0`, and 0 is a
number a reader is entitled to trust:

- `DataSplit::raw_merged_row_count` returned `Some(0)` for a raw-convertible
  split with no deletion files;
- DataFusion turned that into `Precision::Exact(0)` for the whole scan;
- the `aggregate_statistics` rule then rewrote `SELECT COUNT(*)` into the
  constant 0 and never opened a file.

`SELECT *` was unaffected because it reads the files, which is why the gap went
unnoticed: the same table answered 0 rows and returned rows.

The same 0 also silenced column statistics. `ColumnStatsAccumulator::add_file`
skips a file whose `row_count` is 0, so every format-table file was treated as
empty and the accumulator kept its default `null_count = 0`, handed out as
`Precision::Exact` — an exact answer derived from files nobody read.

Make "unknown" expressible instead of overloading a legal value.
`DataFileMeta::ROW_COUNT_UNKNOWN` is -1 and `row_count_known()` tests for it;
`DataSplit::row_counts_known()` reports whether a whole split is known, and
`merged_row_count()` returns `None` as soon as one file is not. `row_count()`
now sums only known files and is documented as a lower bound rather than a
total. Any producer that cannot determine a row count can adopt the same
convention; nothing here is specific to format tables.

Tests: a format table whose `COUNT(*)` must match the rows a scan returns
(0 instead of 5 before this change), an empty table and a table holding an
empty file that must still count 0 exactly, and a partitioned table with a
partition predicate — that path was already correct, because a pushed-down
predicate makes the statistics inexact and the rule does not fire.
@sundapeng sundapeng changed the title fix(table): stop answering COUNT(*) from a placeholder row count fix(table): stop COUNT(*) returning 0 for format tables that have rows Jul 28, 2026
@sundapeng sundapeng changed the title fix(table): stop COUNT(*) returning 0 for format tables that have rows fix(table): compute COUNT(*) for format tables instead of returning 0 Jul 28, 2026

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@JingsongLi
JingsongLi merged commit 82cecf1 into apache:main Jul 28, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants