Return an error on overflow in do_append_val_inner

[liamzwbao]

Which issue does this PR close?

• Closes Return an error instead of panic in ByteGroupValueBuilder::do_append_val_inner #15969.

Rationale for this change

Return proper error message instead of panic

What changes are included in this PR?

Change the trait and implemented methods to return error on overflow, plus a bit of refactoring

Are these changes tested?

Modified related tests for the change

Are there any user-facing changes?

[alamb]

Thank you @liamzwbao -- this looks good to me. I'll start some benchmarks on this PR and as long as that looks good this PR looks nice to me

Thanks again

[alamb]

🤖 ./gh_compare_branch.sh Benchmark Script Running
Linux aal-dev 6.11.0-1013-gcp #13~24.04.1-Ubuntu SMP Wed Apr 2 16:34:16 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Comparing issue-15969-error-on-buffer-overflow (a4bd49e) to aaae4d7 diff
Benchmarks: tpch_mem clickbench_partitioned clickbench_extended
Results will be posted here when complete

[alamb]

🤖: Benchmark completed

Details

Comparing HEAD and issue-15969-error-on-buffer-overflow
--------------------
Benchmark clickbench_extended.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━┓
┃ Query        ┃       HEAD ┃ issue-15969-error-on-buffer-overflow ┃    Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━┩
│ QQuery 0     │  2043.08ms │                            1979.05ms │ no change │
│ QQuery 1     │   685.71ms │                             695.03ms │ no change │
│ QQuery 2     │  1451.05ms │                            1413.97ms │ no change │
│ QQuery 3     │   724.93ms │                             705.65ms │ no change │
│ QQuery 4     │  1501.83ms │                            1485.02ms │ no change │
│ QQuery 5     │ 15286.01ms │                           15279.79ms │ no change │
│ QQuery 6     │  2016.42ms │                            2032.04ms │ no change │
│ QQuery 7     │  2145.82ms │                            2131.17ms │ no change │
│ QQuery 8     │   846.46ms │                             853.37ms │ no change │
└──────────────┴────────────┴──────────────────────────────────────┴───────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┓
┃ Benchmark Summary                                   ┃            ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━┩
│ Total Time (HEAD)                                   │ 26701.30ms │
│ Total Time (issue-15969-error-on-buffer-overflow)   │ 26575.09ms │
│ Average Time (HEAD)                                 │  2966.81ms │
│ Average Time (issue-15969-error-on-buffer-overflow) │  2952.79ms │
│ Queries Faster                                      │          0 │
│ Queries Slower                                      │          0 │
│ Queries with No Change                              │          9 │
└─────────────────────────────────────────────────────┴────────────┘
--------------------
Benchmark clickbench_partitioned.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━┓
┃ Query        ┃       HEAD ┃ issue-15969-error-on-buffer-overflow ┃       Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━┩
│ QQuery 0     │    15.60ms │                              15.17ms │    no change │
│ QQuery 1     │    32.29ms │                              33.24ms │    no change │
│ QQuery 2     │    79.91ms │                              79.88ms │    no change │
│ QQuery 3     │    94.92ms │                              96.00ms │    no change │
│ QQuery 4     │   582.64ms │                             604.45ms │    no change │
│ QQuery 5     │   822.28ms │                             825.16ms │    no change │
│ QQuery 6     │    22.79ms │                              23.38ms │    no change │
│ QQuery 7     │    36.89ms │                              37.41ms │    no change │
│ QQuery 8     │   912.43ms │                             927.29ms │    no change │
│ QQuery 9     │  1207.78ms │                            1203.91ms │    no change │
│ QQuery 10    │   261.50ms │                             261.91ms │    no change │
│ QQuery 11    │   294.64ms │                             301.42ms │    no change │
│ QQuery 12    │   895.86ms │                             923.38ms │    no change │
│ QQuery 13    │  1206.74ms │                            1356.20ms │ 1.12x slower │
│ QQuery 14    │   836.43ms │                             852.90ms │    no change │
│ QQuery 15    │   806.78ms │                             831.66ms │    no change │
│ QQuery 16    │  1724.81ms │                            1735.49ms │    no change │
│ QQuery 17    │  1583.87ms │                            1628.51ms │    no change │
│ QQuery 18    │  3037.83ms │                            3135.34ms │    no change │
│ QQuery 19    │    84.12ms │                              80.66ms │    no change │
│ QQuery 20    │  1129.32ms │                            1158.37ms │    no change │
│ QQuery 21    │  1292.12ms │                            1305.73ms │    no change │
│ QQuery 22    │  2135.78ms │                            2205.85ms │    no change │
│ QQuery 23    │  7930.68ms │                            7993.95ms │    no change │
│ QQuery 24    │   460.72ms │                             466.72ms │    no change │
│ QQuery 25    │   377.81ms │                             386.78ms │    no change │
│ QQuery 26    │   523.08ms │                             532.90ms │    no change │
│ QQuery 27    │  1585.75ms │                            1614.10ms │    no change │
│ QQuery 28    │ 12555.85ms │                           13406.98ms │ 1.07x slower │
│ QQuery 29    │   530.97ms │                             524.66ms │    no change │
│ QQuery 30    │   795.62ms │                             806.85ms │    no change │
│ QQuery 31    │   837.65ms │                             859.07ms │    no change │
│ QQuery 32    │  2648.89ms │                            2707.72ms │    no change │
│ QQuery 33    │  3381.01ms │                            3383.80ms │    no change │
│ QQuery 34    │  3491.93ms │                            3410.35ms │    no change │
│ QQuery 35    │  1324.51ms │                            1326.00ms │    no change │
│ QQuery 36    │   123.38ms │                             126.24ms │    no change │
│ QQuery 37    │    55.32ms │                              58.71ms │ 1.06x slower │
│ QQuery 38    │   120.07ms │                             124.29ms │    no change │
│ QQuery 39    │   189.96ms │                             202.87ms │ 1.07x slower │
│ QQuery 40    │    46.37ms │                              49.92ms │ 1.08x slower │
│ QQuery 41    │    44.84ms │                              46.64ms │    no change │
│ QQuery 42    │    37.09ms │                              38.02ms │    no change │
└──────────────┴────────────┴──────────────────────────────────────┴──────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┓
┃ Benchmark Summary                                   ┃            ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━┩
│ Total Time (HEAD)                                   │ 56158.85ms │
│ Total Time (issue-15969-error-on-buffer-overflow)   │ 57689.92ms │
│ Average Time (HEAD)                                 │  1306.02ms │
│ Average Time (issue-15969-error-on-buffer-overflow) │  1341.63ms │
│ Queries Faster                                      │          0 │
│ Queries Slower                                      │          5 │
│ Queries with No Change                              │         38 │
└─────────────────────────────────────────────────────┴────────────┘
--------------------
Benchmark tpch_mem_sf1.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━┓
┃ Query        ┃     HEAD ┃ issue-15969-error-on-buffer-overflow ┃       Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━┩
│ QQuery 1     │ 126.84ms │                             123.13ms │    no change │
│ QQuery 2     │  22.67ms │                              21.84ms │    no change │
│ QQuery 3     │  33.76ms │                              34.00ms │    no change │
│ QQuery 4     │  19.92ms │                              19.66ms │    no change │
│ QQuery 5     │  51.13ms │                              51.80ms │    no change │
│ QQuery 6     │  11.80ms │                              11.90ms │    no change │
│ QQuery 7     │  93.97ms │                              93.17ms │    no change │
│ QQuery 8     │  25.25ms │                              26.00ms │    no change │
│ QQuery 9     │  59.39ms │                              58.68ms │    no change │
│ QQuery 10    │  56.39ms │                              57.29ms │    no change │
│ QQuery 11    │  11.16ms │                              11.50ms │    no change │
│ QQuery 12    │  40.55ms │                              43.08ms │ 1.06x slower │
│ QQuery 13    │  26.48ms │                              27.47ms │    no change │
│ QQuery 14    │   9.42ms │                               9.54ms │    no change │
│ QQuery 15    │  23.38ms │                              23.74ms │    no change │
│ QQuery 16    │  22.02ms │                              22.00ms │    no change │
│ QQuery 17    │  94.83ms │                              95.83ms │    no change │
│ QQuery 18    │ 214.76ms │                             213.61ms │    no change │
│ QQuery 19    │  25.64ms │                              26.37ms │    no change │
│ QQuery 20    │  38.23ms │                              37.85ms │    no change │
│ QQuery 21    │ 162.00ms │                             159.81ms │    no change │
│ QQuery 22    │  16.48ms │                              16.49ms │    no change │
└──────────────┴──────────┴──────────────────────────────────────┴──────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━┓
┃ Benchmark Summary                                   ┃           ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━┩
│ Total Time (HEAD)                                   │ 1186.08ms │
│ Total Time (issue-15969-error-on-buffer-overflow)   │ 1184.77ms │
│ Average Time (HEAD)                                 │   53.91ms │
│ Average Time (issue-15969-error-on-buffer-overflow) │   53.85ms │
│ Queries Faster                                      │         0 │
│ Queries Slower                                      │         1 │
│ Queries with No Change                              │        21 │
└─────────────────────────────────────────────────────┴───────────┘

