Skip to content

Commit 9a58315

Browse files
RaduBerindeRaduBerinde
committed
sql: fix formatting of prefix casts TIMESTAMP/DATE/INTERVAL
Introducing a FmtFlag that disambiguates constants with a type prefix. This ensures that expressions that contain `DDInterval`, `DDate` etc (without a `CastExpr`) can be formatted and re-parsed correctly. To avoid a redundant cast, we elide the `CastExpr` during type checking if the argument is a constant that can become the required type. The new `FmtParsable` is used when storing expressions in view and when serializing expressions for distsql. Fixes #12532.
1 parent 88cb461 commit 9a58315

File tree

10 files changed

+77
-38
lines changed

10 files changed

+77
-38
lines changed

‎pkg/sql/create.go

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -384,17 +384,21 @@ func (p *planner) CreateView(n *parser.CreateView) (planNode, error) {
384384

385385
var queryBuf bytes.Buffer
386386
var fmtErr error
387-
n.AsSource.Format(&queryBuf, parser.FmtNormalizeTableNames(
388-
parser.FmtSimple,
389-
func(t *parser.NormalizableTableName) *parser.TableName {
390-
tn, err := p.QualifyWithDatabase(t)
391-
if err != nil {
392-
log.Warningf(p.ctx(), "failed to qualify table name %q with database name: %v", t, err)
393-
fmtErr = err
394-
return nil
395-
}
396-
return tn
397-
}))
387+
n.AsSource.Format(
388+
&queryBuf,
389+
parser.FmtNormalizeTableNames(
390+
parser.FmtParsable,
391+
func(t *parser.NormalizableTableName) *parser.TableName {
392+
tn, err := p.QualifyWithDatabase(t)
393+
if err != nil {
394+
log.Warningf(p.ctx(), "failed to qualify table name %q with database name: %v", t, err)
395+
fmtErr = err
396+
return nil
397+
}
398+
return tn
399+
},
400+
),
401+
)
398402
if fmtErr != nil {
399403
return nil, fmtErr
400404
}

‎pkg/sql/distsql_physical_planner.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ func mergePlans(
364364
// exprFmtFlagsBase are FmtFlags used for serializing expressions; a proper
365365
// IndexedVar formatting function needs to be added on.
366366
var exprFmtFlagsBase = parser.FmtStarDatumFormat(
367-
parser.FmtSimple,
367+
parser.FmtParsable,
368368
func(buf *bytes.Buffer, _ parser.FmtFlags) {
369369
fmt.Fprintf(buf, "0")
370370
},

‎pkg/sql/parser/datum.go

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,9 @@ func (d *DFloat) min() (Datum, bool) {
393393

394394
// Format implements the NodeFormatter interface.
395395
func (d *DFloat) Format(buf *bytes.Buffer, f FmtFlags) {
396+
if f.disambiguateDatumTypes {
397+
buf.WriteString("FLOAT ")
398+
}
396399
fl := float64(*d)
397400
if _, frac := math.Modf(fl); frac == 0 && -1000000 < *d && *d < 1000000 {
398401
// d is a small whole number. Ensure it is printed using a decimal point.
@@ -476,6 +479,9 @@ func (d *DDecimal) min() (Datum, bool) {
476479

477480
// Format implements the NodeFormatter interface.
478481
func (d *DDecimal) Format(buf *bytes.Buffer, f FmtFlags) {
482+
if f.disambiguateDatumTypes {
483+
buf.WriteString("DECIMAL ")
484+
}
479485
buf.WriteString(d.Dec.String())
480486
}
481487

@@ -553,6 +559,9 @@ func (d *DString) max() (Datum, bool) {
553559

554560
// Format implements the NodeFormatter interface.
555561
func (d *DString) Format(buf *bytes.Buffer, f FmtFlags) {
562+
if f.disambiguateDatumTypes {
563+
buf.WriteString("STRING ")
564+
}
556565
encodeSQLStringWithFlags(buf, string(*d), f)
557566
}
558567

@@ -850,6 +859,9 @@ func (d *DDate) min() (Datum, bool) {
850859

851860
// Format implements the NodeFormatter interface.
852861
func (d *DDate) Format(buf *bytes.Buffer, f FmtFlags) {
862+
if f.disambiguateDatumTypes {
863+
buf.WriteString("DATE ")
864+
}
853865
if !f.bareStrings {
854866
buf.WriteByte('\'')
855867
}
@@ -1005,6 +1017,9 @@ func (d *DTimestamp) max() (Datum, bool) {
10051017

10061018
// Format implements the NodeFormatter interface.
10071019
func (d *DTimestamp) Format(buf *bytes.Buffer, f FmtFlags) {
1020+
if f.disambiguateDatumTypes {
1021+
buf.WriteString("TIMESTAMP ")
1022+
}
10081023
if !f.bareStrings {
10091024
buf.WriteByte('\'')
10101025
}
@@ -1109,6 +1124,9 @@ func (d *DTimestampTZ) max() (Datum, bool) {
11091124

11101125
// Format implements the NodeFormatter interface.
11111126
func (d *DTimestampTZ) Format(buf *bytes.Buffer, f FmtFlags) {
1127+
if f.disambiguateDatumTypes {
1128+
buf.WriteString("TIMESTAMPTZ ")
1129+
}
11121130
if !f.bareStrings {
11131131
buf.WriteByte('\'')
11141132
}
@@ -1323,9 +1341,16 @@ func (d *DInterval) ValueAsString() string {
13231341

13241342
// Format implements the NodeFormatter interface. Example: "INTERVAL `1h2m`".
13251343
func (d *DInterval) Format(buf *bytes.Buffer, f FmtFlags) {
1326-
buf.WriteString("INTERVAL '")
1344+
if f.disambiguateDatumTypes {
1345+
buf.WriteString("INTERVAL ")
1346+
}
1347+
if !f.bareStrings {
1348+
buf.WriteByte('\'')
1349+
}
13271350
buf.WriteString(d.ValueAsString())
1328-
buf.WriteByte('\'')
1351+
if !f.bareStrings {
1352+
buf.WriteByte('\'')
1353+
}
13291354
}
13301355

13311356
// Size implements the Datum interface.

‎pkg/sql/parser/expr.go

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,8 +1004,6 @@ type castSyntaxMode int
10041004
const (
10051005
castExplicit castSyntaxMode = iota
10061006
castShort
1007-
castPrefix
1008-
castPrefixParens
10091007
)
10101008

10111009
// CastTargetType represents a type that is a valid cast target.
@@ -1055,15 +1053,6 @@ func (node *CastExpr) Format(buf *bytes.Buffer, f FmtFlags) {
10551053
FormatNode(buf, f, node.Expr)
10561054
buf.WriteString("::")
10571055
FormatNode(buf, f, node.Type)
1058-
case castPrefix:
1059-
FormatNode(buf, f, node.Type)
1060-
buf.WriteByte(' ')
1061-
FormatNode(buf, f, node.Expr)
1062-
case castPrefixParens:
1063-
FormatNode(buf, f, node.Type)
1064-
buf.WriteString(" (")
1065-
FormatNode(buf, f, node.Expr)
1066-
buf.WriteByte(')')
10671056
default:
10681057
buf.WriteString("CAST(")
10691058
FormatNode(buf, f, node.Expr)

‎pkg/sql/parser/format.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,16 @@ type fmtFlags struct {
3838
starDatumFormat func(buf *bytes.Buffer, f FmtFlags)
3939
// If true, strings will be rendered without wrapping quotes if possible.
4040
bareStrings bool
41+
// If true, constant values will include type annotations (like INTERVAL,
42+
// DATE) as necessary.
43+
disambiguateDatumTypes bool
4144
}
4245

4346
// FmtFlags enables conditional formatting in the pretty-printer.
4447
type FmtFlags *fmtFlags
4548

4649
// FmtSimple instructs the pretty-printer to produce
47-
// a straightforward representation, ideally using SQL
48-
// syntax that makes prettyprint+parse idempotent.
50+
// a straightforward representation.
4951
var FmtSimple FmtFlags = &fmtFlags{}
5052

5153
// FmtShowTypes instructs the pretty-printer to
@@ -61,6 +63,11 @@ var FmtSymbolicVars FmtFlags = &fmtFlags{symbolicVars: true}
6163
// wrapping quotes, if possible.
6264
var FmtBareStrings FmtFlags = &fmtFlags{bareStrings: true}
6365

66+
// FmtSerialize instructs the pretty-printer to produce a representation that
67+
// can be parsed into an equivalent expression (useful for serialization of
68+
// expressions).
69+
var FmtParsable FmtFlags = &fmtFlags{disambiguateDatumTypes: true}
70+
6471
// FmtNormalizeTableNames returns FmtFlags that instructs the pretty-printer
6572
// to normalize all table names using the provided function.
6673
func FmtNormalizeTableNames(base FmtFlags, fn func(*NormalizableTableName) *TableName) FmtFlags {

‎pkg/sql/parser/sql.go

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/sql/parser/sql.y

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4841,15 +4841,15 @@ a_expr_const:
48414841
| func_name '(' expr_list opt_sort_clause ')' SCONST { return unimplemented(sqllex) }
48424842
| const_typename SCONST
48434843
{
4844-
$$.val = &CastExpr{Expr: &StrVal{s: $2}, Type: $1.colType(), syntaxMode: castPrefix}
4844+
$$.val = &CastExpr{Expr: &StrVal{s: $2}, Type: $1.colType(), syntaxMode: castShort}
48454845
}
48464846
| const_interval SCONST opt_interval
48474847
{
4848-
$$.val = &CastExpr{Expr: &StrVal{s: $2}, Type: $1.colType(), syntaxMode: castPrefix}
4848+
$$.val = &CastExpr{Expr: &StrVal{s: $2}, Type: $1.colType(), syntaxMode: castShort}
48494849
}
48504850
| const_interval '(' ICONST ')' SCONST
48514851
{
4852-
$$.val = &CastExpr{Expr: &StrVal{s: $5}, Type: $1.colType(), syntaxMode: castPrefixParens}
4852+
$$.val = &CastExpr{Expr: &StrVal{s: $5}, Type: $1.colType(), syntaxMode: castShort}
48534853
}
48544854
| TRUE
48554855
{

‎pkg/sql/parser/type_check.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,14 @@ func (expr *CastExpr) TypeCheck(ctx *SemaContext, _ Type) (TypedExpr, error) {
204204
// is in its resolvable type set), we desire the cast's type for the Constant,
205205
// which will result in the CastExpr becoming an identity cast.
206206
desired = returnType
207+
208+
// If the type doesn't have any possible parameters (like length,
209+
// precision), the CastExpr becomes a no-op and can be elided.
210+
switch expr.Type.(type) {
211+
case *BoolColType, *DateColType, *TimestampColType, *TimestampTZColType, *IntervalColType,
212+
*BytesColType:
213+
return expr.Expr.TypeCheck(ctx, returnType)
214+
}
207215
}
208216
case ctx.isUnresolvedPlaceholder(expr.Expr):
209217
// This case will be triggered if ProcessPlaceholderAnnotations found

‎pkg/sql/testdata/datetime

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@ SELECT * FROM t WHERE a = '2015-08-25 05:45:45.53453+01:00'::timestamp
2424
query ITTT
2525
EXPLAIN (DEBUG) SELECT * FROM t WHERE a = '2015-08-25 06:45:45.53453+02:00'::timestamp
2626
----
27-
0 /t/primary/'2015-08-25 04:45:45.53453+00:00' NULL PARTIAL
28-
0 /t/primary/'2015-08-25 04:45:45.53453+00:00'/b '2015-08-25' PARTIAL
29-
0 /t/primary/'2015-08-25 04:45:45.53453+00:00'/c INTERVAL '2h45m2.234s' ROW
27+
0 /t/primary/'2015-08-25 04:45:45.53453+00:00' NULL PARTIAL
28+
0 /t/primary/'2015-08-25 04:45:45.53453+00:00'/b '2015-08-25' PARTIAL
29+
0 /t/primary/'2015-08-25 04:45:45.53453+00:00'/c '2h45m2.234s' ROW
3030

3131
query ITTT
3232
EXPLAIN (DEBUG) SELECT b FROM t WHERE b < '2015-08-29'::date
@@ -36,8 +36,8 @@ EXPLAIN (DEBUG) SELECT b FROM t WHERE b < '2015-08-29'::date
3636
query ITTT
3737
EXPLAIN (DEBUG) SELECT c FROM t WHERE c < '234h45m2s234ms'::interval
3838
----
39-
0 /t/t_c_key/INTERVAL '2h45m2.234s' /'2015-08-25 04:45:45.53453+00:00' ROW
40-
1 /t/t_c_key/INTERVAL '34h0m2s' /'2015-08-30 03:34:45.34567+00:00' ROW
39+
0 /t/t_c_key/'2h45m2.234s' /'2015-08-25 04:45:45.53453+00:00' ROW
40+
1 /t/t_c_key/'34h0m2s' /'2015-08-30 03:34:45.34567+00:00' ROW
4141

4242
# insert duplicate value with different time zone offset
4343
statement error duplicate key value \(a\)=\('2015-08-30 03:34:45\.34567\+00:00'\) violates unique constraint "primary"

‎pkg/sql/testdata/views

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,5 +338,11 @@ CREATE VIEW dt AS SELECT d, t FROM t WHERE d > DATE '1988-11-12' AND t < TIMESTA
338338
statement ok
339339
SELECT * FROM dt
340340

341+
statement ok
342+
CREATE VIEW dt2 AS SELECT d, t FROM t WHERE d > d + INTERVAL '10h'
343+
344+
statement ok
345+
SELECT * FROM dt2
346+
341347
statement ok
342348
DROP TABLE t CASCADE

0 commit comments

Comments
 (0)