Skip to content

fix(storage-postgres): store attribute names containing U+0000 in the catalog - #833

Open
LeeroyHannigan wants to merge 3 commits into
mainfrom
fix/367-pg-catalog-nul-names
Open

LeeroyHannigan wants to merge 3 commits into
mainfrom
fix/367-pg-catalog-nul-names

Conversation

@LeeroyHannigan

@LeeroyHannigan LeeroyHannigan commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

What

This PR lets a PostgreSQL deployment store attribute names that hold U+0000 in its catalog. DynamoDB accepts U+0000 in a key attribute name, an index key attribute name and a projected attribute name. DescribeTable returns each name byte for byte (measured on 2026-09-18 and 2026-10-24, and again on 2026-11-04). PostgreSQL jsonb refuses U+0000. So on main at 25ee109, CreateTable with such a name answers 500 on PostgreSQL.

The stored form

crates/storage-postgres/src/catalog_json.rs defines the stored form of the jsonb documents that carry attribute names. The server stores U+0000 as U+0001 U+0001, and U+0001 as U+0001 U+0002. It stores every other character as itself, so a name without these two characters keeps its old stored form. Every write of these documents goes through the escape, and every read goes through the inverse. The documents are:

  • in the catalog database: tables.key_schema, tables.attribute_definitions, indexes.key_schema, indexes.projection, backups.key_schema, backups.attribute_definitions, backups.vector_indexes, backup_definitions.definition, vector_indexes.vector_attribute, vector_indexes.search_schema and vector_indexes.projection
  • in the data database: gsi_pending.index_context, which holds the index context of the queued index updates

The item documents do not go through the escape. #368 covers U+0000 in item data.

Migration 7, attribute_name_escape

The migration uses the framework of ADR-0023. On PostgreSQL it applies the escape once to each stored document that holds U+0001. Releases before migration 7 stored the names raw. Without the migration, a stored name with U+0001 followed by U+0001 or U+0002 would read back as another name.

  • The migration has a catalog body and a data body. Each body runs in one transaction with its ledger row.
  • It writes one note for each column that it changed, for example tables.key_schema: 1 document rewritten.
  • It is Kind::Breaking. A binary without the decoder reads an escaped name as another name.
  • It is not convergent, because an escaped U+0001 and a raw U+0001 U+0002 are the same bytes. So migrate --yes --reconverge skips it.
  • It does not apply to SQLite, Cassandra and MongoDB. SQLite and Cassandra store these documents as JSON text, where U+0000 is the six characters \^@. MongoDB stores them as BSON, whose strings hold U+0000.

Documentation and CI

  • docs/manuals/07-upgrade-manual.md describes migration 7 in step 4 of the general upgrade procedure.
  • docs/differences-from-dynamodb.md has a row for these names.
  • docs/adr/0023-catalog-migrations.md and the upgrade manual state that each migration is breaking on every backend it applies to. Before this change, both named five migrations.
  • CONTRIBUTING.md renumbers its example migration from 7 to 8.
  • The sample outputs show catalog migration 7.
  • The no-pgvector job in .github/workflows/integration.yml also runs the new test binary migrations_name_escape.

This PR replaces the catalog part of #531. #531 added catalog version 0.0.5 with its own migration code. Main migrates through the ADR-0023 framework, so this PR builds its migration on that framework.

#671 took migration 6 on main, so this PR's migration is now 7. #368, #474, #740, #819 and #823 add migrations too. Each pull request that merges after another one of them must take the next free id.

#700's backup job writes the source table, attribute names included, to backup_index.source_table_details_json through claim_backup and its status write. Both bind the JSON to a JSONB column without the escape. On PostgreSQL a backup of a table with a U+0000 attribute name would fail at that insert. Whichever of #700 and this PR merges second adds the escape on those writes and the unescape on the read.

The merge with main

The merge of main at 38992eb (5a0de15) moves this PR's migration from 6 to 7. It also ports catalog_nul_names.rs and migrations_name_escape.rs to LegacyBackupReader, which replaced BackupEngine in #671. The assertions do not change.

Why

Closes #367. The issue reports two faults on PostgreSQL. #830 fixes the table name fault. This PR fixes the attribute name fault.

Testing done

