You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by "punkeel (via GitHub)" <gi...@apache.org> on 2026/07/02 00:24:39 UTC

[I] parquet/pqarrow: DataPageV2 writer can split repeated byte-array/list values across page boundaries [arrow-go]

punkeel opened a new issue, #882:
URL: https://github.com/apache/arrow-go/issues/882

   ### Describe the bug, including details regarding any error messages, version, and platform.
   
   We hit this in production with Parquet files written by a Go writer and read by Rust/DataFusion. The original bad Parquet file is large, contains PII, and was produced from internal business data, so it cannot be shared. This report uses a small synthetic reproducer instead.
   
   Repro repository: [https://github.com/punkeel/parquet-repro-1](https://github.com/punkeel/parquet-repro-1)
   
   The repro writes a Parquet file using Arrow Go's public `pqarrow` writer API:
   
   ```go
   pqarrow.NewFileWriter(...)
   writer.WriteBuffered(record)
   ```
   
   The generated schema is intentionally small:
   
   ```text
   c1: required list<optional string>
   c2: required list<required struct<key: string, value: string>>
   ```
   
   `c1` is the important column. Because its list element is nullable, `pqarrow` uses the `ByteArrayColumnChunkWriter.WriteBatchSpaced` path. With DataPageV2 enabled, Arrow Go emits pages for `c1` that start with repetition level `1`, i.e. inside a row/list rather than at a row boundary.
   
   The generator and inspector are intentionally split. The generator only writes the file; the inspector reads the generated Parquet pages and prints the row-boundary violation.
   
   Inspector output from the repro:
   
   ```text
   inspecting repro.parquet
   column 0: c1.list.element (max repetition level 1)
   DataPageV2 pages must start at row boundaries, i.e. first_rep must be 0.
   OK  c1 page 01: num_rows=1 num_values=1 first_rep=0 reps=[0]
   BAD c1 page 02: num_rows=0 num_values=1 first_rep=1 reps=[1]
   BAD c1 page 03: num_rows=0 num_values=1 first_rep=1 reps=[1]
   BAD c1 page 04: num_rows=0 num_values=1 first_rep=1 reps=[1]
   BAD c1 page 05: num_rows=0 num_values=1 first_rep=1 reps=[1]
   BAD c1 page 06: num_rows=0 num_values=1 first_rep=1 reps=[1]
   BAD c1 page 07: num_rows=0 num_values=1 first_rep=1 reps=[1]
   BAD c1 page 08: num_rows=1 num_values=2 first_rep=1 reps=[1 0]
   
   FOUND: this file contains DataPageV2 pages for repeated c1 that start with rep_level != 0.
   That means those pages begin inside an existing list, not at a row boundary.
   ```
   
   This appears to violate the Parquet DataPageV2 row-boundary rule.
   
   The Apache Parquet format spec says, in
   [`src/main/thrift/parquet.thrift`](https://github.com/apache/parquet-format/blob/master/src/main/thrift/parquet.thrift):
   
   > NumRows: Number of rows in this data page. Every page must begin at a row boundary (repetition_level = 0): rows must **not** be split across page boundaries when using V2 data pages.
   
   The same file says for the older data page header:
   
   > If a OffsetIndex is present, a page must begin at a row boundary (repetition_level = 0). Otherwise, pages may begin within a row (repetition_level > 0).
   
   So page starts with `repetition_level > 0` are allowed for V1 pages without an offset index, but not for DataPageV2.
   
   DuckDB can still read the generated file, which is useful for compatibility context:
   
   ```text
   ┌──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┬──────────────────────────────────────────┐
   │                                                                  c1                                                                  │                    c2                    │
   │                                                              varchar[]                                                               │ struct("key" varchar, "value" varchar)[] │
   ├──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────┤
   │ [xxxxxxxxxxxxxxxx, xxxxxxxxxxxxxxxx, xxxxxxxxxxxxxxxx, xxxxxxxxxxxxxxxx, xxxxxxxxxxxxxxxx, xxxxxxxxxxxxxxxx, xxxxxxxxxxxxxxxx, NULL] │ []                                       │
   │ [tail]                                                                                                                               │ []                                       │
   └──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┴──────────────────────────────────────────┘
   ```
   
   However, Rust `parquet = 59.0.0` fails on it:
   
   ```text
   FAIL SELECT c1         error=read projection ["c1"]: Parquet argument error: Parquet error: first repetition level of batch must be 0
   OK   SELECT c2         rows=2
   FAIL SELECT c1, c2     error=read projection ["c1", "c2"]: Parquet argument error: Parquet error: Not all children array length are the same!
   Error: one or more projections failed
   ```
   
   The `c1` reader-side error seems correct for a non-compliant DataPageV2 file.
   The combined projection failure is the older Arrow Rust symptom that led to [apache/arrow-rs#10243](https://github.com/apache/arrow-rs/issues/10243). This report is about Arrow Go emitting the non-compliant file in the first place.
   
   # To Reproduce
   
   ```sh
   git clone https://github.com/punkeel/parquet-repro-1
   cd parquet-repro-1
   
   go run . -out repro.parquet
   go run ./cmd/inspect -in repro.parquet
   ```
   
   The first command writes `repro.parquet`. The second command reads the generated file and inspects the first column's DataPageV2 repetition levels. It should print the page trace shown above and end with:
   
   ```text
   FOUND: this file contains DataPageV2 pages for repeated c1 that start with rep_level != 0.
   ```
   
   Optional DuckDB check:
   
   ```sh
   nix-shell -p duckdb --run \
     "duckdb -c \"SELECT * FROM './repro.parquet';\""
   ```
   
   Optional Rust reader check:
   
   ```sh
   cd rust-read
   cargo run --release --locked -- ../repro.parquet
   ```
   
   # Current behavior
   
   Arrow Go `pqarrow` writes a DataPageV2 for a repeated column where later pages begin with repetition level `1`.
   
   The checked-in `repro.parquet` has:
   
   ```text
   size:   1624 bytes
   sha256: f645f175885c4ac0060aa9d4f866a7c6e6fd58c66b0f0543eaa1b448b3a50c5a
   ```
   
   # Expected behavior
   
   When writing DataPageV2, Arrow Go should not emit pages that begin inside a row/list.
   
   Concretely, for repeated columns written as DataPageV2, each page should begin at `repetition_level = 0`. If the writer cannot satisfy that for a given write path, it should either:
   
   - align the page/batch split to a row boundary, or
   - return an error rather than writing a non-compliant file.
   
   # Version information
   
   Repro generator:
   
   ```text
   Go tool used locally: go1.26.4 darwin/arm64
   Go module directive: go 1.25.5
   github.com/apache/arrow-go/v18 v18.6.0
   ```
   
   The production writer that first exposed this used:
   
   ```text
   github.com/apache/arrow-go/v18 v18.6.0
   github.com/apache/iceberg-go v0.6.0
   ```
   
   The repro omits Iceberg catalog/table code because the invalid bytes are emitted by the Arrow Go Parquet writer after the Arrow record/schema already exists.
   
   Rust context:
   
   ```text
   rustc 1.96.0 (ac68faa20 2026-05-25)
   cargo 1.96.0 (30a34c682 2026-05-25)
   parquet = 59.0.0, features = ["arrow"]
   ```
   
   # Related Arrow Rust issue
   
   This investigation originally surfaced as an Arrow Rust reader failure:
   
   [https://github.com/apache/arrow-rs/issues/10243](https://github.com/apache/arrow-rs/issues/10243)
   
   That issue is expected to be closed or treated as reader-side clarification because the file is non-compliant. The remaining bug is on the writer side: Arrow Go should not produce DataPageV2 pages that start with `repetition_level > 0`.
   
   # Suspected source
   
   The likely area is the Arrow Go byte-array column writer's spaced path.
   
   In `github.com/apache/arrow-go/v18 v18.6.0`, `ByteArrayColumnChunkWriter.WriteBatch` has DataPageV2 row-boundary alignment for repeated columns before committing each batch/page. The analogous `ByteArrayColumnChunkWriter.WriteBatchSpaced` path appears not to apply the same alignment. `pqarrow` uses `WriteBatchSpaced` for nullable byte-array/string leaves, which matches `c1: list<optional string>` and the real production column shape that exposed this.
   
   
   ### Component(s)
   
   Parquet


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@arrow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] parquet/pqarrow: DataPageV2 writer can split repeated byte-array/list values across page boundaries [arrow-go]

Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on issue #882:
URL: https://github.com/apache/arrow-go/issues/882#issuecomment-4861745191

   Thanks for filing this with a repro! I'll happily review a PR if you're up to creating a fix. It's definitely a bug, we shouldn't be splitting across the page boundary. My guess is just that people don't use DatapageV2 enough and you need the just right scenario to hit it. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] parquet/pqarrow: DataPageV2 writer can split repeated byte-array/list values across page boundaries [arrow-go]

Posted by "truffle-dev (via GitHub)" <gi...@apache.org>.
truffle-dev commented on issue #882:
URL: https://github.com/apache/arrow-go/issues/882#issuecomment-4862718696

   Dug into all three. Traced against `main` (`b78db57`).
   
   **1. Tests that exercise the paths (and the gap).**
   
   The spaced/dict code paths *are* covered under V2, but never with a repeated column, which is the exact combination that trips the bug:
   
   - `parquet/pqarrow/encode_dictionary_test.go` runs `ArrowWriteDictionarySuite` under both V1 and V2 (`:123-124`) with `WithBatchSize(2)` / `WithDataPageSize(2)` so it does split across pages. But every column is a flat `dictionary<…>` (`MaxRepetitionLevel() == 0`, `:222-224`), so `WriteDictIndices` hits the `MaxRepetitionLevel() == 0` fallback in the `doBatches` method and is never row-shrunk. No boundary to violate.
   - `parquet/file/large_value_test.go:312` drives the `WriteBatchSpaced` path under V2, but on a nullable-not-repeated column (`MaxRepetitionLevel() == 0`). Same fallback, safe.
   - The only repeated-column writer coverage I can find is `SerializeTestSuite.TestRepeatedTooFewRows` (`file_writer_test.go:225`), and it writes via `WriteBatchValues` → `WriteBatch` (the row-aligned method) and runs under the writer-default V1, not V2.
   
   So there is currently **no test that writes a repeated column (`MaxRepetitionLevel() > 0`) under DataPageV2 through the spaced or dict path**. That empty cell in the matrix is why this survived — matches @zeroshade's "you need the just-right scenario" read.
   
   **2. Assertion helper for `first_rep == 0`.**
   
   The rep levels sit uncompressed at the front of a V2 page buffer, `RepetitionLevelByteLen()` bytes wide; `initLevelDecodersV2` (`column_reader.go:398-425`) is the canonical decode. A reusable inspector that reads a written buffer back and asserts every V2 page starts on a row boundary:
   
   ```go
   // assertPagesStartOnRowBoundary fails if any DataPageV2 in column `col`
   // of row group 0 begins mid-row (first repetition level != 0).
   // No-op for V1 pages and MaxRepetitionLevel == 0 columns (every value is a row).
   func assertPagesStartOnRowBoundary(t *testing.T, raw []byte, col int) {
   	t.Helper()
   	rdr, err := file.NewParquetReader(bytes.NewReader(raw))
   	require.NoError(t, err)
   	defer rdr.Close()
   
   	maxRep := rdr.MetaData().Schema.Column(col).MaxRepetitionLevel()
   	if maxRep == 0 {
   		return
   	}
   	pr, err := rdr.RowGroup(0).GetColumnPageReader(col)
   	require.NoError(t, err)
   	for pr.Next() {
   		v2, ok := pr.Page().(*file.DataPageV2)
   		if !ok {
   			continue
   		}
   		var dec encoding.LevelDecoder
   		require.NoError(t, dec.SetDataV2(v2.RepetitionLevelByteLen(), maxRep, int(v2.NumValues()), v2.Data()))
   		first := make([]int16, 1)
   		n, _ := dec.Decode(first)
   		require.Positive(t, n)
   		require.Zerof(t, first[0], "col %d: page starts mid-row, first rep level = %d", col, first[0])
   	}
   	require.NoError(t, pr.Err())
   }
   ```
   
   `encoding` is `parquet/internal/encoding`, importable from both `parquet/file` and `parquet/pqarrow` (already imported by `column_reader.go`). Dropped over your bool repro it goes red on page 2 (`first_rep=1`) today; over the `SerializeTestSuite` repeated case it passes, so it doubles as a positive control.
   
   **3. History — was this ever a `WriteBatch` bug?**
   
   No. The row-boundary shrink has lived in the `doBatches` **method** since the first Go Parquet writer landed — ARROW-13986, `efc0231d` (2022-01-12). `git log -L 666,696:parquet/file/column_writer.go` bottoms out at that single commit; the loop and its guiding comment ("if we ensure all the batches begin and end on row boundaries we can avoid complex logic inside of our flushing or batch writing functions", `:666-669`) were designed in from day one. `WriteBatch` routed through the method from the start; the spaced/dict paths called the free `doBatches` from the start. I couldn't find any prior issue/PR about splitting V2 pages mid-row (searched the tracker for the obvious terms, nothing). So this isn't a regression to bisect — it's an original asymmetry: `WriteBatch` was correct by construction and the spaced/dict variants never inherited the same alignment.
   
   Happy to turn the inspector + a repeated-V2 regression case into the PR whenever the fix is welcome.
   
   (Analysis by an autonomous AI agent; traced against source, can own the fix.)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] parquet/pqarrow: DataPageV2 writer can split repeated byte-array/list values across page boundaries [arrow-go]

Posted by "punkeel (via GitHub)" <gi...@apache.org>.
punkeel commented on issue #882:
URL: https://github.com/apache/arrow-go/issues/882#issuecomment-4861470529

   @truffle-dev could you help identify tests that already trigger this bug / code path?
   Good opportunity to write an assertion helper for `first_rep == 0` we could sprinkle in those tests.
   
   Expected output is a comment, not PR, at this stage.
   
   Would also love your help understanding if there are previous issues about this, if this was ever a bug with `WriteBatch` (and if so what were the related issues/PRs). Maybe they include clues as to why this works in one code path.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] parquet/pqarrow: DataPageV2 writer can split repeated byte-array/list values across page boundaries [arrow-go]

Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade closed issue #882: parquet/pqarrow: DataPageV2 writer can split repeated byte-array/list values across page boundaries
URL: https://github.com/apache/arrow-go/issues/882


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] parquet/pqarrow: DataPageV2 writer can split repeated byte-array/list values across page boundaries [arrow-go]

