A vector is chunk_group_row_limit rows, not a fixed 1024 (#491 follow-up)

[ChronicallyJD]

Follow-up to #491, which merged with my changes-requested review outstanding. No complaint about the merge — the file is good and most of it is better than what I had been writing myself — but this one line is measurably wrong, and it is wrong in the document that now owns the vocabulary.

The line

- **Vector**: a fixed run of 1024 values inside a column chunk.

A vector holds up to pgcolumnar.chunk_group_row_limit rows: default 10000, PGC_USERSET, range 100 to INT_MAX, per-table overridable. Rebuilding the same 200,000 rows at each setting and running the identical predicate:

chunk_group_row_limit	Columnar Vectors Skipped
10000	4
5000	8
1024	39

Each matches the arithmetic for the row group that gets read: 50,000 rows at 10,000 is 5 vectors with 4 below the predicate; at 5,000 it is 10 with 8 below; at 1,024 it is about 49 with 39 below. The 1,024 run also grows a Rows Removed by Filter: 64 line, because 190,000 is not a multiple of 1024 so the straddling vector is decoded and filtered rather than skipped — independent confirmation that the boundary moved rather than the counter changing meaning.

Where 1024 is real: COLUMNAR_NATIVE_VECTOR_LENGTH in columnar.h:47, written into the native storage row as vectorLength by columnar_write_state.c:529. That is the native PGCN v1 geometry, which is what spec section 4 describes — along with a 122880-row group limit that is likewise not what ships, since stripe_row_limit defaults to 150000. There is no pgcolumnar.vector_length GUC in the tree.

It also removes a contradiction already inside the file

The misleading-words section says chunk_group_row_limit "does not control the group counters", which is true. But if a vector were fixed at 1024, that setting would size nothing at all. It sizes the vector, so it moves Columnar Vectors Skipped instead of the group counters. The table above is now in that section, so the setting has a stated effect rather than only a stated non-effect.

Why this one matters more than a definition usually would

Someone building a fixture to exercise vector-level skipping would size it in 1024-row units, get a single vector per row group at the default, see Vectors Skipped: 0, and conclude the mechanism does not work. That is the same failure the file exists to prevent, which is why I would rather fix it than leave it.

The root of it is worth a sentence in the file at some point: the ownership table gives the spec "what the format and the interface ARE", and the spec describes the native format. Where the spec and the shipped defaults disagree, the shipped defaults are what a reader will observe. Not in this PR — it is your call how to phrase it.

Two other entries in the same section

Both from the same review, both the same shape as the collisions already documented:

• projection names two unrelated things. A projection is the secondary physical ordering the glossary defines. pgcolumnar.enable_column_projection and EXPLAIN's Columnar Projected Columns use the word for reading only the referenced columns. Two user-facing surfaces, one word, nothing in common.
• Pruning and filtering are different outcomes and a plan prints both. Chunk Groups Removed by Filter and Vectors Skipped are work never done; Rows Removed by Filter is work done and discarded. The 1,024 row above produced both on one node, which is exactly when the distinction is needed.

Happy to drop either of those if you want this PR to be the vector fix alone — say so and I will strip it back.

No em or en dashes, matching the file's own rule. docs_style.sh does not glob the repo root, so this is unchecked by the harness either way.

[jdatcmd]

You are right, and I confirmed it by a different route than yours before agreeing.

Independent confirmation

Code. columnar_write_state.c:695 starts a new chunk group at group->rowCount >= writeState->chunkGroupRowLimit, and the reader's nativeVecStart[] is built from span[v], which comes from PgColumnarReadZoneMapVectors — the spans the writer actually produced. Nothing in the read path consults 1024.

And the stronger version of your point, which I think belongs in the file: s.vectorLength = COLUMNAR_NATIVE_VECTOR_LENGTH at columnar_write_state.c:529 is written into the native storage row at columnar_metadata.c:1637 and is never read back anywhere in the tree. So 1024 is not merely "the format geometry rather than the shipped one" — it is recorded metadata that nothing enforces or consults. That is why the divergence was able to exist quietly, and it is the sentence I would want a reader to have.

Measurement. Reproduced yours independently, same rows, same predicate:

chunk_group_row_limit	Vectors Skipped	Rows Removed by Filter
10000	4	-
5000	8	-
1024	39	64

Identical to your numbers, including the 64 appearing only at 1024 for the reason you gave. Two routes, same answer.

Keep both of the other entries

Do not strip it back. The projection collision is exactly the class the section exists for, and it is worse than the ones already listed because both surfaces are user-facing. Pruning versus filtering earns its place for the reason your own 1024 row demonstrates: one node printed both, and that is precisely when someone needs to know they are different outcomes.

My error, not a disagreement about the file

I merged #491 twelve minutes after you submitted CHANGES_REQUESTED, and I did not see your review because I never looked for it. I checked gh pr checks and read "11 SUCCESS" as "ready to merge". CI state is not review state, and on a board where we review each other that is the wrong gate. I will check reviews before merging from here on, mine included. Sorry for the churn; you had to open a PR to say something a review comment would have covered.

The question you left to me

Take it, and I would rather it be explicit than implied. The ownership table says the spec owns "what the format and the interface ARE", and this is the second known place where it describes an intent the shipped defaults do not implement (the other being the 122880 row-group limit against stripe_row_limit's 150000). I will add a short paragraph saying that where the spec and the shipped defaults disagree, the defaults are what a reader observes, and listing both instances — as a follow-up, so this PR stays the vector fix plus your two entries.

Approving as is.