Skip to content

Commit

Permalink
Merge #91382 #91411
Browse files Browse the repository at this point in the history
91382: builtins: implement to_char for timestamps and intervals r=rafiss a=otan

Resolves #91156

These commits implement to_char for timestamp and interval types. Most
of the code is copied from the PG implementation.

91411: schemachanger: improve panic- and error handling r=postamar a=postamar

Previously, the declarative schema changer would only recover from runtime errors in certain subsystems like building the targets or planning the execution of operations. Consequently an implementation bug leading to a runtime error in the execution layer would trigger a panic which would not be recovered and which would cause the whole process to crash.

This commit fixes this by introducing a common error handler in the form of scerrors.HandleErrorOrPanic, which recovers from panics, wraps errors, and prints informative log messages in a uniform way, to be used at the top of (or near the top of) the declarative schema changer call stack.

Fixes #91400.

Release note (bug fix): fixed a bug in which panics triggered by certain DDL statements were not properly recovered, leading to the cluster node crashing.

Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
Co-authored-by: Marius Posta <marius@cockroachlabs.com>
  • Loading branch information
3 people committed Nov 8, 2022
3 parents 7891d96 + 17a96ea + 93e620e commit 64e1308
Show file tree
Hide file tree
Showing 36 changed files with 2,846 additions and 194 deletions.
6 changes: 6 additions & 0 deletions docs/generated/sql/functions.md
Original file line number Diff line number Diff line change
Expand Up @@ -661,8 +661,14 @@ has no relationship with the commit order of concurrent transactions.</p>
</span></td><td>Immutable</td></tr>
<tr><td><a name="to_char"></a><code>to_char(interval: <a href="interval.html">interval</a>) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Convert an interval to a string assuming the Postgres IntervalStyle.</p>
</span></td><td>Immutable</td></tr>
<tr><td><a name="to_char"></a><code>to_char(interval: <a href="interval.html">interval</a>, format: <a href="string.html">string</a>) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Convert an interval to a string using the given format.</p>
</span></td><td>Stable</td></tr>
<tr><td><a name="to_char"></a><code>to_char(timestamp: <a href="timestamp.html">timestamp</a>) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Convert an timestamp to a string assuming the ISO, MDY DateStyle.</p>
</span></td><td>Immutable</td></tr>
<tr><td><a name="to_char"></a><code>to_char(timestamp: <a href="timestamp.html">timestamp</a>, format: <a href="string.html">string</a>) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Convert an timestamp to a string using the given format.</p>
</span></td><td>Stable</td></tr>
<tr><td><a name="to_char"></a><code>to_char(timestamptz: <a href="timestamp.html">timestamptz</a>, format: <a href="string.html">string</a>) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Convert a timestamp with time zone to a string using the given format.</p>
</span></td><td>Stable</td></tr>
<tr><td><a name="to_timestamp"></a><code>to_timestamp(timestamp: <a href="float.html">float</a>) &rarr; <a href="timestamp.html">timestamptz</a></code></td><td><span class="funcdesc"><p>Convert Unix epoch (seconds since 1970-01-01 00:00:00+00) to timestamp with time zone.</p>
</span></td><td>Immutable</td></tr>
<tr><td><a name="transaction_timestamp"></a><code>transaction_timestamp() &rarr; <a href="date.html">date</a></code></td><td><span class="funcdesc"><p>Returns the time of the current transaction.</p>
Expand Down
4 changes: 4 additions & 0 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,7 @@ ALL_TESTS = [
"//pkg/util/timetz:timetz_test",
"//pkg/util/timeutil/pgdate:pgdate_test",
"//pkg/util/timeutil:timeutil_test",
"//pkg/util/tochar:tochar_test",
"//pkg/util/tracing/collector:collector_test",
"//pkg/util/tracing/grpcinterceptor:grpcinterceptor_test",
"//pkg/util/tracing/service:service_test",
Expand Down Expand Up @@ -2085,6 +2086,8 @@ GO_TARGETS = [
"//pkg/util/timeutil/ptp:ptp",
"//pkg/util/timeutil:timeutil",
"//pkg/util/timeutil:timeutil_test",
"//pkg/util/tochar:tochar",
"//pkg/util/tochar:tochar_test",
"//pkg/util/tracing/collector:collector",
"//pkg/util/tracing/collector:collector_test",
"//pkg/util/tracing/grpcinterceptor:grpcinterceptor",
Expand Down Expand Up @@ -3051,6 +3054,7 @@ GET_X_DATA_TARGETS = [
"//pkg/util/timeutil/gen:get_x_data",
"//pkg/util/timeutil/pgdate:get_x_data",
"//pkg/util/timeutil/ptp:get_x_data",
"//pkg/util/tochar:get_x_data",
"//pkg/util/tracing:get_x_data",
"//pkg/util/tracing/collector:get_x_data",
"//pkg/util/tracing/grpcinterceptor:get_x_data",
Expand Down
49 changes: 49 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/interval
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,55 @@ SELECT i, i::INTERVAL FROM interval_parsing ORDER BY pk
-10 years 22 months -1 day 01:02:03 -8-2 -1 +1:02:03
-10 years 22 months -1 day -01:02:03 -8-2 -1 -1:02:03

# Could not find any regress tests for to_char in PG, so we're making our own up!
statement ok
CREATE TABLE intvl_tbl (id SERIAL, d1 INTERVAL);
INSERT INTO intvl_tbl (d1) VALUES
('355 months 40 days 123:45:12'),
('-400 months -30 days -100:12:13')

query T
SELECT to_char(d1, 'Y,YYY YYYY YYY YY Y CC Q MM WW DDD DD J')
FROM intvl_tbl ORDER BY id
----
0,029 0029 029 29 9 00 3 07 1528 10690 40 1731873
0,-33 -0033 -033 -33 -3 00 0 -04 -1717 -12030 -30 1708823

query T
SELECT to_char(d1, 'FMY,YYY FMYYYY FMYYY FMYY FMY FMCC FMQ FMMM FMWW FMDDD FMDD FMJ')
FROM intvl_tbl ORDER BY id
----
0,029 29 29 29 9 0 3 7 1528 10690 40 1731873
0,-33 -33 -33 -33 -3 0 0 -4 -1717 -12030 -30 1708823

query T
SELECT to_char(d1, 'HH HH12 HH24 MI SS SSSS')
FROM intvl_tbl ORDER BY id
----
03 03 123 45 12 445512
-04 -04 -100 -12 -13 -360733

query T
SELECT to_char(d1, E'"HH:MI:SS is" HH:MI:SS "\\"text between quote marks\\""')
FROM intvl_tbl ORDER BY id
----
HH:MI:SS is 03:45:12 "text between quote marks"
HH:MI:SS is -04:-12:-13 "text between quote marks"

query T
SELECT to_char(d1, 'HH24--text--MI--text--SS')
FROM intvl_tbl ORDER BY id
----
123--text--45--text--12
-100--text---12--text---13

query T
SELECT to_char(d1, 'YYYYTH YYYYth Jth')
FROM intvl_tbl ORDER BY id
----
0029TH 0029th 1731873rd
-0033RD -0033rd 1708823rd

# Test intervalstyle being respected in pg_indexes.

statement ok
Expand Down
215 changes: 215 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/timestamp
Original file line number Diff line number Diff line change
Expand Up @@ -550,3 +550,218 @@ query T
SELECT (t2 - t1) FROM t
----
1 day

# Rough translation of the to_char tests from PostgreSQL.
statement ok
CREATE TABLE TIMESTAMPTZ_TBL (id SERIAL, d1 timestamp(2) with time zone);
INSERT INTO TIMESTAMPTZ_TBL (d1) VALUES
('1997-06-10 17:32:01 -07:00'),
('2001-09-22T18:19:20');

query T
SELECT to_char(d1, 'DAY Day day DY Dy dy MONTH Month month MON Mon mon')
FROM TIMESTAMPTZ_TBL ORDER BY id
----
wednesday Wednesday wednesday wed Wed wed JUNE June june JUN Jun jun
saturday Saturday saturday sat Sat sat SEPTEMBER September september SEP Sep sep

query T
SELECT to_char(d1, 'FMDAY FMDay FMday FMMONTH FMMonth FMmonth')
FROM TIMESTAMPTZ_TBL ORDER BY id
----
wednesday Wednesday wednesday JUNE June june
saturday Saturday saturday SEPTEMBER September september

query T
SELECT to_char(d1, 'Y,YYY YYYY YYY YY Y CC Q MM WW DDD DD D J')
FROM TIMESTAMPTZ_TBL ORDER BY id
----
1,997 1997 997 97 7 20 2 06 24 162 11 4 2450611
2,001 2001 001 01 1 21 3 09 38 265 22 7 2452175

query T
SELECT to_char(d1, 'FMY,YYY FMYYYY FMYYY FMYY FMY FMCC FMQ FMMM FMWW FMDDD FMDD FMD FMJ')
FROM TIMESTAMPTZ_TBL ORDER BY id
----
1,997 1997 997 97 7 20 2 6 24 162 11 4 2450611
2,001 2001 1 1 1 21 3 9 38 265 22 7 2452175

query T
SELECT to_char(d1::timestamp, 'FMY,YYY FMYYYY FMYYY FMYY FMY FMCC FMQ FMMM FMWW FMDDD FMDD FMD FMJ')
FROM TIMESTAMPTZ_TBL ORDER BY id
----
1,997 1997 997 97 7 20 2 6 24 162 11 4 2450611
2,001 2001 1 1 1 21 3 9 38 265 22 7 2452175

query T
SELECT to_char(d1, 'HH HH12 HH24 MI SS SSSS')
FROM TIMESTAMPTZ_TBL ORDER BY id
----
12 12 00 32 01 1921
06 06 18 19 20 65960

query T
SELECT to_char(d1, E'"HH:MI:SS is" HH:MI:SS "\\"text between quote marks\\""')
FROM TIMESTAMPTZ_TBL ORDER BY id
----
HH:MI:SS is 12:32:01 "text between quote marks"
HH:MI:SS is 06:19:20 "text between quote marks"

query T
SELECT to_char(d1, 'HH24--text--MI--text--SS')
FROM TIMESTAMPTZ_TBL ORDER BY id
----
00--text--32--text--01
18--text--19--text--20

query T
SELECT to_char(d1, 'YYYYTH YYYYth Jth')
FROM TIMESTAMPTZ_TBL ORDER BY id
----
1997TH 1997th 2450611th
2001ST 2001st 2452175th

query T
SELECT to_char(d1, 'YYYY A.D. YYYY a.d. YYYY bc HH:MI:SS P.M. HH:MI:SS p.m. HH:MI:SS pm')
FROM TIMESTAMPTZ_TBL ORDER BY id
----
1997 A.D. 1997 a.d. 1997 ad 12:32:01 A.M. 12:32:01 a.m. 12:32:01 am
2001 A.D. 2001 a.d. 2001 ad 06:19:20 P.M. 06:19:20 p.m. 06:19:20 pm

query T
SELECT to_char(d1, 'IYYY IYY IY I IW IDDD ID')
FROM TIMESTAMPTZ_TBL ORDER BY id
----
1997 997 97 7 24 164 3
2001 001 01 1 38 265 6

query T
SELECT to_char(d1, 'FMIYYY FMIYY FMIY FMI FMIW FMIDDD FMID')
FROM TIMESTAMPTZ_TBL ORDER BY id
----
1997 997 97 7 24 164 3
2001 1 1 1 38 265 6

query T
SELECT to_char(d, 'FF1 FF2 FF3 FF4 FF5 FF6 ff1 ff2 ff3 ff4 ff5 ff6 MS US')
FROM (VALUES
('2018-11-02 12:34:56'::timestamptz),
('2018-11-02 12:34:56.78'::timestamptz),
('2018-11-02 12:34:56.78901'::timestamptz),
('2018-11-02 12:34:56.78901234'::timestamptz)
) d(d)
----
0 00 000 0000 00000 000000 0 00 000 0000 00000 000000 000 000000
7 78 780 7800 78000 780000 7 78 780 7800 78000 780000 780 780000
7 78 789 7890 78901 789010 7 78 789 7890 78901 789010 789 789010
7 78 789 7890 78901 789012 7 78 789 7890 78901 789012 789 789012

query TT
SET timezone = '00:00';
SELECT to_char(now(), 'OF') as of_t, to_char(now(), 'TZH:TZM') as "TZH:TZM";
----
+00 +00:00

query TT
SET timezone = '+02:00';
SELECT to_char(now(), 'OF') as of_t, to_char(now(), 'TZH:TZM') as "TZH:TZM";
----
-02 -02:00

query TT
SET timezone = '-13:00';
SELECT to_char(now(), 'OF') as of_t, to_char(now(), 'TZH:TZM') as "TZH:TZM";
----
+13 +13:00

query TT
SET timezone = '-00:30';
SELECT to_char(now(), 'OF') as of_t, to_char(now(), 'TZH:TZM') as "TZH:TZM";
----
+00:30 +00:30

query TT
SET timezone = '00:30';
SELECT to_char(now(), 'OF') as of_t, to_char(now(), 'TZH:TZM') as "TZH:TZM";
----
-00:30 -00:30

query TT
SET timezone = '-04:30';
SELECT to_char(now(), 'OF') as of_t, to_char(now(), 'TZH:TZM') as "TZH:TZM";
----
+04:30 +04:30

query TT
SET timezone = '04:30';
SELECT to_char(now(), 'OF') as of_t, to_char(now(), 'TZH:TZM') as "TZH:TZM";
----
-04:30 -04:30

query TT
SET timezone = '-04:15';
SELECT to_char(now(), 'OF') as of_t, to_char(now(), 'TZH:TZM') as "TZH:TZM";
----
+04:15 +04:15

query TT
SET timezone = '04:15';
SELECT to_char(now(), 'OF') as of_t, to_char(now(), 'TZH:TZM') as "TZH:TZM";
----
-04:15 -04:15

query TT
RESET timezone;
-- Check of, tzh, tzm with various zone offsets.
SET timezone = '00:00';
SELECT to_char(now(), 'of') as of_t, to_char(now(), 'tzh:tzm') as "tzh:tzm";
----
+00 +00:00

query TT
SET timezone = '+02:00';
SELECT to_char(now(), 'of') as of_t, to_char(now(), 'tzh:tzm') as "tzh:tzm";
----
-02 -02:00

query TT
SET timezone = '-13:00';
SELECT to_char(now(), 'of') as of_t, to_char(now(), 'tzh:tzm') as "tzh:tzm";
----
+13 +13:00

query TT
SET timezone = '-00:30';
SELECT to_char(now(), 'of') as of_t, to_char(now(), 'tzh:tzm') as "tzh:tzm";
----
+00:30 +00:30

query TT
SET timezone = '00:30';
SELECT to_char(now(), 'of') as of_t, to_char(now(), 'tzh:tzm') as "tzh:tzm";
----
-00:30 -00:30

query TT
SET timezone = '-04:30';
SELECT to_char(now(), 'of') as of_t, to_char(now(), 'tzh:tzm') as "tzh:tzm";
----
+04:30 +04:30

query TT
SET timezone = '04:30';
SELECT to_char(now(), 'of') as of_t, to_char(now(), 'tzh:tzm') as "tzh:tzm";
----
-04:30 -04:30

query TT
SET timezone = '-04:15';
SELECT to_char(now(), 'of') as of_t, to_char(now(), 'tzh:tzm') as "tzh:tzm";
----
+04:15 +04:15

query TT
SET timezone = '04:15';
SELECT to_char(now(), 'of') as of_t, to_char(now(), 'tzh:tzm') as "tzh:tzm";
----
-04:15 -04:15
2 changes: 0 additions & 2 deletions pkg/sql/schemachanger/scbuild/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ go_library(
"//pkg/sql/sessiondatapb",
"//pkg/sql/sqlerrors",
"//pkg/sql/types",
"//pkg/util/log",
"//pkg/util/timeutil",
"//pkg/util/uuid",
"@com_github_cockroachdb_errors//:errors",
"@com_github_cockroachdb_redact//:redact",
Expand Down
32 changes: 6 additions & 26 deletions pkg/sql/schemachanger/scbuild/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ package scbuild

import (
"context"
"runtime"

"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
Expand All @@ -27,9 +26,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/errors"
"github.com/cockroachdb/redact"
)

// Build constructs a new state from an initial state and a statement.
Expand All @@ -39,13 +36,11 @@ import (
func Build(
ctx context.Context, dependencies Dependencies, initial scpb.CurrentState, n tree.Statement,
) (_ scpb.CurrentState, err error) {
start := timeutil.Now()
defer func() {
if err != nil || !log.ExpensiveLogEnabled(ctx, 2) {
return
}
log.Infof(ctx, "build for %s took %v", n.StatementTag(), timeutil.Since(start))
}()
defer scerrors.StartEventf(
ctx,
"building declarative schema change targets for %s",
redact.Safe(n.StatementTag()),
).HandlePanicAndLogError(ctx, &err)
initial = initial.DeepCopy()
bs := newBuilderState(ctx, dependencies, initial)
els := newEventLogState(dependencies, initial, n)
Expand All @@ -64,21 +59,6 @@ func Build(
TreeAnnotator: an,
SchemaFeatureChecker: dependencies.FeatureChecker(),
}
defer func() {
switch recErr := recover().(type) {
case nil:
// No error.
case runtime.Error:
err = errors.WithAssertionFailure(recErr)
case error:
err = recErr
default:
err = errors.AssertionFailedf(
"unexpected error encountered while building schema change plan %s",
recErr,
)
}
}()
scbuildstmt.Process(b, an.GetStatement())
an.ValidateAnnotations()
els.statements[len(els.statements)-1].RedactedStatement =
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/schemachanger/scerrors/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ go_library(
"//pkg/sql/catalog",
"//pkg/sql/catalog/descpb",
"//pkg/sql/sem/tree",
"//pkg/util/log",
"//pkg/util/timeutil",
"@com_github_cockroachdb_errors//:errors",
"@com_github_cockroachdb_redact//:redact",
],
)

Expand Down
Loading

0 comments on commit 64e1308

Please sign in to comment.