Skip to content

[core] Carry the row count and byte size a format table writer already counted - #9121

Open
sundapeng wants to merge 2 commits into
apache:masterfrom
sundapeng:upstream/s1-carry-format-table-written-counts
Open

[core] Carry the row count and byte size a format table writer already counted#9121
sundapeng wants to merge 2 commits into
apache:masterfrom
sundapeng:upstream/s1-carry-format-table-written-counts

Conversation

@sundapeng

Copy link
Copy Markdown
Member

Stacked on #9120 — that commit shows up here too until it merges. Review only
[core] Carry the row count and byte size a format table writer already counted.

Purpose

FormatTableRollingFileWriter counts every row it writes and FormatTableSingleFileWriter knows the
byte length of the file it closed. Both numbers are then dropped: closeAndGetCommitters returns only
the committers, and prepareCommit wraps each one in a TwoPhaseCommitMessage that carries nothing
else. Anything downstream that wants to know what a commit wrote has to go back to the filesystem and
list it.

This keeps the two numbers attached to the file they describe, in a new FormatTableWrittenFile that
pairs the committer with them, and lets TwoPhaseCommitMessage carry it.

Nothing reads them yet. That is deliberate — this PR has no behaviour to review, only a data flow
that stops throwing information away.

What is deliberately not touched

TwoPhaseOutputStream.Committer. RenamingTwoPhaseOutputStream is @Public, so adding a method to
the type its committer() returns would break external implementations. The counts ride the
paimon-core commit message instead.

Context

Next in the stack behind #9120. A catalog-managed format table has no snapshot, so it has no channel
for partition statistics; the numbers a commit could report are exact and free here, and are currently
discarded at this point.

API and Format

New paimon-core internal class FormatTableWrittenFile. No public API change. No format change —
nothing new is written to storage.

Documentation

None needed.

PartitionStatistics said only that its fields "may be negative, indicating that
some data has been removed". That covers one of the two planes the class is read
on, and consumers have been getting the other one wrong.

On the delta plane — what a commit changed — a negative value is a decrement the
server adds to what it holds. That is the existing meaning and nothing here
changes it. On the observation plane — what listPartitions returns for a
partition as it stands — a negative value means nobody ever reported that field,
and 0 means an exact zero. Conflating them is not cosmetic: a consumer that reads
unknown as zero plans against an empty partition that may hold a billion rows,
and one that does arithmetic on it gets a number that is wrong rather than
missing.

So the plane is named in the javadoc, unknown gets a name (UNKNOWN, with
isKnown() to test it rather than each caller comparing against -1), and unknown
is documented as per field: a reporter that only knows the file count leaves the
record count unknown and fills the rest.

The fields stay primitive. Boxing them to express unknown as null would be a
breaking change to a @public class, and the encoding above needs no new type.

FileSystemSplitEnumerator now says PartitionStatistics.UNKNOWN where it said -1.
Discovering partitions by listing directories measures nothing about what is
inside them, which is what unknown already meant there; this is the same value
under its own name.
…y counted

FormatTableRollingFileWriter counts every row it writes and
FormatTableSingleFileWriter knows the byte length of the file it closed. Both
numbers are then dropped: closeAndGetCommitters returns only the committers, and
prepareCommit wraps each one in a TwoPhaseCommitMessage that carries nothing
else. Anything downstream that wants to know what a commit wrote has to go back
to the filesystem and list it.

This keeps the two numbers attached to the file they describe, in a new
FormatTableWrittenFile that pairs the committer with them, and lets
TwoPhaseCommitMessage carry it. Nothing reads them yet.

TwoPhaseOutputStream.Committer is untouched. RenamingTwoPhaseOutputStream is
@public, so adding a method to the type its committer() returns would break
external implementations; the counts ride the paimon-core commit message instead.
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.

1 participant