[alamb]

🤖 ./gh_compare_branch.sh Benchmark Script Running
Linux aal-dev 6.11.0-1013-gcp #13~24.04.1-Ubuntu SMP Wed Apr 2 16:34:16 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Comparing issue-15969-error-on-buffer-overflow (a4bd49e) to aaae4d7 diff
Benchmarks: tpch_mem clickbench_partitioned clickbench_extended
Results will be posted here when complete

[alamb]

🤖: Benchmark completed

Details

Comparing HEAD and issue-15969-error-on-buffer-overflow
--------------------
Benchmark clickbench_extended.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Query        ┃       HEAD ┃ issue-15969-error-on-buffer-overflow ┃        Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ QQuery 0     │  1887.33ms │                            1940.58ms │     no change │
│ QQuery 1     │   684.92ms │                             687.43ms │     no change │
│ QQuery 2     │  1444.53ms │                            1456.54ms │     no change │
│ QQuery 3     │   701.76ms │                             704.93ms │     no change │
│ QQuery 4     │  1428.16ms │                            1452.49ms │     no change │
│ QQuery 5     │ 15273.74ms │                           15224.87ms │     no change │
│ QQuery 6     │  2015.92ms │                            2020.32ms │     no change │
│ QQuery 7     │  2199.11ms │                            2069.06ms │ +1.06x faster │
│ QQuery 8     │   857.56ms │                             819.71ms │     no change │
└──────────────┴────────────┴──────────────────────────────────────┴───────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┓
┃ Benchmark Summary                                   ┃            ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━┩
│ Total Time (HEAD)                                   │ 26493.03ms │
│ Total Time (issue-15969-error-on-buffer-overflow)   │ 26375.94ms │
│ Average Time (HEAD)                                 │  2943.67ms │
│ Average Time (issue-15969-error-on-buffer-overflow) │  2930.66ms │
│ Queries Faster                                      │          1 │
│ Queries Slower                                      │          0 │
│ Queries with No Change                              │          8 │
└─────────────────────────────────────────────────────┴────────────┘
--------------------
Benchmark clickbench_partitioned.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Query        ┃       HEAD ┃ issue-15969-error-on-buffer-overflow ┃        Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ QQuery 0     │    15.49ms │                              14.99ms │     no change │
│ QQuery 1     │    32.66ms │                              33.01ms │     no change │
│ QQuery 2     │    80.11ms │                              80.35ms │     no change │
│ QQuery 3     │    94.94ms │                              99.12ms │     no change │
│ QQuery 4     │   590.38ms │                             589.43ms │     no change │
│ QQuery 5     │   835.95ms │                             855.49ms │     no change │
│ QQuery 6     │    23.60ms │                              22.09ms │ +1.07x faster │
│ QQuery 7     │    37.16ms │                              39.37ms │  1.06x slower │
│ QQuery 8     │   925.56ms │                             910.92ms │     no change │
│ QQuery 9     │  1177.20ms │                            1191.00ms │     no change │
│ QQuery 10    │   266.51ms │                             260.19ms │     no change │
│ QQuery 11    │   301.19ms │                             296.09ms │     no change │
│ QQuery 12    │   893.72ms │                             907.61ms │     no change │
│ QQuery 13    │  1382.19ms │                            1400.30ms │     no change │
│ QQuery 14    │   852.77ms │                             838.60ms │     no change │
│ QQuery 15    │   874.85ms │                             820.54ms │ +1.07x faster │
│ QQuery 16    │  1753.65ms │                            1760.22ms │     no change │
│ QQuery 17    │  1627.37ms │                            1634.89ms │     no change │
│ QQuery 18    │  3083.97ms │                            3127.15ms │     no change │
│ QQuery 19    │    83.69ms │                              88.17ms │  1.05x slower │
│ QQuery 20    │  1114.32ms │                            1143.83ms │     no change │
│ QQuery 21    │  1310.41ms │                            1328.95ms │     no change │
│ QQuery 22    │  2163.55ms │                            2216.49ms │     no change │
│ QQuery 23    │  7947.99ms │                            8091.50ms │     no change │
│ QQuery 24    │   458.45ms │                             459.05ms │     no change │
│ QQuery 25    │   390.67ms │                             389.94ms │     no change │
│ QQuery 26    │   523.74ms │                             523.69ms │     no change │
│ QQuery 27    │  1590.71ms │                            1613.82ms │     no change │
│ QQuery 28    │ 12618.24ms │                           13765.25ms │  1.09x slower │
│ QQuery 29    │   520.86ms │                             528.83ms │     no change │
│ QQuery 30    │   806.98ms │                             789.67ms │     no change │
│ QQuery 31    │   835.33ms │                             859.35ms │     no change │
│ QQuery 32    │  2650.88ms │                            2642.04ms │     no change │
│ QQuery 33    │  3329.86ms │                            3396.90ms │     no change │
│ QQuery 34    │  3378.46ms │                            3360.50ms │     no change │
│ QQuery 35    │  1306.07ms │                            1269.40ms │     no change │
│ QQuery 36    │   124.97ms │                             120.25ms │     no change │
│ QQuery 37    │    56.47ms │                              55.71ms │     no change │
│ QQuery 38    │   120.68ms │                             124.15ms │     no change │
│ QQuery 39    │   191.75ms │                             195.43ms │     no change │
│ QQuery 40    │    45.57ms │                              46.57ms │     no change │
│ QQuery 41    │    44.92ms │                              43.46ms │     no change │
│ QQuery 42    │    37.38ms │                              38.82ms │     no change │
└──────────────┴────────────┴──────────────────────────────────────┴───────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┓
┃ Benchmark Summary                                   ┃            ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━┩
│ Total Time (HEAD)                                   │ 56501.27ms │
│ Total Time (issue-15969-error-on-buffer-overflow)   │ 57973.15ms │
│ Average Time (HEAD)                                 │  1313.98ms │
│ Average Time (issue-15969-error-on-buffer-overflow) │  1348.21ms │
│ Queries Faster                                      │          2 │
│ Queries Slower                                      │          3 │
│ Queries with No Change                              │         38 │
└─────────────────────────────────────────────────────┴────────────┘
--------------------
Benchmark tpch_mem_sf1.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Query        ┃     HEAD ┃ issue-15969-error-on-buffer-overflow ┃        Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ QQuery 1     │ 123.90ms │                             120.31ms │     no change │
│ QQuery 2     │  22.23ms │                              22.23ms │     no change │
│ QQuery 3     │  34.61ms │                              34.64ms │     no change │
│ QQuery 4     │  19.75ms │                              20.30ms │     no change │
│ QQuery 5     │  51.63ms │                              52.35ms │     no change │
│ QQuery 6     │  12.24ms │                              12.10ms │     no change │
│ QQuery 7     │  98.16ms │                              93.24ms │ +1.05x faster │
│ QQuery 8     │  26.41ms │                              25.34ms │     no change │
│ QQuery 9     │  59.89ms │                              58.94ms │     no change │
│ QQuery 10    │  56.60ms │                              56.88ms │     no change │
│ QQuery 11    │  11.53ms │                              11.79ms │     no change │
│ QQuery 12    │  44.56ms │                              42.07ms │ +1.06x faster │
│ QQuery 13    │  27.04ms │                              26.97ms │     no change │
│ QQuery 14    │   9.58ms │                               9.61ms │     no change │
│ QQuery 15    │  24.00ms │                              23.76ms │     no change │
│ QQuery 16    │  22.28ms │                              23.00ms │     no change │
│ QQuery 17    │  95.56ms │                              96.82ms │     no change │
│ QQuery 18    │ 206.83ms │                             203.26ms │     no change │
│ QQuery 19    │  25.82ms │                              26.62ms │     no change │
│ QQuery 20    │  36.31ms │                              38.22ms │  1.05x slower │
│ QQuery 21    │ 160.91ms │                             160.78ms │     no change │
│ QQuery 22    │  16.05ms │                              16.34ms │     no change │
└──────────────┴──────────┴──────────────────────────────────────┴───────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━┓
┃ Benchmark Summary                                   ┃           ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━┩
│ Total Time (HEAD)                                   │ 1185.88ms │
│ Total Time (issue-15969-error-on-buffer-overflow)   │ 1175.56ms │
│ Average Time (HEAD)                                 │   53.90ms │
│ Average Time (issue-15969-error-on-buffer-overflow) │   53.43ms │
│ Queries Faster                                      │         2 │
│ Queries Slower                                      │         1 │
│ Queries with No Change                              │        19 │
└─────────────────────────────────────────────────────┴───────────┘

[alamb]

🚀