Skip to content

Store U+0000 in strings on PostgreSQL and MongoDB - #368

Open
LeeroyHannigan wants to merge 13 commits into
mainfrom
fix/nul-in-strings
Open

LeeroyHannigan wants to merge 13 commits into
mainfrom
fix/nul-in-strings

Conversation

@LeeroyHannigan

@LeeroyHannigan LeeroyHannigan commented Sep 18, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #364.

What changes

DynamoDB accepts the character U+0000 anywhere a string appears. Measured against the service on 2026-09-18 with a scratch table: PutItem accepts it inside a partition key, a sort key, a GSI key, a non-key value, a string in a list, a map key, and a top-level attribute name; GetItem returns each byte-identical; it sorts as the byte 0x00 ahead of every other character; it is a different key from the six-character text \u0000; range and prefix conditions treat it as an ordinary byte; the GSI holds the item; DeleteItem on the key removes it. The service also accepts it in key schema attribute names, and rejects it in table and index names by the usual pattern.

ExtendDB's PostgreSQL backend returned 500 InternalServerError for every one of the item cases (TEXT rejects the byte in key columns; jsonb rejects the \u0000 escape in item_data), and for a Query whose key value contains it. The MongoDB backend returned 500 when the character appeared in an attribute name or map key (BSON field names are C strings). SQLite and Cassandra already matched the service.

Both backends now store the character and return it unchanged.

Shared escape

crates/storage/src/util/control_chars.rs adds an order-preserving escape: U+0000 is stored as U+0001 U+0001, U+0001 as U+0001 U+0002, every other character unchanged. Strings without either character are stored exactly as before, so ordinary data does not change on disk. Unit tests enumerate every string up to length 4 over the alphabet {U+0000, U+0001, U+0002, a} and check that the escape round-trips, preserves UTF-8 byte order, preserves prefixes, and never collides, so COLLATE "C" comparisons, BETWEEN, begins_with, and the row-comparison page cursors from #361 and #362 give the same answers on escaped columns as on the raw text.

PostgreSQL

  • Item documents: item_to_json and json_to_item in data/mod.rs apply and reverse the escape on every string in the tree (names, map keys, values). Every item_data write in the crate goes through it (put, update, transactions, index rows, vector index rows), as do stream records, queued index updates and their contexts in gsi_pending, the vector search reader, and the snapshot reader that feat(storage): SnapshotEngine, a consistent table snapshot on every backend [backup/restore 4/4] #395 added (snapshot_engine.rs parsed item_data::text straight into an item, which would have returned the stored form of a string holding U+0000 or U+0001; it now decodes through json_to_item like every other reader of the column).
  • Key columns: data/key_text.rs wraps the shared pk_to_text, composite_pk_to_text, and parse_sk so string key components are escaped before binding; every key-producing site in the crate imports the wrappers, so writes, reads, index maintenance, cursors, and the propagation queue agree on the stored form. Composite partition keys escape each part before netstring joining, matching what the Query builder does with a key condition.
  • SQL that addresses an attribute by name inside item_data uses the escaped name: the TTL index and sweep, and vector search filters (name and value).
  • Catalog migration 6, escape_control_chars: rows written before this change hold their strings raw, and a raw U+0001 followed by U+0001 or U+0002 would read back differently under the new decoder (a lone U+0001 reads back unchanged). Migration 6 has two code bodies on PostgreSQL (crates/storage-postgres/src/migrations/m0006_escape_control_chars/). The catalog body rewrites backup_items; the data body rewrites every data, index and vector index table (text key columns and item_data), gsi_pending and stream_records. The tables are listed from the catalog rows, and a row whose table is DELETING or whose physical table is missing is skipped with a note. Only rows containing U+0001 change. Each body reads its tables once, so it takes time proportional to data size. Legacy rows never contain U+0000, since PostgreSQL refused them. Sort key columns hold one attribute each, so a SQL replace() rewrites them in place. A partition key column of a key schema with more than one HASH attribute holds the netstring join of the parts (<length>:<part>, each), and escaping a part changes its length, so those rows are rebuilt one at a time: the stored value is split into its parts, each part is escaped, and the parts are joined again by key_text::join_escaped_parts, the function composite_pk_to_text uses on the write path. The columns come from the catalog key schemas: pk of a table or index with several HASH attributes, base_pk of the index and vector tables of such a table (limits.allow_multipart_table_keys), and pk of backup_items for a backup of such a table. PostgreSQL checks a primary key as each row changes, and two legacy keys that differ only by a U+0002 after a U+0001 (a\x01 and a\x01\x02 with the same other key columns) escape to two different texts, the first of which is the second's legacy text, so a rewrite that meets the shorter key first fails with a unique violation. Each column's replace() therefore runs under a savepoint; on a unique violation it is rolled back and the column is rewritten row by row in descending byte length of the legacy text, and the composite rebuild uses the same order. Every rewrite makes the text longer and the escape is injective, so no rewrite produces a key another row holds at that moment (order.rs gives the argument). Both read their rows through one cursor, 500 at a time. A duplicate that remains fails the body with PostgreSQL's DETAIL, which names the key, and with the DynamoDB table or index of the physical table. gsi_pending.worker_partition is a hash of the stored base key text and changes with it, so the data body recomputes it for the queued rows whose item holds U+0001, through gsi_queue::worker_partition, the function the enqueue uses; every row of an item moves to the same partition and keeps its id and ready_at, so a row queued before the upgrade and the next write of the same item are applied by the same worker. The candidate predicate spells its backslash as chr(92) so it reads the same under either value of standard_conforming_strings.
  • How migration 6 sits in the runner. The escape is not idempotent: an escaped U+0001 is stored as U+0001 U+0002, which is also a legal legacy sequence, so the migration cannot tell a row this build wrote from a legacy row. Each body runs in the runner's transaction and commits with its store's ledger row, so a run that stops leaves that store as it was, and a recorded store is never rewritten again; the per-table progress table of the earlier revision is gone. The migration is Kind::Breaking and not convergent: a binary that does not carry migration 6 refuses the migrated catalog, a 0.1.x binary refuses it as it refuses every migrated catalog, and extenddb migrate --yes --reconverge skips it. A server of this release refuses a store where migration 6 is pending, which replaces the earlier revision's startup check on the data database, and extenddb init applies it to a new store, which has nothing to rewrite. Because a server refuses a store where migration 6 is pending, the migration only ever sees rows written by earlier releases. CONTRIBUTING.md asks for an idempotent run; migration 6 relies instead on the runner's transaction and on not being convergent, and extenddb migrate --forget 6 followed by a run would escape the strings this release wrote a second time. CONTRIBUTING.md also asks for the functions that define a migration to live in its module; migration 6's bodies call the write path's escape, key text and partition functions instead, so that a migrated row is the row this release writes, and the module's checksum does not cover those functions.
  • Other sessions. migrate --yes waits for the servers that hold the runner's serving key, which a 0.1.x server does not take. Each body's detect, which the runner calls for every pending code body before a run's first write, therefore stops the run with exit 5 while another session is connected to the database that body rewrites: the data database whenever the catalog lists a table or gsi_pending or stream_records holds a row, and the catalog database whenever backup_items holds a row. The item names each session by role, client address and application name. The runner's own sessions (extenddb migrate run <id>), the migration lock's session (extenddb migrate) and status sessions (extenddb status) are left out by application_name; the runner sets those names over any application_name in the connection string, so the migrate command's own naming helper of the earlier revision goes away. The migrator connects as the application role, which is not a superuser, and PostgreSQL hides backend_type on other roles' sessions from it; the filter counts any session with a role whose type is client backend or hidden, so an operator's psql or a server under another role is counted while autovacuum workers are not. A status read does not look for sessions, so the preview still runs beside a server and reports migration 6 as pending, and EXTENDDB_MIGRATE_IGNORE_CONNECTIONS=1 skips the check for idle pooler connections. The test harnesses whose own sessions hold their scratch databases (apply_all_migrations, and Scratch::target in tests/migrations_support) build their target with PostgresTarget::with_other_sessions_allowed, compiled only with test-util or test-hooks; every path an operator runs keeps the check.
  • SQLite, Cassandra and MongoDB record migration 6 as not applicable, each with its reason: SQLite and Cassandra store U+0000 as written, and MongoDB's field names are covered below.

