Use unsigned row counts and source row numbers for TPC-DS - #394
Draft
kevinjqliu wants to merge 5 commits into
Draft
Use unsigned row counts and source row numbers for TPC-DS#394kevinjqliu wants to merge 5 commits into
kevinjqliu wants to merge 5 commits into
Conversation
kevinjqliu
marked this pull request as draft
July 27, 2026 00:21
kevinjqliu
commented
Jul 27, 2026
Comment on lines
-33
to
-41
| // Row counts are always non negative, so this conversion never fails. | ||
| // Clamp rather than panic if that ever changes: a wrong progress total | ||
| // should not abort generation. | ||
| debug_assert!( | ||
| row_count >= 0, | ||
| "negative row count for {}: {row_count}", | ||
| table.get_name() | ||
| ); | ||
| let row_count = u64::try_from(row_count).unwrap_or(0); |
Collaborator
Author
There was a problem hiding this comment.
can remove this now that we're using u64
Collaborator
Author
There was a problem hiding this comment.
this and next 24 files are the same change (25 files total, 3 lines changed each)
| /// Generate a row and its child rows (generateRowAndChildRows) | ||
| /// Generate a row and its child rows (generateRowAndChildRows). | ||
| /// | ||
| /// `row_number` is 1-based. |
Collaborator
Author
There was a problem hiding this comment.
calling out that row number is 1 based
|
|
||
| pub fn compute_scd_key(table: Table, row_number: i64) -> SlowlyChangingDimensionKey { | ||
| pub fn compute_scd_key(table: Table, row_number: u64) -> SlowlyChangingDimensionKey { | ||
| assert!(row_number > 0, "row number must be 1-based"); |
Collaborator
Author
There was a problem hiding this comment.
calling this out, row number should be 1 based here.
compute_scd_key now takes u64, and the row_number - 1 branches require row_number > 0 to avoid underflow.
The row_number - 2 branch is also safe under this precondition because it only runs for row_number % 6 == 0, so the smallest positive value there is 6.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #355
Description
This PR changes TPC-DS row counts and source row ordinals from signed integers to
u64where they are inherently non-negative.Changes include:
u64from TPC-DS scaling APIs:get_row_count,get_id_count, andget_row_count_for_date.u64for row-generator source row numbers, skip counts, Arrow source row ranges, and Parquet generation ranges.u64.Reviewer notes
scaling.rsis the source of the row-count type change.row_generator.rs,abstract_row_generator.rs, andrandom/stream.rscarry the source-row/skip-count type change through generator plumbing.tpcdsgen/src/row/tables/*_row_generator.rschanges are mostly mechanical: sourcerow_numberparameters becomeu64, with locali64::try_from(...)only where generated row fields or signed helper APIs still requirei64.tpcdsgen-arrow/src/tables/*.rschanges are also mechanical: Arrow skip/range APIs now takeu64.plan.rs,parquet.rs, andgenerate.rscarry unsigned source-row ranges through CLI generation and Parquet planning.This stops short of changing generated row fields, join-key APIs, date surrogate keys, and random-key APIs to
u64because those paths still model signed TPC-DS key values, date keys, and-1sentinel behavior. Migrating those would be a separate schema/key-domain change rather than part of the row-count/source-row cleanup.