cockroachdb / cockroach

CockroachDB — the cloud native, distributed SQL database designed for high availability, effortless scale, and control over data placement.
https://www.cockroachlabs.com
Other
29.89k stars 3.77k forks source link

sql: 'infinity'::time should be greater than any other time #129148

Open XiaochenCui opened 1 month ago

XiaochenCui commented 1 month ago

In the current implementation, 'infinity'::time is equal to '23:59:59.999999' and smaller than '24:00:00'. We should make it greater than any other time to fit the semantics of 'infinity', and the updated behaviour will also aligne with pr https://github.com/cockroachdb/cockroach/pull/127141

demo@127.0.0.1:26257/demoapp/movr> SELECT 'infinity'::time = '23:59:59.999999'::time;
  ?column?
------------
     t

demo@127.0.0.1:26257/demoapp/movr> SELECT 'infinity'::time < '24:00:00'::time;
  ?column?
------------
     t

Alternatively, we can align it's behaviour with postgres, which prohibits 'infinity'::time:

postgres=# SELECT 'infinity'::time;
ERROR:  invalid input syntax for type time: "infinity"
LINE 1: SELECT 'infinity'::time;
               ^
postgres=# SELECT 'infinity'::time > '23:59:59.999999'::time;
ERROR:  invalid input syntax for type time: "infinity"
LINE 1: SELECT 'infinity'::time > '23:59:59.999999'::time;

This issue complements issue https://github.com/cockroachdb/cockroach/issues/41564

Jira issue: CRDB-41399

blathers-crl[bot] commented 1 month ago

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

It looks like you have not filled out the issue in the format of any of our templates. To best assist you, we advise you to use one of these templates.

I have CC'd a few people who may be able to assist you:

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

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

XiaochenCui commented 1 month ago

Should I edit this issue to fit the format of the template?

rytaft commented 1 month ago

Should I edit this issue to fit the format of the template?

No need, thanks

fqazi commented 2 weeks ago

@rytaft This seems more like sql-queries change or is there something specific we need on the foundations side to support this (i.e. related to the type or something else)?

XiaochenCui commented 2 weeks ago

hi @fqazi

this issue is legit but it may be hard to implement, "TIME" data type may not have extra space to store special values (especially for values outside [00:00:00 - 24:00:00]) (I'm not sure about that and going to verify it later)

rytaft commented 1 week ago

This felt to me like a SQL Foundations issue since it is related to the TIME datatype. But I agree that the line here is fuzzy. I'll add it to our project as well.

fqazi commented 1 week ago

Ah that makes more sense, and definitely has foundations aspects then. Thank you @rytaft @XiaochenCui