Skip to content

Conversation

hawkfish
Copy link
Contributor

@hawkfish hawkfish commented Aug 3, 2021

Clean up the code and add special cases for the common situation where the part is a constant string.

Richard Wesley added 4 commits August 2, 2021 12:24
Now that there  are structs for all temporal types,
we can dispense with the separate structs and lose
some duplicated code.
Special case constant date part strings.
Special case constant part string.
Refactor to look like the other part functions.
Copy link
Collaborator

@Mytherin Mytherin left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Looks great. Some comments:

Remove constant optimisation in DATEPART because the translator handles it.
Tweak Quater extraction to use Date::Convert.
@hawkfish
Copy link
Contributor Author

hawkfish commented Aug 3, 2021

I expect removing the constant optimisation should implode the coverage statistics...

Copy link
Collaborator

@Mytherin Mytherin left a comment

Choose a reason for hiding this comment

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

Thanks, looks great now! Two more minor coverage requests, then I think it's ready to go:

Add missing DATE_TRUNC constant tests.
Fix DATE_TRUNC ms/µs confusion.
Add missing YEARWEEK date part and tests.
@Mytherin
Copy link
Collaborator

Mytherin commented Aug 5, 2021

Thanks, looks great!

@Mytherin Mytherin merged commit 8c60305 into duckdb:master Aug 5, 2021
@hawkfish hawkfish deleted the hawkfish-datepart branch August 5, 2021 19:18
hawkfish added a commit to hawkfish/duckdb that referenced this pull request May 21, 2024
Disallow streaming window execution for DISTINCT aggregates.

fixes: duckdblabs/duckdb-internal#2095
Mytherin added a commit that referenced this pull request May 21, 2024
hawkfish added a commit to hawkfish/duckdb that referenced this pull request May 26, 2024
Disallow streaming window execution for DISTINCT aggregates.

fixes: duckdblabs/duckdb-internal#2095
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants