Phase 1: Align the TPC-DS generation path with the TPC-H path by qbacpey · Pull Request #400 · rapidsai/velox-testing

qbacpey · 2026-08-10T08:10:06Z

TL;DR

This PR add these features for DuckDB TPC-DS datasets generations:

Row-group sizing: --approx-row-group-bytes is now supported with zero overhead.
Parquet V2: all output uses still use V1 encoding, see Update Velox cuDF pin to support Parquet DELTA encoding #416 .
Multi-file partitioning: tables over --max-rows-per-file split into <table>-<part>.parquet, matching the tpchgen layout.
Metadata: both paths now share one writer, so metadata.json records approx_row_group_bytes alongside scale_factor.

Generation overhead

Measured end to end on machine with 2 x AMD EPYC 9555 64-Core, 2.2TiB HMEM.

Overhead

"Probe wait" is the time the main thread blocked on the probe after materialization finished, i.e. the only part of it that is not hidden:

target SF	total
SF1	5.9s
SF10	13.5s
SF100	90.1s
SF1000	558.6s
SF3000	999.8s

At SF1 the probe costs 0.9s because it generates the same scale factor as the target and so cannot finish first.

Accuracy

Probing yields complete row groups slightly under the requested size, because concurrent writers flush early. At a SF3000 128 MiB target the median is about 90% (~120 MiB): half of the complete groups are at or below that.

Query overhead comparsion

Measured end to end on machine with 8 x RTX PRO 6000 Blackwell

Scale Factor	Upstream	Current branch
SF100	103 s	102 s
SF1000	1,228 s	524 s

The differences come from four sources:

1. Row group size

Upstream uses DuckDB's default of 122,880 rows per row group (median 7.31 MiB compressed).
This branch respects --approx-row-group-bytes and targets 128 MiB per row group instead (median 90 MiB compressed).

2. Partitioning

Upstream always writes one file per table, i.e. exactly 24 files.
This branch respects --max-rows-per-file, producing 83 files at SF1000.

3. Data types

