Skip to content

bench: add date_part benchmark#23350

Open
theirix wants to merge 1 commit into
apache:mainfrom
theirix:bench-date_part
Open

bench: add date_part benchmark#23350
theirix wants to merge 1 commit 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?

Refers #23351

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.

}

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.

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.

2 participants