Skip to content

Columnar Vectors Skipped reads 0 on the batch-fold path while decode skipped 58 of 59 vectors, so the plan denies the skipping #522 added it to show #542

Description

@ChronicallyJD

Columnar Vectors Skipped reads 0 on the batch-fold path while decode skipped 58 of 59 vectors. #522 made every node print the property; it did not make the fold path count, so the plan now affirmatively reports that no per-vector skipping happened, on the one path #512 says it matters most.

Measured

pg18a, clean build of a7d8d92 (make clean first, per #536), one row group. The fixture is test/native_fold_skipguard.sh's: 60,000 rows, chunk_group_row_limit => 1024 (59 vectors), SELECT count(*), sum(v) FROM t WHERE k BETWEEN 30000 AND 30100.

scalar arm (enable_ungrouped_vector_agg=off) batch fold (=on)
Columnar Chunk Groups Read 1 1
Columnar Vectors Skipped 58 0
Columnar Vectors Decoded 1 1
answer 101|3035050 101|3035050

Both arms return the identical, correct answer and both decode exactly one vector. They did the same work. Only one of them says so.

Mechanism

The two counters are incremented in different places:

  • vectorsSkipped++ sits in the row-production skip-ahead path (columnar_reader.c:1842), reached when the reader advances a cursor past a ruled-out vector while producing rows.
  • vectorsDecoded is accumulated in the group decode (columnar_reader.c:1760), which both arms go through.

The batch fold never produces rows one at a time — it takes the group whole through PgColumnarReadFoldNextGroup / PgColumnarReadFoldGroupInfo — so it never enters the function that counts a skip. Decoded: 1 is right on both arms because decode is shared; Skipped is a row-path counter wearing a plan-wide name.

Why this is worse than a missing number

The comment #522 added beside the property says it exactly:

it was printed by the scalar node alone, so a plan could not say whether per-vector skipping happened on the vectorized aggregate path -- the path where it matters most, since #512 is about the fold and the skip vector disagreeing and this is the number that would show it.

That is the right motivation, and the number is now present and wrong. "Not printed" sends a reader to find out; Skipped: 0 answers the question incorrectly. The neighbouring comment makes the same argument about pairs — "Skipped: 30 beside Decoded: 32 is the honest reading, and Skipped: 30 alone invited the reader to assume the work had been avoided". On the fold path the pair reads Skipped: 0, Decoded: 1, which invites the reader to conclude the group only ever had one vector.

It also silently exempts the strongest check we have

test/native_vecdecode.sh:104 is the check with teeth:

and EVERY ruled-out vector is: decoded plus skipped is the group's vectors

Its premises pin it to a columnar custom scan, so it only ever runs on the scalar arm. On the fold arm the same invariant is false by construction: 1 + 0 = 1, against 59 vectors in the group. The check cannot fail there because it never looks there, which is the shape of a guard that protects the path that was already safe.

Suggested fix

Count the skip where decode performs it rather than where rows are produced — pgcolumnar_native_decode_chunk already computes vdecoded against nativeSkipVec, so the skipped count for a group is derivable at the same point and in the same unit (vector positions, max across columns). That keeps decoded + skipped == group vectors true on both arms, which is what makes the pair readable.

Then extend native_vecdecode.sh to run its invariant on the fold arm too. Without that, the counter can regress back to 0 on that path and nothing reddens.

Found while following up the test gap I raised on #523. Related: #537 (a failure that discards its own cause), #536 (stale objects).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions