Skip to content

Conversation

hawkfish
Copy link
Contributor

@hawkfish hawkfish commented Dec 4, 2023

If no weekday is specified for %W, use Monday, not Sunday as the default.
This is less confusing, and since Python doesn't support it, we get to make the rules.

fixes: #9869
fixes: duckdblabs/duckdb-internal#838

If no weekday is specified for %W, use Monday, not Sunday as the default.
This is less confusing, and since Python doesn't support it,
we get to make the rules.

fixes: duckdb#9869
fixes: duckdblabs/duckdb-internal#838
Missing file from commit.
@hawkfish hawkfish requested a review from Mytherin December 4, 2023 23:26
@@ -90,9 +90,9 @@ SELECT try_strptime('30', '%U'), strftime('1900-07-29'::DATE, '%U')
1900-07-29 00:00:00 30

query II
SELECT try_strptime('30', '%W'), strftime('1900-07-29'::DATE, '%W')
SELECT try_strptime('30', '%W'), strftime('1900-07-23'::DATE, '%W')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we also add a test with %U (which results in WEEK_NUMBER_PADDED_SUN_FIRST)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The line above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The date changed because we are now using the first day of the week as a weekday default instead of Sunday. It actually tests the change!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh yeah, makes sense

@Mytherin Mytherin merged commit 3cadd21 into duckdb:main Dec 6, 2023
@hawkfish hawkfish deleted the strptime-week branch December 9, 2023 16:55
krlmlr added a commit to duckdb/duckdb-r that referenced this pull request Dec 14, 2023
Merge pull request duckdb/duckdb#9890 from hawkfish/strptime-week
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The output of strptime may be off by one week
2 participants