On the merge commit 5a0de15, on 2026-11-05: fmt and clippy pass on extenddb-storage, extenddb-storage-postgres, extenddb-storage-sqlite, extenddb-storage-mongodb and extenddb-storage-cassandra. Their lib tests pass: 228, 81, 257, 166 and 128. The MongoDB interpreter_selftests (18) and pushdown_parity (20) pass. CI runs the PostgreSQL, MongoDB and Cassandra test binaries.

Rust, on the branch (base: main at 25ee109):

  • cargo fmt --all -- --check: clean.
  • cargo +1.97.0 clippy --locked --all-targets -- -D warnings: clean.
  • cargo +1.88.0 build --locked --workspace: ok.
  • cargo test --locked --workspace --exclude extenddb-backup-store, with no external store: 3061 passed, 0 failed, 61 ignored.
  • cargo test --locked -p extenddb-storage-postgres --release --tests, against pgvector/pgvector:pg16: 347 passed, 0 failed, 2 ignored, in 27 test binaries. The new binaries are catalog_nul_names (2 passed), catalog_nul_index_writes (3) and migrations_name_escape (2). In an earlier run on this branch, on a loaded host, vector_control_plane::build_ownership_uses_its_own_session_and_releases_on_drop failed once. It passed in the two runs after that, with 42 passed each time. The test probes an advisory lock right after a drop. This PR does not change that code or that test.
  • The migration tests of the no-pgvector job, against postgres:16 without pgvector: 50 passed, 0 failed. By binary: migrations_crash 9, migrations_fresh 6, migrations_golden 7, migrations_guards 7, migrations_name_escape 2, migrations_released 12 and migrations_runs 7.
  • MongoDB (mongo:7 replica set), cargo test --locked -p extenddb-storage-mongodb --tests: 242 passed, 0 failed, 46 ignored. By binary: the library 165 (46 ignored), interpreter_selftests 18, migrations_fresh 8, migrations_guards 7, migrations_holders 10, migrations_locks 10, migrations_released 4 and pushdown_parity 20. In the last run, migrations_holders::a_primary_that_hangs_before_a_renewal_and_answers_again_keeps_the_registration failed once on a loaded host, with "could not renew its registration for 4 s". It passed in the run before and in a rerun of its binary. In that file this PR changes only the expected catalog version of two other tests.
  • Cassandra (cassandra:4.1): --test migrations 21 passed, and --test migrations_stopped 7 passed.

Wire tests through boto3, against a local server per backend, from a release build of the branch. The set is tests/python/test_nul_attribute_names.py (5 rows), test_tables and test_gsi.

Backend Result
SQLite 33 passed
PostgreSQL 33 passed
Cassandra (cassandra:4.1) 33 passed
MongoDB (mongo:7 replica set) 33 passed

The 5 new rows against a main 25ee109 PostgreSQL binary: 4 failed with 500 InternalServerError on CreateTable, and the U+0001 row passed.

Against Amazon DynamoDB in us-east-1 on 2026-11-04, from 22:45 to 22:56 UTC: the 5 new rows passed.

Checklist

  • I have read CONTRIBUTING.md
  • All tests pass (cargo test --workspace)
  • Code is formatted (cargo fmt --check)
  • Clippy is clean (cargo clippy -- -W clippy::pedantic)
  • I have added or updated tests for new functionality
  • I have updated documentation if behavior changed
  • Breaking changes are noted below (if any)
  • If this changes the wire protocol, Storage trait, auth model, on-disk
    format, or public CLI surface, an RFC has been accepted or is linked
    below. Otherwise, an ADR captures the decision (link below).

ADR / RFC: ADR-0023, catalog migrations. This PR adds migration 7 under it.

Breaking changes

Migration 7 is breaking on PostgreSQL. A binary without migration 7 refuses a PostgreSQL catalog that has it, with exit status 4. To roll back, restore the backup taken before extenddb migrate --yes. SQLite, Cassandra and MongoDB record migration 7 as not applicable, which is additive.


By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache License 2.0 and I agree to the Developer Certificate of
Origin (DCO). See CONTRIBUTING.md for details.

… catalog

