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.

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

sql: respect IntervalStyle for parsing #67210

Merged
merged 4 commits into from
Jul 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/ccl/changefeedccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ go_library(
"//pkg/util",
"//pkg/util/bitarray",
"//pkg/util/bufalloc",
"//pkg/util/duration",
"//pkg/util/encoding",
"//pkg/util/errorutil",
"//pkg/util/hlc",
Expand Down
3 changes: 2 additions & 1 deletion pkg/ccl/changefeedccl/avro.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util/bitarray"
"github.com/cockroachdb/cockroach/pkg/util/duration"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/cockroach/pkg/util/timeofday"
"github.com/cockroachdb/errors"
Expand Down Expand Up @@ -493,7 +494,7 @@ func typeToAvroSchema(typ *types.T) (*avroSchemaField, error) {
return d.(*tree.DInterval).ValueAsISO8601String(), nil
},
func(x interface{}) (tree.Datum, error) {
return tree.ParseDInterval(x.(string))
return tree.ParseDInterval(duration.IntervalStyle_ISO_8601, x.(string))
},
)
case types.DecimalFamily:
Expand Down
1 change: 1 addition & 0 deletions pkg/cli/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ go_library(
"//pkg/util",
"//pkg/util/cgroups",
"//pkg/util/contextutil",
"//pkg/util/duration",
"//pkg/util/encoding",
"//pkg/util/encoding/csv",
"//pkg/util/envutil",
Expand Down
14 changes: 9 additions & 5 deletions pkg/cli/sql_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catconstants"
"github.com/cockroachdb/cockroach/pkg/sql/parser"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/util/duration"
"github.com/cockroachdb/cockroach/pkg/util/envutil"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
Expand Down Expand Up @@ -441,13 +442,16 @@ func (c *sqlConn) getLastQueryStatistics() (
errors.Newf("unexpected number of rows in SHOW LAST QUERY STATISTICS: %d", nRows)
}

parsedExecLatency, _ := tree.ParseDInterval(execLatencyRaw)
parsedServiceLatency, _ := tree.ParseDInterval(serviceLatencyRaw)
parsedPlanLatency, _ := tree.ParseDInterval(planLatencyRaw)
parsedParseLatency, _ := tree.ParseDInterval(parseLatencyRaw)
// This should really be the same as the session's IntervalStyle
// but that only effects negative intervals in the magnitude
// of days - and all these latencies should be positive.
parsedExecLatency, _ := tree.ParseDInterval(duration.IntervalStyle_POSTGRES, execLatencyRaw)
parsedServiceLatency, _ := tree.ParseDInterval(duration.IntervalStyle_POSTGRES, serviceLatencyRaw)
parsedPlanLatency, _ := tree.ParseDInterval(duration.IntervalStyle_POSTGRES, planLatencyRaw)
parsedParseLatency, _ := tree.ParseDInterval(duration.IntervalStyle_POSTGRES, parseLatencyRaw)

if containsJobLat {
parsedJobsLatency, _ := tree.ParseDInterval(jobsLatencyRaw)
parsedJobsLatency, _ := tree.ParseDInterval(duration.IntervalStyle_POSTGRES, jobsLatencyRaw)
jobsLat = time.Duration(parsedJobsLatency.Duration.Nanos())
}

Expand Down
1 change: 1 addition & 0 deletions pkg/sql/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,7 @@ go_test(
"//pkg/util/caller",
"//pkg/util/cancelchecker",
"//pkg/util/ctxgroup",
"//pkg/util/duration",
"//pkg/util/encoding",
"//pkg/util/fsm",
"//pkg/util/hlc",
Expand Down
11 changes: 6 additions & 5 deletions pkg/sql/conn_executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
"github.com/cockroachdb/cockroach/pkg/util/ctxgroup"
"github.com/cockroachdb/cockroach/pkg/util/duration"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
Expand Down Expand Up @@ -1002,19 +1003,19 @@ ALTER TABLE t1 ADD COLUMN b INT DEFAULT 1`,
)
require.NoError(t, err, "unexpected error while reading last query statistics")

parseInterval, err := tree.ParseDInterval(parseLatency)
parseInterval, err := tree.ParseDInterval(duration.IntervalStyle_POSTGRES, parseLatency)
require.NoError(t, err)

planInterval, err := tree.ParseDInterval(planLatency)
planInterval, err := tree.ParseDInterval(duration.IntervalStyle_POSTGRES, planLatency)
require.NoError(t, err)

execInterval, err := tree.ParseDInterval(execLatency)
execInterval, err := tree.ParseDInterval(duration.IntervalStyle_POSTGRES, execLatency)
require.NoError(t, err)

serviceInterval, err := tree.ParseDInterval(serviceLatency)
serviceInterval, err := tree.ParseDInterval(duration.IntervalStyle_POSTGRES, serviceLatency)
require.NoError(t, err)

postCommitJobsInterval, err := tree.ParseDInterval(postCommitJobsLatency)
postCommitJobsInterval, err := tree.ParseDInterval(duration.IntervalStyle_POSTGRES, postCommitJobsLatency)
require.NoError(t, err)

if parseInterval.AsFloat64() <= 0 || parseInterval.AsFloat64() > 1 {
Expand Down
48 changes: 40 additions & 8 deletions pkg/sql/logictest/testdata/logic_test/interval
Original file line number Diff line number Diff line change
Expand Up @@ -404,46 +404,62 @@ INSERT INTO intervals VALUES
(2, '1 day 04:06:08.123'),
(3, '2 years 11 mons -2 days +03:25:45.678')

statement ok
create table interval_parsing ( pk INT PRIMARY KEY, i TEXT )

statement ok
INSERT INTO interval_parsing VALUES
(1, '-10 years 22 months 1 day 01:02:03'),
(2, '-10 years -22 months 1 day 01:02:03'),
(3, '-10 years 22 months -1 day 01:02:03'),
(4, '-10 years 22 months -1 day -01:02:03')

query T
SELECT i FROM intervals ORDER BY pk
----
-2 years -11 mons 1 day +04:05:06.123
-2 years -11 mons +1 day 04:05:06.123
1 day 04:06:08.123
2 years 11 mons -2 days +03:25:45.678

query T
SELECT array_to_string(array_agg(i ORDER BY pk), ' ') FROM intervals
----
-2 years -11 mons 1 day +04:05:06.123
1 day 04:06:08.123
2 years 11 mons -2 days +03:25:45.678
-2 years -11 mons +1 day 04:05:06.123 1 day 04:06:08.123 2 years 11 mons -2 days +03:25:45.678

query T
SELECT (array_agg(i ORDER BY pk))::string FROM intervals
----
{"-2 years -11 mons 1 day +04:05:06.123","1 day 04:06:08.123","2 years 11 mons -2 days +03:25:45.678"}
{"-2 years -11 mons +1 day 04:05:06.123","1 day 04:06:08.123","2 years 11 mons -2 days +03:25:45.678"}

query T
SELECT i::string FROM intervals ORDER BY pk
----
-2 years -11 mons 1 day +04:05:06.123
-2 years -11 mons +1 day 04:05:06.123
1 day 04:06:08.123
2 years 11 mons -2 days +03:25:45.678

query T
SELECT (i,) FROM intervals ORDER BY pk
----
("-2 years -11 mons 1 day +04:05:06.123")
("-2 years -11 mons +1 day 04:05:06.123")
("1 day 04:06:08.123")
("2 years 11 mons -2 days +03:25:45.678")

query T
SELECT row_to_json(intervals) FROM intervals ORDER BY pk
----
{"i": "-2 years -11 mons 1 day +04:05:06.123", "pk": 1}
{"i": "-2 years -11 mons +1 day 04:05:06.123", "pk": 1}
{"i": "1 day 04:06:08.123", "pk": 2}
{"i": "2 years 11 mons -2 days +03:25:45.678", "pk": 3}

query TT
SELECT i, i::INTERVAL FROM interval_parsing ORDER BY pk
----
-10 years 22 months 1 day 01:02:03 -8 years -2 mons +1 day 01:02:03
-10 years -22 months 1 day 01:02:03 -11 years -10 mons +1 day 01:02:03
-10 years 22 months -1 day 01:02:03 -8 years -2 mons -1 days +01:02:03
-10 years 22 months -1 day -01:02:03 -8 years -2 mons -1 days -01:02:03

statement ok
SET intervalstyle = 'iso_8601'

Expand Down Expand Up @@ -485,6 +501,14 @@ SELECT row_to_json(intervals) FROM intervals ORDER BY pk
{"i": "P1DT4H6M8.123S", "pk": 2}
{"i": "P2Y11M-2DT3H25M45.678S", "pk": 3}

query TT
SELECT i, i::INTERVAL FROM interval_parsing ORDER BY pk
----
-10 years 22 months 1 day 01:02:03 P-8Y-2M1DT1H2M3S
-10 years -22 months 1 day 01:02:03 P-11Y-10M1DT1H2M3S
-10 years 22 months -1 day 01:02:03 P-8Y-2M-1DT1H2M3S
-10 years 22 months -1 day -01:02:03 P-8Y-2M-1DT-1H-2M-3S

statement ok
SET intervalstyle = 'sql_standard'

Expand Down Expand Up @@ -525,3 +549,11 @@ SELECT row_to_json(intervals) FROM intervals ORDER BY pk
{"i": "-2-11 +1 +4:05:06.123", "pk": 1}
{"i": "1 4:06:08.123", "pk": 2}
{"i": "+2-11 -2 +3:25:45.678", "pk": 3}

query TT
SELECT i, i::INTERVAL FROM interval_parsing ORDER BY pk
----
-10 years 22 months 1 day 01:02:03 -11-10 -1 -1:02:03
-10 years -22 months 1 day 01:02:03 -11-10 +1 +1:02:03
-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
33 changes: 21 additions & 12 deletions pkg/sql/parser/sql.y
Original file line number Diff line number Diff line change
Expand Up @@ -12215,15 +12215,20 @@ iconst64:
interval_value:
INTERVAL SCONST opt_interval_qualifier
{
var err error
var d tree.Datum
var t *types.T
if $3.val == nil {
d, err = tree.ParseDInterval($2)
t = types.Interval
} else {
d, err = tree.ParseDIntervalWithTypeMetadata($2, $3.intervalTypeMetadata())
t = types.MakeInterval($3.intervalTypeMetadata())
}
$$.val = &tree.CastExpr{
Expr: tree.NewStrVal($2),
Type: t,
// TODO(#sql-experience): This should be CastPrepend, but
// that does not work with parenthesized expressions
// (using FmtAlwaysGroupExprs).
SyntaxMode: tree.CastShort,
}
if err != nil { return setErr(sqllex, err) }
$$.val = d
}
| INTERVAL '(' iconst32 ')' SCONST
{
Expand All @@ -12232,12 +12237,16 @@ interval_value:
sqllex.Error(fmt.Sprintf("precision %d out of range", prec))
return 1
}
d, err := tree.ParseDIntervalWithTypeMetadata($5, types.IntervalTypeMetadata{
Precision: prec,
PrecisionIsSet: true,
})
if err != nil { return setErr(sqllex, err) }
$$.val = d
$$.val = &tree.CastExpr{
Expr: tree.NewStrVal($5),
Type: types.MakeInterval(
types.IntervalTypeMetadata{Precision: prec, PrecisionIsSet: true},
),
// TODO(#sql-experience): This should be CastPrepend, but
// that does not work with parenthesized expressions
// (using FmtAlwaysGroupExprs).
SyntaxMode: tree.CastShort,
}
}

// Name classification hierarchy.
Expand Down
51 changes: 25 additions & 26 deletions pkg/sql/parser/testdata/select_exprs
Original file line number Diff line number Diff line change
Expand Up @@ -1035,42 +1035,42 @@ SELECT '0'::INTERVAL -- identifiers removed
parse
SELECT INTERVAL '0'
----
SELECT '00:00:00' -- normalized!
SELECT ('00:00:00') -- fully parenthesized
SELECT _ -- literals removed
SELECT '00:00:00' -- identifiers removed
SELECT '0'::INTERVAL -- normalized!
SELECT (('0')::INTERVAL) -- fully parenthesized
SELECT _::INTERVAL -- literals removed
SELECT '0'::INTERVAL -- identifiers removed

parse
SELECT INTERVAL '1' SECOND
----
SELECT '00:00:01' -- normalized!
SELECT ('00:00:01') -- fully parenthesized
SELECT _ -- literals removed
SELECT '00:00:01' -- identifiers removed
SELECT '1'::INTERVAL SECOND -- normalized!
SELECT (('1')::INTERVAL SECOND) -- fully parenthesized
SELECT _::INTERVAL SECOND -- literals removed
SELECT '1'::INTERVAL SECOND -- identifiers removed

parse
SELECT INTERVAL(3) '12.1234s'
----
SELECT '00:00:12.123' -- normalized!
SELECT ('00:00:12.123') -- fully parenthesized
SELECT _ -- literals removed
SELECT '00:00:12.123' -- identifiers removed
SELECT '12.1234s'::INTERVAL(3) -- normalized!
SELECT (('12.1234s')::INTERVAL(3)) -- fully parenthesized
SELECT _::INTERVAL(3) -- literals removed
SELECT '12.1234s'::INTERVAL(3) -- identifiers removed

parse
SELECT INTERVAL '12.1234s' SECOND(3)
----
SELECT '00:00:12.123' -- normalized!
SELECT ('00:00:12.123') -- fully parenthesized
SELECT _ -- literals removed
SELECT '00:00:12.123' -- identifiers removed
SELECT '12.1234s'::INTERVAL SECOND(3) -- normalized!
SELECT (('12.1234s')::INTERVAL SECOND(3)) -- fully parenthesized
SELECT _::INTERVAL SECOND(3) -- literals removed
SELECT '12.1234s'::INTERVAL SECOND(3) -- identifiers removed

parse
SELECT INTERVAL '14.7899s' SECOND(3)
----
SELECT '00:00:14.79' -- normalized!
SELECT ('00:00:14.79') -- fully parenthesized
SELECT _ -- literals removed
SELECT '00:00:14.79' -- identifiers removed
SELECT '14.7899s'::INTERVAL SECOND(3) -- normalized!
SELECT (('14.7899s')::INTERVAL SECOND(3)) -- fully parenthesized
SELECT _::INTERVAL SECOND(3) -- literals removed
SELECT '14.7899s'::INTERVAL SECOND(3) -- identifiers removed

parse
SELECT '11s'::INTERVAL(3)
Expand Down Expand Up @@ -1112,14 +1112,13 @@ SELECT (('10:00:13.123456')::INTERVAL MINUTE TO SECOND(3)) -- fully parenthesize
SELECT _::INTERVAL MINUTE TO SECOND(3) -- literals removed
SELECT '10:00:13.123456'::INTERVAL MINUTE TO SECOND(3) -- identifiers removed

error
parse
SELECT INTERVAL 'foo'
----
at or near "EOF": syntax error: could not parse "foo" as type interval: interval: missing number at position 0: "foo"
DETAIL: source SQL:
SELECT INTERVAL 'foo'
^

SELECT 'foo'::INTERVAL -- normalized!
SELECT (('foo')::INTERVAL) -- fully parenthesized
SELECT _::INTERVAL -- literals removed
SELECT 'foo'::INTERVAL -- identifiers removed

parse
SELECT 'foo'::BOX2D
Expand Down
35 changes: 17 additions & 18 deletions pkg/sql/parser/testdata/set
Original file line number Diff line number Diff line change
Expand Up @@ -375,35 +375,34 @@ SET timezone = 'Europe/Rome' -- identifiers removed
parse
SET TIME ZONE INTERVAL '-7h'
----
SET timezone = '-07:00:00' -- normalized!
SET timezone = ('-07:00:00') -- fully parenthesized
SET timezone = _ -- literals removed
SET timezone = '-07:00:00' -- identifiers removed
SET timezone = '-7h'::INTERVAL -- normalized!
SET timezone = (('-7h')::INTERVAL) -- fully parenthesized
SET timezone = _::INTERVAL -- literals removed
SET timezone = '-7h'::INTERVAL -- identifiers removed

parse
SET TIME ZONE INTERVAL(3) '-7h'
----
SET timezone = '-07:00:00' -- normalized!
SET timezone = ('-07:00:00') -- fully parenthesized
SET timezone = _ -- literals removed
SET timezone = '-07:00:00' -- identifiers removed
SET timezone = '-7h'::INTERVAL(3) -- normalized!
SET timezone = (('-7h')::INTERVAL(3)) -- fully parenthesized
SET timezone = _::INTERVAL(3) -- literals removed
SET timezone = '-7h'::INTERVAL(3) -- identifiers removed

parse
SET TIME ZONE INTERVAL '-7h0m5s' HOUR TO MINUTE
----
SET timezone = '-06:59:00' -- normalized!
SET timezone = ('-06:59:00') -- fully parenthesized
SET timezone = _ -- literals removed
SET timezone = '-06:59:00' -- identifiers removed
SET timezone = '-7h0m5s'::INTERVAL HOUR TO MINUTE -- normalized!
SET timezone = (('-7h0m5s')::INTERVAL HOUR TO MINUTE) -- fully parenthesized
SET timezone = _::INTERVAL HOUR TO MINUTE -- literals removed
SET timezone = '-7h0m5s'::INTERVAL HOUR TO MINUTE -- identifiers removed

error
parse
SET TIME ZONE INTERVAL 'foobar'
----
at or near "EOF": syntax error: could not parse "foobar" as type interval: interval: missing number at position 0: "foobar"
DETAIL: source SQL:
SET TIME ZONE INTERVAL 'foobar'
^

SET timezone = 'foobar'::INTERVAL -- normalized!
SET timezone = (('foobar')::INTERVAL) -- fully parenthesized
SET timezone = _::INTERVAL -- literals removed
SET timezone = 'foobar'::INTERVAL -- identifiers removed

parse
SET TRANSACTION PRIORITY NORMAL, ISOLATION LEVEL SERIALIZABLE
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/pgwire/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ go_test(
"//pkg/testutils/sqlutils",
"//pkg/testutils/testcluster",
"//pkg/util",
"//pkg/util/duration",
"//pkg/util/leaktest",
"//pkg/util/log",
"//pkg/util/log/channel",
Expand Down
Loading