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

make_timestamp/make_timestamptz returns invalid timestamp when seconds < 1 #126766

Closed
Informatic opened this issue Jul 5, 2024 · 3 comments · Fixed by #136804
Closed

make_timestamp/make_timestamptz returns invalid timestamp when seconds < 1 #126766

Informatic opened this issue Jul 5, 2024 · 3 comments · Fixed by #136804
Labels
A-sql-builtins SQL built-in functions and semantics thereof. branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-community Originated from the community T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@Informatic
Copy link

Informatic commented Jul 5, 2024

Describe the problem

make_timestamp and make_timestamptz seem to return invalid date (years in the past) when sec argument is lower than 1

To Reproduce

What did you do? Describe in your own words.

If possible, provide steps to reproduce the behavior:

  1. Set up CockroachDB cluster
  2. Send SQL: select make_timestamp(1, 1, 1, 0, 0, 0);
  3. Look at result:
         make_timestamp
---------------------------------
  0293-09-22 00:12:43.145224 BC
(1 row)

Expected behavior

    make_timestamp
-----------------------
  0001-01-01 00:00:00
(1 row)

Environment:

  • CockroachDB version v23.1.23
  • Server OS: Docker
  • Client app: cockroach sql

Additional context
Issue seemingly stems from modulo by zero in implementation which returns NaN, which then is casted to int (resulting in a large negative number).

This should be solved with something along the lines of:

truncatedSec := math.Floor(sec)
var nsec float64
if truncatedSec == 0 {
  nsec = sec * float64(time.Second)
} else {
  nsec = math.Mod(sec, truncatedSec) * float64(time.Second)
}
t := time.Date(year, month, day, hour, min, int(truncatedSec), int(nsec), location)

Jira issue: CRDB-40113

Epic CRDB-43310

@Informatic Informatic added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Jul 5, 2024
Copy link

blathers-crl bot commented Jul 5, 2024

Hi @Informatic, please add branch-* labels to identify which branch(es) this C-bug affects.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

Copy link

blathers-crl bot commented Jul 5, 2024

Hello, I am Blathers. I am here to help you get the issue triaged.

Hoot - a bug! Though bugs are the bane of my existence, rest assured the wretched thing will get the best of care here.

I was unable to automatically find someone to ping.

If we have not gotten back to your issue within a few business days, you can try the following:

  • Join our community slack channel and ask on #cockroachdb.
  • Try find someone from here if you know they worked closely on the area and CC them.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@blathers-crl blathers-crl bot added O-community Originated from the community X-blathers-untriaged blathers was unable to find an owner labels Jul 5, 2024
@yuzefovich yuzefovich added T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) and removed X-blathers-untriaged blathers was unable to find an owner labels Jul 8, 2024
@rafiss rafiss added the A-sql-builtins SQL built-in functions and semantics thereof. label Jul 9, 2024
@rafiss
Copy link
Collaborator

rafiss commented Jul 9, 2024

@Informatic Thanks for the bug report!

@Dedej-Bergin I'm assigning this to you since you are currently on-call. It looks like we should be able to fix this by handling the edge case a little more carefully.

@rimadeodhar rimadeodhar added the branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 label Jul 23, 2024
craig bot pushed a commit that referenced this issue Dec 5, 2024
136738: workload/schemachanger: fix drop columns during PK swaps r=fqazi a=fqazi

Previously, if a declarative alter primary key and drop column were executed concurrently this workload did not correctly pick the correct columns. It was possible for the new primary key column to be selected for the drop operation. To address this, this patch allows potential execution errors for invalid column references when a PK swap is in progress.

Fixes: #134056
Fixes: #136661
Fixes: #136405
Fixes: #132298

Release note: None

136754: revert: move revertccl out of ccl r=dt a=dt

Release note: none.
Epic: none.

136803: scop: fix non-redaction of scop Type, Phase, and Stage r=rafiss a=rafiss

Epic: None
Release note: None

136804: builtins: fix handling of <1 seconds in make_timestamp{tz} r=rafiss a=rafiss

On some platforms, `make_timestamp(1, 1, 1, 0, 0, 0)` would return a value of `0293-09-22 00:12:43.145224 BC`. Now all platforms will compute the correct value of `0001-01-01 00:00:00`.

Also, on some platforms a fractional second value that is between 0 and 1 (exclusive), would always be interpeted as zero nanoseconds. Now it will always be converted to nanoseconds correctly.

fixes #126766
Release note (bug fix): Fixed a bug that would cause the make_timestamp and make_timestamptz builtin functions to incorrectly extract the `seconds` argument if the value was less than 1.

Co-authored-by: Faizan Qazi <[email protected]>
Co-authored-by: David Taylor <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
@craig craig bot closed this as completed in #136804 Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-builtins SQL built-in functions and semantics thereof. branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-community Originated from the community T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
No open projects
Status: Triage
Development

Successfully merging a pull request may close this issue.

5 participants