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.

Sign up for GitHub

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

Jump to bottom

sql: bogus int/interval conversions #25241

Closed
knz opened this issue May 2, 2018 · 0 comments · Fixed by #25257
Closed

sql: bogus int/interval conversions #25241

knz opened this issue May 2, 2018 · 0 comments · Fixed by #25257
Assignees
@knz
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. S-3-erroneous-edge-case Database produces or stores erroneous data without visible error/warning, in rare edge cases.

Comments

@knz
Copy link
Contributor

knz commented May 2, 2018

  • conversion from interval to int,decimal,float discards the day/month part silently
  • conversion from interval to int and back again does not roundtrip the value properly.
@knz knz added the fixitday label May 2, 2018
@knz knz self-assigned this May 2, 2018
@knz knz mentioned this issue May 2, 2018
Merged
@knz knz added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label May 2, 2018
@knz knz added this to Triage in (DEPRECATED) SQL Front-end, Lang & Semantics via automation May 2, 2018
@knz knz moved this from Triage to Current milestone in (DEPRECATED) SQL Front-end, Lang & Semantics May 2, 2018
craig bot pushed a commit that referenced this issue May 3, 2018
@knz
Merge #25257
9a54f77 
25257: sql: fix the interval/numeric conversions r=knz a=knz

Fixes #25241.

CockroachDB supports converting intervals to/from `int`. This is a
CockroachDB-specific feature not supported (yet?) in PostgreSQL.

This feature was added in order to simplify analytic queries in a CRL
internal database.

Until now this feature was undocumented and, arguably, non-public.

It was also wrong:

- intervals with a month or day part would be truncated silently. This
  is surprising and arguably incorrect.

- the interval conversion to int would produce the number of seconds,
  whereas the conversion from int to interval would use a number of
  microseconds. This prevented the idempotence of the conversion
  `::int::interval::int` or `::interval::int::interval`.

This patch fixes this internal feature to make it more correct.
It also extends it to other types (float, decimal) for consistency.

Release note (bug fix): the CockroachDB-specific, undocumented
conversion from `interval` to/from numeric types was wrong and has
been corrected. It may become documented in the future.

Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
@craig craig bot closed this as completed in #25257 May 3, 2018
@knz knz moved this from Current milestone to Finished (milestone 0423) in (DEPRECATED) SQL Front-end, Lang & Semantics May 3, 2018
@knz knz added the S-3-erroneous-edge-case Database produces or stores erroneous data without visible error/warning, in rare edge cases. label May 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@knz knz

Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. S-3-erroneous-edge-case Database produces or stores erroneous data without visible error/warning, in rare edge cases.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql: fix the interval/numeric conversions knz/cockroach
1 participant
@knz