sql: remove extra type hint from SHOW CREATE VIEW
#95679
Conversation
|
This change is |
e-mbrown
force-pushed
the
eb/showcreate
branch
from
10e6378 to
f18ef3a
Compare
| table_name create_statement | ||
| t2 CREATE VIEW public.t2 ( | ||
| "array" | ||
| ) AS SELECT ARRAY[1:::INT8, 2:::INT8, 3:::INT8]::INT8[]:::INT8[] |
There was a problem hiding this comment.
it looks like this still has an extra :::INT8[] type annotation at the end. (and same for t1 above this)
pkg/sql/sem/tree/expr.go
Outdated
| @@ -885,7 +885,7 @@ func (node *Array) Format(ctx *FmtCtx) { | |||
| // UNKNOWN[], since that's not a valid annotation. | |||
| if ctx.HasFlags(FmtParsable) && node.typ != nil { | |||
| if node.typ.ArrayContents().Family() != types.UnknownFamily { | |||
| ctx.WriteString(":::") | |||
| ctx.WriteString("::") | |||
There was a problem hiding this comment.
i'm wondering if what we need is to keep track of whether there already was a type annotation wrapping the Array expr. or alternatively, update AnnotateTypeExpr.Format so it doesn't print the annotation if the inner expr already is going to
e-mbrown
force-pushed
the
eb/showcreate
branch
2 times, most recently
from
271fb92 to
2c9c878
Compare
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @rafiss)
pkg/sql/sem/tree/expr.go line 888 at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
i'm wondering if what we need is to keep track of whether there already was a type annotation wrapping the
Arrayexpr. or alternatively, updateAnnotateTypeExpr.Formatso it doesn't print the annotation if the inner expr already is going to
Done.
pkg/sql/logictest/testdata/logic_test/views line 162 at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
it looks like this still has an extra
:::INT8[]type annotation at the end. (and same fort1above this)
Done.
pkg/sql/sem/tree/expr.go
Outdated
| @@ -884,7 +884,7 @@ func (node *Array) Format(ctx *FmtCtx) { | |||
| // If the array has a type, add an annotation. Don't add it if the type is | |||
| // UNKNOWN[], since that's not a valid annotation. | |||
| if ctx.HasFlags(FmtParsable) && node.typ != nil { | |||
| if node.typ.ArrayContents().Family() != types.UnknownFamily { | |||
| if node.typ.ArrayContents().Family() != types.UnknownFamily && node.typeAnnotation.typ == nil { | |||
There was a problem hiding this comment.
to summarize what we discussed offline: node.typ and node.typeAnnotation.typ refer to the same thing, so this change doesn't have the intended affect.
perhaps instead our approach should be to update formatViewQueryForDisplay to detect when an annotation would be duplicated, and prevent that.
e-mbrown
force-pushed
the
eb/showcreate
branch
2 times, most recently
from
329be31 to
2d06063
Compare
Views created with user provided type hints were given extra type annotations. The format functions for `AnnotateTypeExpr` and `Array` were both annotating the expression. This commit alters the format function of `AnnotateTypeExpr` so it doesnt duplicate annotations for arrays. Release note: None
e-mbrown
force-pushed
the
eb/showcreate
branch
from
2d06063 to
e7019b0
Compare
|
Woo! bors r=rafiss |
|
Build succeeded: |
Resolves: #87886
Views created with user provided type hints were given
extra type annotations. The format functions for
AnnotateTypeExprand
Arraywere both annotating the expression. This commit altersthe format function of
AnnotateTypeExprso it doesnt duplicateannotations for arrays.
Release note: None