You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "Abhisheklearn12 (via GitHub)" <gi...@apache.org> on 2026/10/03 11:43:44 UTC
[PR] fix(variant): reject duplicate object field names in shred_variant [arrow-rs]
Abhisheklearn12 opened a new pull request, #11351:
URL: https://github.com/apache/arrow-rs/pull/11351
# Which issue does this PR close?
- Closes #10359.
# Rationale for this change
When an object repeats a field name, `shred_variant` appends both values to that field's builder. That field's column ends up one row longer than the others, and arrow-array panics. If the duplicate field is not shredded, there is no panic: only the last value is kept and the earlier ones are silently dropped.
`Variant::try_new` rejects such objects, but `shred_variant` reads rows with shallow validation only, so they reach the shredder.
# What changes are included in this PR?
A repeated field name now returns `InvalidArgumentError` ("Duplicate field name: ..."), as the issue requests:
- Shredded fields: checked with the existing `seen` scratch buffer. The issue's proposed `HashSet` fix predates #10882, which replaced the per-row set with this buffer.
- Unshredded fields: the object builder now validates unique fields, and `insert_bytes` becomes `try_insert_bytes`, so the error is returned instead of panicking.
Valid input is unaffected: both checks fire only when a field name repeats within one object. Unshredded repeats are detected by field id, so an object whose dictionary is (invalidly) flagged sorted yet repeats a name still passes through unchanged, as before. Catching it would need a name-based check per row, like the per-row HashSet that #10882 removed for performance.
# Are these changes tested?
Yes. `test_object_with_duplicate_field_names_errors` covers a duplicate referenced by the same field id and by two ids naming the same string, each with the field shredded and unshredded. The test fails if either half of the fix is removed.
# Are there any significant user-facing changes?
No API changes. An object with a repeated field name is invalid input; it now returns an error instead of panicking or returning `Ok` with only the last value kept.
--
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: [PR] fix(variant): reject duplicate object field names in shred_variant [arrow-rs]
Posted by "Abhisheklearn12 (via GitHub)" <gi...@apache.org>.
Abhisheklearn12 commented on code in PR #11351:
URL: https://github.com/apache/arrow-rs/pull/11351#discussion_r4285832592
##########
parquet-variant-compute/src/shred_variant.rs:
##########
@@ -423,16 +423,23 @@ impl<'a> VariantToShreddedObjectVariantRowBuilder<'a> {
} = self;
seen.fill(false);
let mut builder = value_builder.builder_ext(value.metadata());
- let mut object_builder = builder.try_new_object()?;
+ // Error on a repeated unshredded field instead of overwriting it
+ let mut object_builder = builder.try_new_object()?.with_validate_unique_fields(true);
Review Comment:
hi @sdf-jkl, done in 503ad4209, thanks :)
--
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: [PR] fix(variant): reject duplicate object field names in shred_variant [arrow-rs]
Posted by "sdf-jkl (via GitHub)" <gi...@apache.org>.
sdf-jkl commented on PR #11351:
URL: https://github.com/apache/arrow-rs/pull/11351#issuecomment-6090548452
run benchmark variant_kernels
--
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: [PR] fix(variant): reject duplicate object field names in shred_variant [arrow-rs]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #11351:
URL: https://github.com/apache/arrow-rs/pull/11351#issuecomment-6090549863
🤖 Benchmark starting (GKE) | [trigger](https://github.com/apache/arrow-rs/pull/11351#issuecomment-6090548452)
**Target:** variant_kernels
**Sharding factor: 1** (1 workers).
Comparing parquet-variant-compute-new (a3310ab1141d9d7f1744ad82c6b5273cde8d769c) to 1d1607a (merge-base) [diff](https://github.com/apache/arrow-rs/compare/1d1607a761e1dff1a104aefcf6ff248f2579818c..a3310ab1141d9d7f1744ad82c6b5273cde8d769c)
<details><summary>Run configuration</summary>
```yaml
run benchmark variant_kernels
shards: 1
```
</details>
Results will be posted when all workers finish.
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
<!-- benchmark-request:6090548452:start:610dd8db80eea6aa5bf4942cc69b76e3 -->
--
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: [PR] fix(variant): reject duplicate object field names in shred_variant [arrow-rs]
Posted by "sdf-jkl (via GitHub)" <gi...@apache.org>.
sdf-jkl commented on code in PR #11351:
URL: https://github.com/apache/arrow-rs/pull/11351#discussion_r4285251122
##########
parquet-variant-compute/src/shred_variant.rs:
##########
@@ -423,16 +423,23 @@ impl<'a> VariantToShreddedObjectVariantRowBuilder<'a> {
} = self;
seen.fill(false);
let mut builder = value_builder.builder_ext(value.metadata());
- let mut object_builder = builder.try_new_object()?;
+ // Error on a repeated unshredded field instead of overwriting it
+ let mut object_builder = builder.try_new_object()?.with_validate_unique_fields(true);
Review Comment:
Small pre-existing documentation nit:
the docs for [`ObjectBuilder::with_validate_unique_fields`](https://github.com/apache/arrow-rs/blob/a3310ab1141d9d7f1744ad82c6b5273cde8d769c/parquet-variant/src/builder/object.rs#L186-L189) and [`VariantBuilder::with_validate_unique_fields`](https://github.com/apache/arrow-rs/blob/a3310ab1141d9d7f1744ad82c6b5273cde8d769c/parquet-variant/src/builder.rs#L769-L773) still say duplicates cause `ObjectBuilder::finish()` to return an error.
The check actually happens during insertion, and `finish()` returns `()`.
--
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: [PR] fix(variant): reject duplicate object field names in shred_variant [arrow-rs]
Posted by "sdf-jkl (via GitHub)" <gi...@apache.org>.
sdf-jkl commented on PR #11351:
URL: https://github.com/apache/arrow-rs/pull/11351#issuecomment-6197717820
Thanks @Abhisheklearn12 :rocket:
--
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: [PR] fix(variant): reject duplicate object field names in shred_variant [arrow-rs]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #11351:
URL: https://github.com/apache/arrow-rs/pull/11351#issuecomment-6090738448
🤖 Benchmark completed (GKE) | [trigger](https://github.com/apache/arrow-rs/pull/11351#issuecomment-6090548452)
**Target:** variant_kernels
Comparing parquet-variant-compute-new (a3310ab1141d9d7f1744ad82c6b5273cde8d769c) to 1d1607a (merge-base) [diff](https://github.com/apache/arrow-rs/compare/1d1607a761e1dff1a104aefcf6ff248f2579818c..a3310ab1141d9d7f1744ad82c6b5273cde8d769c)
<details><summary>Run configuration</summary>
```yaml
run benchmark variant_kernels
shards: 1
```
</details>
<details><summary>Details</summary>
```
group base changed
----- ---- -------
batch_json_string_to_variant json_list 8k string 1.00 19.4±0.17ms ? ?/sec 1.00 19.5±0.07ms ? ?/sec
batch_json_string_to_variant object - 1 depth(100 fields) random_json(2436 bytes per document) 1.00 146.5±0.40ms ? ?/sec 1.00 146.6±0.24ms ? ?/sec
batch_json_string_to_variant object - 1 depth(200 fields) random_json(4871 bytes per document) 1.00 310.0±0.99ms ? ?/sec 1.00 309.8±1.03ms ? ?/sec
batch_json_string_to_variant random_json(2633 bytes per document) 1.00 220.6±1.12ms ? ?/sec 1.00 219.5±0.38ms ? ?/sec
batch_json_string_to_variant repeated_struct 8k string 1.00 6.2±0.05ms ? ?/sec 1.00 6.2±0.02ms ? ?/sec
shred_variant_partial_object_8k_rows 1.00 20.7±0.08ms ? ?/sec 1.02 21.2±0.33ms ? ?/sec
shred_variant_unmatched_object_8k_rows 1.00 25.9±0.14ms ? ?/sec 1.03 26.8±0.52ms ? ?/sec
variant_array_builder_build_262k_small_values 1.00 2.9±0.12ms ? ?/sec 1.01 2.9±0.14ms ? ?/sec
variant_get_binary_from_string 1.03 496.9±26.72µs ? ?/sec 1.00 480.9±0.73µs ? ?/sec
variant_get_binary_from_unshredded_binary 1.04 481.0±31.52µs ? ?/sec 1.00 461.0±2.90µs ? ?/sec
variant_get_list_index_all_oob_int64_64_rows 1.00 1772.8±43.46ns ? ?/sec 1.00 1775.1±42.91ns ? ?/sec
variant_get_list_index_shredded 1.01 46.0±1.76µs ? ?/sec 1.00 45.5±1.54µs ? ?/sec
variant_get_list_index_unshredded 1.05 1041.8±72.40µs ? ?/sec 1.00 991.7±12.29µs ? ?/sec
variant_get_primitive 1.00 573.0±4.82ns ? ?/sec 1.00 571.6±6.76ns ? ?/sec
variant_get_shredded_utf8 1.00 706.2±4.40ns ? ?/sec 1.03 724.5±5.38ns ? ?/sec
variant_get_unshredded_object_path_262k_rows 1.04 38.6±2.05ms ? ?/sec 1.00 37.2±0.48ms ? ?/sec
variant_get_utf8_from_date 1.00 571.7±7.78µs ? ?/sec 1.00 569.5±5.13µs ? ?/sec
variant_get_utf8_from_decimal 1.01 479.6±14.72µs ? ?/sec 1.00 473.1±6.21µs ? ?/sec
variant_get_utf8_from_float 1.02 451.0±17.98µs ? ?/sec 1.00 444.1±1.22µs ? ?/sec
variant_get_utf8_from_int 1.02 451.7±17.73µs ? ?/sec 1.00 444.9±1.17µs ? ?/sec
variant_get_utf8_from_list 1.07 680.3±67.18µs ? ?/sec 1.00 637.7±10.21µs ? ?/sec
variant_get_utf8_from_map_in_list 1.05 706.0±63.57µs ? ?/sec 1.00 670.6±11.94µs ? ?/sec
variant_get_utf8_from_time 1.00 787.1±6.42µs ? ?/sec 1.01 795.8±3.05µs ? ?/sec
variant_get_utf8_from_timestamp_with_timezone 1.02 581.8±15.41µs ? ?/sec 1.00 570.1±3.02µs ? ?/sec
variant_get_utf8_from_timestamp_without_timezone 1.02 583.0±15.38µs ? ?/sec 1.00 571.5±2.99µs ? ?/sec
variant_get_utf8_from_unshredded_string 1.04 510.9±27.84µs ? ?/sec 1.00 493.1±1.20µs ? ?/sec
```
</details>
<details><summary>Per-runner information</summary>
<details><summary>variant_kernels — shard 1/1</summary>
**Node:** gk3-benchmark-cluster-nap-1midrx00-a14d3a6f-nhtm
**Instance:** c4a-highcpu-48 (12 vCPU / 65 GiB)
**uname:**
```text
Linux bench-c6090548452-3263-r9j26 6.12.94+ #1 SMP Fri Aug 21 08:00:16 UTC 2026 aarch64 GNU/Linux
```
**BENCH_COMMAND:**
```sh
cargo bench --features=arrow,async,test_common,experimental,object_store --bench variant_kernels
```
<details><summary>CPU Details (lscpu)</summary>
```text
Architecture: aarch64
CPU op-mode(s): 64-bit
Byte Order: Little Endian
CPU(s): 48
On-line CPU(s) list: 0-47
Vendor ID: ARM
Model name: Neoverse-V2
Model: 1
Thread(s) per core: 1
Core(s) per cluster: 48
Socket(s): -
Cluster(s): 1
Stepping: r0p1
BogoMIPS: 2000.00
Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache: 3 MiB (48 instances)
L1i cache: 3 MiB (48 instances)
L2 cache: 96 MiB (48 instances)
L3 cache: 80 MiB (1 instance)
NUMA node(s): 1
NUMA node0 CPU(s): 0-47
Vulnerability Gather data sampling: Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Reg file data sampling: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; __user pointer sanitization
Vulnerability Spectre v2: Mitigation; CSV2, BHB
Vulnerability Srbds: Not affected
Vulnerability Tsa: Not affected
Vulnerability Tsx async abort: Not affected
Vulnerability Vmscape: Not affected
```
</details>
<details><summary>Resource Usage</summary>
**base (merge-base)**
| Metric | Value |
|--------|-------|
| Wall time | 365.1s |
| Peak memory | 447.1 MiB |
| Avg memory | 228.2 MiB |
| CPU user | 352.8s |
| CPU sys | 6.4s |
| Peak spill | 0 B |
**branch**
| Metric | Value |
|--------|-------|
| Wall time | 355.1s |
| Peak memory | 447.0 MiB |
| Avg memory | 230.4 MiB |
| CPU user | 345.0s |
| CPU sys | 6.4s |
| Peak spill | 0 B |
</details>
</details>
</details>
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
<!-- benchmark-request:6090548452:finish:83fca75ee2b89c951d61713b925d23de -->
--
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: [PR] fix(variant): reject duplicate object field names in shred_variant [arrow-rs]
Posted by "sdf-jkl (via GitHub)" <gi...@apache.org>.
sdf-jkl merged PR #11351:
URL: https://github.com/apache/arrow-rs/pull/11351
--
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