fix(#481): ALTER TABLE ADD COLUMN — new columns invisible to subsequent queries by lraigosov · Pull Request #495 · goccy/bigquery-emulator

lraigosov · 2026-06-17T02:19:49Z

Depends on PR #492. This branch is based on the same commits as #492 (fixes for #470, #482, #483, #493). GitHub therefore shows those shared commits in this diff as well. The only change unique to this PR is the final commit (021f949). Reviewers can focus exclusively on server/handler.go and internal/connection/manager.go for the ALTER TABLE logic, and server/server_test.go for TestDDLAlterTable. Once #492 is merged and this branch is rebased onto the updated main, the diff will show only the ALTER TABLE commit.

Problem

googlesqlite treats ALTER TABLE as a no-op (NoopStmtAction): the underlying SQLite schema and the internal ZetaSQL catalog are never updated, so columns added via ADD COLUMN are invisible to subsequent INSERT/SELECT analysis — the query fails with "Column X is not present in table".

Closes #481

Approach

The fix intercepts ALTER TABLE in both jobsInsertHandler and jobsQueryHandler and applies three coordinated updates within the active transaction:

Raw SQLite DDL (ADD COLUMN, DROP COLUMN, RENAME COLUMN) executed via reflect+unsafe on the private inner *sql.DB inside googlesqlite.Conn.
UPSERT of googlesqlite_catalog with the new TableSpec JSON so the persisted catalog stays consistent.
AddColumn on the WASM-side SimpleTable inside the SimpleCatalog, making the column visible to the ZetaSQL analyzer immediately without reloading the catalog.

After commit, updateTableMetadata persists the updated BigQuery schema so the REST API returns the correct field list. All parsing is driven by the ZetaSQL AST — nothing is hardcoded.

Trade-offs and risks

This fix accesses private fields of googlesqlite.Conn using reflect+unsafe. This is an acknowledged workaround for a gap in the public API of googlesqlite. The approach is self-contained — the unsafe pointer arithmetic is isolated to the ALTER TABLE handler and will need to be revisited if the internal structure of googlesqlite.Conn changes in a future version.

If the team prefers not to merge this until googlesqlite exposes an official hook, this PR can be deferred independently without blocking the other fixes in PR #492.

Test plan

TestDDLAlterTable (server/server_test.go) — covers CREATE TABLE, ALTER TABLE ADD COLUMN, metadata verification via the REST API, INSERT with the new column, and SELECT to confirm round-trip correctness.

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR extends the emulator’s DDL and query handling to better match BigQuery behavior, primarily by adding ALTER TABLE support and improving schema/metadata propagation.

Changes:

Add ALTER TABLE execution path (parse AST, run raw SQLite DDL, update catalogs/metadata).
Improve result formatting for nested/REPEATED fields (notably nested TIMESTAMPs) and add related tests.
Add helper to derive TableReference.TableId from table.Id, plus new tests and local dev ignores.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
server/handler.go	Implements `ALTER TABLE` handling, catalog updates, and `terminalTableID` helper.
internal/connection/manager.go	Adds raw SQLite execution helpers using `reflect/unsafe`.
internal/contentdata/repository.go	Switches repo mode calls used during table creation/data writes.
internal/types/types.go	Refactors formatting to handle nested/repeated cells (TIMESTAMP in RECORDs).
server/server_test.go	Adds integration test covering `ALTER TABLE ADD COLUMN`.
server/handler_tableid_test.go	Adds unit test for `terminalTableID`.
internal/types/types_test.go	Adds unit tests for `formatCell` edge cases and nested timestamp formatting.
.gitignore	Ignores local repro scripts/logs and dev log.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

lraigosov · 2026-06-17T02:51:59Z

+	if err := tx.MetadataRepoMode(); err != nil {
 		return err
 	}


This change is part of the shared commits from PR #492 (fix for #470). The switch to MetadataRepoMode() in these calls is intentional: the table is being registered in the metadata catalog during the streaming-insert resolution path, not written to the content store. PR #492 contains the full explanation and test coverage for this change.

lraigosov · 2026-06-17T02:52:01Z

+	if err := tx.MetadataRepoMode(); err != nil {
 		return err
 	}


Same as above — this is part of the shared commits from PR #492. Addressed there.

lraigosov · 2026-06-17T02:52:16Z

+func (t *Tx) RawExec(ctx context.Context, query string, args ...interface{}) error {
+	var execErr error
+	if err := t.conn.Conn.Raw(func(c interface{}) error {
+		gsqlConn, ok := c.(*googlesqlite.Conn)
+		if !ok {
+			return fmt.Errorf("unexpected driver connection type %T", c)
+		}
+		f := reflect.ValueOf(gsqlConn).Elem().FieldByName("conn")
+		if !f.IsValid() {
+			return fmt.Errorf("googlesqlite.Conn has no 'conn' field")
+		}
+		innerConn := *(**sql.Conn)(unsafe.Pointer(f.UnsafeAddr()))
+		_, execErr = innerConn.ExecContext(ctx, query, args...)
+		return nil
+	}); err != nil {
+		return err
+	}
+	return execErr
+}


Fixed in commit 5b3b693. Extracted the reflect+unsafe access into a dedicated innerSQLConn helper that validates f.Kind() == reflect.Ptr and f.Type() == reflect.TypeOf((*sql.Conn)(nil)) before the pointer reinterpretation. A type mismatch now returns a clear error instead of causing a panic or memory corruption.

lraigosov · 2026-06-17T02:52:31Z

+	for _, action := range actions {
+		if action.kind != "ADD_COLUMN" {
+			continue
+		}
+		colType, err := tf.MakeSimpleType(googlesql.TypeKind(bqTypeKind(action.colType)))
+		if err != nil {
+			return fmt.Errorf("failed to make ZetaSQL type for column %q: %w", action.colName, err)
+		}
+		newCol, err := googlesql.NewSimpleColumn(sqliteTableName, action.colName, colType, false, true)
+		if err != nil {
+			return fmt.Errorf("failed to create SimpleColumn %q: %w", action.colName, err)
+		}
+		if err := target.AddColumn(newCol); err != nil {
+			return fmt.Errorf("failed to add column %q to ZetaSQL table: %w", action.colName, err)
+		}
+	}


Acknowledged. go-googlesql's SimpleTable does not expose a RemoveColumn or RenameColumn method, so rebuilding the in-memory table after DROP/RENAME would require re-registering the table in the SimpleCatalog — which also lacks a RemoveTable/ReplaceTable API. The googlesqlite_catalog UPSERT keeps the persisted schema correct for the next sync cycle. In-memory catalog divergence for DROP/RENAME within the same connection is a known limitation of this approach, documented in the function comment. Full support would require an upstream API addition to go-googlesql.

lraigosov · 2026-06-17T02:52:41Z

+func isAlterTableQuery(query string) bool {
+	q := strings.ToUpper(strings.TrimSpace(query))
+	return strings.HasPrefix(q, "ALTER") && strings.Contains(q, "TABLE")
+}


Fixed in commit 5b3b693. Changed to strings.HasPrefix(q, "ALTER TABLE ") (with trailing space) so only actual ALTER TABLE statements are routed through the DDL path.

lraigosov · 2026-06-17T02:53:11Z

+				kind:    "DROP_COLUMN",
+				colName: colName,
+			})
+		case *googlesql.ASTRenameColumnAction:


Fixed in commit 5b3b693. Added a default branch in parseAlterTable that returns fmt.Errorf("alter table: unsupported action type %T", node). Unknown actions now surface as an explicit error so the job fails cleanly instead of returning a false success.

lraigosov · 2026-06-17T02:53:13Z

+		}
+	}
+	return tablePath, actions, nil


Fixed in commit 5b3b693. Added a default branch in parseAlterTable that returns fmt.Errorf("alter table: unsupported action type %T", node). Unknown actions now surface as an explicit error so the job fails cleanly instead of returning a false success.

lraigosov · 2026-06-17T02:53:35Z

+func bqTypeKind(typeName string) int {
+	switch strings.ToUpper(typeName) {


Fixed in commit 5b3b693. bqTypeKind now returns (int, bool). All three call sites check the boolean and return a descriptive error on unrecognised type names, preventing silent schema corruption.

lraigosov · 2026-06-17T02:53:37Z

+	default:
+		return int(googlesql.TypeKindTypeString)
+	}
+}


Fixed in commit 5b3b693. bqTypeKind now returns (int, bool). All three call sites check the boolean and return a descriptive error on unrecognised type names, preventing silent schema corruption.

lraigosov · 2026-06-17T02:54:16Z

+package server
+
+import "testing"


This file is part of the shared commits from PR #492 (commit 0160fc5). The CRLF issue appears in the diff because the file was written on Windows but the repository uses core.eol=lf. The committed object in the repository already has LF-only line endings — git show HEAD:server/handler_tableid_test.go | file - confirms this. It is a working-tree display artefact on Windows, not a committed problem.

- isAlterTableQuery: use HasPrefix("ALTER TABLE ") instead of separate HasPrefix("ALTER")+Contains("TABLE") to avoid mis-routing unrelated statements. - bqTypeKind: return (int, bool) so callers get an explicit error on unrecognised type names instead of silently falling back to STRING. - parseAlterTable: add default branch returning an error for unsupported action types so unknown actions are rejected, not silently ignored. - innerSQLConn: extract reflect+unsafe cast into a helper that verifies field kind and exact type before the pointer reinterpretation.

lraigosov added 5 commits June 15, 2026 00:56

fix: resolve unqualified query results for streaming inserts

27a407f

fix: nested timestamp format in record fields

3c1be50

chore: ignore local repro directory

e3d09e7

fix: address review feedback for table handling

0160fc5

fix: propagate ALTER TABLE ADD COLUMN to ZetaSQL in-memory catalog

021f949

Copilot AI review requested due to automatic review settings June 17, 2026 02:19

lraigosov mentioned this pull request Jun 17, 2026

Alter table does not seem to work #481

Open

lraigosov changed the title ~~fix: ALTER TABLE ADD COLUMN — new columns invisible to subsequent queries~~ fix(#481): ALTER TABLE ADD COLUMN — new columns invisible to subsequent queries Jun 17, 2026

Copilot AI reviewed Jun 17, 2026

View reviewed changes

lraigosov mentioned this pull request Jun 17, 2026

fix: resolve issues #470, #482, #483 and #493 #492

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(#481): ALTER TABLE ADD COLUMN — new columns invisible to subsequent queries#495

fix(#481): ALTER TABLE ADD COLUMN — new columns invisible to subsequent queries#495
lraigosov wants to merge 6 commits into
goccy:mainfrom
lraigosov:feat/alter-table-fix

lraigosov commented Jun 17, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

lraigosov Jun 17, 2026

Uh oh!

lraigosov Jun 17, 2026

Uh oh!

lraigosov Jun 17, 2026

Uh oh!

lraigosov Jun 17, 2026

Uh oh!

lraigosov Jun 17, 2026

Uh oh!

lraigosov Jun 17, 2026

Uh oh!

lraigosov Jun 17, 2026

Uh oh!

lraigosov Jun 17, 2026

Uh oh!

lraigosov Jun 17, 2026

Uh oh!

lraigosov Jun 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		func bqTypeKind(typeName string) int {
		switch strings.ToUpper(typeName) {

		package server

		import "testing"

Uh oh!

Conversation

lraigosov commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Approach

Trade-offs and risks

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lraigosov commented Jun 17, 2026 •

edited

Loading