From 2e070ede5e801e4c4f2dcb3bd0a4ad755805e285 Mon Sep 17 00:00:00 2001 From: Lee <29015211+LeeroyHannigan@users.noreply.github.com> Date: Wed, 4 Nov 2026 23:52:12 +0000 Subject: [PATCH 1/2] fix(storage-postgres): store attribute names containing U+0000 in the 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 --- .github/workflows/integration.yml | 1 + CONTRIBUTING.md | 41 +- .../storage-cassandra/src/migrations/mod.rs | 5 + crates/storage-mongodb/src/migrations/mod.rs | 4 + .../storage-mongodb/tests/migrations_fresh.rs | 32 +- .../tests/migrations_guards.rs | 4 +- .../tests/migrations_holders.rs | 4 +- .../storage-mongodb/tests/migrations_locks.rs | 6 +- .../tests/migrations_released.rs | 8 +- crates/storage-postgres/src/backup_engine.rs | 43 +- crates/storage-postgres/src/catalog_json.rs | 237 +++++++++++ crates/storage-postgres/src/create_table.rs | 32 +- crates/storage-postgres/src/data/ddl.rs | 19 +- crates/storage-postgres/src/data/gsi_build.rs | 11 +- crates/storage-postgres/src/data/index.rs | 10 +- .../storage-postgres/src/data/vector_index.rs | 13 +- crates/storage-postgres/src/gsi_queue.rs | 11 +- crates/storage-postgres/src/lib.rs | 1 + .../migrations/m0006_attribute_name_escape.rs | 218 ++++++++++ crates/storage-postgres/src/migrations/mod.rs | 28 +- crates/storage-postgres/src/stream_engine.rs | 4 +- crates/storage-postgres/src/table_helpers.rs | 21 +- crates/storage-postgres/src/update_table.rs | 52 +-- crates/storage-postgres/src/vector_search.rs | 10 +- .../tests/catalog_nul_index_writes.rs | 375 +++++++++++++++++ .../tests/catalog_nul_names.rs | 378 ++++++++++++++++++ .../tests/catalog_nul_support/mod.rs | 306 ++++++++++++++ .../golden/0006_attribute_name_escape.txt | 51 +++ .../tests/migrations_golden.rs | 24 +- .../tests/migrations_name_escape.rs | 334 ++++++++++++++++ .../tests/migrations_support/mod.rs | 25 ++ crates/storage-sqlite/src/migrations/mod.rs | 5 + .../src/migrations/released_tests.rs | 9 +- crates/storage-sqlite/src/migrations/tests.rs | 15 +- crates/storage/src/migrations/registry.rs | 6 +- docs/adr/0023-catalog-migrations.md | 4 +- docs/differences-from-dynamodb.md | 1 + docs/getting-started.md | 6 +- docs/manuals/04-quickstart-setup-guide.md | 4 +- docs/manuals/05-admin-guide.md | 2 +- docs/manuals/07-upgrade-manual.md | 6 +- docs/manuals/08-install-linux.md | 2 +- docs/manuals/09-install-macos.md | 2 +- tests/python/test_nul_attribute_names.py | 226 +++++++++++ 44 files changed, 2380 insertions(+), 216 deletions(-) create mode 100644 crates/storage-postgres/src/catalog_json.rs create mode 100644 crates/storage-postgres/src/migrations/m0006_attribute_name_escape.rs create mode 100644 crates/storage-postgres/tests/catalog_nul_index_writes.rs create mode 100644 crates/storage-postgres/tests/catalog_nul_names.rs create mode 100644 crates/storage-postgres/tests/catalog_nul_support/mod.rs create mode 100644 crates/storage-postgres/tests/fixtures/golden/0006_attribute_name_escape.txt create mode 100644 crates/storage-postgres/tests/migrations_name_escape.rs create mode 100644 tests/python/test_nul_attribute_names.py diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 7fd08685e..c3cce6d88 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -516,6 +516,7 @@ jobs: cargo test --release -p extenddb-storage-postgres --test migrations_fresh --test migrations_released --test migrations_crash --test migrations_golden --test migrations_guards --test migrations_runs + --test migrations_name_escape # The daemonized server logs to syslog; dump it so server-side failures # are diagnosable from the job log. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b0d641a98..d4b86e921 100755 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -341,7 +341,7 @@ SQLite, PostgreSQL, Cassandra and MongoDB migrate through this framework. ### Example: a nullable column on `tables` -Migration 6, `table_note`, adds a nullable `note TEXT` column to the catalog's +Migration 7, `table_note`, adds a nullable `note TEXT` column to the catalog's `tables` table on SQLite and PostgreSQL. In `crates/storage/src/migrations/registry.rs`, at the end of the @@ -349,18 +349,18 @@ In `crates/storage/src/migrations/registry.rs`, at the end of the ```rust /// Adds a nullable `note` column to the `tables` catalog table. - TableNote = 6, "table_note", + TableNote = 7, "table_note", "add a nullable note column to the tables catalog"; ``` PostgreSQL has `ADD COLUMN IF NOT EXISTS`, so its body is a file, -`crates/storage-postgres/migrations/0006_table_note.sql`: +`crates/storage-postgres/migrations/0007_table_note.sql`: ```sql -- Copyright 2026 ExtendDB contributors -- SPDX-License-Identifier: Apache-2.0 -- --- Migration 6, table_note, PostgreSQL catalog database: a nullable note +-- Migration 7, table_note, PostgreSQL catalog database: a nullable note -- column on tables. IF NOT EXISTS makes a second run change nothing. ALTER TABLE tables ADD COLUMN IF NOT EXISTS note TEXT; @@ -375,7 +375,7 @@ In `crates/storage-postgres/src/migrations/mod.rs`, the arm in `migration()`: bodies: vec![file_body!( StoreKind::Catalog, "migrations", - "0006_table_note.sql" + "0007_table_note.sql" )], }, ``` @@ -387,19 +387,20 @@ and the id in the transactional arm of `runs_in()`: | MigrationId::ResourceTagConditionKeys | MigrationId::SortKeyDefinitions | MigrationId::DropContinuousBackups + | MigrationId::AttributeNameEscape | MigrationId::TableNote => RunsIn::Transaction, ``` SQLite has no `ADD COLUMN IF NOT EXISTS`, and a file with a plain `ADD COLUMN` fails when it runs a second time, so the SQLite body is code that checks for the column first. -`crates/storage-sqlite/src/migrations/m0006_table_note.rs`: +`crates/storage-sqlite/src/migrations/m0007_table_note.rs`: ```rust // Copyright 2026 ExtendDB contributors // SPDX-License-Identifier: Apache-2.0 -//! Migration 6, `table_note`, SQLite: a nullable `note` column on `tables`. +//! Migration 7, `table_note`, SQLite: a nullable `note` column on `tables`. //! //! SQLite has no `ADD COLUMN IF NOT EXISTS`, so the body reads the table's //! columns first and adds the column only when it is missing. A second run @@ -411,10 +412,10 @@ use extenddb_storage::migrations::{CodeBody, Done, MigrationError}; use super::SqliteStores; /// This file, whose SHA-256 the checksum test pins. -pub(crate) const SOURCE: &str = include_str!("m0006_table_note.rs"); +pub(crate) const SOURCE: &str = include_str!("m0007_table_note.rs"); /// Recorded as this migration's checksum. Changes only with a new migration. -const FINGERPRINT: &str = "0006_table_note/sqlite/1"; +const FINGERPRINT: &str = "0007_table_note/sqlite/1"; pub(crate) struct TableNote; @@ -453,7 +454,7 @@ beside the others: ```rust #[rustfmt::skip] #[allow(clippy::all, deprecated)] -mod m0006_table_note; +mod m0007_table_note; ``` and the arm in `migration()`: @@ -464,8 +465,8 @@ and the arm in `migration()`: convergent: true, bodies: vec![StoreBody::code( StoreKind::Catalog, - &m0006_table_note::TableNote, - m0006_table_note::SOURCE, + &m0007_table_note::TableNote, + m0007_table_note::SOURCE, )], }, ``` @@ -477,8 +478,8 @@ rows of `tables`: ```rust #[tokio::test] -async fn migration_6_adds_the_note_column_to_tables() { - let db = temp_db("m6"); +async fn migration_7_adds_the_note_column_to_tables() { + let db = temp_db("m7"); load_dump(&db, DEV_0_1_14).await; run(&db).await.expect("migrate"); let mut text = dump( @@ -488,12 +489,12 @@ async fn migration_6_adds_the_note_column_to_tables() { ) .await; text.push_str(&dump(&db, "SELECT table_name, note FROM tables ORDER BY table_name").await); - golden("0006_table_note", &text); + golden("0007_table_note", &text); } ``` -`EXTENDDB_BLESS_EXPECTED=1 cargo test -p extenddb-storage-sqlite --lib migration_6` -writes `crates/storage-sqlite/tests/fixtures/golden/0006_table_note.txt`, one +`EXTENDDB_BLESS_EXPECTED=1 cargo test -p extenddb-storage-sqlite --lib migration_7` +writes `crates/storage-sqlite/tests/fixtures/golden/0007_table_note.txt`, one tab-separated line per row under each query: ``` @@ -508,15 +509,15 @@ The PostgreSQL body is a file, so it has no golden test. The example declares `Kind::Breaking`. A nullable column can be additive by the rule in step 4, but declaring it additive needs the old-binary job, which this example does not add. As a breaking migration it makes a 1.x binary -without migration 6 refuse a catalog that has it, and a rollback restores the +without migration 7 refuse a catalog that has it, and a rollback restores the backup. The same PR changes the code that reads and writes `tables.note` in each backend's catalog store, and commits the `docs/schema/sqlite.txt` and `docs/schema/postgres.txt` that the integration jobs regenerate. It changes no per-table DDL, because `tables` is a catalog table, and adds no `RELEASED` -entry. `extenddb migrate --list` then shows migration 6, and on a catalog at -migration 5 `extenddb migrate` lists migration 6 as pending and exits 3. +entry. `extenddb migrate --list` then shows migration 7, and on a catalog at +migration 6 `extenddb migrate` lists migration 7 as pending and exits 3. ### Running the migration tests diff --git a/crates/storage-cassandra/src/migrations/mod.rs b/crates/storage-cassandra/src/migrations/mod.rs index aae33b64f..5ef8bf50a 100644 --- a/crates/storage-cassandra/src/migrations/mod.rs +++ b/crates/storage-cassandra/src/migrations/mod.rs @@ -127,6 +127,11 @@ pub fn migration(id: MigrationId) -> Migration { "0005_drop_continuous_backups.cql" )], }, + MigrationId::AttributeNameEscape => Migration::NotApplicable( + "Cassandra stores the documents that carry attribute names as JSON text in text \ + columns, where U+0000 is the six-character escape \\u0000, so every name reads back \ + as it was written", + ), } } diff --git a/crates/storage-mongodb/src/migrations/mod.rs b/crates/storage-mongodb/src/migrations/mod.rs index 9a3c88598..1fa363dc2 100644 --- a/crates/storage-mongodb/src/migrations/mod.rs +++ b/crates/storage-mongodb/src/migrations/mod.rs @@ -137,6 +137,10 @@ pub fn migration(id: MigrationId) -> Migration { m0005_drop_continuous_backups::SOURCE, )], }, + MigrationId::AttributeNameEscape => Migration::NotApplicable( + "MongoDB stores the catalog's attribute names as BSON string values, which carry their \ + length and so can hold U+0000, and every name reads back as it was written", + ), } } diff --git a/crates/storage-mongodb/tests/migrations_fresh.rs b/crates/storage-mongodb/tests/migrations_fresh.rs index 12cf1838e..338be0b41 100644 --- a/crates/storage-mongodb/tests/migrations_fresh.rs +++ b/crates/storage-mongodb/tests/migrations_fresh.rs @@ -303,7 +303,7 @@ async fn a_fresh_store_gets_every_migration_and_the_expected_definitions() { let Some(uri) = server() else { return }; let d = Deployment::new(&uri, "fresh").await; let report = d.create().await; - assert_eq!((report.from, report.to), (None, Some(5))); + assert_eq!((report.from, report.to), (None, Some(6))); let applied: Vec<(u32, String)> = report .applied .iter() @@ -318,6 +318,7 @@ async fn a_fresh_store_gets_every_migration_and_the_expected_definitions() { (3, d.target.catalog.clone()), (4, d.target.catalog.clone()), (5, d.target.catalog.clone()), + (6, d.target.catalog.clone()), ] ); let not_applicable: Vec = report @@ -326,8 +327,8 @@ async fn a_fresh_store_gets_every_migration_and_the_expected_definitions() { .filter(|s| s.outcome == Outcome::NotApplicable) .map(|s| s.id) .collect(); - assert_eq!(not_applicable, [3, 4], "{report:?}"); - assert_eq!(d.status().await, CatalogStatus::Current { version: 5 }); + assert_eq!(not_applicable, [3, 4, 6], "{report:?}"); + assert_eq!(d.status().await, CatalogStatus::Current { version: 6 }); let rows = d.catalog_ledger().await; let outcomes: Vec<(u32, Outcome)> = rows.iter().map(|r| (r.id, r.outcome)).collect(); assert_eq!( @@ -338,6 +339,7 @@ async fn a_fresh_store_gets_every_migration_and_the_expected_definitions() { (3, Outcome::NotApplicable), (4, Outcome::NotApplicable), (5, Outcome::Applied), + (6, Outcome::NotApplicable), ] ); assert!( @@ -347,7 +349,7 @@ async fn a_fresh_store_gets_every_migration_and_the_expected_definitions() { let data_rows = Deployment::documents(&d.data(), "extenddb_migrations").await; assert_eq!(data_rows.len(), 1); assert_eq!(data_rows[0].get_i64("_id"), Ok(1)); - assert_eq!(d.setting("catalog_version").await.as_deref(), Some("5")); + assert_eq!(d.setting("catalog_version").await.as_deref(), Some("6")); assert_eq!(d.setting("resource_tag_migration").await, None); let history = Deployment::documents(&d.catalog(), "schema_history").await; assert_eq!(history.len(), 1); @@ -389,11 +391,11 @@ async fn a_second_run_applies_nothing_and_reconverge_keeps_the_rows() { let before = d.catalog_ledger().await; let again = d.migrate().await.expect("migrate"); assert!(again.applied.is_empty()); - assert_eq!((again.from, again.to), (Some(5), Some(5))); + assert_eq!((again.from, again.to), (Some(6), Some(6))); let reconverged = d.reconverge().await.expect("reconverge"); // Migration 1's steps in the catalog and the data database, and migration - // 5's in the catalog: 2 is not convergent, and 3 and 4 do not apply on + // 5's in the catalog: 2 is not convergent, and 3, 4 and 6 do not apply on // MongoDB. let ran: Vec<(u32, String)> = reconverged .applied @@ -421,7 +423,7 @@ async fn a_second_run_applies_nothing_and_reconverge_keeps_the_rows() { before, "reconverge leaves the rows as they are" ); - assert_eq!(d.status().await, CatalogStatus::Current { version: 5 }); + assert_eq!(d.status().await, CatalogStatus::Current { version: 6 }); d.drop_databases().await; } @@ -456,7 +458,7 @@ async fn forget_clears_an_unreleased_id_and_migrate_applies_it_again() { assert_eq!(pending.iter().map(|p| p.id).collect::>(), [1, 1]); let report = d.migrate().await.expect("migrate"); assert_eq!(report.applied.len(), 2); - assert_eq!(d.status().await, CatalogStatus::Current { version: 5 }); + assert_eq!(d.status().await, CatalogStatus::Current { version: 6 }); d.drop_databases().await; } @@ -473,19 +475,19 @@ async fn a_changed_checksum_a_newer_catalog_and_an_older_release_are_reported() doc! { "_id": id, "name": "later", "kind": kind, "checksum": "c", "outcome": "applied", "notes": "", "applied_at": bson::DateTime::now(), "applied_by": "9.9.9" } }; - ledger.insert_one(row(6, "breaking")).await.expect("insert"); + ledger.insert_one(row(7, "breaking")).await.expect("insert"); let status = d.status().await; assert_eq!(status.label(), "AheadIncompatible"); assert_eq!(status.exit_code(), exit::AHEAD_INCOMPATIBLE); ledger - .delete_one(doc! { "_id": 6_i64 }) + .delete_one(doc! { "_id": 7_i64 }) .await .expect("delete"); - ledger.insert_one(row(6, "additive")).await.expect("insert"); + ledger.insert_one(row(7, "additive")).await.expect("insert"); assert_eq!(d.status().await.label(), "AheadCompatible"); assert!(d.migrate().await.expect("migrate").applied.is_empty()); ledger - .delete_one(doc! { "_id": 6_i64 }) + .delete_one(doc! { "_id": 7_i64 }) .await .expect("delete"); @@ -500,7 +502,7 @@ async fn a_changed_checksum_a_newer_catalog_and_an_older_release_are_reported() assert_eq!(refused.exit_code(), exit::ATTENTION); d.reconverge().await.expect("reconverge"); assert_eq!(d.setting("resource_tag_migration").await, None); - assert_eq!(d.status().await, CatalogStatus::Current { version: 5 }); + assert_eq!(d.status().await, CatalogStatus::Current { version: 6 }); settings .update_one( doc! { "_id": "catalog_version" }, @@ -510,7 +512,7 @@ async fn a_changed_checksum_a_newer_catalog_and_an_older_release_are_reported() .expect("version"); assert_eq!(d.status().await.label(), "WrittenByOlderRelease"); d.reconverge().await.expect("reconverge"); - assert_eq!(d.setting("catalog_version").await.as_deref(), Some("5")); + assert_eq!(d.setting("catalog_version").await.as_deref(), Some("6")); // A recorded checksum this binary's body does not have. ledger @@ -558,6 +560,6 @@ async fn a_ledger_document_this_binary_cannot_read_stops_the_status() { .expect_err("an error"); migrations.close().await; assert!(error.to_string().contains("\"sideways\""), "{error}"); - assert_eq!(MigrationId::LATEST.number(), 5); + assert_eq!(MigrationId::LATEST.number(), 6); d.drop_databases().await; } diff --git a/crates/storage-mongodb/tests/migrations_guards.rs b/crates/storage-mongodb/tests/migrations_guards.rs index 793f65361..55284c96f 100644 --- a/crates/storage-mongodb/tests/migrations_guards.rs +++ b/crates/storage-mongodb/tests/migrations_guards.rs @@ -207,7 +207,7 @@ async fn migrate_creates_the_indexes_a_deployment_lacks() { .await .contains(&"table_id_1".to_owned()) ); - assert_eq!(d.status().await, CatalogStatus::Current { version: 5 }); + assert_eq!(d.status().await, CatalogStatus::Current { version: 6 }); assert_fresh_definitions(&d).await; d.drop_databases().await; } @@ -232,7 +232,7 @@ async fn stop_at_every_request(uri: &str, state: Option<&str>) -> usize { } assert_eq!( d.status().await, - CatalogStatus::Current { version: 5 }, + CatalogStatus::Current { version: 6 }, "{requests}" ); assert_fresh_definitions(&d).await; diff --git a/crates/storage-mongodb/tests/migrations_holders.rs b/crates/storage-mongodb/tests/migrations_holders.rs index ed0cd866c..5eba917e5 100644 --- a/crates/storage-mongodb/tests/migrations_holders.rs +++ b/crates/storage-mongodb/tests/migrations_holders.rs @@ -113,7 +113,7 @@ async fn migration_2_keeps_the_lock_through_a_scan_longer_than_the_idle_limit() applied.expect("every migration applies"); // A shorter run would apply without the progress the scan records. assert!(took > timing.idle_limit * 4, "the run took {took:?}"); - assert_eq!(d.status().await, CatalogStatus::Current { version: 5 }); + assert_eq!(d.status().await, CatalogStatus::Current { version: 6 }); assert!(d.catalog_ledger().await.iter().any(|row| row.id == 2)); d.drop_databases().await; } @@ -151,7 +151,7 @@ async fn the_status_read_under_the_lock_keeps_it_through_a_long_scan() { "the status read took {took:?}" ); applied.expect("every migration applies"); - assert_eq!(d.status().await, CatalogStatus::Current { version: 5 }); + assert_eq!(d.status().await, CatalogStatus::Current { version: 6 }); d.drop_databases().await; } diff --git a/crates/storage-mongodb/tests/migrations_locks.rs b/crates/storage-mongodb/tests/migrations_locks.rs index 84727523a..3f249d638 100644 --- a/crates/storage-mongodb/tests/migrations_locks.rs +++ b/crates/storage-mongodb/tests/migrations_locks.rs @@ -53,10 +53,10 @@ async fn two_migrators_started_together_apply_once_and_the_other_finds_nothing() applied.sort_unstable(); assert_eq!( applied, - [0, 6], + [0, 7], "one applies every step and the other finds nothing" ); - assert_eq!(d.status().await, CatalogStatus::Current { version: 5 }); + assert_eq!(d.status().await, CatalogStatus::Current { version: 6 }); d.drop_databases().await; } @@ -204,7 +204,7 @@ async fn migrate_waits_for_a_running_server_and_a_server_refuses_while_a_migrati .is_none() ); // The preview and verify read the status beside the server. - assert_eq!(d.status().await, CatalogStatus::Current { version: 5 }); + assert_eq!(d.status().await, CatalogStatus::Current { version: 6 }); serving.release().await; let (held, _) = lock(&d.target, false, Duration::from_secs(2), Timing::PRODUCTION) .await diff --git a/crates/storage-mongodb/tests/migrations_released.rs b/crates/storage-mongodb/tests/migrations_released.rs index c636de7a9..19db70d96 100644 --- a/crates/storage-mongodb/tests/migrations_released.rs +++ b/crates/storage-mongodb/tests/migrations_released.rs @@ -164,12 +164,12 @@ async fn every_released_catalog_upgrades_to_the_definitions_of_a_fresh_install() panic!("{state} is not behind"); }; assert_eq!(version, None, "{state} has no ledger"); - assert_eq!(pending.len(), 6, "{state}"); + assert_eq!(pending.len(), 7, "{state}"); let report = d.migrate().await.expect("migrate"); - assert_eq!((report.from, report.to), (None, Some(5)), "{state}"); + assert_eq!((report.from, report.to), (None, Some(6)), "{state}"); assert_eq!( d.status().await, - CatalogStatus::Current { version: 5 }, + CatalogStatus::Current { version: 6 }, "{state}" ); @@ -401,7 +401,7 @@ async fn a_reconverge_leaves_policies_written_after_migration_2_as_they_are() { .any(|n| n.starts_with("Skipped 2 resource_tag_condition_keys:")), "{report:?}" ); - assert_eq!(d.status().await, CatalogStatus::Current { version: 5 }); + assert_eq!(d.status().await, CatalogStatus::Current { version: 6 }); assert_eq!( documents(Deployment::documents(&d.catalog(), "iam_policies").await), before diff --git a/crates/storage-postgres/src/backup_engine.rs b/crates/storage-postgres/src/backup_engine.rs index 6d93da862..ebc227841 100755 --- a/crates/storage-postgres/src/backup_engine.rs +++ b/crates/storage-postgres/src/backup_engine.rs @@ -7,9 +7,9 @@ use std::collections::HashMap; use std::sync::Arc; use extenddb_core::types::{ - BackupDescription, BackupDetails, BackupSummary, GsiInput, Item, KeySchemaElement, LsiInput, - Projection, RestoreSummary, ScalarAttributeType, SourceTableDetails, StreamSpecification, - TableDescription, + AttributeDefinition, BackupDescription, BackupDetails, BackupSummary, GsiInput, Item, + KeySchemaElement, LsiInput, Projection, RestoreSummary, ScalarAttributeType, + SourceTableDetails, StreamSpecification, TableDescription, }; use extenddb_storage::backup_definition::{ BACKUP_DEFINITION_VERSION, BackupTableDefinition, COPY_BATCH_BYTES, COPY_BATCH_ITEMS, @@ -25,6 +25,7 @@ use futures::future::BoxFuture; use tokio::sync::{OwnedSemaphorePermit, Semaphore}; use crate::PostgresEngine; +use crate::catalog_json::{escape_json, from_catalog_json, to_catalog_json, unescape_json}; use crate::create_table::RestoreRegistration; use crate::data::{ all_sort_key_info, data_table_name, index_table_name, insert_index_rows_multi, @@ -396,9 +397,9 @@ impl PostgresEngine { let mut lsis = Vec::new(); let mut gsi_ids = Vec::new(); for (index_name, index_id, index_type, ks, proj, ipt) in rows { - let key_schema: Vec = serde_json::from_value(ks) + let key_schema: Vec = from_catalog_json(ks) .map_err(|e| StorageError::Internal(format!("index key schema: {e}")))?; - let projection: Projection = serde_json::from_value(proj) + let projection: Projection = from_catalog_json(proj) .map_err(|e| StorageError::Internal(format!("index projection: {e}")))?; if index_type == "LSI" { lsis.push(LsiInput { @@ -490,9 +491,9 @@ impl PostgresEngine { for (index_id, ks, proj) in index_rows { indexes.push(RestoreIndex { table: index_table_name(&index_id), - key_schema: serde_json::from_value(ks) + key_schema: from_catalog_json(ks) .map_err(|e| StorageError::Internal(format!("index key schema: {e}")))?, - projection: serde_json::from_value(proj) + projection: from_catalog_json(proj) .map_err(|e| StorageError::Internal(format!("index projection: {e}")))?, }); } @@ -1140,7 +1141,7 @@ impl BackupEngine for PostgresEngine { // uses, which CreateTable would refuse on the service; drop those. if omitted { let mut used: std::collections::HashSet = - serde_json::from_value::>(key_schema.clone()) + from_catalog_json::>(key_schema.clone()) .map_err(|e| StorageError::Internal(format!("key schema: {e}")))? .into_iter() .map(|k| k.attribute_name) @@ -1158,15 +1159,17 @@ impl BackupEngine for PostgresEngine { { used.insert(k.attribute_name.clone()); } - if let Some(defs) = attr_defs.as_array_mut() { - defs.retain(|d| { - d.get("AttributeName") - .and_then(serde_json::Value::as_str) - .is_some_and(|n| used.contains(n)) - }); - } + // The names are compared decoded: the stored column holds the + // escaped form (`catalog_json`), and the index names above + // were decoded with the captured definition. + let mut defs: Vec = from_catalog_json(attr_defs) + .map_err(|e| StorageError::Internal(format!("attribute definitions: {e}")))?; + defs.retain(|d| used.contains(&d.attribute_name)); + attr_defs = to_catalog_json(&defs)?; } - let definition = definition.to_json()?; + // The definition carries attribute names, so it is stored in the + // form of the catalog columns it was read from. + let definition = escape_json(definition.to_json()?); // All catalog-side writes are one transaction, so a crash cannot // leave a backup AVAILABLE with partial items. The items are read @@ -1321,7 +1324,7 @@ impl BackupEngine for PostgresEngine { ) = row; let key_schema: Vec = - serde_json::from_value(ks_json) + from_catalog_json(ks_json) .map_err(|e| StorageError::Internal(format!("Parse key schema: {e}")))?; Ok(BackupDescription { @@ -1573,7 +1576,7 @@ impl BackupEngine for PostgresEngine { }; let definition = definition - .map(|d| BackupTableDefinition::from_json(d, &backup_arn)) + .map(|d| BackupTableDefinition::from_json(unescape_json(d), &backup_arn)) .transpose()?; // A backup taken before catalog 0.0.4 has no definition; its // vector indexes are known only from the `backups.vector_indexes` @@ -1609,9 +1612,9 @@ impl BackupEngine for PostgresEngine { source_table_name, source_table_arn, source_table_id, - key_schema: serde_json::from_value(ks_json) + key_schema: from_catalog_json(ks_json) .map_err(|e| StorageError::Internal(format!("Parse key schema: {e}")))?, - attribute_definitions: serde_json::from_value(ad_json) + attribute_definitions: from_catalog_json(ad_json) .map_err(|e| StorageError::Internal(format!("Parse attr defs: {e}")))?, billing_mode, provisioned_throughput, diff --git a/crates/storage-postgres/src/catalog_json.rs b/crates/storage-postgres/src/catalog_json.rs new file mode 100644 index 000000000..1d0ada345 --- /dev/null +++ b/crates/storage-postgres/src/catalog_json.rs @@ -0,0 +1,237 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! The stored form of the `jsonb` documents that carry attribute names. +//! +//! DynamoDB accepts U+0000 in an attribute name, a key attribute name +//! included, and `DescribeTable` returns it byte for byte (measured +//! 2026-09-18 and 2026-10-24, and again on 2026-11-04). PostgreSQL `jsonb` +//! refuses the `\u0000` escape (`unsupported Unicode escape sequence`), so a +//! document that holds the character cannot be stored. Every document below +//! is written through [`to_catalog_json`] or [`escape_json`], which replace +//! the two affected characters with sequences `jsonb` accepts, and read +//! through [`from_catalog_json`] or [`unescape_json`], which reverse them: +//! +//! | character | stored as | +//! |-----------|---------------| +//! | U+0000 | U+0001 U+0001 | +//! | U+0001 | U+0001 U+0002 | +//! +//! Every other character is stored as itself, so a name without either +//! character is stored as it was before the escape existed. The escape +//! applies to every string and object key of a document, so a string that is +//! not an attribute name and holds one of the two characters is stored the +//! same way and reads back as written. +//! +//! The documents: 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`. Releases before catalog migration 6 +//! (`attribute_name_escape`) stored these documents raw, and a raw name that +//! holds U+0001 followed by U+0001 or U+0002 would read back as a different +//! name, so migration 6 rewrites every stored document through the escape +//! once (`migrations/m0006_attribute_name_escape.rs`). +//! +//! The item documents (`item_data`, the stream record images, the images in +//! `gsi_pending`) are not written through this module. +//! +//! Decoding reads a U+0001 that is not followed by U+0001 or U+0002 as +//! itself. The encoder never writes one. + +use extenddb_storage::error::StorageError; +use serde::Serialize; +use serde::de::DeserializeOwned; +use serde_json::Value; + +const ESC: char = '\u{1}'; +const NUL_TAIL: char = '\u{1}'; +const ESC_TAIL: char = '\u{2}'; + +/// Serialize `value` into its stored form. +pub(crate) fn to_catalog_json(value: &T) -> Result { + serde_json::to_value(value) + .map(escape_json) + .map_err(|e| StorageError::Internal(e.to_string())) +} + +/// Deserialize a document written by [`to_catalog_json`] or [`escape_json`]. +pub(crate) fn from_catalog_json(v: Value) -> Result { + serde_json::from_value(unescape_json(v)).map_err(|e| StorageError::Internal(e.to_string())) +} + +/// The stored form of a document that is already a JSON tree. +pub(crate) fn escape_json(v: Value) -> Value { + map_strings(v, &escape_str) +} + +/// A stored document with the escape reversed, for a caller that reads its +/// fields without deserializing it. +pub(crate) fn unescape_json(v: Value) -> Value { + map_strings(v, &unescape_str) +} + +fn escape_str(s: &str) -> Option { + if !s.bytes().any(|b| b == 0 || b == 1) { + return None; + } + let mut out = String::with_capacity(s.len() + 8); + for ch in s.chars() { + match ch { + '\u{0}' => { + out.push(ESC); + out.push(NUL_TAIL); + } + '\u{1}' => { + out.push(ESC); + out.push(ESC_TAIL); + } + other => out.push(other), + } + } + Some(out) +} + +fn unescape_str(s: &str) -> Option { + if !s.bytes().any(|b| b == 1) { + return None; + } + let mut out = String::with_capacity(s.len()); + let mut chars = s.chars().peekable(); + while let Some(ch) = chars.next() { + if ch != ESC { + out.push(ch); + continue; + } + match chars.peek() { + Some(&NUL_TAIL) => { + chars.next(); + out.push('\u{0}'); + } + Some(&ESC_TAIL) => { + chars.next(); + out.push('\u{1}'); + } + _ => out.push('\u{1}'), + } + } + Some(out) +} + +/// Apply `f` to every object key and string value of the tree. `f` returns +/// `None` for a string it leaves as it is, so a tree without either character +/// is rebuilt without copying a string. +fn map_strings(v: Value, f: &dyn Fn(&str) -> Option) -> Value { + match v { + Value::String(s) => match f(&s) { + Some(t) => Value::String(t), + None => Value::String(s), + }, + Value::Array(items) => { + Value::Array(items.into_iter().map(|item| map_strings(item, f)).collect()) + } + Value::Object(map) => { + let mut out = serde_json::Map::with_capacity(map.len()); + for (k, val) in map { + let key = f(&k).unwrap_or(k); + out.insert(key, map_strings(val, f)); + } + Value::Object(out) + } + other => other, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use extenddb_core::types::{ + AttributeDefinition, KeySchemaElement, KeyType, ScalarAttributeType, + }; + use serde_json::json; + + fn key_schema(name: &str) -> Vec { + vec![KeySchemaElement { + attribute_name: name.to_owned(), + key_type: KeyType::Hash, + }] + } + + #[test] + fn a_name_without_either_character_is_stored_as_itself() { + let ks = key_schema("pk"); + let stored = to_catalog_json(&ks).unwrap(); + assert_eq!(stored, serde_json::to_value(&ks).unwrap()); + let back: Vec = from_catalog_json(stored).unwrap(); + assert_eq!(back, ks); + } + + #[test] + fn u0000_in_a_key_attribute_name_is_stored_without_u0000_and_read_back() { + let ks = key_schema("p\u{0}k"); + let stored = to_catalog_json(&ks).unwrap(); + let text = stored.to_string(); + assert!(!text.contains("\\u0000"), "{text}"); + assert_eq!(stored[0]["AttributeName"], json!("p\u{1}\u{1}k")); + let back: Vec = from_catalog_json(stored).unwrap(); + assert_eq!(back, ks); + } + + #[test] + fn u0001_in_an_attribute_definition_is_stored_as_a_pair_and_read_back() { + let ad = vec![AttributeDefinition { + attribute_name: "a\u{1}b\u{1}\u{1}c\u{0}".to_owned(), + attribute_type: ScalarAttributeType::S, + }]; + let stored = to_catalog_json(&ad).unwrap(); + assert_eq!( + stored[0]["AttributeName"], + json!("a\u{1}\u{2}b\u{1}\u{2}\u{1}\u{2}c\u{1}\u{1}") + ); + let back: Vec = from_catalog_json(stored).unwrap(); + assert_eq!(back, ad); + } + + #[test] + fn a_u0001_without_a_tail_reads_as_itself() { + let stored = json!([{"AttributeName": "a\u{1}z", "KeyType": "HASH"}]); + let back: Vec = from_catalog_json(stored).unwrap(); + assert_eq!(back[0].attribute_name, "a\u{1}z"); + } + + #[test] + fn object_keys_and_nested_strings_are_escaped() { + let v = json!({"k\u{0}": {"NonKeyAttributes": ["n\u{0}", "plain"]}, "n": 1}); + let stored = escape_json(v.clone()); + assert_eq!( + stored, + json!({"k\u{1}\u{1}": {"NonKeyAttributes": ["n\u{1}\u{1}", "plain"]}, "n": 1}) + ); + assert_eq!(unescape_json(stored), v); + } + + #[test] + fn every_string_of_five_characters_over_the_affected_alphabet_round_trips() { + let alphabet = ['\u{0}', '\u{1}', '\u{2}', 'a']; + let mut frontier = vec![String::new()]; + for _ in 0..5 { + let mut next = Vec::new(); + for s in &frontier { + for ch in alphabet { + let mut t = s.clone(); + t.push(ch); + next.push(t); + } + } + for s in &next { + let stored = escape_str(s).unwrap_or_else(|| s.clone()); + assert!(!stored.contains('\u{0}'), "{s:?}"); + let back = unescape_str(&stored).unwrap_or_else(|| stored.clone()); + assert_eq!(&back, s, "{s:?} stored as {stored:?}"); + } + frontier = next; + } + } +} diff --git a/crates/storage-postgres/src/create_table.rs b/crates/storage-postgres/src/create_table.rs index daabe324e..6257ea297 100755 --- a/crates/storage-postgres/src/create_table.rs +++ b/crates/storage-postgres/src/create_table.rs @@ -11,6 +11,7 @@ use extenddb_storage::error::StorageError; use extenddb_storage::util::{index_arn, stream_arn, table_arn}; use crate::PostgresEngine; +use crate::catalog_json::to_catalog_json; /// What a restore target records about where it came from, written in the /// same catalog transaction that creates it. @@ -110,10 +111,8 @@ impl PostgresEngine { let table_id = uuid::Uuid::new_v4().to_string(); let table_arn = table_arn(&self.region, account_id, &input.table_name); let billing_mode = input.billing_mode.unwrap_or(BillingMode::Provisioned); - let key_schema_json = serde_json::to_value(&input.key_schema) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let attr_defs_json = serde_json::to_value(&input.attribute_definitions) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let key_schema_json = to_catalog_json(&input.key_schema)?; + let attr_defs_json = to_catalog_json(&input.attribute_definitions)?; let billing_str = match billing_mode { BillingMode::Provisioned => "PROVISIONED", BillingMode::PayPerRequest => "PAY_PER_REQUEST", @@ -263,10 +262,8 @@ impl PostgresEngine { let mut gsi_index_ids: Vec = Vec::new(); if let Some(gsis) = &input.global_secondary_indexes { for gsi in gsis { - let gsi_ks = serde_json::to_value(&gsi.key_schema) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let gsi_proj = serde_json::to_value(&gsi.projection) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let gsi_ks = to_catalog_json(&gsi.key_schema)?; + let gsi_proj = to_catalog_json(&gsi.projection)?; let gsi_pt = gsi .provisioned_throughput .as_ref() @@ -306,10 +303,8 @@ impl PostgresEngine { let mut lsi_index_ids: Vec = Vec::new(); if let Some(lsis) = &input.local_secondary_indexes { for lsi in lsis { - let lsi_ks = serde_json::to_value(&lsi.key_schema) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let lsi_proj = serde_json::to_value(&lsi.projection) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let lsi_ks = to_catalog_json(&lsi.key_schema)?; + let lsi_proj = to_catalog_json(&lsi.projection)?; let index_id = uuid::Uuid::new_v4().to_string(); sqlx::query( @@ -363,22 +358,19 @@ impl PostgresEngine { crate::vector::ensure_vector_extension_present(&self.data_pool).await?; } for vi in vis { - let vec_attr = serde_json::to_value(&vi.vector_attribute) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let vec_attr = to_catalog_json(&vi.vector_attribute)?; // The request paths already collapse an empty SearchSchema, so // this is for a caller that reaches the storage trait directly. // Same core rule either way, so the two cannot drift. let search_schema = vi .search_schema_for_storage() - .map(serde_json::to_value) - .transpose() - .map_err(|e| StorageError::Internal(e.to_string()))?; + .map(to_catalog_json) + .transpose()?; let proj = vi .projection .as_ref() - .map(serde_json::to_value) - .transpose() - .map_err(|e| StorageError::Internal(e.to_string()))? + .map(to_catalog_json) + .transpose()? .ok_or_else(|| { // Core validation requires Projection, so reaching here // means the request bypassed validation rather than that diff --git a/crates/storage-postgres/src/data/ddl.rs b/crates/storage-postgres/src/data/ddl.rs index 789447fd4..f28dc2c9b 100755 --- a/crates/storage-postgres/src/data/ddl.rs +++ b/crates/storage-postgres/src/data/ddl.rs @@ -11,6 +11,7 @@ use extenddb_storage::error::StorageError; use extenddb_storage::util::{sk_column, sk_column_n}; use super::{all_sort_key_info, data_table_name, index_table_name, vector_table_name}; +use crate::catalog_json::from_catalog_json; /// Column type for every text key column (`pk`, `sk_s`, `base_pk`, /// `base_sk_s`, and their numbered variants). @@ -397,10 +398,8 @@ impl PostgresEngine { _ => return Err(StorageError::TableNotActive(table_name.to_owned())), } - let key_schema: Vec = - serde_json::from_value(ks_json).map_err(|e| StorageError::Internal(e.to_string()))?; - let attribute_definitions: Vec = - serde_json::from_value(ad_json).map_err(|e| StorageError::Internal(e.to_string()))?; + let key_schema: Vec = from_catalog_json(ks_json)?; + let attribute_definitions: Vec = from_catalog_json(ad_json)?; let stream_specification: Option = stream_spec_json .map(serde_json::from_value) @@ -497,10 +496,8 @@ impl PostgresEngine { ))); } }; - let key_schema: Vec = serde_json::from_value(ks_json) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let projection: Projection = serde_json::from_value(proj_json) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let key_schema: Vec = from_catalog_json(ks_json)?; + let projection: Projection = from_catalog_json(proj_json)?; let info = IndexInfo { index_name, index_id, @@ -593,10 +590,8 @@ impl PostgresEngine { } }; - let key_schema: Vec = - serde_json::from_value(ks_json).map_err(|e| StorageError::Internal(e.to_string()))?; - let projection: Projection = - serde_json::from_value(proj_json).map_err(|e| StorageError::Internal(e.to_string()))?; + let key_schema: Vec = from_catalog_json(ks_json)?; + let projection: Projection = from_catalog_json(proj_json)?; Ok(IndexInfo { index_name: index_name.to_owned(), diff --git a/crates/storage-postgres/src/data/gsi_build.rs b/crates/storage-postgres/src/data/gsi_build.rs index 22ee5bf6f..27965f5c2 100644 --- a/crates/storage-postgres/src/data/gsi_build.rs +++ b/crates/storage-postgres/src/data/gsi_build.rs @@ -66,6 +66,7 @@ use super::index::{ project_item_for_index, }; use super::{all_sort_key_info, data_table_name, index_table_name, json_to_item}; +use crate::catalog_json::from_catalog_json; use crate::gsi_queue::{GsiApplyContext, GsiIndexDef, PendingApplyContext, enqueue_gsi_pending}; /// How long a write may take, from just before its catalog read to just before @@ -1235,12 +1236,12 @@ async fn reconcile_pass( continue; } } - let parse = || -> Result<_, serde_json::Error> { + let parse = || -> Result<_, StorageError> { Ok(( - serde_json::from_value::>(idx_ks.clone())?, - serde_json::from_value::(idx_proj.clone())?, - serde_json::from_value::>(base_ks.clone())?, - serde_json::from_value::>(attr_defs.clone())?, + from_catalog_json::>(idx_ks.clone())?, + from_catalog_json::(idx_proj.clone())?, + from_catalog_json::>(base_ks.clone())?, + from_catalog_json::>(attr_defs.clone())?, )) }; let (key_schema, projection, base_ks, attr_defs) = match parse() { diff --git a/crates/storage-postgres/src/data/index.rs b/crates/storage-postgres/src/data/index.rs index 09c5627d7..d6fe32dd7 100644 --- a/crates/storage-postgres/src/data/index.rs +++ b/crates/storage-postgres/src/data/index.rs @@ -16,6 +16,7 @@ use extenddb_storage::util::SortKeyValue; use extenddb_storage::util::{composite_pk_to_text, parse_sk, sk_column, sk_column_n}; use super::{all_sort_key_info, index_table_name}; +use crate::catalog_json::from_catalog_json; use crate::gsi_queue::{GsiApplyContext, GsiIndexDef, enqueue_gsi_pending}; /// Map a `sqlx` error to `StorageError`, preserving the SQLSTATE code in the @@ -137,16 +138,13 @@ where // Every row carries the same table definitions; parse them once. let attribute_definitions: Vec = match rows.first() { - Some(row) => serde_json::from_value(row.6.clone()) - .map_err(|e| StorageError::Internal(e.to_string()))?, + Some(row) => from_catalog_json(row.6.clone())?, None => Vec::new(), }; rows.into_iter() .map(|(id, index_name, idx_type, ks_json, proj_json, delay, _)| { - let key_schema: Vec = serde_json::from_value(ks_json) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let projection: Projection = serde_json::from_value(proj_json) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let key_schema: Vec = from_catalog_json(ks_json)?; + let projection: Projection = from_catalog_json(proj_json)?; Ok(IndexMeta { index_id: id, index_name, diff --git a/crates/storage-postgres/src/data/vector_index.rs b/crates/storage-postgres/src/data/vector_index.rs index b48170e24..2890d8367 100644 --- a/crates/storage-postgres/src/data/vector_index.rs +++ b/crates/storage-postgres/src/data/vector_index.rs @@ -41,6 +41,7 @@ use extenddb_storage::vector_lifecycle::{ use pgvector::Vector; use super::{all_sort_key_info, vector_table_name}; +use crate::catalog_json::from_catalog_json; /// Read the vector index metadata for a table from the catalog. /// @@ -74,14 +75,14 @@ where let mut out = Vec::with_capacity(rows.len()); for (index_id, dimensions, vector_attribute, search_schema, projection, index_status) in rows { - let attr: extenddb_core::types::VectorAttribute = serde_json::from_value(vector_attribute) + let attr: extenddb_core::types::VectorAttribute = from_catalog_json(vector_attribute) .map_err(|e| StorageError::Internal(format!("vector_attribute: {e}")))?; let search_schema: Vec = match search_schema { - Some(value) => serde_json::from_value(value) + Some(value) => from_catalog_json(value) .map_err(|e| StorageError::Internal(format!("vector search_schema: {e}")))?, None => Vec::new(), }; - let projection: extenddb_core::types::Projection = serde_json::from_value(projection) + let projection: extenddb_core::types::Projection = from_catalog_json(projection) .map_err(|e| StorageError::Internal(format!("vector projection: {e}")))?; let hash_attribute_name = search_schema .iter() @@ -1095,10 +1096,8 @@ pub async fn rebuild_stuck_vector_indexes( ); continue; }; - let base_key_schema: Vec = - serde_json::from_value(ks_json).map_err(|e| StorageError::Internal(e.to_string()))?; - let attribute_definitions: Vec = - serde_json::from_value(ad_json).map_err(|e| StorageError::Internal(e.to_string()))?; + let base_key_schema: Vec = from_catalog_json(ks_json)?; + let attribute_definitions: Vec = from_catalog_json(ad_json)?; let dimensions = u32::try_from(dimensions) .map_err(|_| StorageError::Internal("vector dimensions out of range".to_owned()))?; diff --git a/crates/storage-postgres/src/gsi_queue.rs b/crates/storage-postgres/src/gsi_queue.rs index f068089ed..e8913b7aa 100644 --- a/crates/storage-postgres/src/gsi_queue.rs +++ b/crates/storage-postgres/src/gsi_queue.rs @@ -35,6 +35,8 @@ use serde::{Deserialize, Serialize}; use sqlx::PgPool; use tokio::sync::Notify; +use crate::catalog_json::{from_catalog_json, to_catalog_json}; + /// Number of worker tasks consuming from the persistent queue. /// /// Fixed invariant: a row's `worker_partition` is `hash(base_key) % NUM_WORKERS` @@ -330,8 +332,10 @@ pub(crate) async fn enqueue_gsi_pending( .map(serde_json::to_value) .transpose() .map_err(|e| StorageError::Internal(e.to_string()))?; - let context_json = - serde_json::to_value(context).map_err(|e| StorageError::Internal(e.to_string()))?; + // The context carries attribute names, so it is stored in the form of the + // catalog documents it was read from (`catalog_json`); the item images + // are item data and are not. + let context_json = to_catalog_json(context)?; // Route all updates for a given base item to one worker. // The base key is immutable over an item's lifetime; `new_item` carries it @@ -592,8 +596,7 @@ fn decode_row( .map(serde_json::from_value) .transpose() .map_err(|e| StorageError::Internal(e.to_string()))?; - let context: PendingApplyContext = - serde_json::from_value(ctx_json).map_err(|e| StorageError::Internal(e.to_string()))?; + let context: PendingApplyContext = from_catalog_json(ctx_json)?; Ok((old_item, new_item, context)) } diff --git a/crates/storage-postgres/src/lib.rs b/crates/storage-postgres/src/lib.rs index 81480973c..3ec7d7cb6 100755 --- a/crates/storage-postgres/src/lib.rs +++ b/crates/storage-postgres/src/lib.rs @@ -12,6 +12,7 @@ mod admin_store; mod authorization_store; mod backup_engine; mod bootstrapper; +mod catalog_json; mod catalog_store; pub mod config; mod create_table; diff --git a/crates/storage-postgres/src/migrations/m0006_attribute_name_escape.rs b/crates/storage-postgres/src/migrations/m0006_attribute_name_escape.rs new file mode 100644 index 000000000..03a35736f --- /dev/null +++ b/crates/storage-postgres/src/migrations/m0006_attribute_name_escape.rs @@ -0,0 +1,218 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! Migration 6, `attribute_name_escape`, PostgreSQL: rewrite the stored +//! documents that carry attribute names through the escape that lets them +//! hold U+0000. +//! +//! PostgreSQL `jsonb` refuses U+0000, so from this migration on the server +//! writes these documents with U+0000 stored as U+0001 U+0001 and U+0001 as +//! U+0001 U+0002, and reads them back through the inverse +//! (`crate::catalog_json`). Releases before it stored the documents raw. A +//! raw name holding U+0001 followed by U+0001 or U+0002 would read back as +//! another name, a raw `az` as `az`, and a table +//! keyed on it would no longer find its items. This migration applies the +//! escape once to every stored document that holds U+0001. A raw document +//! cannot hold U+0000, which `jsonb` refused, so applying the escape to it is +//! the whole conversion, and a document without U+0001 is left as it is. +//! +//! The catalog body rewrites `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`. The data body rewrites +//! `gsi_pending.index_context`: a server stopped with queued index updates +//! leaves their rows, and the next server applies them with the names in +//! them. The item documents are not rewritten. +//! +//! The rewrite is not repeatable. An escaped U+0001 and a raw U+0001 U+0002 +//! are the same bytes, so a second run over a document this release wrote +//! would change its names. Each body runs in the runner's transaction and +//! commits with its ledger row, so a store records the migration exactly +//! when its documents were rewritten, and the migration is not convergent: +//! `--reconverge` skips it. `extenddb migrate --forget 6` on a database this +//! migration ran on, followed by a run, rewrites its documents a second time. +//! +//! This file's checksum is pinned once the migration is released. It is +//! excluded from rustfmt and clippy so neither can require an edit, and keeps +//! its own escape instead of calling the crate's, which may change. + +use async_trait::async_trait; +use extenddb_storage::migrations::{CodeBody, Done, MigrationError}; +use serde_json::Value; +use sqlx::{PgPool, Row}; + +use super::PostgresStores; +use super::connect::sql_error; + +/// This file, whose SHA-256 the checksum test pins. +pub(crate) const SOURCE: &str = include_str!("m0006_attribute_name_escape.rs"); + +/// Recorded as this migration's checksum. Changes only with a new migration. +const FINGERPRINT: &str = "0006_attribute_name_escape/postgres/1"; + +/// The catalog database's documents that carry attribute names, by table. +pub(crate) const CATALOG_DOCUMENTS: &[(&str, &[&str])] = &[ + ("tables", &["key_schema", "attribute_definitions"]), + ("indexes", &["key_schema", "projection"]), + ("backups", &["key_schema", "attribute_definitions", "vector_indexes"]), + ("backup_definitions", &["definition"]), + ("vector_indexes", &["vector_attribute", "search_schema", "projection"]), +]; + +/// The data database's documents that carry attribute names. +pub(crate) const DATA_DOCUMENTS: &[(&str, &[&str])] = &[("gsi_pending", &["index_context"])]; + +/// The catalog body. +pub(crate) struct CatalogDocuments; + +/// The data body. +pub(crate) struct QueuedContexts; + +#[async_trait] +impl CodeBody for CatalogDocuments { + fn fingerprint(&self) -> &'static str { + FINGERPRINT + } + + /// Nothing this migration finds stops it: every document has an escaped + /// form. + async fn detect(&self, _stores: &PostgresStores) -> Result, MigrationError> { + Ok(Vec::new()) + } + + async fn run(&self, stores: &PostgresStores) -> Result { + rewrite(&stores.catalog, CATALOG_DOCUMENTS).await.map(|notes| Done { notes }) + } +} + +#[async_trait] +impl CodeBody for QueuedContexts { + fn fingerprint(&self) -> &'static str { + FINGERPRINT + } + + /// Nothing this migration finds stops it. + async fn detect(&self, _stores: &PostgresStores) -> Result, MigrationError> { + Ok(Vec::new()) + } + + async fn run(&self, stores: &PostgresStores) -> Result { + rewrite(&stores.data, DATA_DOCUMENTS).await.map(|notes| Done { notes }) + } +} + +/// The escaped form of one string, or `None` when it holds neither +/// character. +fn escape_str(s: &str) -> Option { + if !s.bytes().any(|b| b == 0 || b == 1) { + return None; + } + let mut out = String::with_capacity(s.len() + 8); + for ch in s.chars() { + match ch { + '\u{0}' => out.push_str("\u{1}\u{1}"), + '\u{1}' => out.push_str("\u{1}\u{2}"), + other => out.push(other), + } + } + Some(out) +} + +/// The escaped form of a document: every object key and string value. +pub(crate) fn escape_document(v: Value) -> Value { + match v { + Value::String(s) => match escape_str(&s) { + Some(t) => Value::String(t), + None => Value::String(s), + }, + Value::Array(items) => Value::Array(items.into_iter().map(escape_document).collect()), + Value::Object(map) => { + let mut out = serde_json::Map::with_capacity(map.len()); + for (k, val) in map { + let key = escape_str(&k).unwrap_or(k); + out.insert(key, escape_document(val)); + } + Value::Object(out) + } + other => other, + } +} + +/// Rewrite every document of `documents` that holds U+0001; one note per +/// column with the number of documents rewritten in it. +/// +/// `jsonb::text` writes a U+0001 inside a string or key as the six +/// characters `\u0001`, so the rows are found by that text, with the +/// backslash spelled `chr(92)` to read the same under either setting of +/// `standard_conforming_strings`. A string holding that text literally is +/// found too; its document escapes to itself and is not written. The rows +/// are locked when they are found, and each is updated once, by the `ctid` +/// read then. +async fn rewrite(pool: &PgPool, documents: &[(&str, &[&str])]) -> Result, MigrationError> { + let mut notes = Vec::new(); + for (table, columns) in documents { + let found = columns + .iter() + .map(|c| format!("position((chr(92) || 'u0001') IN \"{c}\"::text) > 0")) + .collect::>() + .join(" OR "); + let selected = columns.iter().map(|c| format!("\"{c}\"")).collect::>().join(", "); + let rows = sqlx::query(&format!( + "SELECT ctid::text AS row_ctid, {selected} FROM \"{table}\" WHERE {found} FOR UPDATE" + )) + .fetch_all(pool) + .await + .map_err(|e| sql_error(&format!("find the documents to rewrite in {table}"), &e))?; + + let set = columns + .iter() + .enumerate() + .map(|(i, c)| format!("\"{c}\" = ${}", i + 1)) + .collect::>() + .join(", "); + let update = format!( + "UPDATE \"{table}\" SET {set} WHERE ctid = ${}::tid", + columns.len() + 1 + ); + let mut rewritten = vec![0usize; columns.len()]; + for row in rows { + let ctid: String = row + .try_get("row_ctid") + .map_err(|e| sql_error(&format!("read a row of {table}"), &e))?; + let mut values = Vec::with_capacity(columns.len()); + let mut changed = false; + for (i, column) in columns.iter().enumerate() { + let stored: Option = row + .try_get(*column) + .map_err(|e| sql_error(&format!("read {table}.{column}"), &e))?; + let escaped = stored.clone().map(escape_document); + if escaped != stored { + rewritten[i] += 1; + changed = true; + } + values.push(escaped); + } + if !changed { + continue; + } + let mut query = sqlx::query(&update); + for value in values { + query = query.bind(value); + } + query + .bind(&ctid) + .execute(pool) + .await + .map_err(|e| sql_error(&format!("rewrite a row of {table}"), &e))?; + } + for (column, count) in columns.iter().zip(rewritten) { + if count > 0 { + let what = if count == 1 { "document" } else { "documents" }; + notes.push(format!("{table}.{column}: {count} {what} rewritten")); + } + } + } + Ok(notes) +} diff --git a/crates/storage-postgres/src/migrations/mod.rs b/crates/storage-postgres/src/migrations/mod.rs index 740c3469a..7638315c4 100644 --- a/crates/storage-postgres/src/migrations/mod.rs +++ b/crates/storage-postgres/src/migrations/mod.rs @@ -35,6 +35,9 @@ mod m0003_sort_key_definitions; #[rustfmt::skip] #[allow(clippy::all, deprecated)] mod m0004_index_table_base_key; +#[rustfmt::skip] +#[allow(clippy::all, deprecated)] +mod m0006_attribute_name_escape; mod postcheck; mod registrar; mod rows; @@ -127,6 +130,28 @@ pub fn migration(id: MigrationId) -> Migration { "0005_drop_continuous_backups.sql" )], }, + MigrationId::AttributeNameEscape => Migration::Applies { + // A binary without the decoder reads an escaped name as another + // name, and one without the encoder writes a U+0001 raw, which + // this release reads as another name. + kind: Kind::Breaking, + // An escaped U+0001 and a raw U+0001 U+0002 are the same bytes, + // so running the rewrite again would change the names written + // since it ran. + convergent: false, + bodies: vec![ + StoreBody::code( + StoreKind::Catalog, + &m0006_attribute_name_escape::CatalogDocuments, + m0006_attribute_name_escape::SOURCE, + ), + StoreBody::code( + StoreKind::Data, + &m0006_attribute_name_escape::QueuedContexts, + m0006_attribute_name_escape::SOURCE, + ), + ], + }, } } @@ -148,7 +173,8 @@ pub(crate) const fn runs_in(id: MigrationId) -> RunsIn { MigrationId::Baseline | MigrationId::ResourceTagConditionKeys | MigrationId::SortKeyDefinitions - | MigrationId::DropContinuousBackups => RunsIn::Transaction, + | MigrationId::DropContinuousBackups + | MigrationId::AttributeNameEscape => RunsIn::Transaction, MigrationId::IndexTableBaseKey => RunsIn::Autocommit, } } diff --git a/crates/storage-postgres/src/stream_engine.rs b/crates/storage-postgres/src/stream_engine.rs index da7fd122d..eee0c2016 100755 --- a/crates/storage-postgres/src/stream_engine.rs +++ b/crates/storage-postgres/src/stream_engine.rs @@ -14,6 +14,7 @@ use futures::future::BoxFuture; use sqlx::PgPool; use crate::PostgresEngine; +use crate::catalog_json::from_catalog_json; use crate::stream_records_sql::{STREAM_RECORDS_AFTER, STREAM_RECORDS_FROM_START}; /// Number of fixed shards per stream (hash-based assignment). @@ -314,8 +315,7 @@ impl StreamEngine for PostgresEngine { let (ks_json, _ad_json, stream_spec_json, table_status, table_id) = row.ok_or_else(|| extenddb_storage::util::stream_not_found(&stream_arn))?; - let key_schema = serde_json::from_value(ks_json) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let key_schema = from_catalog_json(ks_json)?; let stream_view_type = stream_spec_json .and_then(|v| { diff --git a/crates/storage-postgres/src/table_helpers.rs b/crates/storage-postgres/src/table_helpers.rs index 545f3701e..1f5d5ccb7 100755 --- a/crates/storage-postgres/src/table_helpers.rs +++ b/crates/storage-postgres/src/table_helpers.rs @@ -12,6 +12,7 @@ use extenddb_storage::util::{index_arn, stream_arn}; use extenddb_storage::vector_catalog::{VectorIndexCatalogRow, vector_index_descriptions}; use crate::PostgresEngine; +use crate::catalog_json::{from_catalog_json, unescape_json}; use crate::data; /// Row type for table metadata queries. @@ -68,14 +69,16 @@ pub(crate) struct VectorIndexRow { } impl From for VectorIndexCatalogRow { + /// The three documents carry attribute names, stored through the + /// escape of `catalog_json`; they are handed on decoded. fn from(row: VectorIndexRow) -> Self { Self { index_name: row.index_name, dimensions: i64::from(row.dimensions), distance_function: row.distance_function, - vector_attribute: row.vector_attribute, - search_schema: row.search_schema, - projection: row.projection, + vector_attribute: unescape_json(row.vector_attribute), + search_schema: row.search_schema.map(unescape_json), + projection: unescape_json(row.projection), index_status: row.index_status, backfilling: row.backfilling, } @@ -191,10 +194,8 @@ impl PostgresEngine { let mut lsis: Vec = Vec::new(); for idx in index_rows { - let ks = serde_json::from_value(idx.key_schema) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let proj = serde_json::from_value(idx.projection) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let ks = from_catalog_json(idx.key_schema)?; + let proj = from_catalog_json(idx.projection)?; if idx.index_type == "GSI" { // F-1 fallback: old data may be stored as ProvisionedThroughput @@ -257,10 +258,8 @@ impl PostgresEngine { } } - let key_schema = serde_json::from_value(row.key_schema) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let attr_defs = serde_json::from_value(row.attribute_definitions) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let key_schema = from_catalog_json(row.key_schema)?; + let attr_defs = from_catalog_json(row.attribute_definitions)?; let stream_spec = row .stream_specification .map(serde_json::from_value) diff --git a/crates/storage-postgres/src/update_table.rs b/crates/storage-postgres/src/update_table.rs index dec99a93b..690f4c964 100755 --- a/crates/storage-postgres/src/update_table.rs +++ b/crates/storage-postgres/src/update_table.rs @@ -10,6 +10,7 @@ use extenddb_storage::error::StorageError; use extenddb_storage::util::{building_refuses, effective_attribute_definitions}; use crate::PostgresEngine; +use crate::catalog_json::{from_catalog_json, to_catalog_json}; use crate::data::gsi_build::{ GsiBuild, build_gsis, drop_gsi_tables, is_transient, remove_failed_build, }; @@ -454,10 +455,8 @@ impl PostgresEngine { // `ensure_index_name_free`. ensure_index_name_free(&mut tx, &table_id, &create.index_name).await?; - let gsi_ks = serde_json::to_value(&create.key_schema) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let gsi_proj = serde_json::to_value(&create.projection) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let gsi_ks = to_catalog_json(&create.key_schema)?; + let gsi_proj = to_catalog_json(&create.projection)?; let gsi_pt = create .provisioned_throughput .as_ref() @@ -528,12 +527,8 @@ impl PostgresEngine { // transaction, so `indexes` already holds exactly the surviving set; // the stored definitions were read under the FOR UPDATE lock, so the // read-modify-write is atomic against a concurrent UpdateTable. - let stored_attr_defs: Vec = - serde_json::from_value(ad_json.clone()) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let table_key_schema: Vec = - serde_json::from_value(ks_json.clone()) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let stored_attr_defs: Vec = from_catalog_json(ad_json.clone())?; + let table_key_schema: Vec = from_catalog_json(ks_json.clone())?; let surviving_rows: Vec<(serde_json::Value,)> = sqlx::query_as("SELECT key_schema FROM indexes WHERE table_id = $1") @@ -544,10 +539,7 @@ impl PostgresEngine { let mut surviving_index_key_schemas: Vec> = Vec::with_capacity(surviving_rows.len()); for (ks_value,) in surviving_rows { - surviving_index_key_schemas.push( - serde_json::from_value(ks_value) - .map_err(|e| StorageError::Internal(e.to_string()))?, - ); + surviving_index_key_schemas.push(from_catalog_json(ks_value)?); } let effective = effective_attribute_definitions( @@ -558,8 +550,7 @@ impl PostgresEngine { ); if effective != stored_attr_defs { - let effective_json = serde_json::to_value(&effective) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let effective_json = to_catalog_json(&effective)?; sqlx::query( "UPDATE tables SET attribute_definitions = $1 \ WHERE account_id = $2 AND table_name = $3", @@ -645,11 +636,9 @@ impl PostgresEngine { // naming both shapes, with the vector as type L and its // dimension count. let base_key_schema: Vec = - serde_json::from_value(ks_json.clone()) - .map_err(|e| StorageError::Internal(e.to_string()))?; + from_catalog_json(ks_json.clone())?; let stored_attr_defs: Vec = - serde_json::from_value(ad_json.clone()) - .map_err(|e| StorageError::Internal(e.to_string()))?; + from_catalog_json(ad_json.clone())?; let vec_attr_name = &create.vector_attribute.attribute_name; if let Some(ks) = base_key_schema .iter() @@ -680,15 +669,12 @@ impl PostgresEngine { ensure_index_name_free(&mut tx, &table_id, &create.index_name).await?; let index_id = uuid::Uuid::new_v4().to_string(); - let vec_attr = serde_json::to_value(&create.vector_attribute) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let vec_attr = to_catalog_json(&create.vector_attribute)?; let search_schema = create .search_schema_for_storage() - .map(serde_json::to_value) - .transpose() - .map_err(|e| StorageError::Internal(e.to_string()))?; - let projection = serde_json::to_value(&create.projection) - .map_err(|e| StorageError::Internal(e.to_string()))?; + .map(to_catalog_json) + .transpose()?; + let projection = to_catalog_json(&create.projection)?; let distance = extenddb_storage::vector_catalog::distance_function_token( create.distance_function, )?; @@ -792,10 +778,8 @@ impl PostgresEngine { // recovery sweep never sees a CREATING row with nobody holding it. It is an // efficiency measure only: a second, concurrent build is correct, because a // full backfill pass is enough on its own to publish the index. - let base_key_schema: Vec = serde_json::from_value(ks_json.clone()) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let base_attr_defs: Vec = serde_json::from_value(ad_json.clone()) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let base_key_schema: Vec = from_catalog_json(ks_json.clone())?; + let base_attr_defs: Vec = from_catalog_json(ad_json.clone())?; // Built from the merged and pruned set rather than the request's subset: a // new index may key on an attribute the base table already defined, and on // a conflicting redeclaration the stored type is the one that counts. @@ -926,10 +910,8 @@ impl PostgresEngine { // leaves a CREATING row for the reconciler rather than an ACTIVE index over // a table that was never populated. for (index_id, create) in &created_vector_indexes { - let base_key_schema: Vec = serde_json::from_value(ks_json.clone()) - .map_err(|e| StorageError::Internal(e.to_string()))?; - let base_attr_defs: Vec = serde_json::from_value(ad_json.clone()) - .map_err(|e| StorageError::Internal(e.to_string()))?; + let base_key_schema: Vec = from_catalog_json(ks_json.clone())?; + let base_attr_defs: Vec = from_catalog_json(ad_json.clone())?; let started = self .start_vector_index_build( diff --git a/crates/storage-postgres/src/vector_search.rs b/crates/storage-postgres/src/vector_search.rs index 5b278d467..7f846f947 100644 --- a/crates/storage-postgres/src/vector_search.rs +++ b/crates/storage-postgres/src/vector_search.rs @@ -24,6 +24,7 @@ use extenddb_storage::{ use pgvector::Vector; use crate::PostgresEngine; +use crate::catalog_json::from_catalog_json; use crate::data::vector_table_name; /// The SQL scoring expression for one distance function. @@ -198,11 +199,10 @@ impl VectorSearchEngine for PostgresEngine { row.ok_or_else(|| StorageError::IndexNotFound(index_name.clone()))?; // Stored as the serialized `VectorAttribute` rather than a bare name, so // it is read back the same way the write path wrote it. - let vector_attribute_name = serde_json::from_value::< - extenddb_core::types::VectorAttribute, - >(vector_attribute_json) - .map_err(|e| StorageError::Internal(format!("vector_attribute: {e}")))? - .attribute_name; + let vector_attribute_name = + from_catalog_json::(vector_attribute_json) + .map_err(|e| StorageError::Internal(format!("vector_attribute: {e}")))? + .attribute_name; let dimensions = usize::try_from(dimensions).map_err(|_| { StorageError::Internal(format!("vector dimensions out of range: {dimensions}")) })?; diff --git a/crates/storage-postgres/tests/catalog_nul_index_writes.rs b/crates/storage-postgres/tests/catalog_nul_index_writes.rs new file mode 100644 index 000000000..fd2f1f2ff --- /dev/null +++ b/crates/storage-postgres/tests/catalog_nul_index_writes.rs @@ -0,0 +1,375 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 +//! Item writes through indexes whose names hold U+0000 or U+0001. +//! +//! The write paths read an index's key schema and projection from the +//! catalog, where they are stored through the escape of `catalog_json`, and +//! a write to an index with a propagation delay stores the index definition +//! in `gsi_pending.index_context` for the queue worker. Each test writes +//! items to a table whose GSI is keyed on a raw U+0001 pair and projects a +//! name that holds U+0001 U+0002: a decoder that read either stored form as +//! the name would find no such attribute on the items, and the index would +//! stay empty. A second GSI is keyed on a U+0000 name that the items do not +//! carry, so every write with a delay stores a context that holds U+0000. +//! An item attribute name that holds U+0000 is item data, which the PostgreSQL +//! item columns cannot hold yet, so no item here carries one. +//! +//! See `catalog_nul_support` for the database these tests need. + +mod catalog_nul_support; + +use std::collections::BTreeMap; +use std::time::Duration; + +use catalog_nul_support::{ + ACCOUNT, Options, Scratch, base_conn, include, key, s_attr, scratch, set, skip, stored_text, +}; +use extenddb_core::expression::ExpressionMaps; +use extenddb_core::types::{ + AttributeValue, BillingMode, CreateTableInput, DescribeTableInput, DistanceFunction, GsiInput, + Item, KeyType, Projection, ProjectionType, SearchSchemaElement, SearchSchemaElementType, + TableKeyInfo, UpdateTableInput, VectorAttribute, VectorIndexSpecification, +}; +use extenddb_storage::{DataEngine, TableEngine, VectorSearch, VectorSearchEngine}; + +/// The key of the GSI the items carry: a raw U+0001 pair. +const GK: &str = "g\u{1}\u{1}k"; +/// A projected name: U+0001 U+0002, the stored form of U+0001. +const NK: &str = "n\u{1}\u{2}"; +/// The key of the GSI no item carries: U+0000. +const ZK: &str = "z\u{0}k"; + +fn table_input(table: &str, with_gsis: bool) -> CreateTableInput { + CreateTableInput { + table_name: table.to_owned(), + key_schema: vec![key("pk", KeyType::Hash)], + attribute_definitions: if with_gsis { + vec![s_attr("pk"), s_attr(GK), s_attr(ZK)] + } else { + vec![s_attr("pk")] + }, + billing_mode: Some(BillingMode::PayPerRequest), + global_secondary_indexes: with_gsis.then(|| { + vec![ + GsiInput { + index_name: "by_u1".to_owned(), + key_schema: vec![key(GK, KeyType::Hash)], + projection: include(&[NK]), + provisioned_throughput: None, + }, + GsiInput { + index_name: "by_u0".to_owned(), + key_schema: vec![key(ZK, KeyType::Hash)], + projection: Projection { + projection_type: ProjectionType::All, + non_key_attributes: None, + }, + provisioned_throughput: None, + }, + ] + }), + ..Default::default() + } +} + +fn item(n: u32) -> Item { + BTreeMap::from([ + ("pk".to_owned(), AttributeValue::S(format!("p{n}"))), + (GK.to_owned(), AttributeValue::S(format!("g{}", n % 2))), + (NK.to_owned(), AttributeValue::S(format!("projected {n}"))), + ( + "other".to_owned(), + AttributeValue::S("not projected".to_owned()), + ), + ]) +} + +async fn put(s: &Scratch, key_info: &TableKeyInfo, it: Item) { + s.engine + .put_item(key_info, it, false, None, &ExpressionMaps::default(), None) + .await + .expect("PutItem"); +} + +async fn index_id(s: &Scratch, table: &str, index: &str) -> String { + sqlx::query_scalar( + "SELECT i.index_id FROM indexes i JOIN tables t ON t.table_id = i.table_id \ + WHERE t.table_name = $1 AND i.index_name = $2", + ) + .bind(table) + .bind(index) + .fetch_one(&s.catalog) + .await + .expect("look up the index id") +} + +/// The attribute names of every row of an index, sorted, one row per entry. +async fn index_rows(s: &Scratch, index_id: &str) -> Vec> { + let rows: Vec = + sqlx::query_scalar(&format!("SELECT item_data FROM \"_ddb_{index_id}\"")) + .fetch_all(&s.data) + .await + .expect("read an index table"); + let mut out: Vec> = rows + .iter() + .map(|row| { + let mut names: Vec = + row.as_object().expect("an item").keys().cloned().collect(); + names.sort(); + names + }) + .collect(); + out.sort(); + out +} + +/// Wait until the queue holds no row for the table. +async fn wait_queue_empty(s: &Scratch, table_id: &str) { + for _ in 0..500 { + let n: i64 = sqlx::query_scalar("SELECT count(*) FROM gsi_pending WHERE table_id = $1") + .bind(table_id) + .fetch_one(&s.data) + .await + .expect("count the queued rows"); + if n == 0 { + return; + } + tokio::time::sleep(Duration::from_millis(20)).await; + } + panic!("the propagation queue did not drain"); +} + +/// What `by_u1` holds for three items: the keys and the projected name. +fn expected_rows() -> Vec> { + let mut names = vec![GK.to_owned(), NK.to_owned(), "pk".to_owned()]; + names.sort(); + vec![names; 3] +} + +#[tokio::test] +async fn writes_maintain_indexes_keyed_on_these_names_with_and_without_a_delay() { + let test = "writes_maintain_indexes_keyed_on_these_names_with_and_without_a_delay"; + if base_conn().is_none() { + return skip(test); + } + let options = Options { + gsi_workers: true, + ..Options::default() + }; + let s = scratch(test, options).await.expect("a scratch"); + for delay in ["0", "200"] { + set(&s.catalog, "index_propagation_delay_ms", delay).await; + let table = format!("writes_{delay}"); + s.engine + .create_table(ACCOUNT, table_input(&table, true)) + .await + .expect("CreateTable"); + let key_info = s + .engine + .table_key_info(ACCOUNT, &table) + .await + .expect("table_key_info"); + for n in 0..3 { + put(&s, &key_info, item(n)).await; + } + if delay != "0" { + // The context of each queued row holds the U+0000 name, stored + // through the escape. + let contexts: Vec = sqlx::query_scalar( + "SELECT index_context::text FROM gsi_pending WHERE table_id = $1", + ) + .bind(&key_info.table_id) + .fetch_all(&s.data) + .await + .expect("read the queued contexts"); + for context in &contexts { + assert!(!context.contains("\\u0000"), "{context}"); + } + wait_queue_empty(&s, &key_info.table_id).await; + } + let by_u1 = index_id(&s, &table, "by_u1").await; + assert_eq!( + index_rows(&s, &by_u1).await, + expected_rows(), + "delay {delay}" + ); + let by_u0 = index_id(&s, &table, "by_u0").await; + assert!(index_rows(&s, &by_u0).await.is_empty(), "delay {delay}"); + } +} + +#[tokio::test] +async fn a_backfill_and_the_recovery_sweep_read_the_names_from_the_catalog() { + let test = "a_backfill_and_the_recovery_sweep_read_the_names_from_the_catalog"; + if base_conn().is_none() { + return skip(test); + } + let s = scratch(test, Options::default()).await.expect("a scratch"); + let table = "backfill"; + s.engine + .create_table(ACCOUNT, table_input(table, false)) + .await + .expect("CreateTable"); + let key_info = s + .engine + .table_key_info(ACCOUNT, table) + .await + .expect("table_key_info"); + for n in 0..3 { + put(&s, &key_info, item(n)).await; + } + let input: UpdateTableInput = serde_json::from_value(serde_json::json!({ + "TableName": table, + "AttributeDefinitions": [{"AttributeName": GK, "AttributeType": "S"}], + "GlobalSecondaryIndexUpdates": [{"Create": { + "IndexName": "by_u1", + "KeySchema": [{"AttributeName": GK, "KeyType": "HASH"}], + "Projection": {"ProjectionType": "INCLUDE", "NonKeyAttributes": [NK]} + }}] + })) + .expect("an UpdateTable request"); + s.engine + .update_table(ACCOUNT, input) + .await + .expect("UpdateTable adding the GSI"); + let by_u1 = index_id(&s, table, "by_u1").await; + assert_eq!(index_rows(&s, &by_u1).await, expected_rows()); + + // What a build that died leaves: the index CREATING and its rows gone. + // The recovery sweep reads the definition from the catalog. + sqlx::query("UPDATE indexes SET index_status = 'CREATING' WHERE index_id = $1") + .bind(&by_u1) + .execute(&s.catalog) + .await + .expect("mark the index CREATING"); + sqlx::query(&format!("DELETE FROM \"_ddb_{by_u1}\"")) + .execute(&s.data) + .await + .expect("remove the index rows"); + // The build that ran inside UpdateTable held its ownership lock on a + // connection of its own, released when the server notices the + // connection closed; until then the sweep skips the index. + let mut finished = 0; + for _ in 0..50 { + finished = extenddb_storage_postgres::reconcile_creating_gsis(&s.engine) + .await + .expect("run the recovery sweep"); + if finished > 0 { + break; + } + tokio::time::sleep(Duration::from_millis(100)).await; + } + assert_eq!(finished, 1); + assert_eq!(index_rows(&s, &by_u1).await, expected_rows()); +} + +#[tokio::test] +async fn a_vector_index_keeps_its_attribute_names() { + let test = "a_vector_index_keeps_its_attribute_names"; + if base_conn().is_none() { + return skip(test); + } + let options = Options { + pgvector: true, + ..Options::default() + }; + let Some(s) = scratch(test, options).await else { + return; + }; + // The vector attribute and the search schema's HASH attribute. + let (vector, hash) = ("v\u{1}\u{1}", "t\u{1}\u{2}"); + let input = CreateTableInput { + table_name: "vectors".to_owned(), + key_schema: vec![key("pk", KeyType::Hash)], + attribute_definitions: vec![s_attr("pk"), s_attr(hash)], + billing_mode: Some(BillingMode::PayPerRequest), + vector_indexes: Some(vec![VectorIndexSpecification { + index_name: "vidx".to_owned(), + dimensions: 2, + distance_function: DistanceFunction::Cosine, + vector_attribute: VectorAttribute { + attribute_name: vector.to_owned(), + }, + search_schema: Some(vec![SearchSchemaElement { + attribute_name: hash.to_owned(), + element_type: SearchSchemaElementType::Hash, + }]), + projection: Some(Projection { + projection_type: ProjectionType::All, + non_key_attributes: None, + }), + }]), + ..Default::default() + }; + s.engine + .create_table(ACCOUNT, input) + .await + .expect("CreateTable with a vector index"); + let stored = sqlx::query_scalar::<_, String>( + "SELECT vector_attribute::text || ' ' || search_schema::text FROM vector_indexes \ + WHERE index_name = 'vidx'", + ) + .fetch_one(&s.catalog) + .await + .expect("read the vector index row"); + for name in [vector, hash] { + assert!( + stored.contains(&format!("\"{}\"", stored_text(name))), + "{stored}" + ); + } + let desc = s + .engine + .describe_table( + ACCOUNT, + DescribeTableInput { + table_name: "vectors".to_owned(), + }, + ) + .await + .expect("DescribeTable"); + let vi = &desc.vector_indexes.expect("vector indexes")[0]; + assert_eq!(vi.vector_attribute.attribute_name, vector); + let schema = vi.search_schema.as_ref().expect("a search schema"); + assert_eq!(schema[0].attribute_name, hash); + + let key_info = s + .engine + .table_key_info(ACCOUNT, "vectors") + .await + .expect("table_key_info"); + assert_eq!(key_info.vector_indexes[0].vector_attribute_name, vector); + let tenant = AttributeValue::S("a".to_owned()); + let it: Item = BTreeMap::from([ + ("pk".to_owned(), AttributeValue::S("p1".to_owned())), + (hash.to_owned(), tenant.clone()), + ( + vector.to_owned(), + AttributeValue::L(vec![ + AttributeValue::N("1".to_owned()), + AttributeValue::N("0".to_owned()), + ]), + ), + ]); + put(&s, &key_info, it).await; + // The search reads the vector attribute's name from the catalog and puts + // the stored vector back under it. + let found = s + .engine + .search_vectors(VectorSearch { + key_info: &key_info, + index_name: "vidx", + query_vector: &[1.0, 0.0], + top_k: 5, + hash_key: Some((hash, &tenant)), + filters: &[], + }) + .await + .expect("SearchVectors"); + assert_eq!(found.hits.len(), 1); + assert!( + found.hits[0].item.contains_key(vector), + "{:?}", + found.hits[0].item + ); +} diff --git a/crates/storage-postgres/tests/catalog_nul_names.rs b/crates/storage-postgres/tests/catalog_nul_names.rs new file mode 100644 index 000000000..efce07e87 --- /dev/null +++ b/crates/storage-postgres/tests/catalog_nul_names.rs @@ -0,0 +1,378 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 +//! The catalog round trip of attribute names that hold U+0000 or U+0001. +//! +//! DynamoDB accepts U+0000 in a key attribute name, an attribute definition, +//! an index key attribute name and a projected attribute name, and +//! `DescribeTable` returns each name byte for byte (measured 2026-09-18, +//! 2026-10-24 and 2026-11-04). PostgreSQL `jsonb` refuses U+0000, so the +//! catalog stores these names through the escape of `catalog_json` (U+0000 as +//! U+0001 U+0001, U+0001 as U+0001 U+0002). The table here holds U+0000, a +//! raw U+0001 pair (the stored form of U+0000, which the decoder must not +//! read as U+0000) and U+0001 U+0002 (the stored form of U+0001) in its key +//! schema, its GSI and LSI keys and its projected names. Each place that +//! reads the names back returns them byte for byte: `DescribeTable`, the key +//! info of the data plane, `DescribeStream`, `DescribeBackup`, the restore +//! source, a restored table, `UpdateTable` and `DeleteTable`. +//! +//! See `catalog_nul_support` for the database these tests need. + +mod catalog_nul_support; + +use catalog_nul_support::{ + ACCOUNT, Options, base_conn, def_names, documents, include, key, names, s_attr, scratch, skip, + stored_text, table_documents, wait_active, +}; +use extenddb_core::types::{ + BillingMode, CreateTableInput, DeleteTableInput, DescribeStreamInput, DescribeTableInput, + GsiInput, KeyType, LsiInput, StreamSpecification, StreamViewType, UpdateTableInput, +}; +use extenddb_storage::{ + BackupEngine, RestoreRequest, RestoreTableOverrides, StreamEngine, TableEngine, +}; + +/// U+0000 in the partition key. +const PK: &str = "p\u{0}k"; +/// A raw U+0001 pair in the sort key. +const SK: &str = "s\u{1}\u{1}k"; +/// U+0000 in the GSI partition key. +const GK: &str = "g\u{0}k"; +/// U+0001 U+0002 in the GSI sort key. +const GSK: &str = "h\u{1}\u{2}k"; +/// Both characters in the LSI sort key. +const LK: &str = "l\u{0}\u{1}k"; +/// The projected names of both indexes. +const PROJECTED: [&str; 2] = ["n\u{0}k", "m\u{1}k"]; + +fn input(table: &str) -> CreateTableInput { + CreateTableInput { + table_name: table.to_owned(), + key_schema: vec![key(PK, KeyType::Hash), key(SK, KeyType::Range)], + attribute_definitions: vec![s_attr(PK), s_attr(SK), s_attr(GK), s_attr(GSK), s_attr(LK)], + billing_mode: Some(BillingMode::PayPerRequest), + global_secondary_indexes: Some(vec![GsiInput { + index_name: "gsi1".to_owned(), + key_schema: vec![key(GK, KeyType::Hash), key(GSK, KeyType::Range)], + projection: include(&PROJECTED), + provisioned_throughput: None, + }]), + local_secondary_indexes: Some(vec![LsiInput { + index_name: "lsi1".to_owned(), + key_schema: vec![key(PK, KeyType::Hash), key(LK, KeyType::Range)], + projection: include(&PROJECTED), + }]), + stream_specification: Some(StreamSpecification { + stream_enabled: true, + stream_view_type: Some(StreamViewType::NewAndOldImages), + }), + ..Default::default() + } +} + +fn projected(p: &extenddb_core::types::Projection) -> Vec<&str> { + p.non_key_attributes + .iter() + .flatten() + .map(String::as_str) + .collect() +} + +/// Every name of the table, in its stored form, and no U+0000. +fn assert_stored_escaped(text: &str, what: &str) { + assert!(!text.contains("\\u0000"), "{what} holds U+0000: {text}"); + for name in [PK, SK, GK, GSK, LK, PROJECTED[0], PROJECTED[1]] { + let stored = format!("\"{}\"", stored_text(name)); + assert!(text.contains(&stored), "{what}: no {stored} in {text}"); + } +} + +fn update(table: &str, json: serde_json::Value) -> UpdateTableInput { + let mut json = json; + json["TableName"] = serde_json::Value::String(table.to_owned()); + serde_json::from_value(json).expect("an UpdateTable request") +} + +#[tokio::test] +async fn names_with_u0000_and_u0001_round_trip_through_the_catalog() { + let test = "names_with_u0000_and_u0001_round_trip_through_the_catalog"; + if base_conn().is_none() { + return skip(test); + } + let s = scratch(test, Options::default()).await.expect("a scratch"); + let table = "nul_names"; + let created = s + .engine + .create_table(ACCOUNT, input(table)) + .await + .expect("CreateTable with U+0000 and U+0001 in attribute names"); + assert_eq!(names(&created.key_schema), [PK, SK]); + assert_stored_escaped(&table_documents(&s.catalog, table).await, "the table rows"); + + let desc = s + .engine + .describe_table( + ACCOUNT, + DescribeTableInput { + table_name: table.to_owned(), + }, + ) + .await + .expect("DescribeTable"); + assert_eq!(names(&desc.key_schema), [PK, SK]); + assert_eq!( + def_names(&desc.attribute_definitions), + def_names(&input(table).attribute_definitions) + ); + let gsi = &desc.global_secondary_indexes.as_deref().expect("GSIs")[0]; + assert_eq!(names(&gsi.key_schema), [GK, GSK]); + assert_eq!(projected(&gsi.projection), PROJECTED); + let lsi = &desc.local_secondary_indexes.as_deref().expect("LSIs")[0]; + assert_eq!(names(&lsi.key_schema), [PK, LK]); + assert_eq!(projected(&lsi.projection), PROJECTED); + + // The key info that the data plane and authorization read. + let info = s + .engine + .table_key_info(ACCOUNT, table) + .await + .expect("table_key_info"); + assert_eq!(names(&info.key_schema), [PK, SK]); + assert_eq!( + def_names(&info.attribute_definitions), + def_names(&input(table).attribute_definitions) + ); + assert_eq!( + names(&info.global_secondary_indexes[0].key_schema), + [GK, GSK] + ); + assert_eq!( + projected(&info.global_secondary_indexes[0].projection), + PROJECTED + ); + assert_eq!(names(&info.local_secondary_indexes[0].key_schema), [PK, LK]); + + // DescribeStream reads the key schema on its own path. + let stream = s + .engine + .describe_stream( + ACCOUNT, + &DescribeStreamInput { + stream_arn: desc.latest_stream_arn.clone().expect("the stream ARN"), + limit: None, + exclusive_start_shard_id: None, + }, + ) + .await + .expect("DescribeStream"); + assert_eq!(names(&stream.key_schema), [PK, SK]); + + // A backup copies the documents into `backups` and `backup_definitions`. + let backup = s + .engine + .create_backup(ACCOUNT, table, "bk367") + .await + .expect("CreateBackup"); + let stored = documents( + &s.catalog, + "SELECT b.key_schema::text || ' ' || b.attribute_definitions::text || ' ' || \ + d.definition::text FROM backups b \ + JOIN backup_definitions d ON d.backup_arn = b.backup_arn WHERE b.backup_arn = $1", + &backup.backup_arn, + ) + .await + .join(" "); + assert_stored_escaped(&stored, "the backup rows"); + let described = s + .engine + .describe_backup(ACCOUNT, &backup.backup_arn) + .await + .expect("DescribeBackup"); + assert_eq!(names(&described.source_table_details.key_schema), [PK, SK]); + let source = s + .engine + .backup_restore_source(ACCOUNT, &backup.backup_arn) + .await + .expect("the restore source"); + assert_eq!(names(&source.key_schema), [PK, SK]); + assert_eq!( + def_names(&source.attribute_definitions), + def_names(&input(table).attribute_definitions) + ); + let definition = source.definition.as_ref().expect("the backup definition"); + let bgsi = &definition.global_secondary_indexes[0]; + assert_eq!(names(&bgsi.key_schema), [GK, GSK]); + assert_eq!(projected(&bgsi.projection), PROJECTED); + let blsi = &definition.local_secondary_indexes[0]; + assert_eq!(names(&blsi.key_schema), [PK, LK]); + assert_eq!(projected(&blsi.projection), PROJECTED); + + // The restore target is created from the restore source. + let restored_name = "nul_names_restored"; + let restore_input = source.restore_input(restored_name, &RestoreTableOverrides::default()); + extenddb_core::validation::validate_create_table( + &restore_input, + &extenddb_core::limits::LimitsConfig::default(), + ) + .expect("the restore input validates as CreateTable does"); + s.engine + .restore_table_from_backup( + ACCOUNT, + RestoreRequest { + backup_arn: backup.backup_arn.clone(), + source_table_arn: source.source_table_arn.clone(), + restore_date_time: source.backup_creation_date_time, + input: restore_input, + }, + ) + .await + .expect("RestoreTableFromBackup"); + wait_active(&s.catalog, restored_name).await; + assert_stored_escaped( + &table_documents(&s.catalog, restored_name).await, + "the restored table rows", + ); + let restored = s + .engine + .describe_table( + ACCOUNT, + DescribeTableInput { + table_name: restored_name.to_owned(), + }, + ) + .await + .expect("DescribeTable of the restored table"); + assert_eq!(names(&restored.key_schema), [PK, SK]); + assert_eq!( + def_names(&restored.attribute_definitions), + def_names(&input(table).attribute_definitions) + ); + let rgsi = &restored.global_secondary_indexes.as_deref().expect("GSIs")[0]; + assert_eq!(names(&rgsi.key_schema), [GK, GSK]); + assert_eq!(projected(&rgsi.projection), PROJECTED); + let rlsi = &restored.local_secondary_indexes.as_deref().expect("LSIs")[0]; + assert_eq!(names(&rlsi.key_schema), [PK, LK]); + assert_eq!(projected(&rlsi.projection), PROJECTED); + + // Deleting the GSI prunes its definitions, compared and rewritten in + // the stored form; a GSI is then created on the U+0000 attribute again. + let after_delete = s + .engine + .update_table( + ACCOUNT, + update( + table, + serde_json::json!({ + "GlobalSecondaryIndexUpdates": [{"Delete": {"IndexName": "gsi1"}}] + }), + ), + ) + .await + .expect("UpdateTable deleting the GSI"); + assert!( + after_delete + .global_secondary_indexes + .as_deref() + .unwrap_or(&[]) + .is_empty() + ); + assert_eq!(def_names(&after_delete.attribute_definitions), { + let mut v = vec![PK, SK, LK]; + v.sort_unstable(); + v + }); + let after_create = s + .engine + .update_table( + ACCOUNT, + update( + table, + serde_json::json!({ + "AttributeDefinitions": [{"AttributeName": GK, "AttributeType": "S"}], + "GlobalSecondaryIndexUpdates": [{"Create": { + "IndexName": "gsi2", + "KeySchema": [{"AttributeName": GK, "KeyType": "HASH"}], + "Projection": {"ProjectionType": "INCLUDE", "NonKeyAttributes": PROJECTED} + }}] + }), + ), + ) + .await + .expect("UpdateTable creating a GSI on the U+0000 attribute"); + let gsi2 = &after_create + .global_secondary_indexes + .as_deref() + .expect("GSIs")[0]; + assert_eq!(gsi2.index_name, "gsi2"); + assert_eq!(names(&gsi2.key_schema), [GK]); + assert_eq!(projected(&gsi2.projection), PROJECTED); + assert_eq!(def_names(&after_create.attribute_definitions), { + let mut v = vec![PK, SK, LK, GK]; + v.sort_unstable(); + v + }); + let text = table_documents(&s.catalog, table).await; + assert!(!text.contains("\\u0000"), "{text}"); + assert!(text.contains(&format!("\"{}\"", stored_text(GK))), "{text}"); + + let deleted = s + .engine + .delete_table( + ACCOUNT, + DeleteTableInput { + table_name: table.to_owned(), + }, + ) + .await + .expect("DeleteTable"); + assert_eq!(names(&deleted.key_schema), [PK, SK]); +} + +#[tokio::test] +async fn names_without_either_character_are_stored_as_before() { + let test = "names_without_either_character_are_stored_as_before"; + if base_conn().is_none() { + return skip(test); + } + let s = scratch(test, Options::default()).await.expect("a scratch"); + let table = "plain_names"; + let plain = CreateTableInput { + table_name: table.to_owned(), + key_schema: vec![key("pk", KeyType::Hash), key("sk", KeyType::Range)], + attribute_definitions: vec![s_attr("pk"), s_attr("sk"), s_attr("gk")], + billing_mode: Some(BillingMode::PayPerRequest), + global_secondary_indexes: Some(vec![GsiInput { + index_name: "gsi1".to_owned(), + key_schema: vec![key("gk", KeyType::Hash)], + projection: include(&["nk"]), + provisioned_throughput: None, + }]), + ..Default::default() + }; + s.engine + .create_table(ACCOUNT, plain.clone()) + .await + .expect("CreateTable"); + // The stored documents are the plain serialization. + let (ks, ad): (serde_json::Value, serde_json::Value) = sqlx::query_as( + "SELECT key_schema, attribute_definitions FROM tables WHERE table_name = $1", + ) + .bind(table) + .fetch_one(&s.catalog) + .await + .expect("read the table row"); + assert_eq!(ks, serde_json::to_value(&plain.key_schema).unwrap()); + assert_eq!( + ad, + serde_json::to_value(&plain.attribute_definitions).unwrap() + ); + let (iks, proj): (serde_json::Value, serde_json::Value) = + sqlx::query_as("SELECT key_schema, projection FROM indexes WHERE index_name = 'gsi1'") + .fetch_one(&s.catalog) + .await + .expect("read the index row"); + let gsi = &plain.global_secondary_indexes.as_ref().unwrap()[0]; + assert_eq!(iks, serde_json::to_value(&gsi.key_schema).unwrap()); + assert_eq!(proj, serde_json::to_value(&gsi.projection).unwrap()); + let text = table_documents(&s.catalog, table).await; + assert!(!text.contains("\\u0001"), "{text}"); +} diff --git a/crates/storage-postgres/tests/catalog_nul_support/mod.rs b/crates/storage-postgres/tests/catalog_nul_support/mod.rs new file mode 100644 index 000000000..af3427a44 --- /dev/null +++ b/crates/storage-postgres/tests/catalog_nul_support/mod.rs @@ -0,0 +1,306 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! Shared by the tests of attribute names that hold U+0000 or U+0001 +//! (`catalog_nul_names.rs` and `catalog_nul_index_writes.rs`): a catalog +//! database and a data database that the runner migrated as `extenddb init` +//! does, an engine on them, and the text the catalog stores for a name. +//! +//! Every test needs `EXTENDDB_TEST_PG_CONNECTION_STRING`, a base URL with no +//! database, for a role that may create databases. Without it every test +//! reports a skip and passes; `EXTENDDB_TEST_PG_REQUIRED` turns the skip into +//! a failure. Each test drops its databases when it ends, passing or failing. + +#![allow(dead_code)] + +use std::time::Duration; + +use extenddb_core::types::{ + AttributeDefinition, KeySchemaElement, KeyType, Projection, ProjectionType, ScalarAttributeType, +}; +use extenddb_storage::migrations::Runner; +use extenddb_storage_postgres::migrations::{ + PostgresMigrations, PostgresTarget, try_create_vector_extension, +}; +use extenddb_storage_postgres::{PostgresConfig, PostgresEngine}; +use sqlx::PgPool; +use sqlx::postgres::PgPoolOptions; + +pub const ACCOUNT: &str = "123456789012"; +pub const REGION: &str = "us-east-1"; + +/// The server's base URL, or `None` to skip. +pub fn base_conn() -> Option { + let conn = std::env::var("EXTENDDB_TEST_PG_CONNECTION_STRING").unwrap_or_default(); + if conn.trim().is_empty() { + let required = + std::env::var("EXTENDDB_TEST_PG_REQUIRED").is_ok_and(|v| !v.is_empty() && v != "0"); + assert!( + !required, + "EXTENDDB_TEST_PG_CONNECTION_STRING is not set, and EXTENDDB_TEST_PG_REQUIRED says \ + this job must reach a PostgreSQL server" + ); + return None; + } + Some(conn.trim_end_matches('/').to_owned()) +} + +pub fn skip(test: &str) { + eprintln!("SKIP {test}: EXTENDDB_TEST_PG_CONNECTION_STRING is not set"); +} + +/// One test at a time in each test binary: each opens an engine pool, and +/// the server is shared with other jobs. +static SERIAL: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); + +/// What a scratch deployment runs. +#[derive(Clone, Copy, Default)] +pub struct Options { + /// Start the propagation queue's workers, for a non-zero delay. + pub gsi_workers: bool, + /// Install pgvector in the data database, for vector indexes. + pub pgvector: bool, +} + +/// A catalog database and a data database with an engine on them. +pub struct Scratch { + pub engine: PostgresEngine, + pub catalog: PgPool, + pub data: PgPool, + _guard: Guard, + _serial: tokio::sync::MutexGuard<'static, ()>, +} + +/// Drops the databases when the scratch goes, a failed assertion included. +struct Guard { + base: String, + names: Vec, +} + +impl Drop for Guard { + fn drop(&mut self) { + let (base, names) = (self.base.clone(), self.names.clone()); + let _ = std::thread::spawn(move || { + let Ok(rt) = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + else { + return; + }; + rt.block_on(async { + if let Ok(admin) = PgPoolOptions::new() + .max_connections(1) + .connect(&format!("{base}/postgres")) + .await + { + for n in &names { + let _ = + sqlx::query(&format!("DROP DATABASE IF EXISTS \"{n}\" WITH (FORCE)")) + .execute(&admin) + .await; + } + admin.close().await; + } + }); + }) + .join(); + } +} + +async fn pool(url: &str, size: u32) -> PgPool { + PgPoolOptions::new() + .max_connections(size) + .connect(url) + .await + .unwrap_or_else(|e| panic!("connect to {url}: {e}")) +} + +/// A fresh deployment, or `None` when `options.pgvector` asks for pgvector +/// and the server has no pgvector package. +pub async fn scratch(test: &str, options: Options) -> Option { + let serial = SERIAL.lock().await; + let base = base_conn().expect("the caller checks base_conn() first"); + let stem = format!( + "eddb_nul_{}", + &uuid::Uuid::new_v4().simple().to_string()[..12] + ); + let (catalog_name, data_name) = (format!("{stem}_c"), format!("{stem}_d")); + let guard = Guard { + base: base.clone(), + names: vec![catalog_name.clone(), data_name.clone()], + }; + let admin = pool(&format!("{base}/postgres"), 1).await; + for n in [&catalog_name, &data_name] { + sqlx::query(&format!("CREATE DATABASE \"{n}\"")) + .execute(&admin) + .await + .expect("create a scratch database"); + } + admin.close().await; + let catalog_url = format!("{base}/{catalog_name}"); + let data_url = format!("{base}/{data_name}"); + let target = PostgresTarget::new(catalog_url.parse().expect("a postgres URL"), &data_name) + .with_data_connection_row(&data_url); + let runner = PostgresMigrations::open_unlocked(&target) + .await + .expect("open the migration runner"); + let report = Runner::new(&runner, "test").migrate().await; + runner.close().await; + report.expect("migrate the scratch databases"); + if options.pgvector { + let line = + try_create_vector_extension(&data_url.parse().expect("a postgres URL"), None).await; + if !line.starts_with("pgvector available") { + eprintln!("SKIP {test}: {line}"); + return None; + } + } + let catalog = pool(&catalog_url, 2).await; + let data = pool(&data_url, 2).await; + for key in ["control_plane_delay_seconds", "index_propagation_delay_ms"] { + set(&catalog, key, "0").await; + } + sqlx::query("INSERT INTO accounts (account_id, account_name) VALUES ($1, $2)") + .bind(ACCOUNT) + .bind(format!("acct-{stem}")) + .execute(&catalog) + .await + .expect("seed the account row"); + let mut engine = PostgresEngine::new( + &PostgresConfig { + connection_string: catalog_url, + pool_size: 4, + max_item_size_bytes: 400_000, + }, + REGION, + ) + .await + .expect("open an engine on the scratch databases"); + if options.gsi_workers { + engine = engine.start_gsi_workers(); + } + Some(Scratch { + engine, + catalog, + data, + _guard: guard, + _serial: serial, + }) +} + +/// Write a settings row, as main's PostgreSQL tests do: the migrations seed +/// none. +pub async fn set(catalog: &PgPool, key: &str, value: &str) { + sqlx::query( + "INSERT INTO settings (key, value) VALUES ($1, $2) \ + ON CONFLICT (key) DO UPDATE SET value = EXCLUDED.value", + ) + .bind(key) + .bind(value) + .execute(catalog) + .await + .expect("write a setting"); +} + +pub fn key(name: &str, key_type: KeyType) -> KeySchemaElement { + KeySchemaElement { + attribute_name: name.to_owned(), + key_type, + } +} + +pub fn s_attr(name: &str) -> AttributeDefinition { + AttributeDefinition { + attribute_name: name.to_owned(), + attribute_type: ScalarAttributeType::S, + } +} + +pub fn include(names: &[&str]) -> Projection { + Projection { + projection_type: ProjectionType::Include, + non_key_attributes: Some(names.iter().map(|n| (*n).to_owned()).collect()), + } +} + +pub fn names(ks: &[KeySchemaElement]) -> Vec<&str> { + ks.iter().map(|k| k.attribute_name.as_str()).collect() +} + +/// The names of attribute definitions, sorted. +pub fn def_names(ad: &[AttributeDefinition]) -> Vec<&str> { + let mut v: Vec<&str> = ad.iter().map(|a| a.attribute_name.as_str()).collect(); + v.sort_unstable(); + v +} + +/// How `jsonb::text` shows `name` stored through the escape: U+0000 as +/// U+0001 U+0001, U+0001 as U+0001 U+0002, and each control character +/// written `\u00NN`. The names in these tests hold no control character but +/// U+0000, U+0001 and U+0002. +pub fn stored_text(name: &str) -> String { + let mut out = String::new(); + for c in name.chars() { + match c { + '\u{0}' => out.push_str("\\u0001\\u0001"), + '\u{1}' => out.push_str("\\u0001\\u0002"), + '\u{2}' => out.push_str("\\u0002"), + c => out.push(c), + } + } + out +} + +/// The `jsonb::text` of the documents a query returns, one per row. +pub async fn documents(pool: &PgPool, sql: &str, bind: &str) -> Vec { + sqlx::query_scalar(sql) + .bind(bind) + .fetch_all(pool) + .await + .unwrap_or_else(|e| panic!("{sql}: {e}")) +} + +/// The catalog documents of a table and its indexes, as stored. +pub async fn table_documents(catalog: &PgPool, table: &str) -> String { + let mut out = documents( + catalog, + "SELECT key_schema::text || ' ' || attribute_definitions::text FROM tables \ + WHERE account_id = '123456789012' AND table_name = $1", + table, + ) + .await; + out.extend( + documents( + catalog, + "SELECT i.key_schema::text || ' ' || i.projection::text FROM indexes i \ + JOIN tables t ON t.table_id = i.table_id \ + WHERE t.account_id = '123456789012' AND t.table_name = $1 ORDER BY i.index_name", + table, + ) + .await, + ); + out.join(" ") +} + +/// Wait until the table is `ACTIVE` in the catalog. +pub async fn wait_active(catalog: &PgPool, table: &str) { + let deadline = tokio::time::Instant::now() + Duration::from_secs(90); + loop { + let status: Option = sqlx::query_scalar( + "SELECT table_status FROM tables WHERE account_id = $1 AND table_name = $2", + ) + .bind(ACCOUNT) + .bind(table) + .fetch_optional(catalog) + .await + .expect("read the table status"); + if status.as_deref() == Some("ACTIVE") { + return; + } + assert!( + tokio::time::Instant::now() < deadline, + "{table} still {status:?} after 90 s" + ); + tokio::time::sleep(Duration::from_millis(20)).await; + } +} diff --git a/crates/storage-postgres/tests/fixtures/golden/0006_attribute_name_escape.txt b/crates/storage-postgres/tests/fixtures/golden/0006_attribute_name_escape.txt new file mode 100644 index 000000000..723538fd8 --- /dev/null +++ b/crates/storage-postgres/tests/fixtures/golden/0006_attribute_name_escape.txt @@ -0,0 +1,51 @@ +tables raw_tbl + key_schema: [{"KeyType": "HASH", "AttributeName": "a\u0001\u0002\u0001\u0002z"}] + attribute_definitions: [{"AttributeName": "a\u0001\u0002\u0001\u0002z", "AttributeType": "S"}, {"AttributeName": "g\u0001\u0002\u0002k", "AttributeType": "S"}] +tables seed_simple + key_schema: [{"KeyType": "HASH", "AttributeName": "id"}] + attribute_definitions: [{"AttributeName": "id", "AttributeType": "N"}] +tables seed_tbl + key_schema: [{"KeyType": "HASH", "AttributeName": "pk"}, {"KeyType": "RANGE", "AttributeName": "sk"}] + attribute_definitions: [{"AttributeName": "pk", "AttributeType": "S"}, {"AttributeName": "sk", "AttributeType": "S"}, {"AttributeName": "gpk", "AttributeType": "S"}, {"AttributeName": "lsk", "AttributeType": "N"}] +indexes gsi1 + key_schema: [{"KeyType": "HASH", "AttributeName": "gpk"}, {"KeyType": "RANGE", "AttributeName": "sk"}] + projection: {"ProjectionType": "ALL"} +indexes lsi1 + key_schema: [{"KeyType": "HASH", "AttributeName": "pk"}, {"KeyType": "RANGE", "AttributeName": "lsk"}] + projection: {"ProjectionType": "ALL"} +indexes raw_gsi + key_schema: [{"KeyType": "HASH", "AttributeName": "g\u0001\u0002\u0002k"}] + projection: {"ProjectionType": "INCLUDE", "NonKeyAttributes": ["n\u0001\u0002\u0001\u0002"]} +indexes text_gsi + key_schema: [{"KeyType": "HASH", "AttributeName": "lit\\u0001"}] + projection: {"ProjectionType": "KEYS_ONLY"} +backups raw_backup + key_schema: [{"KeyType": "HASH", "AttributeName": "a\u0001\u0002\u0001\u0002z"}] + attribute_definitions: [{"AttributeName": "a\u0001\u0002\u0001\u0002z", "AttributeType": "S"}, {"AttributeName": "g\u0001\u0002\u0002k", "AttributeType": "S"}] + vector_indexes: {"Version": 1, "VectorIndexes": [{"IndexName": "raw_vidx", "Dimensions": 2, "Projection": {"ProjectionType": "ALL"}, "SearchSchema": [{"AttributeName": "t\u0001\u0002\u0002", "SearchSchemaElementType": "HASH"}], "VectorAttribute": {"AttributeName": "v\u0001\u0002\u0001\u0002"}, "DistanceFunction": "COSINE"}]} +backups seed_backup + key_schema: [{"KeyType": "HASH", "AttributeName": "pk"}, {"KeyType": "RANGE", "AttributeName": "sk"}] + attribute_definitions: [{"AttributeName": "pk", "AttributeType": "S"}, {"AttributeName": "sk", "AttributeType": "S"}, {"AttributeName": "gpk", "AttributeType": "S"}, {"AttributeName": "lsk", "AttributeType": "N"}] + vector_indexes: {"Version": 1, "VectorIndexes": []} +backup_definitions raw_backup + definition: {"Version": 1, "BillingMode": "PAY_PER_REQUEST", "LocalSecondaryIndexes": [], "GlobalSecondaryIndexes": [{"IndexName": "raw_gsi", "KeySchema": [{"KeyType": "HASH", "AttributeName": "g\u0001\u0002\u0002k"}], "Projection": {"ProjectionType": "INCLUDE", "NonKeyAttributes": ["n\u0001\u0002\u0001\u0002"]}}]} +vector_indexes raw_vidx + vector_attribute: {"AttributeName": "v\u0001\u0002\u0001\u0002"} + search_schema: [{"AttributeName": "t\u0001\u0002\u0002", "SearchSchemaElementType": "HASH"}] + projection: {"ProjectionType": "ALL"} +gsi_pending 00000000-0000-4000-8000-000000000006 + index_context: {"index": {"index_id": "00000000-0000-4000-8000-000000000016", "key_schema": [{"KeyType": "HASH", "AttributeName": "g\u0001\u0002\u0002k"}], "projection": {"ProjectionType": "INCLUDE", "NonKeyAttributes": ["n\u0001\u0002\u0001\u0002"]}}, "base_key_schema": [{"KeyType": "HASH", "AttributeName": "a\u0001\u0002\u0001\u0002z"}], "attribute_definitions": [{"AttributeName": "a\u0001\u0002\u0001\u0002z", "AttributeType": "S"}, {"AttributeName": "g\u0001\u0002\u0002k", "AttributeType": "S"}]} + new_item: {"a\u0001\u0001z": {"S": "x"}, "g\u0001\u0002k": {"S": "y"}} +catalog notes: +tables.key_schema: 1 document rewritten +tables.attribute_definitions: 1 document rewritten +indexes.key_schema: 1 document rewritten +indexes.projection: 1 document rewritten +backups.key_schema: 1 document rewritten +backups.attribute_definitions: 1 document rewritten +backups.vector_indexes: 1 document rewritten +backup_definitions.definition: 1 document rewritten +vector_indexes.vector_attribute: 1 document rewritten +vector_indexes.search_schema: 1 document rewritten +data notes: +gsi_pending.index_context: 1 document rewritten diff --git a/crates/storage-postgres/tests/migrations_golden.rs b/crates/storage-postgres/tests/migrations_golden.rs index 47d36bb25..5dd412aca 100644 --- a/crates/storage-postgres/tests/migrations_golden.rs +++ b/crates/storage-postgres/tests/migrations_golden.rs @@ -13,31 +13,9 @@ use std::time::Duration; use extenddb_storage::migrations::{CatalogStatus, MigrationError, MigrationId, Runner}; use extenddb_storage_postgres::migrations::{PostgresMigrations, migrate}; -use migrations_support::{ACCOUNT, LATEST, Scratch, base_conn, id_names, skip}; +use migrations_support::{ACCOUNT, LATEST, Scratch, base_conn, golden, id_names, notes, skip}; use sqlx::PgPool; -fn golden(name: &str, actual: &str) { - let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) - .join("tests/fixtures/golden") - .join(name); - if migrations_support::bless() { - std::fs::create_dir_all(path.parent().unwrap()).unwrap(); - std::fs::write(&path, actual).unwrap(); - return; - } - let expected = - std::fs::read_to_string(&path).unwrap_or_else(|e| panic!("{}: {e}", path.display())); - assert_eq!(actual, expected, "{name} differs from its golden file"); -} - -async fn notes(catalog: &PgPool, id: i32) -> String { - sqlx::query_scalar("SELECT notes FROM extenddb_migrations WHERE id = $1") - .bind(id) - .fetch_one(catalog) - .await - .unwrap() -} - const TAGGED: &str = r#"{"Version":"2012-10-17","Statement":[{"Effect":"Deny","Action":"dynamodb:*","Resource":"*","Condition":{"StringEquals":{"dynamodb:ResourceTag/env":"prod"}}}]}"#; /// Both forms on one key with different values, which migration 2 refuses. const COLLIDING: &str = r#"{"Version":"2012-10-17","Statement":[{"Effect":"Deny","Action":"dynamodb:*","Resource":"*","Condition":{"StringEquals":{"dynamodb:ResourceTag/env":"prod","aws:ResourceTag/env":"dev"}}}]}"#; diff --git a/crates/storage-postgres/tests/migrations_name_escape.rs b/crates/storage-postgres/tests/migrations_name_escape.rs new file mode 100644 index 000000000..9c156fb6a --- /dev/null +++ b/crates/storage-postgres/tests/migrations_name_escape.rs @@ -0,0 +1,334 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 +//! Golden tests of migration 6, `attribute_name_escape`, on PostgreSQL: the +//! migration rewrites the raw names a release before it stored, and a +//! reconverge leaves the names written after it as they are. The rewritten +//! rows and the ledger notes are compared with +//! `tests/fixtures/golden/0006_attribute_name_escape.txt`; +//! `EXTENDDB_BLESS_EXPECTED=1` writes the file instead. See +//! `migrations_support` for the database these tests need. + +mod migrations_support; + +use std::fmt::Write as _; + +use extenddb_core::types::{CreateTableInput, DescribeTableInput}; +use extenddb_storage::migrations::{CatalogStatus, MigrationId, Runner}; +use extenddb_storage::{BackupEngine, TableEngine}; +use extenddb_storage_postgres::migrations::PostgresMigrations; +use extenddb_storage_postgres::{PostgresConfig, PostgresEngine}; +use migrations_support::{ACCOUNT, LATEST, Scratch, base_conn, golden, notes, skip}; +use serde_json::json; +use sqlx::PgPool; + +/// Names a release before migration 6 stored raw: a U+0001 pair, which the +/// escape's decoder reads as U+0000, and U+0001 U+0002, which it reads as +/// U+0001. +const RAW_KEY: &str = "a\u{1}\u{1}z"; +const RAW_GSI_KEY: &str = "g\u{1}\u{2}k"; +const RAW_PROJECTED: &str = "n\u{1}\u{1}"; +const RAW_VECTOR: &str = "v\u{1}\u{1}"; +const RAW_HASH: &str = "t\u{1}\u{2}"; +const RAW_TABLE_ID: &str = "00000000-0000-4000-8000-000000000006"; +const RAW_INDEX_ID: &str = "00000000-0000-4000-8000-000000000016"; + +/// An engine on the scratch catalog. +async fn engine(s: &Scratch) -> PostgresEngine { + PostgresEngine::new( + &PostgresConfig { + connection_string: s.catalog_url(), + pool_size: 2, + max_item_size_bytes: 400_000, + }, + "us-east-1", + ) + .await + .expect("open an engine") +} + +/// The raw documents of a table, its GSI, a backup, a vector index and a +/// queued index update, written by SQL as a release before migration 6 +/// wrote them. A second GSI's name holds the six characters `\u0001`: the +/// rewrite finds its documents and leaves them as they are. +async fn seed_raw_names(catalog: &PgPool, data: &PgPool) { + let ks = json!([{"AttributeName": RAW_KEY, "KeyType": "HASH"}]); + let ad = json!([{"AttributeName": RAW_KEY, "AttributeType": "S"}, + {"AttributeName": RAW_GSI_KEY, "AttributeType": "S"}]); + let gsi_ks = json!([{"AttributeName": RAW_GSI_KEY, "KeyType": "HASH"}]); + let projection = json!({"ProjectionType": "INCLUDE", "NonKeyAttributes": [RAW_PROJECTED]}); + sqlx::query( + "INSERT INTO tables (account_id, table_name, key_schema, attribute_definitions, \ + table_status, table_arn, table_id) VALUES ($1, 'raw_tbl', $2, $3, 'ACTIVE', \ + 'arn:raw_tbl', $4)", + ) + .bind(ACCOUNT) + .bind(&ks) + .bind(&ad) + .bind(RAW_TABLE_ID) + .execute(catalog) + .await + .unwrap(); + for (index_id, name, key_schema, projection) in [ + (RAW_INDEX_ID, "raw_gsi", gsi_ks.clone(), projection.clone()), + ( + "00000000-0000-4000-8000-000000000017", + "text_gsi", + json!([{"AttributeName": "lit\\u0001", "KeyType": "HASH"}]), + json!({"ProjectionType": "KEYS_ONLY"}), + ), + ] { + sqlx::query( + "INSERT INTO indexes (table_id, index_id, index_name, index_type, key_schema, \ + projection) VALUES ($1, $2, $3, 'GSI', $4, $5)", + ) + .bind(RAW_TABLE_ID) + .bind(index_id) + .bind(name) + .bind(&key_schema) + .bind(&projection) + .execute(catalog) + .await + .unwrap(); + } + let vector_attribute = json!({"AttributeName": RAW_VECTOR}); + let search_schema = json!([{"AttributeName": RAW_HASH, "SearchSchemaElementType": "HASH"}]); + let all = json!({"ProjectionType": "ALL"}); + sqlx::query( + "INSERT INTO vector_indexes (table_id, index_name, index_id, dimensions, \ + distance_function, vector_attribute, search_schema, projection, index_status, \ + backfilling) VALUES ($1, 'raw_vidx', '00000000-0000-4000-8000-000000000026', 2, \ + 'COSINE', $2, $3, $4, 'ACTIVE', NULL)", + ) + .bind(RAW_TABLE_ID) + .bind(&vector_attribute) + .bind(&search_schema) + .bind(&all) + .execute(catalog) + .await + .unwrap(); + let snapshot = json!({"Version": 1, "VectorIndexes": [{"IndexName": "raw_vidx", + "Dimensions": 2, "DistanceFunction": "COSINE", "VectorAttribute": vector_attribute, + "SearchSchema": search_schema, "Projection": all}]}); + sqlx::query( + "INSERT INTO backups (backup_arn, backup_name, table_id, table_name, account_id, \ + key_schema, attribute_definitions, vector_indexes) \ + VALUES ('arn:raw_backup', 'raw_backup', $1, 'raw_tbl', $2, $3, $4, $5)", + ) + .bind(RAW_TABLE_ID) + .bind(ACCOUNT) + .bind(&ks) + .bind(&ad) + .bind(&snapshot) + .execute(catalog) + .await + .unwrap(); + let definition = json!({"Version": 1, "BillingMode": "PAY_PER_REQUEST", + "GlobalSecondaryIndexes": [{"IndexName": "raw_gsi", "KeySchema": gsi_ks, + "Projection": projection}], + "LocalSecondaryIndexes": []}); + sqlx::query( + "INSERT INTO backup_definitions (backup_arn, definition) VALUES ('arn:raw_backup', $1)", + ) + .bind(&definition) + .execute(catalog) + .await + .unwrap(); + // The item image is item data, which the migration leaves raw. + let context = json!({"base_key_schema": ks, "attribute_definitions": ad, + "index": {"index_id": RAW_INDEX_ID, "key_schema": gsi_ks, "projection": projection}}); + let new_item = json!({RAW_KEY: {"S": "x"}, RAW_GSI_KEY: {"S": "y"}}); + sqlx::query( + "INSERT INTO gsi_pending (table_id, worker_partition, old_item, new_item, \ + index_context) VALUES ($1, 0, NULL, $2, $3)", + ) + .bind(RAW_TABLE_ID) + .bind(&new_item) + .bind(&context) + .execute(data) + .await + .unwrap(); +} + +/// Each row's documents as `jsonb::text`, under a label, sorted by byte +/// order. +async fn dump_documents(pool: &PgPool, sql: &str, out: &mut String) { + let rows: Vec<(String, Vec)> = sqlx::query_as(sql).fetch_all(pool).await.unwrap(); + for (label, columns) in rows { + writeln!(out, "{label}").unwrap(); + for column in columns { + writeln!(out, " {column}").unwrap(); + } + } +} + +#[tokio::test] +async fn migration_6_rewrites_the_raw_names_of_every_document_that_carries_them() { + let Some(base) = base_conn() else { + return skip("golden 6"); + }; + let s = Scratch::new(&base, "pgm_g6").await; + s.load("0.1.14").await; + s.migrate_through(MigrationId::DropContinuousBackups).await; + let (catalog, data) = (s.catalog().await, s.data().await); + seed_raw_names(&catalog, &data).await; + s.migrate_through(MigrationId::AttributeNameEscape).await; + + let mut out = String::new(); + for sql in [ + "SELECT 'tables ' || table_name, ARRAY['key_schema: ' || key_schema::text, \ + 'attribute_definitions: ' || attribute_definitions::text] FROM tables \ + ORDER BY table_name COLLATE \"C\"", + "SELECT 'indexes ' || index_name, ARRAY['key_schema: ' || key_schema::text, \ + 'projection: ' || projection::text] FROM indexes ORDER BY index_name COLLATE \"C\"", + "SELECT 'backups ' || backup_name, ARRAY['key_schema: ' || key_schema::text, \ + 'attribute_definitions: ' || attribute_definitions::text, \ + 'vector_indexes: ' || coalesce(vector_indexes::text, 'NULL')] FROM backups \ + ORDER BY backup_name COLLATE \"C\"", + "SELECT 'backup_definitions ' || b.backup_name, ARRAY['definition: ' || \ + d.definition::text] FROM backup_definitions d JOIN backups b \ + ON b.backup_arn = d.backup_arn ORDER BY b.backup_name COLLATE \"C\"", + "SELECT 'vector_indexes ' || index_name, ARRAY['vector_attribute: ' || \ + vector_attribute::text, 'search_schema: ' || coalesce(search_schema::text, 'NULL'), \ + 'projection: ' || projection::text] FROM vector_indexes \ + ORDER BY index_name COLLATE \"C\"", + ] { + dump_documents(&catalog, sql, &mut out).await; + } + dump_documents( + &data, + "SELECT 'gsi_pending ' || table_id, ARRAY['index_context: ' || index_context::text, \ + 'new_item: ' || coalesce(new_item::text, 'NULL')] FROM gsi_pending \ + ORDER BY table_id COLLATE \"C\", id", + &mut out, + ) + .await; + writeln!(out, "catalog notes:\n{}", notes(&catalog, 6).await).unwrap(); + writeln!(out, "data notes:\n{}", notes(&data, 6).await).unwrap(); + golden("0006_attribute_name_escape.txt", &out); + + // This binary reads the rewritten names as the release wrote them. + let engine = engine(&s).await; + let desc = engine + .describe_table( + ACCOUNT, + DescribeTableInput { + table_name: "raw_tbl".to_owned(), + }, + ) + .await + .expect("DescribeTable"); + assert_eq!(desc.key_schema[0].attribute_name, RAW_KEY); + let gsi = desc + .global_secondary_indexes + .iter() + .flatten() + .find(|g| g.index_name == "raw_gsi") + .expect("raw_gsi"); + assert_eq!(gsi.key_schema[0].attribute_name, RAW_GSI_KEY); + assert_eq!( + gsi.projection.non_key_attributes.as_deref(), + Some(&[RAW_PROJECTED.to_owned()][..]) + ); + let text = desc + .global_secondary_indexes + .iter() + .flatten() + .find(|g| g.index_name == "text_gsi") + .expect("text_gsi"); + assert_eq!(text.key_schema[0].attribute_name, "lit\\u0001"); + let vector = &desc.vector_indexes.expect("vector indexes")[0]; + assert_eq!(vector.vector_attribute.attribute_name, RAW_VECTOR); + let schema = vector.search_schema.as_deref().expect("a search schema"); + assert_eq!(schema[0].attribute_name, RAW_HASH); + let source = engine + .backup_restore_source(ACCOUNT, "arn:raw_backup") + .await + .expect("the restore source"); + assert_eq!(source.key_schema[0].attribute_name, RAW_KEY); + let definition = source.definition.expect("the definition"); + assert_eq!( + definition.global_secondary_indexes[0].key_schema[0].attribute_name, + RAW_GSI_KEY + ); + drop(engine); + catalog.close().await; + data.close().await; +} + +/// Migration 6 is not convergent: a reconverge leaves the documents this +/// release wrote after it, through the escape, as they are, and the names +/// still read back as written. +#[tokio::test] +async fn a_reconverge_leaves_the_names_written_after_migration_6_as_they_are() { + let Some(base) = base_conn() else { + return skip("reconverge after migration 6"); + }; + let s = Scratch::new(&base, "pgm_g6rc").await; + s.load("0.1.14").await; + s.migrate_through(MigrationId::LATEST).await; + let catalog = s.catalog().await; + let engine = engine(&s).await; + let input: CreateTableInput = serde_json::from_value(json!({ + "TableName": "escaped_tbl", + "KeySchema": [{"AttributeName": "a\u{0}z", "KeyType": "HASH"}, + {"AttributeName": RAW_KEY, "KeyType": "RANGE"}], + "AttributeDefinitions": [{"AttributeName": "a\u{0}z", "AttributeType": "S"}, + {"AttributeName": RAW_KEY, "AttributeType": "S"}], + "BillingMode": "PAY_PER_REQUEST" + })) + .unwrap(); + engine + .create_table(ACCOUNT, input) + .await + .expect("CreateTable"); + let stored = || async { + sqlx::query_scalar::<_, String>( + "SELECT key_schema::text || ' ' || attribute_definitions::text FROM tables \ + WHERE table_name = 'escaped_tbl'", + ) + .fetch_one(&catalog) + .await + .unwrap() + }; + let before = stored().await; + assert!(before.contains("\"a\\u0001\\u0001z\""), "{before}"); + + sqlx::query("INSERT INTO settings (key, value) VALUES ('resource_tag_migration', 'applied')") + .execute(&catalog) + .await + .unwrap(); + assert_eq!(s.status().await.label(), "WrittenByOlderRelease"); + let m = PostgresMigrations::open_unlocked(&s.target()) + .await + .unwrap(); + let report = Runner::new(&m, "test").reconverge().await; + m.close().await; + let report = report.expect("reconverge"); + assert!(report.applied.iter().all(|step| step.id != 6), "{report:?}"); + assert!( + report + .notes + .iter() + .any(|n| n.starts_with("Skipped 6 attribute_name_escape:")), + "{report:?}" + ); + assert_eq!(s.status().await, CatalogStatus::Current { version: LATEST }); + assert_eq!(stored().await, before); + let desc = engine + .describe_table( + ACCOUNT, + DescribeTableInput { + table_name: "escaped_tbl".to_owned(), + }, + ) + .await + .expect("DescribeTable"); + let names: Vec<&str> = desc + .key_schema + .iter() + .map(|k| k.attribute_name.as_str()) + .collect(); + assert_eq!(names, ["a\u{0}z", RAW_KEY]); + drop(engine); + catalog.close().await; +} diff --git a/crates/storage-postgres/tests/migrations_support/mod.rs b/crates/storage-postgres/tests/migrations_support/mod.rs index 31646f90e..f224eabd8 100644 --- a/crates/storage-postgres/tests/migrations_support/mod.rs +++ b/crates/storage-postgres/tests/migrations_support/mod.rs @@ -566,3 +566,28 @@ pub fn note_steps(note: &str) -> Vec<(String, String)> { } steps } + +/// Compare `actual` with `tests/fixtures/golden/`, or write the file +/// when `EXTENDDB_BLESS_EXPECTED=1`. +pub fn golden(name: &str, actual: &str) { + let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures/golden") + .join(name); + if bless() { + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(&path, actual).unwrap(); + return; + } + let expected = + std::fs::read_to_string(&path).unwrap_or_else(|e| panic!("{}: {e}", path.display())); + assert_eq!(actual, expected, "{name} differs from its golden file"); +} + +/// The ledger notes of migration `id` in the store `catalog` connects to. +pub async fn notes(catalog: &PgPool, id: i32) -> String { + sqlx::query_scalar("SELECT notes FROM extenddb_migrations WHERE id = $1") + .bind(id) + .fetch_one(catalog) + .await + .unwrap() +} diff --git a/crates/storage-sqlite/src/migrations/mod.rs b/crates/storage-sqlite/src/migrations/mod.rs index 06647fbb4..4c316aa40 100644 --- a/crates/storage-sqlite/src/migrations/mod.rs +++ b/crates/storage-sqlite/src/migrations/mod.rs @@ -106,6 +106,11 @@ pub fn migration(id: MigrationId) -> Migration { "0005_drop_continuous_backups.sql" )], }, + MigrationId::AttributeNameEscape => Migration::NotApplicable( + "SQLite stores the documents that carry attribute names as JSON text in TEXT columns, \ + where U+0000 is the six-character escape \\u0000, so every name reads back as it was \ + written", + ), } } diff --git a/crates/storage-sqlite/src/migrations/released_tests.rs b/crates/storage-sqlite/src/migrations/released_tests.rs index 71dcee691..48d658040 100644 --- a/crates/storage-sqlite/src/migrations/released_tests.rs +++ b/crates/storage-sqlite/src/migrations/released_tests.rs @@ -9,7 +9,7 @@ use std::collections::BTreeMap; use std::path::Path; use extenddb_storage::MetadataEngine; -use extenddb_storage::migrations::{CatalogStatus, MigrationId, Runner, exit}; +use extenddb_storage::migrations::{CatalogStatus, Migration, MigrationId, Runner, exit}; use super::SqliteMigrations; use super::postcheck; @@ -390,11 +390,12 @@ async fn a_catalog_an_older_release_wrote_to_is_reported_and_reconverged() { next_second().await; let report = reconverge(&db).await.expect("reconverge"); - // Every convergent migration ran again; migration 2, which is not - // convergent, is named as skipped. + // Every convergent migration with a body on this backend ran again; + // migration 2, which is not convergent, is named as skipped. let convergent = MigrationId::ALL .into_iter() - .filter(|id| super::migration(*id).convergent()) + .map(super::migration) + .filter(|m| matches!(m, Migration::Applies { .. }) && m.convergent()) .count(); assert_eq!(report.applied.len(), convergent, "{trace}: {report:?}"); let skipped = format!("Skipped {}:", MigrationId::ResourceTagConditionKeys); diff --git a/crates/storage-sqlite/src/migrations/tests.rs b/crates/storage-sqlite/src/migrations/tests.rs index a837e802e..05e8a03b6 100644 --- a/crates/storage-sqlite/src/migrations/tests.rs +++ b/crates/storage-sqlite/src/migrations/tests.rs @@ -14,8 +14,9 @@ use std::time::Duration; use async_trait::async_trait; use extenddb_storage::migrations::{ - Body, CatalogStatus, CodeBody, Done, LedgerRow, MigrationBackend, MigrationError, MigrationId, - Outcome, RELEASED_THROUGH, Runner, Step, StoreBody, StoreKind, StoreRef, check_released, + Body, CatalogStatus, CodeBody, Done, LedgerRow, Migration, MigrationBackend, MigrationError, + MigrationId, Outcome, RELEASED_THROUGH, Runner, Step, StoreBody, StoreKind, StoreRef, + check_released, }; use sqlx::SqlitePool; use sqlx::sqlite::SqlitePoolOptions; @@ -264,12 +265,22 @@ async fn a_fresh_database_is_current_after_one_run_and_seeds_nothing() { status(&db).await, CatalogStatus::Current { version: LATEST } ); + // A row for every id: `applied` for the ids with a body on this backend, + // `not_applicable` for the others. + let applies = MigrationId::ALL + .into_iter() + .filter(|id| matches!(migration(*id), Migration::Applies { .. })) + .count(); assert_eq!( count( &db, "SELECT COUNT(*) FROM extenddb_migrations WHERE outcome = 'applied'" ) .await, + i64::try_from(applies).expect("a count fits i64") + ); + assert_eq!( + count(&db, "SELECT COUNT(*) FROM extenddb_migrations").await, i64::from(LATEST) ); assert_eq!(version_row(&db).await, Some(LATEST.to_string())); diff --git a/crates/storage/src/migrations/registry.rs b/crates/storage/src/migrations/registry.rs index dc426fd18..5e8c750b3 100644 --- a/crates/storage/src/migrations/registry.rs +++ b/crates/storage/src/migrations/registry.rs @@ -67,11 +67,15 @@ migration_ids! { /// it. DropContinuousBackups = 5, "drop_continuous_backups", "drop the continuous_backups table, which no request reads or writes since #335"; + /// Rewrites the stored documents that carry attribute names through the + /// escape that lets a PostgreSQL catalog hold U+0000 in a name (#367). + AttributeNameEscape = 6, "attribute_name_escape", + "store attribute names through the escape that lets PostgreSQL jsonb hold U+0000"; } /// Highest id that has shipped in a release. Bodies at or below it are frozen: /// each backend pins their checksums, kinds and convergent flags, and -/// `forget` refuses them. The v1.0 release PR raises it to 5. +/// `forget` refuses them. The v1.0 release PR raises it to 6. pub const RELEASED_THROUGH: u32 = 0; impl MigrationId { diff --git a/docs/adr/0023-catalog-migrations.md b/docs/adr/0023-catalog-migrations.md index eeeb98a54..b83f1e3ac 100644 --- a/docs/adr/0023-catalog-migrations.md +++ b/docs/adr/0023-catalog-migrations.md @@ -365,8 +365,8 @@ that each backend implements, as follows. to run. - Rollback after additive migrations runs the previous 1.x binary. Rollback after a breaking migration restores the backup taken before the upgrade. - All five v1.0 migrations are breaking, so rolling back from 1.0 is a - restore. + Each v1.0 migration is breaking on every backend it applies to, so + rolling back from 1.0 is a restore. - Releases before 1.0 refuse a 1.0 catalog or leave a trace in it. PostgreSQL `serve` refuses the integer version row, and v0.1.14 `serve` on every backend refuses a catalog without the `resource_tag_migration` row, which diff --git a/docs/differences-from-dynamodb.md b/docs/differences-from-dynamodb.md index 0686d8445..2b95929c6 100755 --- a/docs/differences-from-dynamodb.md +++ b/docs/differences-from-dynamodb.md @@ -52,6 +52,7 @@ from a request sent to that service or backend. |------|----------|------| | Table creation delay | Returns `CREATING` immediately; transitions to `ACTIVE` typically within seconds. Same behavior for on-demand and provisioned | Configurable via `control_plane_delay_seconds` runtime setting (default: 5s) | | DeletionProtectionEnabled | Enforced | Enforced (accepted and stored, DeleteTable rejects when enabled) | +| U+0000 in a key attribute name, an index key attribute name or a projected attribute name | Accepted; `DescribeTable` returns each name byte for byte (measured 2026-09-18 and 2026-10-24, and again on 2026-11-04) | The same on all four backends. PostgreSQL `jsonb` cannot hold U+0000, so a PostgreSQL deployment stores these names through an escape, U+0000 as U+0001 U+0001 and U+0001 as U+0001 U+0002. Catalog migration 6 applies the escape to the names an earlier release stored (see the [Upgrade Manual](manuals/07-upgrade-manual.md#general-upgrade-procedure)) | ## Time to Live (TTL) diff --git a/docs/getting-started.md b/docs/getting-started.md index 712f34cfe..b3e6ef366 100755 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -269,7 +269,7 @@ Config: extenddb.toml Catalog database: extenddb_catalog --- Checking catalog migrations... - OK: Current: the catalog is at migration 5 + OK: Current: the catalog is at migration 6 --- Checking catalog connection... OK: Connected to catalog. --- Checking data database... @@ -287,7 +287,7 @@ extenddb runs as a daemon (background process) and logs to syslog. On startup it ```bash ./target/release/extenddb serve --config extenddb.toml -# extenddb 0.1.14 (catalog migration 5) starting on 127.0.0.1:18443 +# extenddb 0.1.14 (catalog migration 6) starting on 127.0.0.1:18443 # storage: postgres (postgresql://extenddb:***@localhost:5432/extenddb_catalog) ``` @@ -1366,7 +1366,7 @@ Each runner requires its tools to be installed. The runner checks prerequisites ```bash ./target/release/extenddb version # extenddb 0.1.14 -# catalog migration 5 (postgres) +# catalog migration 6 (postgres) # commit abc1234 # built 2026-04-17T12:00:00Z ``` diff --git a/docs/manuals/04-quickstart-setup-guide.md b/docs/manuals/04-quickstart-setup-guide.md index 6e8339c6b..08333d9dd 100755 --- a/docs/manuals/04-quickstart-setup-guide.md +++ b/docs/manuals/04-quickstart-setup-guide.md @@ -130,7 +130,7 @@ Check the version: ```bash ./target/release/extenddb version # extenddb 0.1.14 -# catalog migration 5 (postgres) +# catalog migration 6 (postgres) # commit abc1234 # built 2026-04-17T12:00:00Z ``` @@ -172,7 +172,7 @@ Config: extenddb.toml Catalog database: extenddb_catalog --- Checking catalog migrations... - OK: Current: the catalog is at migration 5 + OK: Current: the catalog is at migration 6 --- Checking catalog connection... OK: Connected to catalog. --- Checking data database... diff --git a/docs/manuals/05-admin-guide.md b/docs/manuals/05-admin-guide.md index e51da206c..e0ff8162f 100755 --- a/docs/manuals/05-admin-guide.md +++ b/docs/manuals/05-admin-guide.md @@ -925,7 +925,7 @@ Check that PostgreSQL is running and the connection string in `extenddb.toml` is **The catalog has pending migrations, or is ahead of the binary:** -On PostgreSQL, SQLite, Cassandra and MongoDB, `serve` reads the catalog's migration status before it opens the engine and refuses a catalog it cannot serve, printing the status and the command that resolves it. A catalog with pending migrations exits 3, for example `the catalog is at migration 4, and ExtendDB needs migration 5`, followed by the pending migrations and `extenddb migrate --config extenddb.toml --yes`. A catalog that a newer release migrated with a breaking migration exits 4 and names that release's migrations; upgrade the binary rather than the catalog. The [Upgrade Manual](07-upgrade-manual.md#the-catalog-status) lists every status with its exit code and the stop / migrate / start sequence. +On PostgreSQL, SQLite, Cassandra and MongoDB, `serve` reads the catalog's migration status before it opens the engine and refuses a catalog it cannot serve, printing the status and the command that resolves it. A catalog with pending migrations exits 3, for example `the catalog is at migration 5, and ExtendDB needs migration 6`, followed by the pending migrations and `extenddb migrate --config extenddb.toml --yes`. A catalog that a newer release migrated with a breaking migration exits 4 and names that release's migrations; upgrade the binary rather than the catalog. The [Upgrade Manual](07-upgrade-manual.md#the-catalog-status) lists every status with its exit code and the stop / migrate / start sequence. ### Authentication Errors diff --git a/docs/manuals/07-upgrade-manual.md b/docs/manuals/07-upgrade-manual.md index 147890c71..79218ca2e 100755 --- a/docs/manuals/07-upgrade-manual.md +++ b/docs/manuals/07-upgrade-manual.md @@ -4,7 +4,7 @@ ## Current Status -This release versions the PostgreSQL, SQLite, Cassandra and MongoDB catalogs by a migration ledger and carries catalog migrations 1 to 5, which `extenddb migrate --list` prints. **Every deployment created by a 0.1.x release runs `extenddb migrate --yes` once** before a server of this release starts on it: the server refuses a catalog with pending migrations. Migration 1 brings the catalog of every 0.1.x release to the v1.0 schema, and migration 5 drops `continuous_backups`, a table and on MongoDB a collection (see [Catalog 0.0.5](#catalog-005)). +This release versions the PostgreSQL, SQLite, Cassandra and MongoDB catalogs by a migration ledger and carries catalog migrations 1 to 6, which `extenddb migrate --list` prints. **Every deployment created by a 0.1.x release runs `extenddb migrate --yes` once** before a server of this release starts on it: the server refuses a catalog with pending migrations. Migration 1 brings the catalog of every 0.1.x release to the v1.0 schema, and migration 5 drops `continuous_backups`, a table and on MongoDB a collection (see [Catalog 0.0.5](#catalog-005)). Migration 6 rewrites the attribute names that a PostgreSQL deployment stores in its catalog, so that a name can hold U+0000. It does not apply to the other backends (see step 4 of the [general upgrade procedure](#general-upgrade-procedure)). A backup that a 0.1.x release took can restore without its global and local secondary indexes: a SQLite backup taken by 0.1.14 records only the table's key schema and attribute definitions, and its restore creates the table with its items and no index. Take a new backup of each table after upgrading to v1.0, and keep the 0.1.x backups for their items only. @@ -39,7 +39,7 @@ Exit status 6 means the migration lock, or a running server, was still held when `verify` also checks the connection to the data database. When that check fails, `verify` exits 1 on a Behind catalog instead of 3, because `migrate --yes` cannot apply the pending migrations while the data database cannot be reached. It still exits 4 or 5 for the statuses that need a person. -A migration is additive when the previous release serves the catalog correctly with it applied, and breaking otherwise. Migrations 1 to 5 are breaking: do not run a 0.1.x server against a migrated catalog. A 0.1.x binary that writes to a migrated catalog (its `migrate --yes`, or a SQLite dev server) leaves it WrittenByOlderRelease. `migrate --yes --reconverge` repairs it by running the convergent migrations again. Migration 2, which rewrites stored policies, is not convergent: the repair skips it and prints a line that says so (see the v0.1.14 notes below). +A migration is additive when the previous release serves the catalog correctly with it applied, and breaking otherwise. Each migration is breaking on every backend it applies to: do not run a 0.1.x server against a migrated catalog. A 0.1.x binary that writes to a migrated catalog (its `migrate --yes`, or a SQLite dev server) leaves it WrittenByOlderRelease. `migrate --yes --reconverge` repairs it by running the convergent migrations again. Migration 2, which rewrites stored policies, is not convergent: the repair skips it and prints a line that says so (see the v0.1.14 notes below). ## Writing a New Migration @@ -93,6 +93,8 @@ A server notices at its next renewal, within 15 s, that the database ended the s The two transactions that hold the keys, the server's registration and the migration lock, turn off `idle_in_transaction_session_timeout` for themselves, and `transaction_timeout` on PostgreSQL 17 and later, so a role or database setting of either does not end them. PgBouncer's `idle_transaction_timeout` is a setting of PgBouncer, which a transaction cannot change. Each holder runs `SELECT 1` every 15 s, so set `idle_transaction_timeout` to 0, its default, or to 30 s or more. A value under 15 s closes the server's connection between two renewals, and the server then registers again every time. Each time it also stops its background workers and starts a new set, and each worker waits its interval before its first run, so a worker whose interval is longer than the 15 s between two renewals never runs: among them the TTL scan (60 s), the table size refresh (300 s), and the idempotency token and stream record cleanups (600 s and 3600 s). +On PostgreSQL, migration 6, `attribute_name_escape`, rewrites the stored documents that carry attribute names. DynamoDB accepts U+0000 in an attribute name, and PostgreSQL `jsonb` cannot hold it. So a server of this release stores U+0000 in these documents as U+0001 U+0001, and U+0001 as U+0001 U+0002. It reverses the change when it reads them. Earlier releases stored the documents as written. In such a document, a name that holds U+0001 followed by U+0001 or U+0002 would read back as another name. A table keyed on that name would no longer find its items. Migration 6 applies the escape once to each document that holds U+0001 and leaves the others as they are. 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, which holds the index updates a stopped server left queued. Items are not rewritten. `migrate --yes` prints a note for each column it rewrote, for example `tables.key_schema: 1 document rewritten`, and the ledger keeps the notes. Each database's body commits in one transaction with its ledger row. A second run would change the names written since the first, so migration 6 is not convergent: `migrate --yes --reconverge` skips it. Migration 6 does not apply to SQLite, Cassandra and MongoDB. SQLite and Cassandra store these documents as JSON text, where U+0000 is the six characters `\u0000`, and MongoDB's BSON strings hold U+0000. + On MongoDB, `migrate` connects with the connection string in the config file, which the server uses too. A user with the `readWrite` role on `extenddb_catalog` and on `extenddb_data` and no other role can run `init`, `migrate --yes`, `verify` and `serve`. `migrate --yes` takes the migration lock, a document in the catalog's `settings` collection, and then waits for the registration every running server keeps in the catalog's `extenddb_servers` collection, and after `--lock-timeout` exits 6 naming each server's host and pid. A server renews its registration every 5 s, and every second after a renewal failed, and deletes it when it stops, once every background worker has ended (it leaves it to expire when one could not be stopped); a server that was killed leaves it until it expires, at most 45 s after its last renewal. A server whose registration was deleted, or that has not renewed it for 30 s, for example because the primary stopped answering, answers new requests with `ServiceUnavailable` and registers again, as a PostgreSQL server does after its session ends (above): it serves again once it holds a new registration and the catalog's status is the one it started with, and it stops with exit 1 when a migration holds the lock, when the status changed, or when 30 s pass without a registration. The lock and the registrations expire by the clock of the replica set's primary, so keep the clocks of its members in sync with NTP. Migration 1 deletes the `resource_tag_migration` settings document in the transaction that records its catalog step, so a 0.1.14 server refuses the migrated catalog, and a run that fails before that transaction leaves the document in place. 0.1.12 and 0.1.13 servers read neither that document nor the catalog version, so they start on a migrated catalog and serve it: do not run them against it. To go back to a 0.1.x release, restore the backup taken before `migrate --yes`. A 0.1.x `migrate` on a migrated catalog reports a pending change, and its `--yes` writes the settings document back; this release's status then reports the catalog as written by an older release, and `migrate --yes --reconverge` repairs it. On Cassandra, `migrate` connects as the role the config file names, which the server connects as too. `--cassandra-user ` connects as another role, with its password in `EXTENDDB_CASSANDRA_PASSWORD` or `--cassandra-pass`; a wrong password is reported as a connection timeout. Migration 1 adopts a 0.1.x catalog: it keeps every setting except the `resource_tag_migration` row, which it deletes, and records itself in a ledger in the catalog keyspace and in every account keyspace. On a cluster that runs `CassandraAuthorizer`, `init` grants the application role SELECT, MODIFY, CREATE and ALTER on the catalog keyspace, those four and DROP on the default account's keyspace, and CREATE ON ALL KEYSPACES, which CreateAccount needs to create a keyspace per account and which also lets the role create tables in every keyspace on the cluster; a run of `migrate --yes --cassandra-user ` that creates a missing account keyspace grants the application role the five permissions on it. A 0.1.x `init` granted the application role nothing, so a deployment created before 1.0 on such a cluster needs the statements below before `migrate`. `migrate --yes` takes the migration lock, a row in the catalog's `settings` table written by lightweight transactions, and then waits for the registration every running server of this release keeps in the catalog's `extenddb_servers` table, and after `--lock-timeout` exits 6 naming each server. A server renews its registration every 5 s, and every second after a renewal failed, and deletes it when it stops, once every background worker has ended (it leaves it to expire when one could not be stopped); a server that was killed leaves it until it expires, at most 45 s after its last renewal. A server tries its renewals for 30 s after the last one that succeeded, so it keeps serving through an outage of 25 to 30 s, depending on when the outage starts, for example while the only replica of the catalog restarts. After that, 30 to 35 s after its last successful renewal, it answers new requests with `ServiceUnavailable` and registers again, as a PostgreSQL server does after its session ends (above), and stops with exit 1 when a migration holds the lock, when the status changed, or when 30 s pass without a registration. A migration needs every replica of the catalog keyspace reachable: the lock is written at `SERIAL`, the registrations are read at `ALL`, and after each schema statement the runner waits until every live node reports the same schema version. `serve` refuses a catalog with pending migrations, exiting 3 under `--foreground` with the pending migrations and the `extenddb migrate --config --yes` command to run. A 0.1.x server refuses a migrated catalog. diff --git a/docs/manuals/08-install-linux.md b/docs/manuals/08-install-linux.md index 928ba957d..2b868522e 100755 --- a/docs/manuals/08-install-linux.md +++ b/docs/manuals/08-install-linux.md @@ -118,7 +118,7 @@ Expected: ``` === extenddb verify === ... - OK: Current: the catalog is at migration 5 + OK: Current: the catalog is at migration 6 ... === HEALTHY: All checks passed === ``` diff --git a/docs/manuals/09-install-macos.md b/docs/manuals/09-install-macos.md index 85ba0f1e8..b1ca6de2d 100755 --- a/docs/manuals/09-install-macos.md +++ b/docs/manuals/09-install-macos.md @@ -96,7 +96,7 @@ Expected: ``` === extenddb verify === ... - OK: Current: the catalog is at migration 5 + OK: Current: the catalog is at migration 6 ... === HEALTHY: All checks passed === ``` diff --git a/tests/python/test_nul_attribute_names.py b/tests/python/test_nul_attribute_names.py new file mode 100644 index 000000000..5b66b9129 --- /dev/null +++ b/tests/python/test_nul_attribute_names.py @@ -0,0 +1,226 @@ +# Copyright 2026 ExtendDB contributors +# SPDX-License-Identifier: Apache-2.0 + +"""U+0000 and U+0001 in the attribute names of a table definition. + +DynamoDB accepts U+0000 inside a key schema attribute name (the table's or a +secondary index's), an attribute definition and a projected attribute name, +and DescribeTable returns each name byte for byte. Measured against the +service on 2026-09-18 (partition key and GSI key rows) and 2026-10-24 (LSI key +and projected name row), and again on 2026-11-04 in us-east-1 for those four +rows. The SDK's parameter validation checks no pattern on an attribute name, +so the request reaches the server with the name as written. + +On PostgreSQL the catalog stores these names through an escape (U+0000 as +U+0001 U+0001, U+0001 as U+0001 U+0002). The U+0001 row holds the stored +forms themselves as names: a raw U+0001 pair, which the decoder must not read +as U+0000, and U+0001 U+0002. DynamoDB returned those names byte for byte +too (measured in us-east-1 on 2026-11-04). +""" + +from __future__ import annotations + +import contextlib +import time + +NUL = "\u0000" +SOH = "\u0001" +STX = "\u0002" + + +def _defs(table: dict) -> dict: + return {d["AttributeName"]: d["AttributeType"] for d in table["AttributeDefinitions"]} + + +def _wait_for_table_and_indexes(client, name: str, timeout: float = 900.0) -> dict: + """Poll DescribeTable until the table and every GSI on it are ACTIVE, and + return the description. + + A GSI that UpdateTable adds is still CREATING after the table is ACTIVE + again, and one that UpdateTable deletes is listed as DELETING until it is + gone. `helpers.wait_for_active` reads only the table status. + """ + deadline = time.monotonic() + timeout + delay = 0.1 + while True: + table = client.describe_table(TableName=name)["Table"] + indexes = {g["IndexName"]: g["IndexStatus"] for g in table.get("GlobalSecondaryIndexes", [])} + if table["TableStatus"] == "ACTIVE" and all(s == "ACTIVE" for s in indexes.values()): + return table + if time.monotonic() >= deadline: + raise AssertionError( + f"{name}: table {table['TableStatus']}, indexes {indexes} after {timeout:.0f} s" + ) + time.sleep(delay) + delay = min(delay * 2, 5.0) + + +class TestNulInKeyAttributeNames: + def test_partition_key_attribute_name_with_nul_is_stored_and_returned( + self, dynamodb_client, table_factory + ): + pk = f"p{NUL}k" + name = table_factory(hash_key=pk) + table = dynamodb_client.describe_table(TableName=name)["Table"] + assert table["TableStatus"] == "ACTIVE" + assert table["KeySchema"] == [{"AttributeName": pk, "KeyType": "HASH"}] + assert table["AttributeDefinitions"] == [{"AttributeName": pk, "AttributeType": "S"}] + + def test_gsi_key_attribute_name_with_nul_is_stored_and_returned( + self, dynamodb_client, table_factory + ): + gk = f"g{NUL}k" + name = table_factory( + AttributeDefinitions=[ + {"AttributeName": "pk", "AttributeType": "S"}, + {"AttributeName": gk, "AttributeType": "S"}, + ], + GlobalSecondaryIndexes=[ + { + "IndexName": "gsi1", + "KeySchema": [{"AttributeName": gk, "KeyType": "HASH"}], + "Projection": {"ProjectionType": "ALL"}, + } + ], + ) + table = dynamodb_client.describe_table(TableName=name)["Table"] + gsi = table["GlobalSecondaryIndexes"][0] + assert gsi["IndexName"] == "gsi1" + assert gsi["KeySchema"] == [{"AttributeName": gk, "KeyType": "HASH"}] + assert _defs(table) == {"pk": "S", gk: "S"} + + def test_gsi_on_nul_attribute_can_be_removed_and_added_again( + self, dynamodb_client, table_factory + ): + gk = f"g{NUL}k" + name = table_factory( + AttributeDefinitions=[ + {"AttributeName": "pk", "AttributeType": "S"}, + {"AttributeName": gk, "AttributeType": "S"}, + ], + GlobalSecondaryIndexes=[ + { + "IndexName": "gsi1", + "KeySchema": [{"AttributeName": gk, "KeyType": "HASH"}], + "Projection": {"ProjectionType": "ALL"}, + } + ], + ) + # The fixture's cleanup ignores a DeleteTable that fails, and three + # runs of this test against DynamoDB that ended while gsi2 was still + # CREATING left their table behind. So the test waits for every index + # before it ends, whether it passes or fails. + try: + dynamodb_client.update_table( + TableName=name, + GlobalSecondaryIndexUpdates=[{"Delete": {"IndexName": "gsi1"}}], + ) + table = _wait_for_table_and_indexes(dynamodb_client, name) + assert not table.get("GlobalSecondaryIndexes") + dynamodb_client.update_table( + TableName=name, + AttributeDefinitions=[{"AttributeName": gk, "AttributeType": "S"}], + GlobalSecondaryIndexUpdates=[ + { + "Create": { + "IndexName": "gsi2", + "KeySchema": [{"AttributeName": gk, "KeyType": "HASH"}], + "Projection": {"ProjectionType": "ALL"}, + } + } + ], + ) + table = _wait_for_table_and_indexes(dynamodb_client, name) + gsi = table["GlobalSecondaryIndexes"][0] + assert gsi["IndexName"] == "gsi2" + assert gsi["KeySchema"] == [{"AttributeName": gk, "KeyType": "HASH"}] + assert _defs(table) == {"pk": "S", gk: "S"} + finally: + with contextlib.suppress(AssertionError): + _wait_for_table_and_indexes(dynamodb_client, name) + + def test_lsi_key_and_gsi_projected_attribute_names_with_nul_are_stored_and_returned( + self, dynamodb_client, table_factory + ): + """An LSI keyed on a U+0000 attribute and a GSI whose NonKeyAttributes + entry holds U+0000: both returned byte for byte (measured 2026-10-24 + and 2026-11-04).""" + lk = f"l{NUL}k" + nk = f"n{NUL}k" + name = table_factory( + AttributeDefinitions=[ + {"AttributeName": "pk", "AttributeType": "S"}, + {"AttributeName": "sk", "AttributeType": "S"}, + {"AttributeName": "gk", "AttributeType": "S"}, + {"AttributeName": lk, "AttributeType": "S"}, + ], + KeySchema=[ + {"AttributeName": "pk", "KeyType": "HASH"}, + {"AttributeName": "sk", "KeyType": "RANGE"}, + ], + LocalSecondaryIndexes=[ + { + "IndexName": "lsi1", + "KeySchema": [ + {"AttributeName": "pk", "KeyType": "HASH"}, + {"AttributeName": lk, "KeyType": "RANGE"}, + ], + "Projection": {"ProjectionType": "ALL"}, + } + ], + GlobalSecondaryIndexes=[ + { + "IndexName": "gsi1", + "KeySchema": [{"AttributeName": "gk", "KeyType": "HASH"}], + "Projection": {"ProjectionType": "INCLUDE", "NonKeyAttributes": [nk]}, + } + ], + ) + table = dynamodb_client.describe_table(TableName=name)["Table"] + lsi = table["LocalSecondaryIndexes"][0] + assert lsi["KeySchema"] == [ + {"AttributeName": "pk", "KeyType": "HASH"}, + {"AttributeName": lk, "KeyType": "RANGE"}, + ] + gsi = table["GlobalSecondaryIndexes"][0] + assert gsi["Projection"] == {"ProjectionType": "INCLUDE", "NonKeyAttributes": [nk]} + assert _defs(table) == {"pk": "S", "sk": "S", "gk": "S", lk: "S"} + + +class TestSohInKeyAttributeNames: + def test_key_and_projected_attribute_names_with_u0001_are_stored_and_returned( + self, dynamodb_client, table_factory + ): + """A raw U+0001 pair in the partition key and the GSI key, and + U+0001 U+0002 in the sort key and a projected name.""" + pk = f"a{SOH}{SOH}z" + sk = f"b{SOH}{STX}y" + gk = f"g{SOH}{SOH}k" + nk = f"n{SOH}{STX}" + name = table_factory( + AttributeDefinitions=[ + {"AttributeName": pk, "AttributeType": "S"}, + {"AttributeName": sk, "AttributeType": "S"}, + {"AttributeName": gk, "AttributeType": "S"}, + ], + KeySchema=[ + {"AttributeName": pk, "KeyType": "HASH"}, + {"AttributeName": sk, "KeyType": "RANGE"}, + ], + GlobalSecondaryIndexes=[ + { + "IndexName": "gsi1", + "KeySchema": [{"AttributeName": gk, "KeyType": "HASH"}], + "Projection": {"ProjectionType": "INCLUDE", "NonKeyAttributes": [nk]}, + } + ], + ) + table = dynamodb_client.describe_table(TableName=name)["Table"] + assert table["KeySchema"] == [ + {"AttributeName": pk, "KeyType": "HASH"}, + {"AttributeName": sk, "KeyType": "RANGE"}, + ] + assert _defs(table) == {pk: "S", sk: "S", gk: "S"} + gsi = table["GlobalSecondaryIndexes"][0] + assert gsi["KeySchema"] == [{"AttributeName": gk, "KeyType": "HASH"}] + assert gsi["Projection"] == {"ProjectionType": "INCLUDE", "NonKeyAttributes": [nk]} From 6fda801a4331de08885a1c8194ca22118dddd19e Mon Sep 17 00:00:00 2001 From: Lee <29015211+LeeroyHannigan@users.noreply.github.com> Date: Thu, 5 Nov 2026 16:24:06 +0000 Subject: [PATCH 2/2] test(storage-cassandra): accept later pending migrations 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. --- crates/storage-cassandra/tests/direct/backup_engine.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/storage-cassandra/tests/direct/backup_engine.rs b/crates/storage-cassandra/tests/direct/backup_engine.rs index 6c8c4da0d..15c1381d5 100644 --- a/crates/storage-cassandra/tests/direct/backup_engine.rs +++ b/crates/storage-cassandra/tests/direct/backup_engine.rs @@ -4164,12 +4164,17 @@ async fn test_a_migration_6_that_fails_in_its_backfill_stays_pending_and_resumes let (engine, account_id, _drop_keyspaces) = isolated_engine("mf", true).await; let catalog = engine.catalog_keyspace(); let prefix = prefix_of(&engine); + // Migration 6 is pending, and so is every later migration this binary + // carries. let behind_at_5 = |status: CatalogStatus| match status { CatalogStatus::Behind { version, pending } => { version == Some(MigrationId::DropContinuousBackups.number()) && pending .iter() - .all(|p| p.id == MigrationId::BackupCatalogIndex.number()) + .any(|p| p.id == MigrationId::BackupCatalogIndex.number()) + && pending + .iter() + .all(|p| p.id >= MigrationId::BackupCatalogIndex.number()) } _ => false, };