MongoDB

BSON string values are length-prefixed and already held the character; field names could not. item_to_document and its inverse escape object keys only. Conditions and native updates whose attribute names contain U+0000 or U+0001 fall back to the existing in-process paths, the same way names containing . or starting with $ already do. The native $setOnInsert upsert from #473 writes the key attributes as item_data.<name> paths, so a table whose key attribute name contains either character takes the session path too, through the same is_plain_path_name check that already routed names containing . or $. Field paths with no fallback (the TTL index, sweep, and field reads) use the escaped name.

The MongoDB ledger records migration 6 as not applicable, naming the field-name limitation below.

Documentation

docs/design/04-component-storage.md describes the escape beside key and item storage; docs/manuals/07-upgrade-manual.md documents migration 6 under Behavior Changes (the upgrade order, the sessions check, the rollback and the MongoDB field-name limitation) and counts it among the migrations this release carries. The install and quickstart guides show a new deployment at migration 6.

Merge with main

Merged main at ad8945976 (merge commit 2048b5dd4), 122 commits ahead of the earlier base f49c5f823. Main replaced the per-backend upgrade mechanisms with the shared migration runner and its ledger (#666, #681), so the merge takes main's side for the framework: crates/storage-postgres/src/migrations.rs, its CATALOG_MIGRATIONS and DATA_CODE_MIGRATIONS, CATALOG_VERSION and the catalog version check, tests/catalog_converge.rs, migrations/004_control_chars_encoding.sql and the migrate command's exclusive-access step are gone. The next commit (a620d8567) ports the re-encode onto the runner as migration 6, as CONTRIBUTING.md describes: the registry entry, the arm on each backend, the code bodies, the golden test and its fixture file, and the upgrade manual entry. 636d6242a updates the SQLite and MongoDB migration tests that counted five migrations, d83f4ba06 renumbers CONTRIBUTING.md's worked example, table_note, to migration 7, and c3a229f69 adds the CLI test of migration 6 that replaces the one the merge dropped. Migration 6 creates no schema object, so docs/schema/ does not change. What carried over from 0bd7324a9 with its logic unchanged, together with its tests: the escape, every PostgreSQL and MongoDB read and write path, the snapshot reader, the restore and key column fixes, the composite key rebuild, the worker_partition recompute and the longest-first rewrite. The unit test of reencoded_json is gone; reencodes_legacy_rows covers its cases on a live database. The sessions check still covers both databases and sessions of other roles, but it now checks a database only while the body that rewrites it has rows to rewrite. The merge took main's side of tests/test_cli_vector_catalog_migration.py, which dropped the earlier revision's CLI test of the re-encode, test_migrate_applies_the_pending_string_encoding_data_migration; test_migrate_applies_migration_6_beside_its_own_sessions replaces it.

Merging

  • feat: store and report WarmThroughput on tables and global secondary indexes #474 also adds a migration 6 (WarmThroughput). Whichever of the two merges second renumbers its migration to 7: the registry entry, the module and golden file names, the fingerprint, the upgrade manual, install guide, quickstart and getting-started lines that name the number and, for this PR, the sessions check's message in guard.rs, the doc comments and the design document that name catalog migration 6, and the tests whose names carry the number (migration_6_* in migrations_golden.rs and test_migrate_applies_migration_6_beside_its_own_sessions). CONTRIBUTING.md's worked example then becomes migration 8.

Tests

  • tests/test_nul_strings.py: 14 wire tests encoding the measurements above (round trip of an item with the character in every position, partition byte order with U+0000 first, U+0000 distinct from its escape text, distinct partition keys, range and prefix conditions, Limit 1 cursor walk, GSI query on a U+0000 key, filter and condition expressions on a U+0000 attribute name, UpdateItem SET and REMOVE with U+0000 names and values, BatchWriteItem, TransactWriteItems, BatchGetItem, TransactGetItems, Scan count, DeleteItem), plus a stream test (INSERT, MODIFY, REMOVE records for such an item, with Keys, NewImage, and OldImage byte-identical) that runs against ExtendDB servers only, as the existing stream tests do. The 14 pass against DynamoDB itself (measured on the earlier revision; the file did not change). On servers built from this head: the 15 pass on a PostgreSQL server and on a MongoDB server. CI also runs the file on SQLite and Cassandra; the cassandra-pytest job passed at 0bd7324a9. Before the change: 14 errors on PostgreSQL and MongoDB, 14 passing on SQLite; the stream test fails on both changed backends.
  • Unit tests: 11 on the shared escape (exhaustive order, prefix, collision, round trip, lenient legacy decode, JSON tree variants); 5 on the PostgreSQL key text wrappers, including the netstring split and re-escape the migration uses; 1 on the snapshot row decoder; 6 in the MongoDB crate (round trip with the character in a top-level name, a nested map key, and a map key inside a list, with no BSON field name containing a NUL; control-free items store raw names; pushdown, native update and the native upsert refuse such names).
  • Migration 6 in crates/storage-postgres/src/migrations/m0006_*tests.rs, live on scratch databases apart from one unit test of the composite key check; the rewrite tests run the bodies' rewrites in one transaction: legacy rows seeded in every affected table are rewritten, with the notes the ledger records, while a literal-backslash \u0001 text row and plain rows stay byte-identical; jsonb rows are re-encoded with standard_conforming_strings off; rebuilds_composite_partition_keys_part_by_part and recomputes_the_worker_partition_of_rows_queued_before_the_upgrade cover the two review items (with replace() the two-HASH table's pk is left as 2:a\x01\x02,1:b,; without the recompute the queued rows stay at [0, 0, 3] where the build computes [2, 2, 3]); eight tests in m0006_order_tests.rs cover the longest-first rewrite, the batches, the byte-order reversal, a duplicate that remains and the error naming the DynamoDB table; four in m0006_guard_tests.rs cover the sessions check: an unnamed session on the data database, a session on a separate catalog database, another role's session seen from a non-superuser pool, and a store with nothing to rewrite.
  • Migration 6 through the runner, in tests/migrations_golden.rs over two databases: migration_6_escapes_the_strings_an_earlier_release_stored loads the 0.1.14 fixture, migrates it through migration 5, gives every row of seed_tbl's table, GSI and LSI and of the backup's backup_items a raw U+0001 in its key columns and an attribute holding U+0001 in its name and U+0001 U+0002 in its value, migrates through 6 and compares the rows and the ledger notes of both stores with tests/fixtures/golden/0006_escape_control_chars.txt. migration_6_stops_before_any_write_while_another_session_holds_the_data opens an unnamed session on the data database: the status still reads Behind, migrate stops with exit 5 naming the session and leaves both ledgers without a migration 6 row, and once the session is gone the run completes, with connection strings that set application_name=psql on both databases. With the data body's detect returning no item, the second test fails because the run applies migration 6; with the data body's rewrite removed, the golden test fails on the raw rows.
  • Migration 6 through the CLI, in tests/test_cli_vector_catalog_migration.py: test_migrate_applies_migration_6_beside_its_own_sessions makes a deployment with this binary's init, creates a table, an item and a backup through the server, so that the sessions check runs for both bodies, then takes migration 6 out of both ledgers and sets the catalog version back to 5. The preview exits 3 and lists both bodies; with another session on the catalog database migrate --yes exits 5, names that database and records migration 6 in neither ledger; once the session closes the run exits 0 and applies migration 6 in both stores. With the migrate command's lock session counted as another session, the last run exits 5 and the test fails.
  • Gates on this head: fmt; clippy with -D warnings over all targets on extenddb-storage, extenddb-storage-postgres (with test-util and test-hooks), extenddb-storage-sqlite, extenddb-storage-cassandra and extenddb-storage-mongodb, the crates in the diff; lib tests on Rust 1.88.0 of extenddb-storage (208), extenddb-storage-cassandra (115), extenddb-storage-sqlite (234, 1 ignored) and extenddb-storage-postgres (98, live); the 23 integration test files of extenddb-storage-postgres, live (262 passed, 2 ignored); the lib tests of extenddb-storage-mongodb against a one-node replica set with --include-ignored (217), and its five migration test files the same way (39); four of the PostgreSQL CLI test files (test_cli_lifecycle.py, test_cli_container_readiness.py, test_cli_migrate_concurrency.py and test_cli_vector_catalog_migration.py) with a PostgreSQL binary built from this head's code (34 passed, 1 skipped).
  • The end-to-end upgrade run of the earlier revision (a deployment initialised by a f49c5f823 binary, upgraded with migrate --yes, then served) covers 0bd7324a9 and was not repeated for this head; the golden test covers the migration of a 0.1.14 deployment through the runner.