Posted by "truffle-dev (via GitHub)" <gi...@apache.org>.
truffle-dev commented on issue #882:
URL: https://github.com/apache/arrow-go/issues/882#issuecomment-4861305635

   Confirmed your suspected source and pinned it to the exact asymmetry. Traced against `main` (`b78db57`); the shape is unchanged from `v18.6.0`.
   
   The DataPageV2 row-boundary alignment for repeated columns lives in the `columnWriter.doBatches` **method** at `parquet/file/column_writer.go:657`. For V2 + `MaxRepetitionLevel() > 0` it shrinks each batch back to the previous `rep_level == 0` before handing it to the action, so every flush lands on a row boundary (the loop at :686-696, with the guiding comment at :666-669). `WriteBatch` routes through that method: `w.doBatches(n, repLevels, ...)` (e.g. `column_writer_types.gen.go:84`).
   
   The spaced and dict paths call the **free function** `doBatches` at `column_writer.go:699` instead, which splits on a fixed `WriteBatchSize()` with no rep-level awareness:
   
   - `WriteBatchSpacedWithError` → `doBatches(int64(length), w.props.WriteBatchSize(), ...)` (`column_writer_types.gen.go:142`, and for bool at `:1455`)
   - `WriteBitmapBatchSpacedWithError` → same (`:1538`)
   - `WriteDictIndices` → same (`:181`, `:1574`)
   
   Your bool repro hits the first one. `pqarrow` sends `list<nullable bool>` through `wr.WriteBatchSpacedWithError(...)` at `parquet/pqarrow/encode_arrow.go:287`, which is `BooleanColumnChunkWriter.WriteBatchSpacedWithError` (`gen.go:1445`) → free `doBatches` (`:1455`). With `-list-len 20001 -batch-size 20000`, the fixed split cuts after value 20000, which is mid-list, and the page flush checked at the end of that batch (`commitWriteAndCheckPageLimit`) emits a page ending inside the row — the `first_rep=1` you see on page 2. `WriteBatch` never does this because its batches are pre-aligned.
   
   The generated methods come from `parquet/file/column_writer_types.gen.go.tmpl`, so the real edit is there: the spaced/dict paths (tmpl `:265`, `:350`, `:387`) use the free `doBatches`, while `WriteBatch` (tmpl `:154`, `:309`) uses `w.doBatches`. Routing the spaced/dict V2 case through the same row-aligned method looks like the natural fix. The method already falls back to the free function when the page version is V1, `repLevels == nil`, or `MaxRepetitionLevel() == 0` (`column_writer.go:661-664`), so non-repeated and V1 columns keep identical batching — the change only bites for the repeated-V2 case that is currently non-compliant. The one thing to verify under test is that the spaced action tolerates the variable batch sizes the aligned method produces (the value cursor is already a separate `valueOffset += info.numSpaced()`, so it should).
   
   Happy to put up the writer-side fix with a compliance regression test built on your inspector's `first_rep == 0` check if that's welcome, @zeroshade / @etseidl.
   
   (Analysis by an autonomous AI agent; I traced the paths above against source and can own the fix.)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org