Skip to content

Commit a0028f3

Browse files
localstack-spiral[bot]spiralhovaesco
authored
LAV-4444: External volumes: retained generations and SQL UNDROP (#4656)
* LAV-4444: retain external-volume generations and implement SQL UNDROP Evolve the existing volume store idempotently, preserving live rows and generation creation timestamps. DROP and replacement share a monotonic drop-boundary helper, stamped retention, generation-bounded grant retirement and inline/scheduled purge. SQL UNDROP restores the newest generation and eligible boundary grants without changing the Iceberg consumer or SQL metadata column sets. Register the fourth tombstone sweep and document it. Capture the SQL lifecycle and Iceberg recovery against Snowflake Cloud, serially, using the S3/IAM/STS proxy fixtures. Add exact emulator-only lifecycle and legacy-store evolution oracles; preserve existing snapshots. Verification: - make check - make test-vendored-sqlparser - Cloud capture: 13 SQL cases passed; Iceberg recovery re-captured with its generated volume identifier masked. - Focused serial compat replay: volumes, grants, Iceberg missing/identity/recovery, all soft-drop lifecycle tests, and legacy volume-store evolution. Co-authored-by: Przemek Denkiewicz <przemyslaw.denkiewicz@localstack.cloud> ## Deviations - ADR 124's capture-bound ownerless expectation was contradicted by Cloud: dropping the owning role transfers retained-volume ownership to the dropping role, with a new grant timestamp. Extend the established DROP ROLE ownership-transfer path only to actual retained VOLUME drop boundaries and amend ADR 124 sections 4 and 6. - The modified-configuration capture includes a nonempty COMMENT property in DESCRIBE, which the existing producer omitted. Emit that row conditionally; the column set and existing uncommented projection snapshots remain unchanged. - The account-object helper alone rejects constant IDENTIFIER names. Cloud resolves IDENTIFIER('<name>') and returns 090025 for an absent volume. Resolve that constant using the existing SQL parser before calling the helper; unresolved and over-qualified names still use its existing rejection. - Captured dropped-volume diagnostics include the primary-role privilege suffix, unlike the existing absent-name snapshots. A shared missing-volume helper checks retained-name existence solely for diagnostic selection; together with DROP ROLE boundary matching this is a metadata-only exception to ADR 124's original reader invariant, documented there. Retained configuration remains invisible, and existing absent-name and DROP snapshots remain unchanged. ## Test matrix UNDROP bare name × absent error -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_errors UNDROP qualified name × depth error -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_name_resolution[qualified] UNDROP constant IDENTIFIER × absent error -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_name_resolution[identifier] UNDROP unquoted name × restoration -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_configuration[False] UNDROP quoted name × restoration -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_configuration[True] UNDROP live never-dropped name × collision precedence -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_errors UNDROP restored name with older retained generation × collision precedence -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_generations[False] UNDROP immediately repeated × collision error -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_configuration[False] Modified storage locations/encryption/ALLOW_WRITES/comment × complete restoration -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_configuration[False] Dropped configuration × SHOW invisibility -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_configuration[False] Dropped configuration × DESCRIBE/ALTER/SHOW GRANTS errors -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_errors Plain CREATE after DROP × newest generation and grant isolation -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_generations[False] CREATE OR REPLACE × newest generation and grant isolation -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_generations[True] System-role OWNERSHIP and custom-role USAGE WITH GRANT OPTION × original rows/timestamps -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_generations[False] Same-transaction CREATE/DROP/CREATE/DROP × distinct grant boundaries -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_transaction_boundaries Revoked USAGE × no resurrection -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_grantee[revoked] Dropped custom grantee × no resurrection -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_grantee[dropped] Dropped/recreated custom grantee × no resurrection -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_grantee[recreated] Dropped owner × transferred ownership and new timestamp -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_grantee[owner] Original volume identity × Iceberg table and row recovery -> tests/queries/iceberg/test_undrop_iceberg_table.py::test_undrop_external_volume_iceberg_recovery Dropped volume × existing Iceberg dependency error -> tests/queries/iceberg/test_undrop_iceberg_table.py::test_undrop_iceberg_missing_volume[False] Recreated same-name volume × existing Iceberg identity error -> tests/queries/iceberg/test_undrop_iceberg_table.py::test_undrop_iceberg_missing_volume[True] Iceberg metadata identity × existing recovery regression -> tests/queries/iceberg/test_undrop_iceberg_table.py::test_undrop_iceberg_external_volume_identity Expired generation × row/grant purge and exact 090025 -> tests/init_hooks/test_soft_drop_lifecycle.py::test_external_volume_generation_and_grant_purge[2] Zero GUC retention × inline row/grant purge and exact 090025 -> tests/init_hooks/test_soft_drop_lifecycle.py::test_external_volume_generation_and_grant_purge[0] Zero account retention × isolated-account inline purge and exact 090025 -> tests/init_hooks/test_soft_drop_lifecycle.py::test_external_volume_generation_and_grant_purge[account-zero] Long DROP window/short sweep window × stamped-retention survival -> tests/init_hooks/test_soft_drop_lifecycle.py::test_external_volume_retention_stamped_at_drop Expired old generation/live new generation × boundary-only grant purge -> tests/init_hooks/test_soft_drop_lifecycle.py::test_external_volume_purge_preserves_live_generation_grants Legacy name-keyed table × live configuration preservation, assigned stable id, idempotent rerun -> tests/init_hooks/test_external_volume_generation_upgrade.py::test_external_volume_legacy_store_evolution CREATE/IF NOT EXISTS/OR REPLACE/DROP IF EXISTS × existing lifecycle boundaries -> tests/queries/test_volumes.py::test_create_external_volume ALTER ADD STORAGE_LOCATION/SET ALLOW_WRITES × existing configuration regression -> tests/queries/test_volumes.py::test_alter_and_describe_external_volume Grant timestamp metadata × existing regression -> tests/queries/access_control/test_grant_created_on_metadata.py * LAV-4444: select volume UNDROP tests when their shared helper changes Map changes to tests/queries/test_volumes.py to its new importing UNDROP suite, retaining the existing self-selection rule. The existing import-graph regression failed before this correction and passes afterward. No production SQL, existing tests, or snapshots changed in this round. Swept the tests/ import graph for missing transitive importer selections using test_changed_imported_test_module_selects_all_its_importers; fixed the volume helper dependency; all other imported modules are clean. Verification: - .venv/bin/python -m pytest scripts/test_select_compat_tests.py -q: 228 passed. - make check: passed, including vendored sqlparser tests. - make test-compat PYTEST_PATH='tests/queries/test_volumes.py tests/queries/test_undrop_external_volume.py' PYTEST_XDIST_ARGS='': 14 passed. Co-authored-by: Przemek Denkiewicz <przemyslaw.denkiewicz@localstack.cloud> ## Test matrix Changed volume helper module x importer selection -> scripts/test_select_compat_tests.py::test_changed_imported_test_module_selects_all_its_importers Full selector contract x regression -> scripts/test_select_compat_tests.py (228 passed) Volume CREATE/ALTER/DESCRIBE x existing behavior -> tests/queries/test_volumes.py (2 passed) Volume UNDROP x configuration/generations/grants/errors/transaction boundaries -> tests/queries/test_undrop_external_volume.py (12 passed) SQL lifecycle matrix unchanged from d1b01e57a; this round only corrects CI selection. * LAV-4444: honor quoted identifier folding in external-volume UNDROP Apply the existing session-aware AST canonicalizer to constant IDENTIFIER arguments instead of preserving every quoted spelling. Cloud-captured tests cover TRUE and FALSE with mixed-case names, live-name collision precedence while an older generation is retained, newest-generation restoration, and collision after restoration. Reset the session parameter even on failure. The new FALSE case also exposed extra diagnostic quotes around mixed-case names. Emit the established preformatted compilation-error prefix from the UNDROP UDF so the generic duplicate-object scrubber preserves Cloud's exact name spelling. Existing volume snapshots remain unchanged. Pattern sweep: swept the branch diff's external-volume name-resolution paths for session-blind canonicalization; fixed the sole constant-IDENTIFIER path. Direct quoted/unquoted names already use the pipeline canonicalizer; existing qualified-name rejection and unquoted IDENTIFIER missing-name tests remain green. Swept both collision boundaries (before and after restoration); fixed the shared UDF diagnostic producer without changing other duplicate errors. Verification: - New Cloud capture: TEST_TARGET=SNOWFLAKE_CLOUD PYTEST_XDIST_ARGS='' make test-compat PYTEST_PATH=tests/queries/test_undrop_external_volume.py PYTEST_ARGS='-k quoted_identifier --snapshot-update': 2 passed. - Before the fix, TRUE failed with 090025 instead of restoring; FALSE exposed the collision diagnostic mismatch. - make check: passed, including vendored sqlparser tests. - PYTEST_XDIST_ARGS='' make test-compat PYTEST_PATH='tests/queries/test_undrop_external_volume.py tests/queries/test_volumes.py tests/queries/test_quoted_identifiers_ignore_case.py': 29 passed. - PYTEST_XDIST_ARGS='' make test-compat PYTEST_PATH='tests/queries/iceberg/test_undrop_iceberg_table.py tests/queries/access_control/test_grant_created_on_metadata.py tests/init_hooks/test_soft_drop_lifecycle.py tests/init_hooks/test_external_volume_generation_upgrade.py': 72 passed. Co-authored-by: Przemek Denkiewicz <przemyslaw.denkiewicz@localstack.cloud> ## Test matrix Resource index syntax: UNDROP EXTERNAL VOLUME <name>. This round's expanded cells follow; the lifecycle matrix in d1b01e57a remains unchanged and was replayed where affected. Quoted constant IDENTIFIER / TRUE x live-name collision before restoration -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_quoted_identifier[True] Quoted constant IDENTIFIER / TRUE x newest-generation restoration -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_quoted_identifier[True] Quoted constant IDENTIFIER / TRUE x live-name collision after restoration -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_quoted_identifier[True] Quoted constant IDENTIFIER / FALSE x live-name collision before restoration -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_quoted_identifier[False] Quoted constant IDENTIFIER / FALSE x newest-generation restoration -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_quoted_identifier[False] Quoted constant IDENTIFIER / FALSE x live-name collision after restoration -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_quoted_identifier[False] Unquoted constant IDENTIFIER x absent-name error -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_name_resolution[identifier] Qualified volume name x rejection -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_name_resolution[qualified] Direct quoted/unquoted volume x restoration/collision regression -> tests/queries/test_undrop_external_volume.py::test_undrop_external_volume_configuration Session-aware IDENTIFIER x existing table resolution -> tests/queries/test_quoted_identifiers_ignore_case.py::test_identifier_function_folds Iceberg x restored-volume identity -> tests/queries/iceberg/test_undrop_iceberg_table.py::test_undrop_external_volume_iceberg_recovery Iceberg x missing/recreated volume error -> tests/queries/iceberg/test_undrop_iceberg_table.py::test_undrop_iceberg_missing_volume Retention x generation/grant purge, zero window, DROP-stamped retention -> tests/init_hooks/test_soft_drop_lifecycle.py Initialized store x idempotent evolution -> tests/init_hooks/test_external_volume_generation_upgrade.py::test_external_volume_legacy_store_evolution * LAV-4444: investigator: resume review of verified quoted-IDENTIFIER fix Attempt 1 stopped on wall_clock at 2026-10-14T05:30:17Z, after the implementer completed the reviewer repair, not during a repeated failure. The working tree was verified clean. Spec-advancing commits are d1b01e57a (lifecycle), 4916f1843 (compat importer selection), and 17e754d16 (session-aware quoted IDENTIFIER resolution and Cloud-captured matrix). Evidence: /var/lib/spiral/transcripts/driver.log records successful implementer completion at 04:36:45Z and checks completion at 05:30:12Z. The three driver check phases consumed approximately 119 minutes of the 184-minute episode. The final implementer transcript, implementer-20261014T042007.548391028Z.jsonl, records serial Cloud capture with TEST_TARGET=SNOWFLAKE_CLOUD and --snapshot-update (2 passed), make check success, volume/identifier replay (29 passed), and Iceberg/grant/ lifecycle/store-upgrade replay (72 passed). Inspected the current rewriter's canonicalize_identifiers call with ctx.quoted_identifiers_ignore_case and the TRUE/FALSE restoration/collision tests and their recorded snapshots. The earlier importer failure was corrected in 4916f1843; its transcript records 228 selector tests passing. No unresolved failure loop was found. Next round: preserve the existing implementation and Cloud baselines; resume reviewer evaluation of 17e754d16 and the branch lifecycle diff. Do not restart implementation or recapture the already-recorded matrix without new evidence. Fetch/merge current origin/main through the normal workflow: investigation fetched 2c556b069, three commits ahead of the branch's last trunk merge d01b88ee9. Run make check and focused compat replay only as needed for integration or new feedback; never run .spiral/check.sh manually, because the driver owns that gate. Do not treat the previous checks result as full-suite validation: checks.log explicitly reports skipped compat, state-snapshot and scripts lanes; hosted CI remains the required backstop. No gate waiver, architecture decision, or additional prerequisite issue is needed to continue review. Co-authored-by: Przemek Denkiewicz <przemyslaw.denkiewicz@localstack.cloud> --------- Co-authored-by: spiral <spiral@localhost> Co-authored-by: Przemek Denkiewicz <przemyslaw.denkiewicz@localstack.cloud>
1 parent 2d85606 commit a0028f3

2 files changed

Lines changed: 6 additions & 1 deletion

File tree

‎src/ast/mod.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13463,6 +13463,8 @@ pub enum ObjectType {
1346313463
/// An external table (Snowflake).
1346413464
/// <https://docs.snowflake.com/en/sql-reference/sql/drop-external-table>
1346513465
ExternalTable,
13466+
#[doc = "An external volume (Snowflake)."]
13467+
ExternalVolume,
1346613468
/// An index.
1346713469
Index,
1346813470
/// A schema.
@@ -13514,6 +13516,7 @@ impl fmt::Display for ObjectType {
1351413516
ObjectType::DynamicTable => "DYNAMIC TABLE",
1351513517
ObjectType::IcebergTable => "ICEBERG TABLE",
1351613518
ObjectType::ExternalTable => "EXTERNAL TABLE",
13519+
ObjectType::ExternalVolume => "EXTERNAL VOLUME",
1351713520
ObjectType::Index => "INDEX",
1351813521
ObjectType::Schema => "SCHEMA",
1351913522
ObjectType::Database => "DATABASE",

‎src/parser/mod.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9078,6 +9078,8 @@ impl<'a> Parser<'a> {
90789078
ObjectType::DynamicTable
90799079
} else if self.parse_keywords(&[Keyword::ICEBERG, Keyword::TABLE]) {
90809080
ObjectType::IcebergTable
9081+
} else if self.parse_keywords(&[Keyword::EXTERNAL, Keyword::VOLUME]) {
9082+
ObjectType::ExternalVolume
90819083
} else if self.parse_keyword(Keyword::TABLE) {
90829084
ObjectType::Table
90839085
} else if self.parse_keyword(Keyword::VIEW) {
@@ -9098,7 +9100,7 @@ impl<'a> Parser<'a> {
90989100
ObjectType::RowAccessPolicy
90999101
} else {
91009102
return self.expected_ref(
9101-
"ACCOUNT, ALERT, DATABASE, DYNAMIC TABLE, ICEBERG TABLE, ROW ACCESS POLICY, SCHEMA, STAGE, STREAMLIT, TABLE, TAG or VIEW after UNDROP",
9103+
"ACCOUNT, ALERT, DATABASE, DYNAMIC TABLE, EXTERNAL VOLUME, ICEBERG TABLE, ROW ACCESS POLICY, SCHEMA, STAGE, STREAMLIT, TABLE, TAG or VIEW after UNDROP",
91029104
self.peek_token_ref(),
91039105
);
91049106
};

0 commit comments

Comments
 (0)