Related behaviour

  • U+0000 in key schema attribute names on PostgreSQL: the service accepts it; PostgreSQL returns 500 because the catalog stores the key schema as jsonb. Table and index names cannot contain it on the service either, but PostgreSQL answers a U+0000 table name with 500 Internal error during authorization where the service and the other backends answer the pattern ValidationException. Both are catalog and authorization paths, separate from item storage: [Bug] PostgreSQL: U+0000 in key schema attribute names returns 500; U+0000 in a table name fails in authorization instead of validation #367.
  • Legacy MongoDB field names containing a raw U+0001 followed by U+0001 or U+0002 are not rewritten; no migration exists for that backend. A field name containing U+0001 at all is the precondition, and a lone U+0001 reads back unchanged; values and key strings are unaffected. A startup acknowledgement with nothing to rewrite would add a step to every MongoDB upgrade for that exposure, so the tradeoff is stated at the decoder (item_data_to_json) and in the upgrade manual instead.

Deploy note

PostgreSQL deployments upgrade in this order: stop every server that uses the deployment, including servers of earlier releases, run extenddb migrate --yes with the new binary, start the new servers. While the deployment holds a table or a backup, migrate --yes stops with exit 5 before writing anything while another session holds the data database or the catalog database, and names the sessions. The new server refuses to start while migration 6 is pending; a server of an earlier release refuses the migrated catalog. Downgrading means restoring both databases from pre-migration backups. Fresh deployments are unaffected.

