Skip to content

Commit

Permalink
Merge #96833
Browse files Browse the repository at this point in the history 
96833: tree: cleanup time type formatting r=rafiss a=otan

fixes #41563

Remove all the duplicate time formatting solutions from tree,
centralising on `Format` / `PGWireFormat` instead.

Release note (bug fix): We previously have included the minute/second
offset for TimestampTZ in certain places when casting it to string even
when they were zero. This does not match PostgreSQL and now has been resolved.

Release note (bug fix): We previously used negative years instead of BC
when casting a timestamptz to a string. This has now been resolved.



Co-authored-by: Oliver Tan <[email protected]>
  • Loading branch information
@otan
craig[bot] and otan committed Feb 9, 2023
2 parents faeebee + 255bc76 commit 7b272ff
Show file tree
Hide file tree
Showing 41 changed files with 398 additions and 451 deletions.
2 changes: 1 addition & 1 deletion pkg/ccl/changefeedccl/cdceval/expr_eval_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ $$`)
expectMainFamily: []decodeExpectation{
{
keyValues: []string{" spaced out ", "1"},
allValues: map[string]string{"btrim": "spaced out", "past": "01:00:00+00:00:00"},
allValues: map[string]string{"btrim": "spaced out", "past": "01:00:00+00"},
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/changefeedccl/cdceval/functions_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ func TestEvaluatesCDCFunctionOverloads(t *testing.T) {
p, err := e.Eval(ctx, testRow, cdcevent.Row{})
require.NoError(t, err)

expectedTZ := fmt.Sprintf("%s-01:33:00",
expectedTZ := fmt.Sprintf("%s-01:33",
futureTS.GoTime().Add(-93*time.Minute).Format("15:04:05"))
require.Equal(t, map[string]string{"timezone": expectedTZ}, slurpValues(t, p))
})
Expand Down
34 changes: 17 additions & 17 deletions pkg/kv/kvserver/reports/constraint_stats_report_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -757,15 +757,15 @@ func TestConstraintReport(t *testing.T) {

require.ElementsMatch(t, TableData(ctx, "system.replication_constraint_stats", con), [][]string{
{"1", "3", "'constraint'", "'+country=CH'", "1", "NULL", "0"},
{"2", "3", "'constraint'", "'+country=CH'", "1", "'2001-01-01 10:00:00+00:00'", "1"},
{"5", "6", "'constraint'", "'+ssd'", "1", "'2001-01-01 10:00:00+00:00'", "2"},
{"7", "8", "'constraint'", "'+dc=west'", "1", "'2001-01-01 10:00:00+00:00'", "1"},
{"2", "3", "'constraint'", "'+country=CH'", "1", "'2001-01-01 10:00:00+00'", "1"},
{"5", "6", "'constraint'", "'+ssd'", "1", "'2001-01-01 10:00:00+00'", "2"},
{"7", "8", "'constraint'", "'+dc=west'", "1", "'2001-01-01 10:00:00+00'", "1"},
{"7", "8", "'constraint'", "'+dc=east'", "1", "NULL", "0"},
{"8", "9", "'constraint'", "'+dc=west'", "1", "'2001-01-01 10:00:00+00:00'", "1"},
{"8", "9", "'constraint'", "'+dc=west'", "1", "'2001-01-01 10:00:00+00'", "1"},
{"8", "9", "'constraint'", "'+dc=east'", "1", "NULL", "0"},
})
require.ElementsMatch(t, TableData(ctx, "system.reports_meta", con), [][]string{
{"1", "'2001-01-01 10:00:00+00:00'"},
{"1", "'2001-01-01 10:00:00+00'"},
})
require.Equal(t, 7, r.LastUpdatedRowCount())

Expand All @@ -785,22 +785,22 @@ func TestConstraintReport(t *testing.T) {

require.ElementsMatch(t, TableData(ctx, "system.replication_constraint_stats", con), [][]string{
// Wasn't violated before - is violated now.
{"1", "3", "'constraint'", "'+country=CH'", "1", "'2001-01-01 11:00:00+00:00'", "1"},
{"1", "3", "'constraint'", "'+country=CH'", "1", "'2001-01-01 11:00:00+00'", "1"},
// Was violated before - isn't violated now.
{"5", "6", "'constraint'", "'+ssd'", "1", "NULL", "0"},
// Didn't exist before - new for this run and violated.
{"6", "8", "'constraint'", "'+dc=east'", "1", "'2001-01-01 11:00:00+00:00'", "1"},
{"6", "8", "'constraint'", "'+dc=east'", "1", "'2001-01-01 11:00:00+00'", "1"},
// Didn't exist before - new for this run and not violated.
{"6", "8", "'constraint'", "'+dc=west'", "1", "NULL", "0"},
// Was violated before - and it still is but the range count changed.
{"7", "8", "'constraint'", "'+dc=west'", "1", "'2001-01-01 10:00:00+00:00'", "2"},
{"7", "8", "'constraint'", "'+dc=west'", "1", "'2001-01-01 10:00:00+00'", "2"},
// Was violated before - and it still is but the range count didn't change.
{"8", "9", "'constraint'", "'+dc=west'", "1", "'2001-01-01 10:00:00+00:00'", "1"},
{"8", "9", "'constraint'", "'+dc=west'", "1", "'2001-01-01 10:00:00+00'", "1"},
// Wasn't violated before - and is still not violated.
{"8", "9", "'constraint'", "'+dc=east'", "1", "NULL", "0"},
})
require.ElementsMatch(t, TableData(ctx, "system.reports_meta", con), [][]string{
{"1", "'2001-01-01 11:00:00+00:00'"},
{"1", "'2001-01-01 11:00:00+00'"},
})
require.Equal(t, 7, r.LastUpdatedRowCount())

Expand Down Expand Up @@ -837,16 +837,16 @@ func TestConstraintReport(t *testing.T) {
report = make(ConstraintReport)

require.ElementsMatch(t, TableData(ctx, "system.replication_constraint_stats", con), [][]string{
{"1", "3", "'constraint'", "'+country=CH'", "1", "'2001-01-01 12:00:00+00:00'", "1"},
{"1", "3", "'constraint'", "'+country=CH'", "1", "'2001-01-01 12:00:00+00'", "1"},
{"5", "6", "'constraint'", "'+ssd'", "1", "NULL", "0"},
{"6", "8", "'constraint'", "'+dc=east'", "1", "'2001-01-01 11:00:00+00:00'", "1"},
{"6", "8", "'constraint'", "'+dc=east'", "1", "'2001-01-01 11:00:00+00'", "1"},
{"6", "8", "'constraint'", "'+dc=west'", "1", "NULL", "0"},
{"7", "8", "'constraint'", "'+dc=west'", "1", "'2001-01-01 12:00:00+00:00'", "2"},
{"8", "9", "'constraint'", "'+dc=west'", "1", "'2001-01-01 10:00:00+00:00'", "1"},
{"7", "8", "'constraint'", "'+dc=west'", "1", "'2001-01-01 12:00:00+00'", "2"},
{"8", "9", "'constraint'", "'+dc=west'", "1", "'2001-01-01 10:00:00+00'", "1"},
{"8", "9", "'constraint'", "'+dc=east'", "1", "NULL", "0"},
})
require.ElementsMatch(t, TableData(ctx, "system.reports_meta", con), [][]string{
{"1", "'2001-01-01 12:00:00+00:00'"},
{"1", "'2001-01-01 12:00:00+00'"},
})
require.Equal(t, 3, r.LastUpdatedRowCount())

Expand All @@ -859,10 +859,10 @@ func TestConstraintReport(t *testing.T) {
require.NoError(t, r.Save(ctx, report, time5, db, con))

require.ElementsMatch(t, TableData(ctx, "system.replication_constraint_stats", con), [][]string{
{"1", "3", "'constraint'", "'+country=CH'", "1", "'2001-01-01 12:00:00+00:00'", "1"},
{"1", "3", "'constraint'", "'+country=CH'", "1", "'2001-01-01 12:00:00+00'", "1"},
})
require.ElementsMatch(t, TableData(ctx, "system.reports_meta", con), [][]string{
{"1", "'2001-01-01 12:30:00+00:00'"},
{"1", "'2001-01-01 12:30:00+00'"},
})
require.Equal(t, 6, r.LastUpdatedRowCount())
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/kv/kvserver/reports/critical_localities_report_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ func TestCriticalLocalitiesSaving(t *testing.T) {
{"7", "8", "'dc=B'", "2", "2"},
})
require.ElementsMatch(t, TableData(ctx, "system.reports_meta", con), [][]string{
{"2", "'2001-01-01 10:00:00+00:00'"},
{"2", "'2001-01-01 10:00:00+00'"},
})
require.Equal(t, 3, saver.LastUpdatedRowCount())

Expand All @@ -227,7 +227,7 @@ func TestCriticalLocalitiesSaving(t *testing.T) {
{"15", "6", "'dc=A'", "2", "1"},
})
require.ElementsMatch(t, TableData(ctx, "system.reports_meta", con), [][]string{
{"2", "'2001-01-01 11:00:00+00:00'"},
{"2", "'2001-01-01 11:00:00+00'"},
})
require.Equal(t, 3, saver.LastUpdatedRowCount())

Expand Down Expand Up @@ -267,7 +267,7 @@ func TestCriticalLocalitiesSaving(t *testing.T) {
{"15", "6", "'dc=A'", "2", "1"},
})
require.ElementsMatch(t, TableData(ctx, "system.reports_meta", con), [][]string{
{"2", "'2001-01-01 12:00:00+00:00'"},
{"2", "'2001-01-01 12:00:00+00'"},
})
require.Equal(t, 2, saver.LastUpdatedRowCount())

Expand All @@ -282,7 +282,7 @@ func TestCriticalLocalitiesSaving(t *testing.T) {
{"5", "6", "'dc=A'", "2", "1"},
})
require.ElementsMatch(t, TableData(ctx, "system.reports_meta", con), [][]string{
{"2", "'2001-01-01 12:30:00+00:00'"},
{"2", "'2001-01-01 12:30:00+00'"},
})
require.Equal(t, 3, saver.LastUpdatedRowCount())
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/kv/kvserver/reports/replication_stats_report_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func TestRangeReport(t *testing.T) {
{"2", "4", "3", "1", "0", "1", "0"},
})
require.ElementsMatch(t, TableData(ctx, "system.reports_meta", con), [][]string{
{"3", "'2001-01-01 10:00:00+00:00'"},
{"3", "'2001-01-01 10:00:00+00'"},
})
require.Equal(t, 3, r.LastUpdatedRowCount())

Expand Down Expand Up @@ -120,7 +120,7 @@ func TestRangeReport(t *testing.T) {
{"4", "4", "3", "1", "0", "1", "1"},
})
require.ElementsMatch(t, TableData(ctx, "system.reports_meta", con), [][]string{
{"3", "'2001-01-01 11:00:00+00:00'"},
{"3", "'2001-01-01 11:00:00+00'"},
})
require.Equal(t, 3, r.LastUpdatedRowCount())

Expand Down Expand Up @@ -170,7 +170,7 @@ func TestRangeReport(t *testing.T) {
{"4", "4", "3", "1", "0", "1", "1"},
})
require.ElementsMatch(t, TableData(ctx, "system.reports_meta", con), [][]string{
{"3", "'2001-01-01 12:00:00+00:00'"},
{"3", "'2001-01-01 12:00:00+00'"},
})
require.Equal(t, 3, r.LastUpdatedRowCount())

Expand All @@ -189,7 +189,7 @@ func TestRangeReport(t *testing.T) {
{"1", "3", "3", "1", "0", "1", "1"},
})
require.ElementsMatch(t, TableData(ctx, "system.reports_meta", con), [][]string{
{"3", "'2001-01-01 12:30:00+00:00'"},
{"3", "'2001-01-01 12:30:00+00'"},
})
require.Equal(t, 2, r.LastUpdatedRowCount())
}
Expand Down
32 changes: 12 additions & 20 deletions pkg/sql/colexec/colexecbase/cast.eg.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 5 additions & 7 deletions pkg/sql/colexec/execgen/cmd/execgen/cast_gen_util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -612,20 +612,18 @@ func stringToUUID(to, from, _, _, _ string) string {
}

func timestampToString(to, from, _, toType, _ string) string {
return toString(fmt.Sprintf("%s = []byte(tree.FormatTimestamp(%s))", to, from), to, toType)
return toString(fmt.Sprintf("%s = tree.PGWireFormatTimestamp(%s, nil, r)", to, from), to, toType)
}

func timestampTZToString(to, from, evalCtx, toType, buf string) string {
convStr := `
// Convert to context timezone for correct display.
_t := %[2]s.In(%[3]s.GetLocation())
%[5]s
%[4]s.Reset()
tree.FormatTimestampTZ(_t, %[4]s)
%[1]s = []byte(%[4]s.String())
_t := %[2]s
%[4]s
r = tree.PGWireFormatTimestamp(_t, %[3]s.GetLocation(), r)
`
roundT := roundTimestamp("_t", "_t", "time.Microsecond")
return toString(fmt.Sprintf(convStr, to, from, evalCtx, buf, roundT), to, toType)
return toString(fmt.Sprintf(convStr, to, from, evalCtx, roundT), to, toType)
}

func uuidToString(to, from, _, toType, _ string) string {
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/builtin_function
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2461,7 +2461,7 @@ be09b235155bae6cb96b94ce4645260937e856ac3907d710850256e6351f50b428f948a7af339374
f7fbba6e0636f890e56fbbf3283e524c6fa3204ae298382d624741d0dc6638326e282c41be5e4254d8820772c5518a2c5a8c0c7f7eda19594a7eb539453e1ed7
95bce0fdbcf48ba9c944dae46238d89bbd6df696a0d0b7cc8fc16eeabd30c03d6d2506cfcce81de320b37bc677df1bd045ac9231b43ae11807773db3909d1220
b2d173023893f71caadf7cb2f9557355462570de2c9c971b9cfa5494936e28df8e13d0db4d550aab66d5e7a002f678ddb02def092c069ce473cf5fb293953986
960b0fed9378be1e9adefd91e1be6ac9c1de7208008dfec438ff845135727bebea0f7458a5181079f61288176e0168cfea501b900c3e495b3ab9bbe4d372486d
17ecb6d43868f7502f588817661f5edb85b8e408d304ffbdeddd6b1abbec00dc5dda907b930356f1a031be368ea053626af59a9e74fbe55cf9cb7855acfd5f17
d82c4eb5261cb9c8aa9855edd67d1bd10482f41529858d925094d173fa662aa91ff39bc5b188615273484021dfb16fd8284cf684ccf0fc795be3aa2fc1e6c181

# We only support one encoding, UTF8, which is hardcoded to id 6 just like in
Expand Down
Loading

0 comments on commit 7b272ff

Please sign in to comment.