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. The error of every table but one lands within −5.9% to +7.1%.
Parquet V2: all output uses V2 encodings. This needs DuckDB moves 1.3.2 -> 1.5.5.
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.

Results

Measured end to end on lab machine, TPC-DS.

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	materialize	probe wait	write	total
SF1	10.9s	0.9s	4.8s	16.6s
SF10	99.7s	0.0s	24.5s	124.2s
SF30	227.8s	0.0s	36.6s	264.4s
SF100	801.2s	0.0s	436.0s	1237.2s

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

Accuracy

Most probing error lands within −5.9% to +7.1%

inventory is the one exception, it has a discontinuity the other tables do not; see appendix B.

Tests

All 15 tests pass with DuckDB 1.5.5.

The CI row-group test uses a 1 MiB target so that SF1 tables contain enough row groups to validate sizing. The larger-target accuracy matrix above comes from manual runs.

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.

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

		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'"

		_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

Results

Overhead

Accuracy

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

qbacpey commented Aug 10, 2026 •

edited

Loading

qbacpey Aug 11, 2026 •

edited

Loading

qbacpey Aug 12, 2026 •

edited

Loading