Skip to content

Commit

Permalink
sql: granular detection of non-immutable constant casts
Browse files Browse the repository at this point in the history
Interpreting a timestamp (and similar types) can depend on the current
timezone or even the current time. A recent change prevented these
casts from happening during type-checking.

Unfortunately, this includes a lot of common cases which aren't
actually context-dependent. The most important are dates and
timestamps without time zone (except rare cases like 'now' or
'tomorrow'). In these cases, the type-checked expressions won't seem
constant and won't be able to be used for partitioning, index
predicates, etc.

This change improves the parsing functions to return a
`dependsOnContext` boolean. When this flag is false, the result is
immutable and is replaced during type-checking.

The volatility of the version of date_trunc that returns a TIMESTAMPTZ
is corrected to Stable.

Release note: None
  • Loading branch information
RaduBerinde committed Jun 29, 2020
1 parent 2c77a8b commit 557e9db
Show file tree
Hide file tree
Showing 36 changed files with 661 additions and 401 deletions.
3 changes: 2 additions & 1 deletion pkg/ccl/changefeedccl/avro.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,8 @@ func columnDescToAvroSchema(colDesc *sqlbase.ColumnDescriptor) (*avroSchemaField
return d.(*tree.DTimeTZ).TimeTZ.String(), nil
}
schema.decodeFn = func(x interface{}) (tree.Datum, error) {
return tree.ParseDTimeTZ(nil, x.(string), time.Microsecond)
d, _, err := tree.ParseDTimeTZ(nil, x.(string), time.Microsecond)
return d, err
}
case types.TimestampFamily:
avroType = avroLogicalType{
Expand Down
4 changes: 2 additions & 2 deletions pkg/ccl/importccl/read_import_mysql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,14 +228,14 @@ func TestMysqlValueToDatum(t *testing.T) {
defer leaktest.AfterTest(t)()

date := func(s string) tree.Datum {
d, err := tree.ParseDDate(nil, s)
d, _, err := tree.ParseDDate(nil, s)
if err != nil {
t.Fatal(err)
}
return d
}
ts := func(s string) tree.Datum {
d, err := tree.ParseDTimestamp(nil, s, time.Microsecond)
d, _, err := tree.ParseDTimestamp(nil, s, time.Microsecond)
if err != nil {
t.Fatal(err)
}
Expand Down
9 changes: 5 additions & 4 deletions pkg/cli/dump.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ WHERE
if enumMembersS != nil {
// The driver sends back arrays as bytes, so we have to parse the array
// if we want to access its elements.
arr, err := tree.ParseDArrayFromString(
arr, _, err := tree.ParseDArrayFromString(
tree.NewTestingEvalContext(serverCfg.Settings), string(enumMembersS), types.String)
if err != nil {
return nil, err
Expand Down Expand Up @@ -439,7 +439,7 @@ func getAsOf(conn *sqlConn, asOf string) (string, error) {
clusterTS = string(vals[0].([]byte))
} else {
// Validate the timestamp. This prevents SQL injection.
if _, err := tree.ParseDTimestamp(nil, asOf, time.Nanosecond); err != nil {
if _, _, err := tree.ParseDTimestamp(nil, asOf, time.Nanosecond); err != nil {
return "", err
}
clusterTS = asOf
Expand Down Expand Up @@ -601,7 +601,8 @@ func extractArray(val interface{}) ([]string, error) {
if !ok {
return nil, fmt.Errorf("unexpected value: %T", b)
}
arr, err := tree.ParseDArrayFromString(tree.NewTestingEvalContext(serverCfg.Settings), string(b), types.String)
evalCtx := tree.NewTestingEvalContext(serverCfg.Settings)
arr, _, err := tree.ParseDArrayFromString(evalCtx, string(b), types.String)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -932,7 +933,7 @@ func dumpTableData(
}
case types.ArrayFamily:
// We can only observe ARRAY types by their [] suffix.
d, err = tree.ParseDArrayFromString(
d, _, err = tree.ParseDArrayFromString(
tree.NewTestingEvalContext(serverCfg.Settings), string(t), ct.ArrayContents())
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion pkg/internal/sqlsmith/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ FROM
// If enum members were provided, parse the result into a string array.
var members []string
if len(membersRaw) != 0 {
arr, err := tree.ParseDArrayFromString(&evalCtx, string(membersRaw), types.String)
arr, _, err := tree.ParseDArrayFromString(&evalCtx, string(membersRaw), types.String)
if err != nil {
return nil, err
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/opt/constraint/testutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,11 @@ func parseDatumPath(evalCtx *tree.EvalContext, str string, typs []types.Family)
case types.DecimalFamily:
val, err = tree.ParseDDecimal(valStr)
case types.DateFamily:
val, err = tree.ParseDDate(evalCtx, valStr)
val, _, err = tree.ParseDDate(evalCtx, valStr)
case types.TimestampFamily:
val, err = tree.ParseDTimestamp(evalCtx, valStr, time.Microsecond)
val, _, err = tree.ParseDTimestamp(evalCtx, valStr, time.Microsecond)
case types.TimestampTZFamily:
val, err = tree.ParseDTimestampTZ(evalCtx, valStr, time.Microsecond)
val, _, err = tree.ParseDTimestampTZ(evalCtx, valStr, time.Microsecond)
case types.StringFamily:
val = tree.NewDString(valStr)
case types.OidFamily:
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/opt/memo/testdata/typing
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ project
│ └── columns: x:1(int!null) y:2(int)
└── projections
├── x:1 + 1.5 [as=r:3, type=decimal]
├── '2000-01-01'::DATE - 15 [as=s:4, type=date]
├── '2000-01-01' - 15 [as=s:4, type=date]
├── 10.10 * x:1 [as=t:5, type=decimal]
├── 1 / y:2 [as=u:6, type=decimal]
└── x:1 // 1.5 [as=v:7, type=decimal]
Expand Down
6 changes: 2 additions & 4 deletions pkg/sql/opt/optbuilder/testdata/scalar
Original file line number Diff line number Diff line change
Expand Up @@ -371,8 +371,7 @@ cast: INT8 [type=int]
build-scalar
'2010-05-12'::timestamp
----
cast: TIMESTAMP [type=timestamp]
└── const: '2010-05-12' [type=string]
const: '2010-05-12 00:00:00+00:00' [type=timestamp]

build-scalar
'now'::timestamp
Expand All @@ -384,8 +383,7 @@ build-scalar
'2010-05-12'::timestamp - 'now'
----
minus [type=interval]
├── cast: TIMESTAMP [type=timestamp]
│ └── const: '2010-05-12' [type=string]
├── const: '2010-05-12 00:00:00+00:00' [type=timestamp]
└── cast: TIMESTAMP [type=timestamp]
└── const: 'now' [type=string]

Expand Down
12 changes: 6 additions & 6 deletions pkg/sql/opt/props/histogram_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -513,11 +513,11 @@ func TestFilterBucket(t *testing.T) {
})

t.Run("date", func(t *testing.T) {
upperBound, err := tree.ParseDDate(&evalCtx, "2019-08-01")
upperBound, _, err := tree.ParseDDate(&evalCtx, "2019-08-01")
if err != nil {
t.Fatal(err)
}
lowerBound, err := tree.ParseDDate(&evalCtx, "2019-07-01")
lowerBound, _, err := tree.ParseDDate(&evalCtx, "2019-07-01")
if err != nil {
t.Fatal(err)
}
Expand All @@ -526,7 +526,7 @@ func TestFilterBucket(t *testing.T) {
{NumEq: 1, NumRange: 62, DistinctRange: 31, UpperBound: upperBound},
}}

ub1, err := tree.ParseDDate(&evalCtx, "2019-07-02")
ub1, _, err := tree.ParseDDate(&evalCtx, "2019-07-02")
if err != nil {
t.Fatal(err)
}
Expand All @@ -546,11 +546,11 @@ func TestFilterBucket(t *testing.T) {
})

t.Run("timestamp", func(t *testing.T) {
upperBound, err := tree.ParseDTimestamp(&evalCtx, "2019-08-01 12:00:00.000000", time.Microsecond)
upperBound, _, err := tree.ParseDTimestamp(&evalCtx, "2019-08-01 12:00:00.000000", time.Microsecond)
if err != nil {
t.Fatal(err)
}
lowerBound, err := tree.ParseDTimestamp(&evalCtx, "2019-07-01 12:00:00.000000", time.Microsecond)
lowerBound, _, err := tree.ParseDTimestamp(&evalCtx, "2019-07-01 12:00:00.000000", time.Microsecond)
if err != nil {
t.Fatal(err)
}
Expand All @@ -559,7 +559,7 @@ func TestFilterBucket(t *testing.T) {
{NumEq: 1, NumRange: 62, DistinctRange: 31, UpperBound: upperBound},
}}

ub1, err := tree.ParseDTimestamp(&evalCtx, "2019-07-02 00:00:00.000000", time.Microsecond)
ub1, _, err := tree.ParseDTimestamp(&evalCtx, "2019-07-02 00:00:00.000000", time.Microsecond)
if err != nil {
t.Fatal(err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/opt/testutils/testcat/test_catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -991,7 +991,7 @@ var _ cat.TableStatistic = &TableStat{}

// CreatedAt is part of the cat.TableStatistic interface.
func (ts *TableStat) CreatedAt() time.Time {
d, err := tree.ParseDTimestamp(nil, ts.js.CreatedAt, time.Microsecond)
d, _, err := tree.ParseDTimestamp(nil, ts.js.CreatedAt, time.Microsecond)
if err != nil {
panic(err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/pgwire/encoding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func TestEncodings(t *testing.T) {
// the case, manually do the conversion to array.
darr, isdarr := tc.Datum.(*tree.DArray)
if isdarr && d.ResolvedType().Family() == types.StringFamily {
d, err = tree.ParseDArrayFromString(&evalCtx, string(value), darr.ParamTyp)
d, _, err = tree.ParseDArrayFromString(&evalCtx, string(value), darr.ParamTyp)
if err != nil {
t.Fatal(err)
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/sql/pgwire/pgwirebase/encoding.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,31 +277,31 @@ func DecodeOidDatum(
}
return tree.NewDBytes(tree.DBytes(res)), nil
case oid.T_timestamp:
d, err := tree.ParseDTimestamp(ctx, string(b), time.Microsecond)
d, _, err := tree.ParseDTimestamp(ctx, string(b), time.Microsecond)
if err != nil {
return nil, pgerror.Newf(pgcode.Syntax, "could not parse string %q as timestamp", b)
}
return d, nil
case oid.T_timestamptz:
d, err := tree.ParseDTimestampTZ(ctx, string(b), time.Microsecond)
d, _, err := tree.ParseDTimestampTZ(ctx, string(b), time.Microsecond)
if err != nil {
return nil, pgerror.Newf(pgcode.Syntax, "could not parse string %q as timestamptz", b)
}
return d, nil
case oid.T_date:
d, err := tree.ParseDDate(ctx, string(b))
d, _, err := tree.ParseDDate(ctx, string(b))
if err != nil {
return nil, pgerror.Newf(pgcode.Syntax, "could not parse string %q as date", b)
}
return d, nil
case oid.T_time:
d, err := tree.ParseDTime(nil, string(b), time.Microsecond)
d, _, err := tree.ParseDTime(nil, string(b), time.Microsecond)
if err != nil {
return nil, pgerror.Newf(pgcode.Syntax, "could not parse string %q as time", b)
}
return d, nil
case oid.T_timetz:
d, err := tree.ParseDTimeTZ(ctx, string(b), time.Microsecond)
d, _, err := tree.ParseDTimeTZ(ctx, string(b), time.Microsecond)
if err != nil {
return nil, pgerror.Newf(pgcode.Syntax, "could not parse string %q as timetz", b)
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/sql/pgwire/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func TestParseTs(t *testing.T) {
}

for i, test := range parseTsTests {
parsed, err := tree.ParseDTimestamp(nil, test.strTimestamp, time.Nanosecond)
parsed, _, err := tree.ParseDTimestamp(nil, test.strTimestamp, time.Nanosecond)
if err != nil {
t.Errorf("%d could not parse [%s]: %v", i, test.strTimestamp, err)
continue
Expand All @@ -65,7 +65,7 @@ func TestTimestampRoundtrip(t *testing.T) {
ts := time.Date(2006, 7, 8, 0, 0, 0, 123000, time.FixedZone("UTC", 0))

parse := func(encoded []byte) time.Time {
decoded, err := tree.ParseDTimestamp(nil, string(encoded), time.Nanosecond)
decoded, _, err := tree.ParseDTimestamp(nil, string(encoded), time.Nanosecond)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -93,7 +93,7 @@ func TestWriteBinaryArray(t *testing.T) {
// writeBuffer is equivalent to writing to two different writeBuffers and
// then concatenating the result.
st := cluster.MakeTestingClusterSettings()
ary, _ := tree.ParseDArrayFromString(tree.NewTestingEvalContext(st), "{1}", types.Int)
ary, _, _ := tree.ParseDArrayFromString(tree.NewTestingEvalContext(st), "{1}", types.Int)

defaultConv := makeTestingConvCfg()

Expand Down Expand Up @@ -321,23 +321,23 @@ func benchmarkWriteString(b *testing.B, format pgwirebase.FormatCode) {
}

func benchmarkWriteDate(b *testing.B, format pgwirebase.FormatCode) {
d, err := tree.ParseDDate(nil, "2010-09-28")
d, _, err := tree.ParseDDate(nil, "2010-09-28")
if err != nil {
b.Fatal(err)
}
benchmarkWriteType(b, d, format)
}

func benchmarkWriteTimestamp(b *testing.B, format pgwirebase.FormatCode) {
ts, err := tree.ParseDTimestamp(nil, "2010-09-28 12:00:00.1", time.Microsecond)
ts, _, err := tree.ParseDTimestamp(nil, "2010-09-28 12:00:00.1", time.Microsecond)
if err != nil {
b.Fatal(err)
}
benchmarkWriteType(b, ts, format)
}

func benchmarkWriteTimestampTZ(b *testing.B, format pgwirebase.FormatCode) {
tstz, err := tree.ParseDTimestampTZ(nil, "2010-09-28 12:00:00.1", time.Microsecond)
tstz, _, err := tree.ParseDTimestampTZ(nil, "2010-09-28 12:00:00.1", time.Microsecond)
if err != nil {
b.Fatal(err)
}
Expand Down
14 changes: 6 additions & 8 deletions pkg/sql/sem/builtins/builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -2305,7 +2305,7 @@ may increase either contention or retry errors, or both.`,
Fn: func(ctx *tree.EvalContext, args tree.Datums) (tree.Datum, error) {
timeSpan := strings.ToLower(string(tree.MustBeDString(args[0])))
fromTS := args[1].(*tree.DTimestamp)
tsTZ, err := truncateTimestamp(ctx, fromTS.Time, timeSpan)
tsTZ, err := truncateTimestamp(fromTS.Time, timeSpan)
if err != nil {
return nil, err
}
Expand All @@ -2328,13 +2328,13 @@ may increase either contention or retry errors, or both.`,
if err != nil {
return nil, err
}
return truncateTimestamp(ctx, fromTSTZ.Time, timeSpan)
return truncateTimestamp(fromTSTZ.Time, timeSpan)
},
Info: "Truncates `input` to precision `element`. Sets all fields that are less\n" +
"significant than `element` to zero (or one, for day and month)\n\n" +
"Compatible elements: millennium, century, decade, year, quarter, month,\n" +
"week, day, hour, minute, second, millisecond, microsecond.",
Volatility: tree.VolatilityImmutable,
Volatility: tree.VolatilityStable,
},
tree.Overload{
Types: tree.ArgTypes{{"element", types.String}, {"input", types.Time}},
Expand All @@ -2359,7 +2359,7 @@ may increase either contention or retry errors, or both.`,
Fn: func(ctx *tree.EvalContext, args tree.Datums) (tree.Datum, error) {
fromTSTZ := args[1].(*tree.DTimestampTZ)
timeSpan := strings.ToLower(string(tree.MustBeDString(args[0])))
return truncateTimestamp(ctx, fromTSTZ.Time.In(ctx.GetLocation()), timeSpan)
return truncateTimestamp(fromTSTZ.Time.In(ctx.GetLocation()), timeSpan)
},
Info: "Truncates `input` to precision `element`. Sets all fields that are less\n" +
"significant than `element` to zero (or one, for day and month)\n\n" +
Expand Down Expand Up @@ -2412,7 +2412,7 @@ may increase either contention or retry errors, or both.`,
Fn: func(ctx *tree.EvalContext, args tree.Datums) (tree.Datum, error) {
tzArg := string(tree.MustBeDString(args[0]))
tsArg := string(tree.MustBeDString(args[1]))
ts, err := tree.ParseDTimestampTZ(ctx, tsArg, time.Microsecond)
ts, _, err := tree.ParseDTimestampTZ(ctx, tsArg, time.Microsecond)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -5709,9 +5709,7 @@ func decodeEscape(input string) ([]byte, error) {
return result, nil
}

func truncateTimestamp(
_ *tree.EvalContext, fromTime time.Time, timeSpan string,
) (*tree.DTimestampTZ, error) {
func truncateTimestamp(fromTime time.Time, timeSpan string) (*tree.DTimestampTZ, error) {
year := fromTime.Year()
month := fromTime.Month()
day := fromTime.Day()
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/sem/builtins/builtins_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ func TestExtractTimeSpanFromTimeTZ(t *testing.T) {

for _, tc := range testCases {
t.Run(fmt.Sprintf("%s_%s", tc.timeSpan, tc.timeTZString), func(t *testing.T) {
timeTZ, err := tree.ParseDTimeTZ(nil, tc.timeTZString, time.Microsecond)
timeTZ, _, err := tree.ParseDTimeTZ(nil, tc.timeTZString, time.Microsecond)
assert.NoError(t, err)

datum, err := extractTimeSpanFromTimeTZ(timeTZ, tc.timeSpan)
Expand Down Expand Up @@ -467,7 +467,7 @@ func TestTruncateTimestamp(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.timeSpan, func(t *testing.T) {
result, err := truncateTimestamp(nil, tc.fromTime, tc.timeSpan)
result, err := truncateTimestamp(tc.fromTime, tc.timeSpan)
require.NoError(t, err)
assert.Equal(t, tc.expected, result)
})
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/sem/tree/as_of.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func DatumToHLC(evalCtx *EvalContext, stmtTimestamp time.Time, d Datum) (hlc.Tim
case *DString:
s := string(*d)
// Attempt to parse as timestamp.
if dt, err := ParseDTimestamp(evalCtx, s, time.Nanosecond); err == nil {
if dt, _, err := ParseDTimestamp(evalCtx, s, time.Nanosecond); err == nil {
ts.WallTime = dt.Time.UnixNano()
break
}
Expand Down
Loading

0 comments on commit 557e9db

Please sign in to comment.