GoogleCloudPlatform / GoogleCloudPlatform/cloud-spanner-emulator

ALTER CHANGE STREAM ... SET FOR leaves a column-tracked table tracked, blocking DROP TABLE (1.5.54)

Open
#352 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
334
Forks
77
Avg merge
8m
Merged PRs (30d)
2

Description

## Summary

Removing a **column-tracked** table (`FOR T(col)`) from a change stream's `FOR` clause via `ALTER CHANGE STREAM ... SET FOR` updates `information_schema` correctly (the table no longer shows as tracked), but does **not** clear the table's internal explicit-tracking back-reference. A subsequent `DROP TABLE` is then wrongly rejected:

> Cannot drop table Z. Table Z is tracked by 1 Change Stream: CS. Tables explicitly tracked by a Change Stream cannot be dropped...

Whole-table tracking (`FOR T`) is unaffected, and `DROP CHANGE STREAM` clears tracking correctly — so the bug is specific to **untracking a column-tracked table via `SET FOR`**. It blocks normal schema evolution: a table that was ever column-tracked by a change stream can't be dropped, even after it's removed from the `FOR` clause. (Distinct from the now-fixed false per-column-limit bug, #298.)

## Environment

- `gcr.io/cloud-spanner-emulator/emulator:1.5.54` (latest; reproduced)
- Still present on current `main` by source inspection — see *Root cause*.

## Minimal repro

```bash
docker run -d --rm --name emu -p 9010:9010 -p 9020:9020 gcr.io/cloud-spanner-emulator/emulator:1.5.54
gcloud config configurations create emu --no-activate
gcloud config set --configuration=emu auth/disable_credentials true
gcloud config set --configuration=emu project test-project
gcloud config set --configuration=emu api_endpoint_overrides/spanner http://localhost:9020/
gcloud config configurations activate emu
gcloud spanner instances create test-instance --config=emulator-config --nodes=1

gcloud spanner databases create db --instance=test-instance \
--ddl='CREATE TABLE K (Id INT64, c INT64) PRIMARY KEY(Id)' \
--ddl='CREATE TABLE Z (Id INT64, c INT64) PRIMARY KEY(Id)' \
--ddl='CREATE CHANGE STREAM CS FOR K(c), Z(c)'

# Remove Z from the change stream's FOR clause:
gcloud spanner databases ddl update db --instance=test-instance \
--ddl='ALTER CHANGE STREAM CS SET FOR K(c)'

# information_schema confirms CS now tracks only K:
gcloud spanner databases execute-sql db --instance=test-instance \
--sql="SELECT table_name FROM information_schema.change_stream_tables WHERE change_stream_name='CS'"
# -> K (Z is gone)

# ...but dropping Z fails:
gcloud spanner databases ddl update db --instance=test-instance --ddl='DROP TABLE Z'
```

**Expected:** `DROP TABLE Z` succeeds — Z is no longer in `CS`'s `FOR` clause.

**Actual:**
```
ERROR: ... Cannot drop table Z. Table Z is tracked by 1 Change Stream: CS.
Tables explicitly tracked by a Change Stream cannot be dropped. Please drop the
Change Stream or modify its FOR clause to stop tracking the table explicitly
before dropping the table.
```

### Controls (isolate the trigger)

- **Whole-table tracking works:** `CREATE CHANGE STREAM CS FOR K, Z` → `ALTER CHANGE STREAM CS SET FOR K` → `DROP TABLE Z` **succeeds**.
- **`DROP CHANGE STREAM` works:** `CREATE CHANGE STREAM CS FOR Z(c)` → `DROP CHANGE STREAM CS` → `DROP TABLE Z` **succeeds**.

Only removing a *column-tracked* table from the `FOR` clause via `SET FOR` triggers it.

## Root cause

In `backend/schema/updater/schema_updater.cc`, `RegisterTrackedObjects` records explicit table tracking **asymmetrically**:

- it always adds the table to `change_streams_explicitly_tracking_table_`;
- it adds the table to the table's general `change_streams_` list **only** in the `has_all_columns()` (whole-table) branch — not in the `has_tracked_columns()` (column-only) branch.

`UnregisterChangeStreamFromTrackedObjects` guards the table cleanup on `Table::FindChangeStream`, which searches **only** the general `change_streams_` list:

```cpp
const Table* table = latest_schema_->FindTable(table_name);
if (table->FindChangeStream(change_stream->Name())) { // false for a column-only tracked table
GOOGLESQL_RETURN_IF_ERROR(AlterNode(table, table_cb)); // remove_change_stream -> also clears explicit list
}
```

For a column-only tracked table the guard is false, so `change_streams_explicitly_tracking_table_` is never cleared — and the `DROP TABLE` validation, which reads that list, keeps failing.

## Suggested fix

Always run the table-unregister callback. `Table::Editor::remove_change_stream` is idempotent (both erases are guarded by `if (itr != end())`), so it's safe when the stream isn't in the general list:

```diff
const Table* table = latest_schema_->FindTable(table_name);
- if (table->FindChangeStream(change_stream->Name())) {
- GOOGLESQL_RETURN_IF_ERROR(AlterNode(table, table_cb));
- }
+ GOOGLESQL_RETURN_IF_ERROR(AlterNode(table, table_cb));
```

A reference fix — this one-line change plus a conformance regression test in `tests/conformance/data/schema_changes/change_streams.test` — is on a fork for convenience:
https://github.com/darryljjennings/cloud-spanner-emulator/commit/9b2632fd

(I see CONTRIBUTING.md notes external code contributions aren't currently accepted — this is just for reference/repro; happy to provide it in whatever form is most useful.)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.