Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The output of strptime may be off by one week #9869

Closed
1 task done
daniarleagk opened this issue Dec 1, 2023 · 3 comments · Fixed by #9890
Closed
1 task done

The output of strptime may be off by one week #9869

daniarleagk opened this issue Dec 1, 2023 · 3 comments · Fixed by #9890
Assignees
Labels
Needs Documentation Use for issues or PRs that require changes in the documentation reproduced

Comments

@daniarleagk
Copy link

What happens?

Hello,

I observed this behavior for the strptime/try_strptime functions:

 duckdb.sql("""
        select 
            try_strptime('2009-W53', '%Y-W%W') e_d_2010_01_03,   
            week(try_strptime('2009-W53', '%Y-W%W')) e_53, 
            week(try_strptime('2009-W52', '%Y-W%W'))  e_52, 
            week(to_timestamp(1262476800)) e_53,
            strftime(to_timestamp(1262476800), '%Y-W%W') as_expected, 
               
    """).show()

Result:

┌─────────────────────┬───────┬───────┬───────┬─────────────┐
│   e_d_2010_01_03    │ e_53  │ e_52  │ e_53  │ as_expected │
│      timestamp      │ int64 │ int64 │ int64 │   varchar   │
├─────────────────────┼───────┼───────┼───────┼─────────────┤
│ 2010-01-10 00:00:00 │     1 │    53 │    53 │ 2010-W00    │
└─────────────────────┴───────┴───────┴───────┴─────────────┘

I expected that the timestamp value be either 2009-12-28 or 2010-01-03 for strptime function, however, it returns one week after.
The result strftime on the otherhand is consistent with strftime c function (applying %G-W%V in c/python return 2009-W53). Could you please check the output for these functions?

To Reproduce

 duckdb.sql("""
        select 
            try_strptime('2009-W53', '%Y-W%W') e_d_2010_01_03,   
            week(try_strptime('2009-W53', '%Y-W%W')) e_53, 
            week(try_strptime('2009-W52', '%Y-W%W'))  e_52, 
            week(to_timestamp(1262476800)) e_53,
            strftime(to_timestamp(1262476800), '%Y-W%W') as_expected, 
               
    """).show()

OS:

Ubuntu 22.04.1 LTS, Windows 10/11 all x64

DuckDB Version:

0.9.2, 0.9.1

DuckDB Client:

Python

Full Name:

Daniar Achakeev

Affiliation:

HMS Analytical Software GmbH

Have you tried this on the latest main branch?

I have tested with a release build (and could not test with a main build)

Have you tried the steps to reproduce? Do they include all relevant data and configuration? Does the issue you report still appear there?

  • Yes, I have
@szarnyasg szarnyasg changed the title strptime The output of strptime may be off by one week Dec 1, 2023
@hawkfish hawkfish self-assigned this Dec 3, 2023
@hawkfish
Copy link
Contributor

hawkfish commented Dec 4, 2023

There is no ISO week 0. We seem to be confused between 0 and 1 based numbering.

@hawkfish
Copy link
Contributor

hawkfish commented Dec 4, 2023

OK, I think part of the problem here is that the %W/%U format codes are not ISO week numbers - they are simply week numbers based on the year. Week 1 is the first Sunday/Monday in the year, so there can be a week 0. Python has added a %V format code that would maybe make this all less confusing.

@hawkfish
Copy link
Contributor

hawkfish commented Dec 4, 2023

There was a small problem here caused by the %W format code using Sunday as the default weekday instead of Monday, which was very confusing. Since Python doesn't support %W without a weekday but we do, this should be fixed.

hawkfish added a commit to hawkfish/duckdb that referenced this issue 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: duckdb#9869
fixes: duckdblabs/duckdb-internal#838
hawkfish added a commit to hawkfish/duckdb that referenced this issue Dec 4, 2023
Missing file from commit.
Mytherin added a commit that referenced this issue Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Documentation Use for issues or PRs that require changes in the documentation reproduced
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants