sql: align the behavior of 'infinity' timestamp with Postgres #127141

XiaochenCui · 2024-07-15T01:47:54Z

Description

Previously, SELECT 'infinity'::timestamp will return a concrete timestamp instead of 'infinity', which is incompatible with Postgres, causing issue #41564.

This commit addresses this issue by:

Returning "infinity" for 'infinity'::timestamp, supporting both text and binary formats.

This commit also updates the cmp/eval behavior of "infinity" timestamps. The updated behavior is the same as Postgres:

"infinity" is always larger than other timestamps, and "infinity" equals itself.
"-infinity" is always smaller than other timestamps, and "-infinity" equals itself.
"infinity"/"-infinity" add or subtract any duration results in itself.

Fixes: #41564

Release note (bug fix): Fixed a bug where 'infinity'::timestamp returns different result than postgres.

How to Review

The reviewer is invited to first review the new test cases in pkg/sql/logictest/testdata/logic_test/timestamp to make sure this pr address the issue sql: 'infinity' returned as timestamp rather than 'infinity' #41564 properly.
Then consider the changes to TimeInfinity and TimeNegativeInfinity in pkg/util/timeutil/pgdate/parsing.go.
The changes in pkg/sql/pgwire/types.go is the procedure we encode 'infinity' and '-infinity' timestamp to postgers binary format, and pkg/sql/sem/tree/pgwire_encode.go is the encoding procedure for postgres text format.
Changes in pkg/util/duration/duration.go is the logic for the computational evaluation of 'infinity'.
Other changes are condition checks, test updates, aesthetic changes and block list updates, which can be reviewed quickly.

Tasks

TODO (sql behavior):

Return "infinity" and "-infinity" timestamp.

"infinity" is always larger than other timestamps, "infinity" equals "infinity".

postgres=# SELECT 'infinity'::timestamp > '2004-10-19 10:23:54'::timestamp;
?column?
----------
t

postgres=#  SELECT 'infinity'::timestamp > 'infinity'::timestamp;
?column?
----------
f

postgres=#  SELECT 'infinity'::timestamp = 'infinity'::timestamp;
?column?
----------
t

"-infinity" is always smaller than other timestamps. "-infinity" equals to "-infinity".
"infinity" and "-infinity" add/subtract any value result in itself.
Keep the behaviour of 'infinity'::TIME and '-infinity'::TIME intact.
Update "-4713-11-24 00:00:00 +0000 +0000" in the test lookup_join to "-infinity", update "294276-12-31 23:59:59.999999 +0000 +0000" to "infinity".

TODO (implementation):

binary format: return "math.MaxInt64" for 'infinity'::timestamp (Postgres uses math.MaxInt64 microseconds as the infinity value. https://github.com/postgres/postgres/blob/42aa1f0ab321fd43cbfdd875dd9e13940b485900/src/include/datatype/timestamp.h#L107)
test format: return "infinity" (see: https://github.com/lib/pq/blob/3d613208bca2e74f2a20e04126ed30bcb5c4cc27/encode.go#L365)
Update decoding code.
Add binary format test for it. ("logictest" only tests the text format)
Clear legacy code about this issue. (comments, temporary workaround code, tests)
Remove issue sql: 'infinity' returned as timestamp rather than 'infinity' #41564 from block lists.
Add test for "-infinity" in the file /pkg/sql/pgwire/testdata/encodings.json.

Make to_timestamp function align with type cast.

postgres=#  SELECT 'infinity'::timestamp;
postgres=#  SELECT to_timestamp('infinity');

Unify the code of function MakeDTimestamp and MakeDTimestampTZ.

cockroach/pkg/sql/sem/tree/datum.go

Lines 2592 to 2599 in 1ebd9e1

    
           // MakeDTimestamp creates a DTimestamp with specified precision. 
        
           func MakeDTimestamp(t time.Time, precision time.Duration) (*DTimestamp, error) { 
        
           	ret := t.Round(precision) 
        
           	if ret.After(MaxSupportedTime) || ret.Before(MinSupportedTime) { 
        
           		return nil, NewTimestampExceedsBoundsError(ret) 
        
           	} 
        
           	return &DTimestamp{Time: ret}, nil 
        
           }

cockroach/pkg/sql/sem/tree/datum.go

Lines 2877 to 2883 in 1ebd9e1

    
           // MakeDTimestampTZ creates a DTimestampTZ with specified precision. 
        
           func MakeDTimestampTZ(t time.Time, precision time.Duration) (_ *DTimestampTZ, err error) { 
        
           	if t, err = checkTimeBounds(t, precision); err != nil { 
        
           		return nil, err 
        
           	} 
        
           	return &DTimestampTZ{Time: t}, nil 
        
           }

TODO (review):

Assess the impact of the new logic on performance.

blathers-crl · 2024-07-15T01:47:58Z

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

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

cockroach-teamcity · 2024-07-15T01:48:02Z

This change is

blathers-crl · 2024-07-19T01:46:19Z

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

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

XiaochenCui · 2024-07-19T21:16:48Z

hi, @rafiss
This pr is ready for review.

blathers-crl · 2024-07-19T21:20:45Z

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
Please ensure your git commit message contains a release note.
When CI has completed, please ensure no errors have appeared.

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

rytaft · 2024-07-24T14:31:22Z

Thanks for the contribution, @XiaochenCui! I'll let @rafiss take a look at most of this, but please ping me when you address the TODO in the stats code.

XiaochenCui

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @herkolategan, @nameisbhaskar, @rafiss, and @rytaft)

pkg/sql/logictest/testdata/logic_test/lookup_join line 1343 at r8 (raw file):

Previously, XiaochenCui (Xiaochen Cui) wrote…

don't worry, the behaviour of 'infinity'::TIME and 'infinity'::TIME has been keep intact carefully, see here for details:

https://github.com/cockroachdb/cockroach/pull/127141/files#diff-b5f391eef116d4f2cef73aedd8872a29338217f090831600ab094383720cfd17

	// 24 hours after "MaxSupportedTime". (294277-01-01 23:59:59.999999 +0000 UTC)
	// We keep the "time" part as "23:59:59.999999" to keep the behavior of 'Infinity'::time
	// intact. (In the previous implementation, 'Infinity'::time becomes '23:59:59.999999')
	TimeInfinity    = timeutil.Unix(9224318102399, 999999000)
	TimeInfinitySec = float64(TimeInfinity.Unix())
	// 24 hours before "MinSupportedTime". (-4714-11-23 00:00:00 +0000 UTC)
	// We keep the "time" part as "00:00:00" to keep the behavior of '-Infinity'::time
	// intact. (In the previous implementation, '-Infinity'::time becomes '00:00:00')
	TimeNegativeInfinity    = timeutil.Unix(-210898425600, 0)
	TimeNegativeInfinitySec = float64(TimeNegativeInfinity.Unix())

typo: the behaviour of 'infinity'::TIME and '-infinity'::TIME

XiaochenCui

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @herkolategan, @nameisbhaskar, @rafiss, and @rytaft)

pkg/sql/logictest/testdata/logic_test/lookup_join line 1343 at r8 (raw file):

Previously, XiaochenCui (Xiaochen Cui) wrote…

typo: the behaviour of 'infinity'::TIME and '-infinity'::TIME

I have moved the discussion of 'infinity'::TIME to a separate issue:

#129148

rytaft

Reviewed 2 of 2 files at r10, 1 of 2 files at r11, 2 of 2 files at r12, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @herkolategan, @nameisbhaskar, @rafiss, and @XiaochenCui)

pkg/sql/stats/quantile.go line 366 at r12 (raw file):

//     support round-trip conversions.
//
// TODO(michae2): Add support for DECIMAL and INTERVAL.

why did you remove these values from the comment? Looks like they still aren't supported

pkg/sql/stats/quantile.go line 418 at r12 (raw file):

//     rather than failing the conversion (and thus the entire histogram).
//
// TODO(michae2): Add support for DECIMAL, TIME, TIMETZ, and INTERVAL.

ditto

pkg/util/timeutil/pgdate/parsing.go line 72 at r9 (raw file):

Previously, XiaochenCui (Xiaochen Cui) wrote…

It's the args used to construct "294277-01-01 23:59:59.999999 +0000 UTC", we have to click "EXPAND" to see the full comments.

I see the comment, but it's not obvious to me how you got this number from "294277-01-01 23:59:59.999999 +0000 UTC"

XiaochenCui

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @herkolategan, @nameisbhaskar, @rafiss, and @rytaft)

pkg/sql/stats/quantile.go line 366 at r12 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

why did you remove these values from the comment? Looks like they still aren't supported

Is this the support for TIME and TIMETZ?

code in the master branch:

cockroach/pkg/sql/stats/quantile.go

Lines 382 to 392 in 446bc36

    
           case *tree.DTimestamp: 
        
           	if v.Equal(pgdate.TimeInfinity) || v.Equal(pgdate.TimeNegativeInfinity) { 
        
           		return 0, tree.ErrFloatOutOfRange 
        
           	} 
        
           	return float64(v.Unix()) + float64(v.Nanosecond())*1e-9, nil 
        
           case *tree.DTimestampTZ: 
        
           	// TIMESTAMPTZ doesn't store a timezone, so this is the same as TIMESTAMP. 
        
           	if v.Equal(pgdate.TimeInfinity) || v.Equal(pgdate.TimeNegativeInfinity) { 
        
           		return 0, tree.ErrFloatOutOfRange 
        
           	} 
        
           	return float64(v.Unix()) + float64(v.Nanosecond())*1e-9, nil

code in this pr:

	case *tree.DTimestamp:
		if v.Equal(pgdate.TimeInfinity) {
			return pgdate.TimeInfinitySec, nil
		}
		if v.Equal(pgdate.TimeNegativeInfinity) {
			return pgdate.TimeNegativeInfinitySec, nil
		}
		return float64(v.Unix()) + float64(v.Nanosecond())*1e-9, nil
	case *tree.DTimestampTZ:
		// TIMESTAMPTZ doesn't store a timezone, so this is the same as TIMESTAMP.
		if v.Equal(pgdate.TimeInfinity) {
			return pgdate.TimeInfinitySec, nil
		}
		if v.Equal(pgdate.TimeNegativeInfinity) {
			return pgdate.TimeNegativeInfinitySec, nil
		}
		return float64(v.Unix()) + float64(v.Nanosecond())*1e-9, nil

pkg/sql/stats/quantile.go line 418 at r12 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

ditto

Is this the support for TIME and TIMETZ?

code in the master branch:

cockroach/pkg/sql/stats/quantile.go

Lines 471 to 486 in 446bc36

    
           case types.TimestampFamily, types.TimestampTZFamily: 
        
           	sec, frac := math.Modf(val) 
        
           	var t time.Time 
        
           	// Clamp to (our alternative finite) DTimestamp bounds. 
        
           	if sec <= quantileMinTimestampSec { 
        
           		t = quantileMinTimestamp 
        
           	} else if sec >= quantileMaxTimestampSec { 
        
           		t = quantileMaxTimestamp 
        
           	} else { 
        
           		t = timeutil.Unix(int64(sec), int64(frac*1e9)) 
        
           	} 
        
           	roundTo := tree.TimeFamilyPrecisionToRoundDuration(colType.Precision()) 
        
           	if colType.Family() == types.TimestampFamily { 
        
           		return tree.MakeDTimestamp(t, roundTo) 
        
           	} 
        
           	return tree.MakeDTimestampTZ(t, roundTo)

code in this pr:

	case types.TimestampFamily, types.TimestampTZFamily:
		sec, frac := math.Modf(val)
		var t time.Time
		// Clamp to DTimestamp bounds.
		if sec <= pgdate.TimeNegativeInfinitySec {
			t = pgdate.TimeNegativeInfinity
		} else if sec >= pgdate.TimeInfinitySec {
			t = pgdate.TimeInfinity
		} else {
			t = timeutil.Unix(int64(sec), int64(frac*1e9))
		}
		roundTo := tree.TimeFamilyPrecisionToRoundDuration(colType.Precision())
		if colType.Family() == types.TimestampFamily {
			return tree.MakeDTimestamp(t, roundTo)
		}
		return tree.MakeDTimestampTZ(t, roundTo)

pkg/util/timeutil/pgdate/parsing.go line 72 at r9 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

I see the comment, but it's not obvious to me how you got this number from "294277-01-01 23:59:59.999999 +0000 UTC"

Yeah, it's not easy, I wrote a script to convert a time literal to the args of "timeuitl.Unix":

https://github.com/XiaochenCui/playground/blob/05b6974cc54444b7f25e9e04e283900077dd1d6d/golang/time/main.go

We can prove the result is right after that:

https://go.dev/play/p/z5S_Z4R1q4L

Do you have any ideas for this code? It will be great if we can make if clearer.

rytaft

This is getting really close! Once you fix up the comments I think it's ready to merge.

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @herkolategan, @nameisbhaskar, @rafiss, and @XiaochenCui)

pkg/sql/stats/quantile.go line 366 at r12 (raw file):

Previously, XiaochenCui (Xiaochen Cui) wrote…

Is this the support for TIME and TIMETZ?

code in the master branch:

cockroach/pkg/sql/stats/quantile.go

Lines 382 to 392 in 446bc36

case *tree.DTimestamp:

if v.Equal(pgdate.TimeInfinity) || v.Equal(pgdate.TimeNegativeInfinity) {

return 0, tree.ErrFloatOutOfRange

}

return float64(v.Unix()) + float64(v.Nanosecond())*1e-9, nil

case *tree.DTimestampTZ:

// TIMESTAMPTZ doesn't store a timezone, so this is the same as TIMESTAMP.

if v.Equal(pgdate.TimeInfinity) || v.Equal(pgdate.TimeNegativeInfinity) {

return 0, tree.ErrFloatOutOfRange

}

return float64(v.Unix()) + float64(v.Nanosecond())*1e-9, nil

code in this pr:
	case *tree.DTimestamp:
		if v.Equal(pgdate.TimeInfinity) {
			return pgdate.TimeInfinitySec, nil
		}
		if v.Equal(pgdate.TimeNegativeInfinity) {
			return pgdate.TimeNegativeInfinitySec, nil
		}
		return float64(v.Unix()) + float64(v.Nanosecond())*1e-9, nil
	case *tree.DTimestampTZ:
		// TIMESTAMPTZ doesn't store a timezone, so this is the same as TIMESTAMP.
		if v.Equal(pgdate.TimeInfinity) {
			return pgdate.TimeInfinitySec, nil
		}
		if v.Equal(pgdate.TimeNegativeInfinity) {
			return pgdate.TimeNegativeInfinitySec, nil
		}
		return float64(v.Unix()) + float64(v.Nanosecond())*1e-9, nil

No, those are for Timestamp and TimesampTZ, which are different data types from Time and TimeTZ

pkg/sql/stats/quantile.go line 418 at r12 (raw file):

Previously, XiaochenCui (Xiaochen Cui) wrote…

Is this the support for TIME and TIMETZ?

code in the master branch:

cockroach/pkg/sql/stats/quantile.go

Lines 471 to 486 in 446bc36

case types.TimestampFamily, types.TimestampTZFamily:

sec, frac := math.Modf(val)

var t time.Time

// Clamp to (our alternative finite) DTimestamp bounds.

if sec <= quantileMinTimestampSec {

t = quantileMinTimestamp

} else if sec >= quantileMaxTimestampSec {

t = quantileMaxTimestamp

} else {

t = timeutil.Unix(int64(sec), int64(frac*1e9))

}

roundTo := tree.TimeFamilyPrecisionToRoundDuration(colType.Precision())

if colType.Family() == types.TimestampFamily {

return tree.MakeDTimestamp(t, roundTo)

}

return tree.MakeDTimestampTZ(t, roundTo)

code in this pr:
	case types.TimestampFamily, types.TimestampTZFamily:
		sec, frac := math.Modf(val)
		var t time.Time
		// Clamp to DTimestamp bounds.
		if sec <= pgdate.TimeNegativeInfinitySec {
			t = pgdate.TimeNegativeInfinity
		} else if sec >= pgdate.TimeInfinitySec {
			t = pgdate.TimeInfinity
		} else {
			t = timeutil.Unix(int64(sec), int64(frac*1e9))
		}
		roundTo := tree.TimeFamilyPrecisionToRoundDuration(colType.Precision())
		if colType.Family() == types.TimestampFamily {
			return tree.MakeDTimestamp(t, roundTo)
		}
		return tree.MakeDTimestampTZ(t, roundTo)

Same response as above -- those are different datatypes.

pkg/util/timeutil/pgdate/parsing.go line 72 at r9 (raw file):

Previously, XiaochenCui (Xiaochen Cui) wrote…

Yeah, it's not easy, I wrote a script to convert a time literal to the args of "timeuitl.Unix":

https://github.com/XiaochenCui/playground/blob/05b6974cc54444b7f25e9e04e283900077dd1d6d/golang/time/main.go

We can prove the result is right after that:

https://go.dev/play/p/z5S_Z4R1q4L

Do you have any ideas for this code? It will be great if we can make if clearer.

I think if you could put the key pieces of that code into the comment it would help. i.e. describe what the getBinary function does in your script, and include the proof that conversion back to human-readable format works.

It would also help to write this as timeutil.Unix(9224318102399 /* sec */, 999999000 /* nsec */) (ditto below).

I'd also keep some of the previous comment, which includes info about the Postgres behavior and how we differ. Someone reading this code without the context of this PR won't know what you mean by "the previous implementation".

You might also want to add a reference to the issue you opened here.

XiaochenCui

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @herkolategan, @nameisbhaskar, @rafiss, and @rytaft)

pkg/sql/stats/quantile.go line 366 at r12 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

No, those are for Timestamp and TimesampTZ, which are different data types from Time and TimeTZ

Done.

pkg/sql/stats/quantile.go line 418 at r12 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

Same response as above -- those are different datatypes.

Done.

pkg/util/timeutil/pgdate/parsing.go line 72 at r9 (raw file):
I've updated the comments for "TimeInfinity" and "TimeNegativeInfinity", the comments follow a structure of:

what the value is
the reason behind the date part
the reason behind the time part
the implementation of postgres
the process to derive the arguments "sec" and "nsec"

I tried to clarify 'the reason behind the time part' and the explanation still feels a bit unclear. Do you have any suggestions?

For 'the the process to derive the arguments "sec" and "nsec"', I documented the process in the doc comments of function "timeutil.Unix" and referenced it. I didn't write it at "TimeInfinity" since there are at least 4 special timestamps in crdb which need this comment:

TimeInfinity
TimeNegativeInfinity
MaxSupportedTime
MinSupportedTime

describe what the getBinary function does in your script

the getBinary function has nothing to do with this logic, it's used to verify updates in the file "pkg/sql/pgwire/testdata/encodings.json"

rytaft

Thanks for the contribution, @XiaochenCui ! Sorry this took so long for us to get merged.

bors r+

Reviewed 4 of 4 files at r13, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @herkolategan, @nameisbhaskar, @rafiss, and @XiaochenCui)

pkg/util/timeutil/pgdate/parsing.go line 72 at r9 (raw file):

Previously, XiaochenCui (Xiaochen Cui) wrote…

I've updated the comments for "TimeInfinity" and "TimeNegativeInfinity", the comments follow a structure of:

what the value is

the reason behind the date part

the reason behind the time part

the implementation of postgres

the process to derive the arguments "sec" and "nsec"

I tried to clarify 'the reason behind the time part' and the explanation still feels a bit unclear. Do you have any suggestions?

For 'the the process to derive the arguments "sec" and "nsec"', I documented the process in the doc comments of function "timeutil.Unix" and referenced it. I didn't write it at "TimeInfinity" since there are at least 4 special timestamps in crdb which need this comment:

TimeInfinity

TimeNegativeInfinity

MaxSupportedTime

MinSupportedTime

describe what the getBinary function does in your script

the getBinary function has nothing to do with this logic, it's used to verify updates in the file "pkg/sql/pgwire/testdata/encodings.json"

Thanks so much, this helps a lot! I think this is ready to go.

craig · 2024-08-26T18:37:28Z

Build failed (retrying...):

check_generated_code

craig · 2024-08-26T19:31:55Z

Build failed (retrying...):

unit_tests

craig · 2024-08-26T20:14:16Z

Build failed (retrying...):

lint

renatolabs · 2024-08-26T20:18:59Z

bors r-

This PR is failing on the lint step (gofmt). Canceling the bors merge for now.

craig · 2024-08-26T20:19:02Z

Canceled.

Previously, "SELECT 'infinity'::timestamp" would return a concrete timestamp instead of "infinity", which is incompatible with Postgres, causing issue cockroachdb#41564. This commit addresses this issue by: - Returning "infinity" for 'infinity'::timestamp, supporting both text and binary formats. This commit also updates the cmp/eval behavior of "infinity" timestamps. The updated behavior is the same as Postgres: - "infinity" is always larger than other timestamps, and "infinity" equals itself. - "-infinity" is always smaller than other timestamps, and "-infinity" equals itself. - "infinity"/"-infinity" add or subtract any duration results in itself. Fixes: cockroachdb#41564 Release note (bug fix): Fixed a bug where 'infinity'::timestamp returned a different result than Postgres.

XiaochenCui · 2024-08-26T20:34:29Z

lint fixed

rytaft · 2024-08-26T21:06:17Z

Thanks!

bors r+

craig · 2024-08-26T21:49:04Z

Build succeeded:

XiaochenCui

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale)

pkg/sql/pgwire/pgwirebase/encoding.go line 929 at r14 (raw file):

	if i == math.MinInt64 {
		return pgdate.TimeNegativeInfinity
	}

hi, @rytaft , I would like to add comments to explain the meaning of these two special numbers, what do you think? should we open a new PR to add tiny comments?

rytaft · 2024-09-26T15:08:48Z

pkg/sql/pgwire/pgwirebase/encoding.go line 929 at r14 (raw file):

Previously, XiaochenCui (Xiaochen Cui) wrote…

hi, @rytaft , I would like to add comments to explain the meaning of these two special numbers, what do you think? should we open a new PR to add tiny comments?

Hi @XiaochenCui -- you're welcome and encouraged to open a PR to improve comments! Please feel free to tag me as a reviewer.

XiaochenCui requested review from a team as code owners July 15, 2024 01:47

blathers-crl bot added the O-community Originated from the community label Jul 15, 2024

rafiss self-requested a review July 17, 2024 20:16

XiaochenCui marked this pull request as draft July 18, 2024 02:23

XiaochenCui force-pushed the infinity-41564 branch from 8816411 to 9fb7b4d Compare July 19, 2024 01:46

XiaochenCui force-pushed the infinity-41564 branch 2 times, most recently from ecba23d to f2de34b Compare July 19, 2024 21:06

XiaochenCui marked this pull request as ready for review July 19, 2024 21:15

XiaochenCui force-pushed the infinity-41564 branch 2 times, most recently from f2de34b to f7c9283 Compare July 19, 2024 23:50

XiaochenCui requested review from a team as code owners July 19, 2024 23:50

XiaochenCui requested review from herkolategan, nameisbhaskar and rytaft and removed request for a team July 19, 2024 23:50

XiaochenCui force-pushed the infinity-41564 branch 3 times, most recently from b122b3c to 79a3639 Compare July 20, 2024 00:03

XiaochenCui changed the title ~~sql: return 'infinity' literal as type timestamp (align with postgres)~~ sql: align the behavior of 'infinity' timestamp with Postgres Jul 20, 2024

XiaochenCui marked this pull request as draft July 24, 2024 17:29

XiaochenCui commented Aug 20, 2024

View reviewed changes

rytaft requested a review from rafiss August 20, 2024 19:29

rytaft reviewed Aug 20, 2024

View reviewed changes

XiaochenCui commented Aug 20, 2024

View reviewed changes

rytaft reviewed Aug 22, 2024

View reviewed changes

XiaochenCui force-pushed the infinity-41564 branch 3 times, most recently from 6df392c to e2f56dd Compare August 22, 2024 22:23

XiaochenCui commented Aug 22, 2024

View reviewed changes

rytaft approved these changes Aug 26, 2024

View reviewed changes

XiaochenCui force-pushed the infinity-41564 branch from e2f56dd to 63f8a13 Compare August 26, 2024 20:31

craig bot merged commit 0122d9c into cockroachdb:master Aug 26, 2024
7 checks passed

This was referenced Aug 27, 2024

sql: make VALID UNTIL 'infinity' work the same way as VALID UNTIL NULL #116714 #118012

Closed

.NET DateTime.MaxValue Exceeds Supported Timestamp Bounds #125054

Closed

michae2 mentioned this pull request Sep 9, 2024

sql/stats: TestRandomQuantileRoundTrip failed #130371

Closed

XiaochenCui commented Sep 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sql: align the behavior of 'infinity' timestamp with Postgres #127141

sql: align the behavior of 'infinity' timestamp with Postgres #127141

XiaochenCui commented Jul 15, 2024 •

edited

Loading

blathers-crl bot commented Jul 15, 2024

cockroach-teamcity commented Jul 15, 2024

blathers-crl bot commented Jul 19, 2024

XiaochenCui commented Jul 19, 2024 •

edited

Loading

blathers-crl bot commented Jul 19, 2024

rytaft commented Jul 24, 2024

XiaochenCui left a comment

XiaochenCui left a comment

rytaft left a comment

XiaochenCui left a comment

rytaft left a comment

XiaochenCui left a comment

rytaft left a comment

craig bot commented Aug 26, 2024

craig bot commented Aug 26, 2024

craig bot commented Aug 26, 2024

renatolabs commented Aug 26, 2024

craig bot commented Aug 26, 2024

XiaochenCui commented Aug 26, 2024

rytaft commented Aug 26, 2024

craig bot commented Aug 26, 2024

XiaochenCui left a comment

rytaft commented Sep 26, 2024

	// MakeDTimestamp creates a DTimestamp with specified precision.
	func MakeDTimestamp(t time.Time, precision time.Duration) (*DTimestamp, error) {
	ret := t.Round(precision)
	if ret.After(MaxSupportedTime) \|\| ret.Before(MinSupportedTime) {
	return nil, NewTimestampExceedsBoundsError(ret)
	}
	return &DTimestamp{Time: ret}, nil
	}

	// MakeDTimestampTZ creates a DTimestampTZ with specified precision.
	func MakeDTimestampTZ(t time.Time, precision time.Duration) (_ *DTimestampTZ, err error) {
	if t, err = checkTimeBounds(t, precision); err != nil {
	return nil, err
	}
	return &DTimestampTZ{Time: t}, nil
	}

	case *tree.DTimestamp:
	if v.Equal(pgdate.TimeInfinity) \|\| v.Equal(pgdate.TimeNegativeInfinity) {
	return 0, tree.ErrFloatOutOfRange
	}
	return float64(v.Unix()) + float64(v.Nanosecond())*1e-9, nil
	case *tree.DTimestampTZ:
	// TIMESTAMPTZ doesn't store a timezone, so this is the same as TIMESTAMP.
	if v.Equal(pgdate.TimeInfinity) \|\| v.Equal(pgdate.TimeNegativeInfinity) {
	return 0, tree.ErrFloatOutOfRange
	}
	return float64(v.Unix()) + float64(v.Nanosecond())*1e-9, nil

	case types.TimestampFamily, types.TimestampTZFamily:
	sec, frac := math.Modf(val)
	var t time.Time
	// Clamp to (our alternative finite) DTimestamp bounds.
	if sec <= quantileMinTimestampSec {
	t = quantileMinTimestamp
	} else if sec >= quantileMaxTimestampSec {
	t = quantileMaxTimestamp
	} else {
	t = timeutil.Unix(int64(sec), int64(frac*1e9))
	}
	roundTo := tree.TimeFamilyPrecisionToRoundDuration(colType.Precision())
	if colType.Family() == types.TimestampFamily {
	return tree.MakeDTimestamp(t, roundTo)
	}
	return tree.MakeDTimestampTZ(t, roundTo)

sql: align the behavior of 'infinity' timestamp with Postgres #127141

sql: align the behavior of 'infinity' timestamp with Postgres #127141

Conversation

XiaochenCui commented Jul 15, 2024 • edited Loading

Description

How to Review

Tasks

blathers-crl bot commented Jul 15, 2024

cockroach-teamcity commented Jul 15, 2024

blathers-crl bot commented Jul 19, 2024

XiaochenCui commented Jul 19, 2024 • edited Loading

blathers-crl bot commented Jul 19, 2024

rytaft commented Jul 24, 2024

XiaochenCui left a comment

Choose a reason for hiding this comment

XiaochenCui left a comment

Choose a reason for hiding this comment

rytaft left a comment

Choose a reason for hiding this comment

XiaochenCui left a comment

Choose a reason for hiding this comment

rytaft left a comment

Choose a reason for hiding this comment

XiaochenCui left a comment

Choose a reason for hiding this comment

rytaft left a comment

Choose a reason for hiding this comment

craig bot commented Aug 26, 2024

craig bot commented Aug 26, 2024

craig bot commented Aug 26, 2024

renatolabs commented Aug 26, 2024

craig bot commented Aug 26, 2024

XiaochenCui commented Aug 26, 2024

rytaft commented Aug 26, 2024

craig bot commented Aug 26, 2024

XiaochenCui left a comment

Choose a reason for hiding this comment

rytaft commented Sep 26, 2024

XiaochenCui commented Jul 15, 2024 •

edited

Loading

XiaochenCui commented Jul 19, 2024 •

edited

Loading