DynamoDB accepts U+0000 in a key attribute name, an index key attribute
name and a projected attribute name. DescribeTable returns each name
byte for byte (measured on 2026-09-18 and 2026-10-24, and again on
2026-11-04). PostgreSQL jsonb refuses U+0000, so CreateTable with such a
name answered 500 on PostgreSQL.

catalog_json stores U+0000 in the jsonb documents that carry attribute
names as U+0001 U+0001, and U+0001 as U+0001 U+0002. Each read applies
the inverse. A name without either character keeps its stored form. The
documents are tables.key_schema, tables.attribute_definitions,
indexes.key_schema, indexes.projection, backups.key_schema,
backups.attribute_definitions, backups.vector_indexes,
backup_definitions.definition, vector_indexes.vector_attribute,
vector_indexes.search_schema and vector_indexes.projection in the
catalog database, and gsi_pending.index_context in the data database.
Item data does not go through the escape.

Migration 6, attribute_name_escape, applies the escape once to each
stored PostgreSQL document that holds U+0001. It has a catalog body and
a data body, and each body runs in one transaction with its ledger row.
It writes one note for each column that it changed. It is breaking,
because a binary without the decoder reads an escaped name as another
name. It is not convergent, because an escaped U+0001 and a raw U+0001
U+0002 are the same bytes. It does not apply to SQLite and Cassandra,
which store these documents as JSON text, or to MongoDB, whose BSON
strings hold U+0000.

The upgrade manual, ADR-0023, CONTRIBUTING.md and the sample outputs
name migration 6. The CONTRIBUTING.md example becomes migration 7. The
no-pgvector CI job also runs the new migrations_name_escape tests.

Closes #367
Main added migration 6, backup_catalog_index, which this branch had
used for attribute_name_escape. The merge keeps main's migration 6 as
it is and moves attribute_name_escape to id 7 in the registry, in the
PostgreSQL module (the file, module, SOURCE and FINGERPRINT are now
m0007_attribute_name_escape), in the golden fixture, which is now
0007_attribute_name_escape.txt, and in the SQLite, MongoDB and
Cassandra NotApplicable arms. The RELEASED_THROUGH comment names 7.

crates/storage/src/migrations/registry.rs: both sides added an entry at
id 6. The result keeps main's BackupCatalogIndex at 6 and adds
AttributeNameEscape at 7.

crates/storage-postgres/src/migrations/mod.rs: both sides added an arm
in migration() and an id in the transactional arm of runs_in(). The
result has both arms and both ids.

crates/storage-sqlite, storage-mongodb and storage-cassandra
src/migrations/mod.rs: both sides added an arm. The result has main's
BackupCatalogIndex arm followed by this branch's NotApplicable arm.

crates/storage-mongodb/tests: main replaced the literal catalog
version with MigrationId::LATEST.number(), which this branch had moved
to 6. The result keeps main's form. migrations_fresh.rs lists id 7 as
a not-applicable step and ledger row and names it in the reconverge
comment, the later-binary rows use id 8, and the step counts in
migrations_locks.rs and migrations_released.rs are 8.

CONTRIBUTING.md: both sides renumbered the table_note example to 7. The
result renumbers it to 8, the next free id after attribute_name_escape,
and lists both new ids in the runs_in() example.

docs/manuals/07-upgrade-manual.md: the result keeps main's sentence on
migration 6 and its section on the backup catalog index, this branch's
sentence that each migration is breaking on every backend it applies
to, and this branch's description of the escape as migration 7. The
sample outputs in the getting started guide, the quickstart, the admin
guide and the install guides, and the row in the differences table,
name migration 7.

crates/storage-postgres/tests/catalog_nul_names.rs and
migrations_name_escape.rs: main replaced BackupEngine with
LegacyBackupReader, whose create and restore take a JobPermit. The two
tests call create, describe, restore_source and restore with
JobPermit::detached().
test_a_migration_6_that_fails_in_its_backfill_stays_pending_and_resumes
expected migration 6 to be the only pending migration of a catalog at
migration 5. This branch adds migration 7, so such a catalog has 6 and
7 pending. The check now asks for migration 6 among the pending
migrations and for no pending migration below it.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] PostgreSQL: U+0000 in key schema attribute names returns 500; U+0000 in a table name fails in authorization instead of validation

1 participant