hcrosse opened a new issue, #1445: URL: https://github.com/apache/arrow-go/issues/1445
### Describe the enhancement requested Reading an optional dictionary column does extra work even when none of its values are null. With `SetReadDict` enabled, 1,000 distinct values and 65,536-row batches, it takes about 1.8x the required column's read time (8.37 vs 4.62 ns/row). [`recordReader.ReadRecordData`](https://github.com/apache/arrow-go/blob/22c9f26df0abbc2066f5d9ae45f31720d29a4300/parquet/file/record_reader.go#L673-L685) chooses spaced or dense decoding from the schema, not the batch's null count. Every batch of an optional column therefore takes the spaced path. [`DecodeIndicesSpaced`](https://github.com/apache/arrow-go/blob/22c9f26df0abbc2066f5d9ae45f31720d29a4300/parquet/internal/encoding/decoder.go#L198-L230) fills a per-value `[]bool` and passes it to `AppendIndices`, even when the batch has no nulls. The shared record reader makes this choice for binary and int32 readers too. **Measurements** I compared main with a patch that uses dense decoding for batches with no nulls. The input was 2 million synthetic 24-byte strings in one dictionary-encoded row group. I ran six interleaved rounds of each version and analyzed the results with benchstat. All runs used one Linux amd64 VM with 8 AMD EPYC vCPUs, 16 GiB RAM and Go 1.26.10. `SetReadDict` was enabled and every data page was `RLE_DICTIONARY`. These are medians in ns/row for optional columns with no nulls. | Distinct values | Rows/batch | Main → patched (ns/row) | Read time reduction | | --- | --- | --- | --- | | 1k | 8,192 | 10.05 → 6.69 | 33% | | 1k | 65,536 | 8.37 → 4.85 | 42% | | 100k | 65,536 | 36.2 → 32.6 | 10% | All three reductions have `p=0.002` (`n=6`). Required columns had no significant time change. After the patch, the 1k cases were close to required columns at 6.42 and 4.69 ns/row respectively. Memory allocated per read was effectively unchanged (less than 0.3% difference for these cases), because the scratch slice was already reused. At 100k distinct values and 8,192 rows/batch, the per-batch dictionary copy described in https://github.com/apache/arrow-go/issues/1443 dominates, where main reads at about 178 to 180 ns/row. The change from dense decoding alone was not significant there. With both patches, read time fell from 180.0 to 12.3 ns/row. **Reproduction** This program is separate from the benchmark above. It writes 2 million random 24-byte strings drawn from 1,000 distinct values to an in-memory Parquet file twice, once as a required column and once as an optional column with no nulls. It then reads each file nine times with `SetReadDict` on and 65,536-row batches, and prints the median time per row. ```go // Reads the same dictionary-encoded string column, stored once as required and // once as optional with no nulls, with SetReadDict on. package main import ( "bytes" "context" "fmt" "log" "math/rand/v2" "slices" "time" "github.com/apache/arrow-go/v18/arrow" "github.com/apache/arrow-go/v18/arrow/array" "github.com/apache/arrow-go/v18/arrow/memory" "github.com/apache/arrow-go/v18/parquet/file" "github.com/apache/arrow-go/v18/parquet/pqarrow" ) const ( numRows = 2_000_000 distinct = 1_000 batchSize = 65_536 runs = 9 ) func main() { for _, nullable := range []bool{false, true} { data := write(nullable) perRow := make([]float64, runs) for i := range perRow { start := time.Now() read(data) perRow[i] = float64(time.Since(start).Nanoseconds()) / numRows } slices.Sort(perRow) name := "required" if nullable { name = "optional, no nulls" } fmt.Printf("%-18s %5.2f ns/row\n", name, perRow[runs/2]) } } func write(nullable bool) []byte { schema := arrow.NewSchema([]arrow.Field{{Name: "s", Type: arrow.BinaryTypes.String, Nullable: nullable}}, nil) bldr := array.NewStringBuilder(memory.DefaultAllocator) defer bldr.Release() rng := rand.New(rand.NewPCG(1, 2)) for range numRows { bldr.Append(fmt.Sprintf("value-%018d", rng.IntN(distinct))) } col := bldr.NewArray() defer col.Release() rec := array.NewRecordBatch(schema, []arrow.Array{col}, numRows) defer rec.Release() var buf bytes.Buffer w, err := pqarrow.NewFileWriter(schema, &buf, nil, pqarrow.DefaultWriterProps()) check(err) check(w.Write(rec)) check(w.Close()) return buf.Bytes() } func read(data []byte) { pf, err := file.NewParquetReader(bytes.NewReader(data)) check(err) defer pf.Close() props := pqarrow.ArrowReadProperties{BatchSize: batchSize} props.SetReadDict(0, true) fr, err := pqarrow.NewFileReader(pf, props, memory.DefaultAllocator) check(err) rr, err := fr.GetRecordReader(context.Background(), nil, nil) check(err) defer rr.Release() for rr.Next() { } check(rr.Err()) } func check(err error) { if err != nil { log.Fatal(err) } } ``` Run it with this `go.mod`: ```text module repro go 1.25.0 require github.com/apache/arrow-go/v18 v18.8.0 ``` ```sh go mod tidy && go run . ``` Output on v18.8.0 from three runs, on a VM of the same type as the benchmark with Go 1.26.10: ```text required 5.64 ns/row optional, no nulls 9.87 ns/row required 5.39 ns/row optional, no nulls 9.70 ns/row required 5.28 ns/row optional, no nulls 9.80 ns/row ``` Unpatched main at `22c9f26df0abbc2066f5d9ae45f31720d29a4300` showed the same gap. With the patch applied to main, the optional column read in 5.14 to 5.56 ns/row, against 4.84 to 5.09 ns/row for the required column. The benchmark used the same main revision. Release v18.8.0 has the [same schema-based branch](https://github.com/apache/arrow-go/blob/72e9a5695337bab9ac01ec1e4d58177bc4949dc3/parquet/file/record_reader.go#L673-L685). **Expected behavior** Batches with no nulls should use the dense path, while batches that need spaced decoding should keep it. `DefLevelsToBitmap` has already written the validity bits before this choice. For comparison, Arrow C++ at `0ab78e2` [also takes the spaced path for nullable columns](https://github.com/apache/arrow/blob/0ab78e2df489055b71a7d26a666a52823f87f01c/cpp/src/parquet/column_reader.cc#L2204-L2267). Its [dictionary decoder](https://github.com/apache/arrow/blob/0ab78e2df489055b71a7d26a666a52823f87f01c/cpp/src/parquet/decoder.cc#L954-L983) still fills validity bytes, but the [no-null loop never reads the bitmap](https://github.com/apache/arrow/blob/0ab78e2df489055b71a7d26a666a52823f87f01c/cpp/src/arrow/visit_data_inline.h#L314-L340). arrow-rs at `b231cf1` [decodes densely and pads nulls only when values are missing](https://github.com/apache/arrow-rs/blob/b231cf15efa5dbbbe4ae8503849aa6ab3cfc4e0c/parquet/src/arrow/record_reader/mod.rs#L379-L396). **What I did not check** - `noasm`, TinyGo, arm64, macOS, or Windows. - Fixed-length byte array or boolean benchmarks. - Dedicated benchmarks or new tests for repeated (list) columns, though the existing suite covers them. - Real-world files. I have a patch with tests and can open a PR. ### 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