Upstream writes every int column as INT32.
This branch picks up a DuckDB upgrade (duckdb/duckdb#18098) that promotes some of those columns to INT64.

4. Encoding

This branch forces Parquet V1 encodings, so columns that were RLE_DICTIONARY upstream are now written as PLAIN_DICTIONARY.
Dictionary coverage also increased as a result.

Tests

All 15 tests pass with DuckDB 1.6dev.

The CI row-group test uses a 1 MiB target so that SF1 tables contain enough row groups to validate sizing.

copy-pr-bot · 2026-08-10T08:10:11Z

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

- Updated `init_benchmark_tables` to accept a DuckDB connection parameter, allowing for more flexible database interactions. - Introduced `get_select_query` and `get_column_projection` functions to handle SQL queries and column type conversions, improving data handling in the generation process. - Modified `generate_data_files_with_duckdb` to utilize the new connection parameter and updated logic for writing metadata and table partitions. - Enhanced tests to validate the new functionality and ensure compatibility with both TPCH and TPCDS benchmarks.

qbacpey · 2026-08-10T09:49:29Z

+import pyarrow.parquet as pq
+from duckdb_utils import get_select_query, init_benchmark_tables
+
+_ROW_GROUP_GRANULARITY = 2048  # DuckDB rounds ROW_GROUP_SIZE to its vector size


Need statistic from TPC-DS 1K & 3K generation process to adjust these number

paul-aiyedun

Changes overall look good to me. However, I had a few questions and code cleanup comments.

paul-aiyedun · 2026-08-10T21:54:21Z

-def test_generated_files_use_v2_page_format(setup_and_teardown):
-    """Verify every generated Parquet file is written with the v2 format."""
+@pytest.mark.parametrize(
+    "benchmark_type,use_duckdb",


I don't think the use_duckdb parameter is needed here. We can have the test execution be based on the default value. Also, can we apply the parameterization globally at the setup_and_teardown level, so that all tests cover TPC-DS?

Same comment applies to the row_group_size_test.py update.

paul-aiyedun · 2026-08-10T22:06:22Z

+from generate_data_files import generate_data_files
+
+
+def test_max_rows_per_file_splits_tables_tpcds(setup_and_teardown):


Please update the test case to cover both TPC-H and TPC-DS.

paul-aiyedun · 2026-08-10T22:18:04Z

 charset-normalizer==3.4.3
 click==8.2.1
-duckdb==1.3.2
+# DuckDB 1.5.5 supports Parquet V2; parallel dsdgen is not yet in a stable release.


Nit: Remove comment about parallel dsdgen or add a TODO for this.

paul-aiyedun · 2026-08-10T22:58:15Z

+
+def get_column_projection(column_metadata):
+    col_name, col_type, *_ = column_metadata
+    if is_decimal_column(col_type):


Why do we always do a conversion here?

The conversion is not unconditional. get_column_projection() is only called inside the convert_decimals_to_floats branch of get_select_query(). Without -c, get_select_query() returns SELECT *, so no casts are generated.

Renamed to get_column_projection_with_decimals_as_double

paul-aiyedun · 2026-08-10T23:14:44Z

-    with open(f"{args.data_dir_path}/metadata.json", "w") as file:
-        json.dump({"scale_factor": args.scale_factor}, file, indent=2)
-        file.write("\n")
+    write_metadata(args)


Can we write the metadata on a shared path (i.e. outside of generate_data_files_with_duckdb or generate_data_files_with_duckdb)?

paul-aiyedun · 2026-08-11T00:00:07Z

+    for part in range(num_partitions):
+        # Avoid a redundant LIMIT/OFFSET for a single part.
+        partition_query = (
+            f"{select_query} LIMIT {max_rows_per_file} OFFSET {part * max_rows_per_file}"


Can you try comparing the performance with an implementation that runs this in parallel and does WHERE rowid >= {part * max_rows_per_file} AND rowid < {max_rows_per_file}?

Observed performance scaling roughly proportional to the number of threads. Also set the concurrency to max_workers = min(num_threads, num_partitions) to avoid excessive memory growth and I/O contention.

rowid query is consistently about 3× faster than LIMIT / OFFSET query across 1/4/8 outer writers

Table Outer Writers LIMIT / OFFSET Median Copy Time rowid Median Copy Time

inventory 1 7.46s 2.42s

inventory 4 2.07s 0.70s

inventory 8 1.36s 0.44s

store_sales 1 17.89s 6.20s

store_sales 4 7.00s 2.33s

store_sales 8 3.86s 1.29s

paul-aiyedun · 2026-08-11T00:16:04Z

+
+
+def _write_probe(conn, query, path, row_group_rows=None):
+    options = "FORMAT parquet, PARQUET_VERSION 'V2'"


Can we move this to a function that is shared with _write_table_partitions?

paul-aiyedun · 2026-08-11T00:25:56Z

+            # A second write cannot fill the estimated row group or improve the result.
+            return rows
+
+        # Second pass: measure one full row group near the requested size.


Why is this second pass needed?

Encoded bytes/row changes with row-group size, so the first pass only gives a rough estimate based on DuckDB’s default-sized groups. The second pass uses that estimate to write one complete group near the requested size and measures bytes/row again there.

paul-aiyedun · 2026-08-11T00:26:48Z

+
+_ROW_GROUP_GRANULARITY = 2048  # DuckDB rounds ROW_GROUP_SIZE to its vector size
+_MAX_PROBE_SCALE_FACTOR = 10
+_STAGE1_ROWS = 200_000


How was this number derived?

200,000 was originally chosen to ensure DuckDB wrote at least one complete row group. I will change it to 122,880 (DuckDB’s default row-group size).

_MAX_PROBE_SCALE_FACTOR = 10 is for MEM budget, plan to adjust this when able to run SF1K &3K generation.

About the 1.2x: Use 1x can produce the same level of accuracy, so I removed it.

qbacpey · 2026-08-11T14:31:53Z

 TEST_NON_DEFAULT_COMPRESSION_PATH = TESTS_DIR / "test_codec_definitions_non_default_compression.json"
 TEST_INVALID_COMPRESSION_PATH = TESTS_DIR / "test_codec_definitions_invalid_compression.json"

+pytestmark = pytest.mark.parametrize("setup_and_teardown", ["tpch"], indirect=True)


Makes every test in codec_definitions_test.py run only with TPC-H (currently codec definitions are supported only for TPC-H)

- Updated `get_select_query` to use `get_column_projection_with_decimals_as_double` for improved handling of decimal columns. - Added installation command for DuckDB in `generate_data_files_with_duckdb` to prevent concurrent installations. - Simplified row group sizing logic in `row_group_sizing.py` for better performance and clarity.

qbacpey · 2026-08-12T14:49:10Z

+_MAX_PROBE_SCALE_FACTOR = 10
+_PROBE_MEMORY_LIMIT = "8GB"


Probe MEM budget, perhaps a better value? Or remove it?

- Revised the docstring of `_rows_for_target` to specify that it rounds to the nearest 2,048-row multiple instead of allowing DuckDB to round up, enhancing clarity for future developers.

qbacpey · 2026-08-12T15:03:09Z

+def _rows_for_target(bytes_per_row, target_bytes):
+    """Round to the nearest 2,048-row multiple instead of letting DuckDB round up."""
+    if not bytes_per_row:
+        return None
+    rows = round(target_bytes / bytes_per_row / _ROW_GROUP_GRANULARITY)
+    return max(rows, 1) * _ROW_GROUP_GRANULARITY


This matters mainly for small row-group targets. DuckDB rounds ROW_GROUP_SIZE up to a 2,048-row multiple, and one step can be a large percentage when a group contains only a few thousand rows. Rounding to the nearest multiple reduced the SF1/1 MiB errors from −10% to +0.5% for web_sales and from −29% to +4.3% for item.

copy-pr-bot · 2026-09-03T08:27:36Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

…rmat - Deleted AGENTS.md as it was no longer needed. - Updated `generate_data_files.py` to use Parquet version 2 for file generation. - Modified tests to verify that TPC-H files use the v2 page format and TPC-DS files use the v1 page format. - Adjusted row group size tests for improved accuracy and clarity.

…nality - Renamed test function to reflect expected page format verification for benchmarks. - Consolidated assertions to dynamically check for expected Parquet version based on benchmark type. - Removed outdated tests for TPC-DS and DuckDB copy functionality to streamline test suite.

qbacpey · 2026-09-03T09:30:29Z


-def init_benchmark_tables(benchmark_type, scale_factor):
-    tables = duckdb.sql("SHOW TABLES").fetchall()
+def init_benchmark_tables(benchmark_type, scale_factor, conn=duckdb):


Ensure we reuse the same connection as generation duckdb

- Renamed parameter in `configure_duckdb_export` from `num_tasks` to `num_threads` for clarity. - Updated `export_tables` to use `args.num_threads` directly for DuckDB configuration. - Adjusted thread pool size in `ThreadPoolExecutor` to match the number of tasks, improving concurrency handling. - Enhanced help message in argument parser for better understanding of thread usage.

- Added validation for the `--memory-limit` argument to ensure it is only used with TPC-DS generation and is a positive value. - Implemented default memory limit calculation based on available system RAM when `--memory-limit` is not specified for TPC-DS and DuckDB usage. - Updated argument parser to reflect changes in memory limit handling and removed the deprecated `--duckdb-memory-limit` parameter. - Adjusted DuckDB connection configuration to utilize the new memory limit format.

- Added specific issue links to TODO comments in `generate_data_files.py`, `requirements.txt`, and `parquet_file_metadata_test.py` for improved traceability of tasks related to memory limit handling, DuckDB updates, and Parquet file compatibility.

shrshi · 2026-09-04T19:43:09Z

+            write_part(task, conn)
+        return
+
+    with ThreadPoolExecutor(max_workers=num_tasks) as executor:


Should we limit export concurrency here by args.num_threads? IIUC, every output file gets its own worker thread so in the case of many writers, it might make sense to limit number of worker threads to something like min(args.num_threads, num_tasks)

Export concurrency (writers) and args.num_threads control different layers of parallelism:

args.num_threads configures DuckDB's thread pool capacity.

writers controls how many COPY statements can be in flight concurrently.

So coupling them might not be so desirable. But it do make sense to add a cap for the writer (max_workers = min(num_tasks, available_cpus)).

For example, the following SF1000 results show COPY time in seconds (the machine has 128 physical cores):

Maximum rows per file Files (N) args.num_threads writers=N writers=N/2 writers=256 writers=128 writers=32

500M 33 128 178 175 — — 171

500M 33 256 200 186 — — 175

100M (default) 83 128 167 174 — — 173

100M 83 256 196 187 — — 200

20M 337 128 201 201 191 187 192

20M 337 256 249 255 262 273 270

5M 1,288 128 256 237 229 215 277

5M 1,288 256 318 321 293 275 330

When the number of output files is small or moderate, limiting writers to between 32 and 256 has little effect. Writer capping becomes more useful when the file count is very large: with 1,288 files, 128 writers outperformed both writers=N and 32 writers.

Added in 06da867

…. Updated `export_tables` to limit `ThreadPoolExecutor` workers to the lesser of specified tasks and available CPUs, enhancing performance and resource management during Parquet file exports.

Upgrade DuckDB

356d9c7

qbacpey changed the title ~~Phase 1: bring the DuckDB (TPC-DS) generation path in line with the TPC-H path~~ Phase 1: bring TPC-DS generation path align with TPC-H path Aug 10, 2026

qbacpey commented Aug 10, 2026

View reviewed changes

qbacpey changed the title ~~Phase 1: bring TPC-DS generation path align with TPC-H path~~ Phase 1: Align the TPC-DS generation path with the TPC-H path Aug 10, 2026

paul-aiyedun reviewed Aug 11, 2026

View reviewed changes

Clean code structure

6763e52

qbacpey commented Aug 11, 2026

View reviewed changes

qbacpey requested a review from paul-aiyedun August 12, 2026 08:03

qbacpey marked this pull request as ready for review August 12, 2026 14:43

qbacpey requested a review from a team as a code owner August 12, 2026 14:43

qbacpey commented Aug 12, 2026

View reviewed changes

Update row group sizing function documentation for clarity

56a7656

- Revised the docstring of `_rows_for_target` to specify that it rounds to the nearest 2,048-row multiple instead of allowing DuckDB to round up, enhancing clarity for future developers.

qbacpey commented Aug 12, 2026

View reviewed changes

qbacpey and others added 5 commits August 17, 2026 18:44

Reduce MEM peak by using temp disk file

cb16d2b

proifle snapshot

af929a0

Add per-task export processes and lifecycle test updates

e618a20

Code clean up

8b773b5

Function renaming & Clean comment

a43a84b

qbacpey added 2 commits September 3, 2026 11:07

qbacpey commented Sep 3, 2026

View reviewed changes

qbacpey and others added 4 commits September 3, 2026 12:02

Merge branch 'main' into duckdb-tpcds-rowgroup-sizing

10b3e4a

shrshi reviewed Sep 4, 2026

View reviewed changes

Cap concurrent writers in data export based on available logical CPUs…

06da867

…. Updated `export_tables` to limit `ThreadPoolExecutor` workers to the lesser of specified tasks and available CPUs, enhancing performance and resource management during Parquet file exports.

		from generate_data_files import generate_data_files


		def test_max_rows_per_file_splits_tables_tpcds(setup_and_teardown):

Table	Outer Writers	`LIMIT` / `OFFSET` Median Copy Time	`rowid` Median Copy Time
`inventory`	1	7.46s	2.42s
`inventory`	4	2.07s	0.70s
`inventory`	8	1.36s	0.44s
`store_sales`	1	17.89s	6.20s
`store_sales`	4	7.00s	2.33s
`store_sales`	8	3.86s	1.29s



		def _write_probe(conn, query, path, row_group_rows=None):
		options = "FORMAT parquet, PARQUET_VERSION 'V2'"

Maximum rows per file	Files (`N`)	`args.num_threads`	`writers=N`	`writers=N/2`	`writers=256`	`writers=128`	`writers=32`
500M	33	128	178	175	—	—	171
500M	33	256	200	186	—	—	175
100M (default)	83	128	167	174	—	—	173
100M	83	256	196	187	—	—	200
20M	337	128	201	201	191	187	192
20M	337	256	249	255	262	273	270
5M	1,288	128	256	237	229	215	277
5M	1,288	256	318	321	293	275	330

		_MAX_PROBE_SCALE_FACTOR = 10
		_PROBE_MEMORY_LIMIT = "8GB"

Uh oh!

Conversation

qbacpey commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TL;DR

Generation overhead

Overhead

Accuracy

Query overhead comparsion

Tests

Uh oh!

copy-pr-bot Bot commented Aug 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paul-aiyedun left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qbacpey Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qbacpey Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

copy-pr-bot Bot commented Sep 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qbacpey Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qbacpey commented Aug 10, 2026 •

edited

Loading

qbacpey Aug 11, 2026 •

edited

Loading

qbacpey Aug 12, 2026 •

edited

Loading

qbacpey Sep 8, 2026 •

edited

Loading