@robinnsc robinnsc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One blocking item (the connection guard's coverage, inline on migrations.rs), one strong question on the downgrade hazard, one non-blocking question on the MongoDB asymmetry. The escape design itself checks out: I traced the order/prefix/collision argument, the migration's non-idempotence handling (per-table transaction plus progress marker, with the startup gate guaranteeing only legacy rows are ever scanned), and the backup path — create-backup copies stored rows verbatim from the data tables, so escaped form flows through backup_items transitively and there is no unescaped write path I could find.

Comment thread crates/storage-mongodb/src/data/mod.rs Resolved
jcshepherd
jcshepherd previously approved these changes Sep 21, 2026
@jcshepherd
jcshepherd added this pull request to stack #369 September 21, 2026 21:32
Base automatically changed from fix/hash-only-gsi-reverse-cursor to main September 21, 2026 22:12
@LeeroyHannigan

Copy link
Copy Markdown
Collaborator Author

Rebased on main at abf8082; conflicts in crates/storage-postgres/src/gsi_queue.rs, resolved by keeping #401's queue code and decoding its rows through json_to_item and stored_json_to.

#401's queue apply builds the base key text in key_from_item (crates/storage-postgres/src/data/gsi_build.rs) with the unescaped shared helpers. With this branch's escaping, a GSI write on a key holding U+0000 then failed in the worker with SQLSTATE 22021 and test_gsi_holds_the_item_with_a_nul_key failed. key_from_item now uses the key_text wrappers, and tests/test_nul_strings.py passes on PostgreSQL.

The merge of main that was on this branch is folded into the first commit with its resolutions unchanged (migration 004, catalog 0.0.5).

@navareamol navareamol left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, Lee. The escape design is really nice: order, prefixes and the jsonb and BSON paths all hold up as far as I could trace them, and moving every key-text site in storage-postgres onto key_text makes the change easy to follow.

I found two things, both on the upgrade path for rows written before this change:

  1. 004 breaks the netstring length prefix on GSI pk when the index has more than one HASH attribute (details inline on migrations.rs).
  2. Rows still queued in gsi_pending keep their old worker_partition after the hash changes (details inline on gsi_queue.rs).

A test for each that seeds the legacy form and fails without the fix would be great.

I also checked the description against the code at 9ae7a01, and a few parts have drifted since the rebase:

  1. The catalog numbers are from before #384 merged. The description says catalog 0.0.4 and migrations/003_control_chars_encoding.sql, and that #384 and #335 also claim 0.0.4. The code has 004_control_chars_encoding.sql and CATALOG_VERSION 0.0.5, and #384 is on main. The "Upgrade path" and "Deploy note" sections still say 0.0.4 and quote expected 0.0.3, found 0.0.4. The upgrade manual already says 0.0.5, so only the description is behind.
  2. The end-to-end upgrade run in "Gates" was done before the renumbering. Your 24 Oct reply says the old binary was not run again against a migrated deployment after the merge. Could you rerun it, or note which revision it covers?
  3. "Composite partition keys escape each part before netstring joining" holds for writes and queries, but 004 does not do it for legacy rows (point 1). Likewise, "the propagation queue agree[s] on the stored form" holds for the queued documents but not for worker_partition (point 2).
  4. "All three backends" reads as if there are only three. Cassandra runs tests/test_nul_strings.py in CI and passes, so it would be worth naming it.

With 1 and 2 fixed and the description brought up to date, I will approve.

DynamoDB accepts the character U+0000 anywhere a string appears: keys, index
keys, values, strings in lists, map keys, attribute names (measured against the
service 2026-09-18; it sorts as the byte 0x00 and is a different key from the
six-character text "\u0000"). PostgreSQL TEXT rejects the byte and jsonb
rejects the escape, so the backend answered 500 for every such item and for a
Query whose key value carried it. BSON field names are C strings, so MongoDB
answered 500 when the character appeared in an attribute name or map key.

Shared escape (crates/storage/src/util/control_chars.rs): U+0000 is stored as
U+0001 U+0001, U+0001 as U+0001 U+0002, everything else unchanged. Exhaustive
tests over every string up to length 4 on {U+0000, U+0001, U+0002, a} pin round
trip, UTF-8 byte order, prefix preservation, and distinctness, so COLLATE "C"
comparisons, BETWEEN, begins_with, and the row-comparison page cursors return
the rows the raw text would.

PostgreSQL: item documents go through item_to_json/json_to_item (every
item_data write, stream records, gsi_pending rows and contexts, vector rows,
and the rows the snapshot engine's cursor stream reads); key column text goes
through data/key_text.rs wrappers at every producing site; the TTL index and
sweep and the vector search filters address attributes by the escaped name.
Migration 004_escape_control_chars re-encodes rows written by earlier releases
that contain U+0001, one table per transaction with a per-table progress
marker, only rows containing U+0001 touched, predicate independent of
standard_conforming_strings. The server refuses to start on a data database
that does not record the migration (rows this build writes are
indistinguishable from legacy rows to the migration) and extenddb init records
it on fresh deployments.

Catalog 0.0.5 (migrations/004_control_chars_encoding.sql) changes no schema; it
moves the version so that a binary from an earlier release, which reads escaped
rows as literal data and reports nothing, refuses a migrated deployment through
the version check it already performs. The two gates close both directions.

extenddb migrate refuses to run 004 while any other session holds the data
database or, when the catalog is a separate database, the catalog database
(backup_items lives there). The check runs before the first write, so a refusal
leaves the catalog version where it was, and again inside the migration. The
command connects as the application role, for which pg_stat_activity hides
backend_type on other roles' sessions; the filter counts any session with a role
whose type is client backend or hidden, so an operator's psql or a server under
another role is counted and autovacuum workers are not. The test-hooks entry
point apply_all_migrations, which the live test harness runs on scratch
databases it created and holds alone, runs 004 without that check; every path
an operator runs keeps it.

MongoDB: item_to_document and its inverse escape object keys only (BSON string
values already hold the byte); conditions and native updates on names with
these characters use the existing in-process fallback paths, as names with "."
or "$" already do; the native $setOnInsert upsert writes key attributes as
item_data.<name> paths, so a table whose key attribute name holds either
character takes the session path through the same is_plain_path_name check;
field paths with no fallback use the escaped name. Legacy field names are read
through the decoder without a migration; the decoder's documentation states the
exposure (a name containing U+0001 followed by U+0001 or U+0002) and points at
the upgrade manual.

Tests: tests/test_nul_strings.py (14 wire cases passing against DynamoDB and
on PostgreSQL, SQLite, MongoDB and Cassandra, plus a stream record case on
ExtendDB servers); unit tests on the escape, the key wrappers, the MongoDB
document conversion and predicates, the migration count and version; live
migration tests for re-encoding, the second-run skip, the
standard_conforming_strings setting, the startup gate, the connection guard on
the data database, on a separate catalog database, and from a non-superuser
pool against another role's session; a live snapshot test that streams an item
with U+0000 in an attribute name and U+0001 in a value as it was written;
tests/test_cli_vector_catalog_migration.py covers 0.0.2 to 0.0.5 and the gate
in both directions. Docs: storage design, upgrade manual, version literals.

Closes #364
…m the decoded item

Main's restore (#384) reads each backup_items document as text, parses it
into an Item and writes the text into the target's item_data unchanged. On
this branch that text carries the control-character escape, and the key
columns and index rows are built by helpers that escape their input, so
the parsed item was escaped a second time: a restored table whose items
hold U+0000 or U+0001 got key columns and index rows in a form the read
path does not produce. The item is now decoded with json_to_item before
the key helpers see it, and the key helpers are the escaping ones from
key_text, so every column is escaped exactly once; the item_data text is
still copied as it is. The backup copy sizes each item from the decoded
form for the same reason.

The parallel Scan cross-segment check (#203) hashes the candidate key's
text and compares it with the hash of the stored pk column; it now uses
the escaping pk_to_text so both sides hash the same text for keys with
control characters.

A storage-level test restores a table whose keys, index keys, attribute
names and nested values hold U+0000 and U+0001 and compares the stored
rows of the base table and both index tables with the source's, then
reads each item back.
… check ignores them

On PostgreSQL, `migrate --yes` with 004_escape_control_chars pending refused
every upgrade from 0.1.14: "3 other connection(s) hold the catalog database:
extenddb from ... (no application name)". The migrate command opens catalog
management and settings stores for the resource-tag step through the
configured connection string, and those sessions carry no application name,
so the check for foreign sessions that the data code migration runs before
rewriting rows counted the command's own connections. The bootstrapper's
pools carry the name and were ignored.

The connection string handed to the catalog stores now carries
application_name=extenddb-migrate when it is a PostgreSQL URL. Other
backends' connection strings and a URL that already names an application
pass through unchanged.

The CLI test file gains the upgrade path a fresh init never reaches: the
data ledger row for 004_escape_control_chars removed and the catalog rolled
back, then migrate without --yes lists it, migrate --yes applies it and the
deployment serves. The existing pre-vector test did not cover this because
init records the data migration, so the exclusive-access check never ran.
…module

Pure move of with_migrate_application_name, its constant and its four tests
from cmd_migrate.rs into migrate_connection.rs. No behaviour change.
cmd_migrate.rs goes back from 856 to 793 lines.
…on string

The exclusive-access check for 004_escape_control_chars skips only sessions
named extenddb-migrate. A connection string that already set
application_name kept the operator's name, so migrate --yes counted its own
catalog sessions as foreign and refused the upgrade. The helper now drops any
application_name parameter and appends extenddb-migrate. The unit test for an
existing name fails on the previous helper and passes now.
… scratch setup

serve_refuses_an_unreadable_catalog_version_with_the_shared_refusal wrote
'garbage' into catalog_version from the scratch setup's third migration
step. The setup also applies 004_control_chars_encoding.sql after that
step, and 004 sets catalog_version to 0.0.5, so the test read "0.0.5" and
failed its first assertion before it reached serve.

The test now passes the shipped 003 as the setup's third step and writes
the unreadable version once the setup has returned. The test failed on the
previous revision (left "0.0.5", right "garbage") and passes now in 5 runs
of 5, with the other four tests in catalog_converge.rs.
… migration 004

Migration 004 escaped every text key column with one SQL replace(). A
partition key column of a key schema with more than one HASH attribute
holds the netstring join of the parts, and the replace grew a part
without fixing its length prefix: the legacy index key 2:a\x01,1:b,
became 2:a\x01\x02,1:b, where this build writes and looks up
3:a\x01\x02,1:b,. A Query on such a GSI missed the legacy row, and the
next write of the item left it behind as a duplicate.

Those rows are now rebuilt one at a time: the legacy value is split into
its netstring parts, each part is escaped, and the parts are joined again
by key_text::join_escaped_parts, the function composite_pk_to_text uses on
the write path. The columns come from the catalog key schemas: pk of a
table or index with several HASH attributes, base_pk of the index and
vector tables of a table with several (limits.allow_multipart_table_keys),
and pk of backup_items for a backup of such a table. Sort key columns hold
one attribute each (sk_s, sk2_s, ...) and keep the replace().

gsi_pending.worker_partition is a hash of the stored base key text, which
the escape changes for a key holding U+0001, while 004 rewrote only the
jsonb columns of the queue. A row queued at upgrade time and the next
write of the same item then went to different workers, which breaks the
one item, one worker rule of the queue. 004 now recomputes the column for
the queued rows whose item holds U+0001, through gsi_queue::worker_partition,
the function enqueue_gsi_pending uses. Every row of an item moves to the
same partition and keeps its id and ready_at.

Live tests: rebuilds_composite_partition_keys_part_by_part seeds a table
with two HASH attributes, a two-HASH GSI, a one-HASH GSI on the two-HASH
table and a backup of each, with a raw U+0001 in a HASH part, and checks
each rebuilt column against composite_pk_to_text.
recomputes_the_worker_partition_of_rows_queued_before_the_upgrade seeds
queued rows whose worker_partition was computed from the raw key, checks
that they move to the partition this build computes, and that the next
enqueue of the item lands there. Each fails with the previous migration.
… 004

Two legacy keys that differ only by a U+0002 after a U+0001, such as
a\x01 and a\x01\x02 with the same sort key, escape to two different
texts, but the first one's new text, a\x01\x02, is the second one's
legacy text. PostgreSQL checks the primary key as each row changes, so
the one statement 004 issued per key column failed with a unique
violation when it met the shorter key first, which depends on where the
two rows are stored. The migration stopped with the catalog at 0.0.5 and
004 not recorded, and a retry failed the same way. The composite key
rebuild had the same failure on 2:a\x01,1:b, beside 3:a\x01\x02,1:b,.

The statement now runs under a savepoint. On a unique violation it is
rolled back and the column is rewritten row by row, longest legacy text
first, and the composite key rebuild uses the same order. Every rewrite
makes the text longer and the escape is injective, so when a row's new
text is another row's legacy text, that other row is longer and was
rewritten first; migration_004_order.rs states the argument in full.
Both paths read the rows through one cursor in that order, 500 rows at a
time, where the rebuild loaded every matching key of the table at once.
A duplicate that remains is reported with PostgreSQL's DETAIL, which
names the key, and with the DynamoDB table or index of the physical
table.

Live tests store each pair with the shorter key first in the heap: a
partition key pair with one sort key, a sort key pair, a two-HASH
composite pair, a base_pk pair on an index table, and one table on each
path with more rows than one batch and its pair in two batches. Each
checks that 004 completes and that every final text is the escape of
its legacy text, and each fails with the previous migration. A pair
whose byte order is the reverse of its length order, a\x01b beside
a\x01\x02b, fails when the rows are ordered by their text instead of
their length. a_duplicate_that_remains_is_reported_with_its_key checks
the error text, and an_error_names_the_dynamodb_table checks that an
error names the DynamoDB table.
@LeeroyHannigan

Copy link
Copy Markdown
Collaborator Author

Rebased onto main at f49c5f8 and pushed 0bd7324. What changed at this head:

  • @navareamol both items are in this push, with a live test each: composite partition key columns are rebuilt part by part instead of through replace(), and gsi_pending.worker_partition is recomputed for the rows the migration touches (details in the two threads). The body now says catalog 0.0.5 and 004_control_chars_encoding.sql throughout, names Cassandra with the other three backends, and its Tests section carries the gates and the end-to-end upgrade run, redone for this push.
  • The last commit fixes 004 for two legacy keys that differ only by a U+0002 after a U+0001, like a\x01 and a\x01\x02 with the same sort key. The migration failed on the primary key when the shorter one came first in the heap, since its new text is the other one's old text. 004 now rewrites them longest first, and the composite rebuild uses the same order and reads its rows in batches.
  • The rebase picked up three things from main. fix(mongodb): store the key when UpdateItem creates an item #473's native MongoDB upsert writes item_data.<key name> paths, so its name check now also refuses names containing U+0000 or U+0001 and such a table takes the session path. feat(storage): SnapshotEngine, a consistent table snapshot on every backend [backup/restore 4/4] #395's snapshot reader parsed item_data::text straight into an item and now decodes through json_to_item like every other reader of the column. feat(storage): SnapshotEngine, a consistent table snapshot on every backend [backup/restore 4/4] #395's apply_all_migrations test entry point runs 004 without the exclusive-access check, because the harness's own pools were counted as another session and all 81 table_snapshot tests failed on it.
  • @robinnsc the three threads hold at this head: the exclusive-access check on both databases is unchanged, the downgrade gate compares 0.0.4 with 0.0.5 (a main binary refuses the migrated catalog with Catalog version mismatch: expected 0.0.4, found 0.0.5, measured in the upgrade run), and the MongoDB field-name tradeoff is as stated at item_data_to_json.

Could you both take another look?

@navareamol navareamol added the v1 Needed for the 1.0 release label Nov 3, 2026
@navareamol navareamol added this to the 1.0 milestone Nov 3, 2026
Main replaced the per-backend upgrade mechanisms with the shared migration
runner (#666, #681). The merge drops this branch's data migration
004_escape_control_chars, catalog 0.0.5, the server's startup check for 004
and the migrate command's exclusive-access step; this commit gives the
re-encode its place in the runner as migration 6, escape_control_chars.

- The registry carries migration 6. PostgreSQL applies it with two code
  bodies: the catalog body rewrites backup_items, and the data body rewrites
  the data, index and vector index tables, gsi_pending (recomputing
  worker_partition) and stream_records. The rewrites are those of 004: the
  part by part rebuild of composite partition keys, the longest first rewrite
  after a unique violation, and the jsonb re-encode. SQLite, Cassandra and
  MongoDB record the migration as not applicable.
- Each body runs in the runner's transaction and commits with its ledger
  row, so the per-table progress table is gone: a run that stops leaves the
  store as it was, and a recorded store is not rewritten again. The
  migration is breaking and not convergent, so an earlier binary refuses the
  migrated catalog and --reconverge skips it.
- A server refuses a store where migration 6 is pending, which replaces the
  startup check for 004, and init applies it to a new store, which has
  nothing to rewrite.
- The check for other sessions moves to each body's detect, which the
  runner calls before a run's first write. It leaves out the runner's
  sessions, the migration lock's session and status sessions by their
  application_name, which the runner sets over any name in the connection
  string, so the migrate command's own naming helper goes away. It runs in a
  migrating run only, not in a status read, so the preview still runs
  beside a server, and only while the store holds something to rewrite.
  EXTENDDB_MIGRATE_IGNORE_CONNECTIONS still skips it. A test harness whose
  own sessions hold its scratch databases builds its target with
  with_other_sessions_allowed.

The live tests of 004 now run the bodies' rewrites. A golden test runs
migration 6 on the 0.1.14 fixture after giving its rows legacy strings, and
a runner test checks that a session on the data database stops the run with
exit 5 before any write, and that the run completes once the session is
gone, with connection strings that name an application.
…tests

The SQLite tests counted every migration as applied on SQLite and every
convergent migration as run again by --reconverge; migration 6 is the
first that SQLite records as not applicable, so both counts now take the
migrations that apply on SQLite. The MongoDB tests named migration 5 as
the latest and used id 6 for a later binary's ledger row; they now name 6
as the latest, list it among the steps of a fresh run, and use id 7 for
the later binary.
Migration 6 is now escape_control_chars, so the worked example in
CONTRIBUTING.md's "Adding a catalog migration", table_note, becomes
migration 7, with its file names, fingerprint, test name and the pending
listing on a catalog at migration 6.
The merge took main's side of tests/test_cli_vector_catalog_migration.py,
which dropped the earlier revision's CLI test of the string re-encode, the
only test that ran `extenddb migrate --yes` with the re-encode pending.

The new test makes a deployment with this binary's init, creates a table,
an item and a backup through the server, then takes migration 6 out of
both ledgers and sets the catalog version back to 5. The preview lists
both bodies and exits 3. With another session on the catalog database,
`migrate --yes` exits 5, names that database and records migration 6 in
neither ledger. Once the session closes, the run exits 0 and applies
migration 6 in both stores. With the migrate command's lock session
counted as another session, the last run exits 5 and the test fails.
@LeeroyHannigan

Copy link
Copy Markdown
Collaborator Author

@navareamol I merged main into the branch and moved the re-encode onto the shared migration runner from #666 and #681, so this is ready for another look.

What the port changed:

  • The merge (2048b5dd4) takes main's side for the framework. The branch's data migration 004_escape_control_chars, catalog 0.0.5 (004_control_chars_encoding.sql), the server's startup check for 004, the migrate command's exclusive-access step and its application_name helper are gone.
  • a620d8567 adds catalog migration 6, escape_control_chars, as CONTRIBUTING.md describes. On PostgreSQL it has two code bodies: the catalog body rewrites backup_items, and the data body rewrites the data, index and vector index tables, gsi_pending and stream_records. SQLite, Cassandra and MongoDB record it as not applicable. It is breaking and not convergent.
  • Each body runs in the runner's transaction and commits with its ledger row, so the per-table progress table is gone. A run that stops leaves that store as it was, and a recorded store is not rewritten again.
  • A server refuses a store where migration 6 is pending, which replaces the startup check of 004, and init applies it to a new store.
  • The check for other sessions is now each body's detect, which the runner calls before a run's first write; a refusal exits 5 and names the sessions. It leaves out the runner's sessions, the migration lock's session and status sessions by the application_name the runner sets, and the runner sets those names over any name in the connection string. It does not run in a status read, so the preview still runs beside a server, and it runs only while the store holds something to rewrite. EXTENDDB_MIGRATE_IGNORE_CONNECTIONS=1 still skips it. Test harnesses whose own sessions hold their scratch databases build their target with PostgresTarget::with_other_sessions_allowed (compiled only with test-util or test-hooks); tests/migrations_support does that in Scratch::target, which the existing runner tests use while they hold lock holders and observers open, and Scratch::target_checking_sessions keeps the check.
  • 636d6242a updates the SQLite and MongoDB migration tests that assumed five migrations, or that every migration applies on SQLite.
  • d83f4ba06 renumbers CONTRIBUTING.md's worked migration example, table_note, to 7.
  • c3a229f69 adds test_migrate_applies_migration_6_beside_its_own_sessions to tests/test_cli_vector_catalog_migration.py. The merge took main's side of that file, which dropped the earlier revision's CLI test of the re-encode, the only test that ran extenddb migrate --yes with it pending. The new test seeds a table and a backup through the server, so that the sessions check runs for both bodies, and checks that the command's own sessions are not counted and that another session on the catalog database stops the run with exit 5.
  • The description again says that SQLite and Cassandra already matched the service and that CI runs tests/test_nul_strings.py on them (your sixth point).

What stayed as you reviewed it at 0bd7324a9: the escape, every PostgreSQL and MongoDB read and write path, the composite partition key rebuild (your first point), the worker_partition recompute (your second point), the longest-first rewrite, and the sessions check's coverage of both databases and of other roles, which now checks a database only while the body that rewrites it has rows to rewrite. Their tests moved into crates/storage-postgres/src/migrations/m0006_*tests.rs and pass against the bodies: rebuilds_composite_partition_keys_part_by_part and recomputes_the_worker_partition_of_rows_queued_before_the_upgrade among them. The unit test of reencoded_json is gone; reencodes_legacy_rows covers its cases on a live database.

New tests: migration_6_escapes_the_strings_an_earlier_release_stored runs migration 6 through the runner on the 0.1.14 fixture after giving its rows legacy strings, and compares the rows and both ledgers' notes with tests/fixtures/golden/0006_escape_control_chars.txt. migration_6_stops_before_any_write_while_another_session_holds_the_data checks that a session on the data database leaves the status at Behind, stops migrate with exit 5 and no migration 6 ledger row in either store, and that the run completes once it is gone, with connection strings that set application_name=psql. With the data body's detect returning nothing, the second test fails because the run applies; with the data body's rewrite removed, the golden test fails on the raw rows. test_migrate_applies_migration_6_beside_its_own_sessions fails when the migrate command's lock session is counted as another session.

Tests run on this head: fmt; clippy with -D warnings over all targets on extenddb-storage, extenddb-storage-postgres (with test-util and test-hooks), extenddb-storage-sqlite, extenddb-storage-cassandra and extenddb-storage-mongodb, the crates in the diff; lib tests on Rust 1.88.0 of extenddb-storage (208), extenddb-storage-cassandra (115), extenddb-storage-sqlite (234, 1 ignored) and extenddb-storage-postgres (98, live); the 23 integration test files of extenddb-storage-postgres, live (262 passed, 2 ignored); the lib tests of extenddb-storage-mongodb against a one-node replica set with --include-ignored (217), and its five migration test files the same way (39); four of the PostgreSQL CLI test files, test_cli_vector_catalog_migration.py among them, with a PostgreSQL binary built from this head's code (34 passed, 1 skipped). Wire: tests/test_nul_strings.py on a PostgreSQL server and on a MongoDB server built from this head, 15 passed on each.

The end-to-end upgrade run in the description covers 0bd7324a9; I did not repeat it for the port.

#474 also adds a migration 6. Whichever of the two merges second renumbers its migration.

@LeeroyHannigan

Copy link
Copy Markdown
Collaborator Author

@robinnsc the port onto the shared migration runner (c3a229f) replaced the mechanism the 13 Oct and 24 Oct replies describe, so here is where each of your three points stands now.

  • Connection guard on both databases: migration 6 has a catalog body, which rewrites backup_items, and a data body, which rewrites the data, index and vector index tables, gsi_pending and stream_records. Each body's detect checks for other sessions on the database it rewrites, so a session that holds only the catalog database stops the run with exit 5 while backup_items has a row. a_session_on_a_separate_catalog_is_reported_for_the_catalog covers that case, and test_migrate_applies_migration_6_beside_its_own_sessions holds a session on the catalog database and checks the exit 5 through the CLI.
  • Downgrade: migration 6 is breaking, so a binary that does not carry it refuses a catalog that records it, and a 0.1.x binary refuses every migrated catalog. The catalog version markers of the earlier revision are gone with the old framework.
  • MongoDB field names: unchanged. item_data_to_json still states the tradeoff, the upgrade manual describes it, and MongoDB records migration 6 as not applicable with that reason.

Your review from 19 Sep is the one that still blocks the PR. Could you take another look?

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

v1 Needed for the 1.0 release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] PostgreSQL and MongoDB: an item containing U+0000 returns InternalServerError; the service accepts it

4 participants