googleapis / googleapis/go-sql-spanner
QueryContext forwards empty and comment-only fragments during multi-statement execution
- Dominant language
- Go
- Stars
- 125
- Forks
- 35
- Avg merge
- 2h 20m
- Merged PRs (30d)
- 4
Description
#### Environment details
- Programming language: Go
- OS: macOS (Darwin/arm64)
- Language runtime version: Go 1.26.5
- Package version: `github.com/googleapis/go-sql-spanner` v1.26.0
- Cloud Spanner Emulator: v1.5.56
- Database dialect: GoogleSQL
#### Steps to reproduce
1. Start a fresh Cloud Spanner Emulator.
2. Save the following as `main.go` and run it with `github.com/googleapis/go-sql-spanner` v1.26.0:
```go
package main
import (
"context"
"database/sql"
"fmt"
"cloud.google.com/go/spanner/admin/database/apiv1/databasepb"
_ "github.com/googleapis/go-sql-spanner"
"github.com/googleapis/go-sql-spanner/parser"
)
func main() {
p, err := parser.NewStatementParser(databasepb.DatabaseDialect_GOOGLE_STANDARD_SQL, 0)
if err != nil {
panic(err)
}
for _, q := range []string{"-- comment only\n", "NOT A STATEMENT"} {
fmt.Printf("%q: %v\n", q, p.DetectStatementType(q).StatementType)
}
db, err := sql.Open("spanner", "projects/test-project/instances/test-instance/databases/test-database?autoConfigEmulator=true")
if err != nil {
panic(err)
}
defer db.Close()
for _, q := range []string{"-- comment only\n", "SELECT 1;;SELECT 2"} {
fmt.Printf("%q: %v\n", q, query(context.Background(), db, q))
}
}
func query(ctx context.Context, db *sql.DB, q string) error {
rows, err := db.QueryContext(ctx, q)
if err != nil {
return err
}
defer rows.Close()
for {
for rows.Next() {
}
if err := rows.Err(); err != nil {
return err
}
if !rows.NextResultSet() {
return rows.Err()
}
}
}
```
#### Actual behavior
`DetectStatementType` reports both comment-only input and invalid SQL as `StatementTypeUnknown`. `QueryContext` sends comment-only input to Spanner, where it fails with `InvalidArgument: Unexpected end of statement`. In `SELECT 1;;SELECT 2`, the first query executes, the empty statement fails, and the second query is not reached. Because later errors are reported through `Rows.Err`, `QueryContext` itself can return without an error.
This is more consequential for DDL. For example, in `CREATE TABLE T1 (...);;CREATE TABLE T2 (...)`, `T1` can be applied before the empty fragment fails and prevents `T2` from running. The preceding DDL is not rolled back.
#### Expected behavior
There is no universal cross-database contract for a wholly empty query. This issue is limited to empty or comment-only fragments encountered while handling multi-statement input.
`QueryContext` should handle those fragments consistently before executing any non-empty fragment instead of forwarding them to Spanner partway through iteration. Discarding them, as PostgreSQL's top-level parser does, is one reasonable behavior; rejecting the complete input before execution would also avoid the current partial-processing behavior. Non-empty invalid SQL must remain distinguishable.
#### Cause
The simple parser retains empty fragments when splitting statements, while `DetectStatementType` classifies both empty/comment-only input and non-empty unrecognized SQL as `StatementTypeUnknown`. Callers therefore cannot distinguish SQL trivia from an invalid statement.
`QueryContext` treats these unknown fragments as queries and sends them to Spanner. A parser API that identifies whether input contains no statement tokens would let `QueryContext` handle empty/comment-only fragments without hiding invalid SQL.
#### References
- Go's [`database/sql.QueryContext`](https://pkg.go.dev/database/sql#DB.QueryContext) documentation does not define empty-query or multi-statement behavior.
- Spanner applies DDL statements in order, stops at the first error, and does not roll back statements that were already applied, as described in [Schema update best practices](https://cloud.google.com/spanner/docs/schema-updates-best-practices#order_of_execution_of_statements_in_batches).
- PostgreSQL's [top-level grammar](https://github.com/postgres/postgres/blob/master/src/backend/parser/gram.y#L948-L971) explicitly discards empty statements, while its [simple-query protocol](https://www.postgresql.org/docs/current/protocol-flow.html#PROTOCOL-FLOW-SIMPLE-QUERY) returns a distinct `EmptyQueryResponse` for a wholly empty query string. PostgreSQL comments are [equivalent to whitespace](https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-COMMENTS).
- SQLite reports successful preparation with no statement object when the input contains [no SQL, including an empty string or a comment](https://www.sqlite.org/c3ref/prepare.html).
- MySQL instead defines [`ER_EMPTY_QUERY`](https://dev.mysql.com/doc/mysql-errors/8.4/en/server-error-reference.html#error_er_empty_query) (`Query was empty`).
The empty and multi-statement cases above were also reproduced with PostgreSQL 18.4, MySQL 8.4.11, and SQLite 3.51.0.
#### Relation to #461
This behavior affects the simple-parser use cases described in #461.
Contributor guide
Research direction
Start with QueryContext and the simple parser path used by DetectStatementType, then reproduce the comment-only and SELECT 1;;SELECT 2 cases from the issue. Trace how statement fragments are split and classified, and inspect the behavior described in #461. Done means empty or comment-only fragments are handled consistently before execution while non-empty invalid SQL remains distinguishable, with coverage for multi-statement behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend-api-design, databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100