GoogleCloudPlatform / GoogleCloudPlatform/cloud-spanner-emulator
ALTER CHANGE STREAM ... SET FOR leaves a column-tracked table tracked, blocking DROP TABLE (1.5.54)
- 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
Assessment
This issue has not been assessed yet.