Skip to content

bench: add date_part benchmark#23350

Merged
alamb merged 4 commits into
apache:mainfrom
theirix:bench-date_part
Jul 9, 2026
Merged

bench: add date_part benchmark#23350
alamb merged 4 commits into
apache:mainfrom
theirix:bench-date_part

Conversation

@theirix

@theirix theirix commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

date_part UDF and other datetime functions have non-trivial logic and should be benchmarked

What changes are included in this PR?

Brand new bench for date_part. Covers a lot of code paths and input combinations

Are these changes tested?

I've run it locally; it passes.

Are there any user-facing changes?

@github-actions github-actions Bot added the functions Changes to functions implementation label Jul 6, 2026
@theirix theirix marked this pull request as ready for review July 6, 2026 21:29

@kosiew kosiew left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@theirix
Looks good to me.
I left a couple of small suggestions that could make the benchmarks a bit more representative and easier to compare across runs.

)
}

fn generate_date64_array(rng: &mut ThreadRng) -> Date64Array {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition. One small thought: Date64Array values are milliseconds since epoch, but this generator uses 0..30_000, so every value lands within the first 30 seconds of 1970-01-01. That still exercises the Date64 branch, but the calendar-part benchmarks like year, month, week, and day may be less representative than Date32.

Could be worth generating something like days * SEC_DAY * 1_000, or another millisecond date range, so Date64 measures more realistic date extraction work.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, updated them using the same set of constants.

}

fn criterion_benchmark(c: &mut Criterion) {
let mut rng = rand::rng();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small benchmark hygiene suggestion: the data is regenerated from ThreadRng on each run. That can make Criterion comparisons a bit noisier when looking into regressions.

Using a seeded StdRng would keep the benchmark data stable across runs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched to StdRng. I believe ThreadRng in other tests should also be replaced with a seeded generator - maybe later

@alamb alamb added this pull request to the merge queue Jul 9, 2026
@alamb

alamb commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Thanks @theirix and @kosiew

Merged via the queue into apache:main with commit 1dc73dc Jul 9, 2026
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functions Changes to functions implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants