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

Error while parsing timestamps with times between midnight and 1am formatted in the 12-hour clock convention #4502

Closed
1 task done
jerrinot opened this issue May 14, 2024 · 2 comments · Fixed by #4506
Closed
1 task done
Assignees

Comments

@jerrinot
Copy link
Contributor

To reproduce

This test fails:

@Test
public void test12HourClockAfterMidnight() throws Exception {
  DateFormat fmt = compiler.compile("MM/dd/yyyy HH:mm:ss a", false);
  long millis = fmt.parse("04/07/2018 12:30:00 AM", defaultLocale);
}

QuestDB expects hours in timestamps in the 12-hour clock system to be between 00 - 11
.
It interprets 00:01 AM as one minute after midnight. However, the usual way to format 1 minute after midnight is 12:01 AM and not 00:01 AM. The 12-hour clock transitions from 12:59 AM to 01:00 AM. 🤯

It sounds counter-intuitive, the transition 1 hour after midnight looks abrupt. but it makes sense if you think about good ol' analog clock. See: https://en.wikipedia.org/wiki/12-hour_clock

A fix should change:

  1. both the generic DateFormat parser and the specialized compiler
  2. also the reverse part: timestamp printing

QuestDB version:

7.4.2

OS, in case of Docker specify Docker and the Host OS:

irrelevant

File System, in case of Docker specify Host File System:

irrelevant

Full Name:

Jaromir Hamala

Affiliation:

QuestDB

Have you followed Linux, MacOs kernel configuration steps to increase Maximum open files and Maximum virtual memory areas limit?

  • Yes, I have

Additional context

No response

@jerrinot
Copy link
Contributor Author

jerrinot commented May 14, 2024

https://questdb.io/docs/reference/function/date-time/#timestamp-format shows another placeholder for hours in the 12-hour convention:

h - Hour in am/pm (1-12)

However this has some funny behaviour too. This test is failing:

@Test
public void testFirstHourIn12HourClock() throws Exception {
  assertThat("MM/dd/yyyy hh:mm:ss a", "2017-04-09T00:01:00.000Z", "04/09/2017 12:01:00 am");
}

It parses the timestamp as 2017-04-09T11:01:00.000Z. It's just subtracting an hour, instead of doing the right thing.

@jerrinot jerrinot self-assigned this May 14, 2024
jerrinot added a commit that referenced this issue May 15, 2024
The 'hh' pattern stands 'Hour in am/pm (1-12)'
The original implementation was subtracting 1.

As a result it parsed e.g. 05:00 as if it was 04:00,
which is clearly wrong. It adds tests comparing 12-hour
clock parser/formatter with a refernce SimpleDateFormat from JDK.

Fixes #4502
@jerrinot
Copy link
Contributor Author

another related issue: #4421 - I discovered this while trying to import the CSV file mentioned in #4421

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 a pull request may close this issue.

1 participant