Skip to content

Conversation

@simonvandel
Copy link
Contributor

Which issue does this PR close?

Rationale for this change

The table functions range and generate_series now matches the functionality of the scalar functions of the same name.

What changes are included in this PR?

  • Introduce SeriesValue which abstracts how different value-types are generated
  • Implement SeriesValue for integers and timestamps. The timestamp logic is also used for date generation

Are these changes tested?

Yes, added more slt tests.

Are there any user-facing changes?

Yes, range and generate_series table functions are now more powerful.

@github-actions github-actions bot added sqllogictest SQL Logic Tests (.slt) functions Changes to functions implementation labels Jun 25, 2025
@alamb
Copy link
Contributor

alamb commented Jun 26, 2025

FYI @Abdullahsab3 perhaps you would be interested in reviewing this PR as well

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @simonvandel -- I went through this PR pretty carefully and I found it a joy to review. It was well structured, well commented, and well tested. 🏆 for sure

I have nothing to suggest

I actually ran a code coverage analysis here:

cargo llvm-cov --html test --test sqllogictests -- table_functions

report.zip

And pretty much all the code is covered except for some argument error checking which I think is totally find.

}

/// Trait for values that can be generated in a series
trait SeriesValue: fmt::Debug + Clone + Send + Sync + 'static {
Copy link
Contributor

Choose a reason for hiding this comment

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

this is quite clever

@alamb alamb merged commit 2999e41 into apache:main Jun 27, 2025
27 checks passed
Copy link
Contributor

@Abdullahsab3 Abdullahsab3 left a comment

Choose a reason for hiding this comment

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

Thanks @simonvandel Very nice PR. I too enjoyed going through it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functions Changes to functions implementation sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

generate_series UDTF only supports integers

3 participants