fix(spanner/spansql): PROTO BUNDLE and protobuf type parsing fixes (#…

…11279)

* fix: spansql: fix NOT NULL protobuf column type parsing

The protobuf type-name parser was so greedy that it failed on NOT NULL
columns. In particular, it wasn't aware that unquoted tokens should be
separated by dots, and that quoted tokens shouldn't be concatenated with
anything else.

Fix this by adding a boolean to handle that alternation and only
allowing quoted IDs if nothing else has been consumed. (then bailing
immediately after a quoted ID so we don't try to consume anything
else)

* fix: spansql: fix invalid CAST tests

A misreading of the spanner docs lead to tests that indicated that
casting `AS ENUM` or `AS PROTO` was valid syntax (despite not specifying
_which_ protobuf enum or message type to cast to). Replace these cases
with ones that validate casting to specific enum/message types.

Thanks to @apstndb for calling this out on #10945.

* fix spansql: CREATE PROTO BUNDLE SQL with 0 types

Fix a bug in CreateProtoBundle.SQL() which unintentionally generated the
DDL when there were no types listed:
```
CREATE PROTO BUNDLE (``)
```

---------

Co-authored-by: Sri Harsha CH <[email protected]>

spanner/spansql/parser.go

@@ -3151,21 +3151,40 @@ func (p *parser) parseProtobufTypeName(consumed string) (string, *parseError) {
 	possibleProtoTypeName := strings.Builder{}
 	possibleProtoTypeName.WriteString(consumed)
 	ntok := p.next()
+	// Pretend the last token was a dot if either the "consumed" portion we
+	// were given was either empty, or it actually ended in a dot.
+	lastTokIsDot := len(consumed) == 0 || consumed[len(consumed)-1] == '.'
 PROTO_TOK_CONSUME:
 	for ; ntok.err == nil; ntok = p.next() {
 		appendVal := ntok.value
 		switch ntok.typ {
 		case unquotedID:
+			// only consume an unquoted token if the last one was a dot
+			if !lastTokIsDot {
+				p.back()
+				break PROTO_TOK_CONSUME
+			}
+			lastTokIsDot = false
 		case quotedID:
+			// It isn't valid to only quote part of a protobuf
+			// type-name, back out if we encounter another quoted
+			// value.
+			if possibleProtoTypeName.Len() > 0 {
+				p.back()
+				break PROTO_TOK_CONSUME
+			}
 			if !fqProtoMsgName.MatchString(ntok.string) {
 				return "", p.errorf("got %q, want fully qualified protobuf type", ntok.string)
 			}
-			appendVal = ntok.string
+			// Once we've encountered a quoted type-name, we can't consume anything else for this type-name
+			possibleProtoTypeName.WriteString(ntok.string)
+			break PROTO_TOK_CONSUME
 		case unknownToken:
 			if ntok.value != "." {
 				p.back()
 				break PROTO_TOK_CONSUME
 			}
+			lastTokIsDot = true
 		default:
 			p.back()
 			break PROTO_TOK_CONSUME

spanner/spansql/parser_test.go

@@ -444,8 +444,8 @@ func TestParseExpr(t *testing.T) {
 		// Functions
 		{`STARTS_WITH(Bar, 'B')`, Func{Name: "STARTS_WITH", Args: []Expr{ID("Bar"), StringLiteral("B")}}},
 		{`CAST(Bar AS STRING)`, Func{Name: "CAST", Args: []Expr{TypedExpr{Expr: ID("Bar"), Type: Type{Base: String}}}}},
-		{`CAST(Bar AS ENUM)`, Func{Name: "CAST", Args: []Expr{TypedExpr{Expr: ID("Bar"), Type: Type{Base: Enum}}}}},
-		{`CAST(Bar AS PROTO)`, Func{Name: "CAST", Args: []Expr{TypedExpr{Expr: ID("Bar"), Type: Type{Base: Proto}}}}},
+		{`CAST(Bar AS fizzle.bit)`, Func{Name: "CAST", Args: []Expr{TypedExpr{Expr: ID("Bar"), Type: Type{Base: Proto, ProtoRef: "fizzle.bit"}}}}},
+		{`CAST(Bar AS fizzle.bit.baz)`, Func{Name: "CAST", Args: []Expr{TypedExpr{Expr: ID("Bar"), Type: Type{Base: Proto, ProtoRef: "fizzle.bit.baz"}}}}},
 		{`SAFE_CAST(Bar AS INT64)`, Func{Name: "SAFE_CAST", Args: []Expr{TypedExpr{Expr: ID("Bar"), Type: Type{Base: Int64}}}}},
 		{`EXTRACT(DATE FROM TIMESTAMP AT TIME ZONE "America/Los_Angeles")`, Func{Name: "EXTRACT", Args: []Expr{ExtractExpr{Part: "DATE", Type: Type{Base: Date}, Expr: AtTimeZoneExpr{Expr: ID("TIMESTAMP"), Zone: "America/Los_Angeles", Type: Type{Base: Timestamp}}}}}},
 		{`EXTRACT(DAY FROM DATE)`, Func{Name: "EXTRACT", Args: []Expr{ExtractExpr{Part: "DAY", Expr: ID("DATE"), Type: Type{Base: Int64}}}}},
@@ -1942,6 +1942,26 @@ func TestParseDDL(t *testing.T) {
 				},
 			},
 		},
+		{
+			"CREATE TABLE IF NOT EXISTS tname (id INT64, name `foo.bar.baz.ProtoName` NOT NULL) PRIMARY KEY (id)",
+			&DDL{
+				Filename: "filename",
+				List: []DDLStmt{
+					&CreateTable{
+						Name:        "tname",
+						IfNotExists: true,
+						Columns: []ColumnDef{
+							{Name: "id", Type: Type{Base: Int64}, Position: line(1)},
+							{Name: "name", NotNull: true, Type: Type{Base: Proto, ProtoRef: "foo.bar.baz.ProtoName"}, Position: line(1)},
+						},
+						PrimaryKey: []KeyPart{
+							{Column: "id"},
+						},
+						Position: line(1),
+					},
+				},
+			},
+		},
 		{
 			`CREATE INDEX IF NOT EXISTS iname ON tname (cname)`,
 			&DDL{

spanner/spansql/sql.go

@@ -99,8 +99,12 @@ func (ci CreateIndex) SQL() string {
 }
 
 func (cp CreateProtoBundle) SQL() string {
+	typeList := ""
+	if len(cp.Types) > 0 {
+		typeList = "`" + strings.Join(cp.Types, "`, `") + "`"
+	}
 	// Backtick-quote all the types so we don't need to check for SQL keywords
-	return "CREATE PROTO BUNDLE (`" + strings.Join(cp.Types, "`, `") + "`)"
+	return "CREATE PROTO BUNDLE (" + typeList + ")"
 }
 
 func (cv CreateView) SQL() string {

spanner/spansql/sql_test.go

@@ -944,6 +944,14 @@ func TestSQL(t *testing.T) {
 			"CREATE PROTO BUNDLE (`a.b.c`, `b.d.e`)",
 			reparseDDL,
 		},
+		{
+			&CreateProtoBundle{
+				Types:    []string(nil),
+				Position: line(1),
+			},
+			"CREATE PROTO BUNDLE ()",
+			reparseDDL,
+		},
 		{
 			&CreateProtoBundle{
 				Types:    []string{"a"},