Fix #226: defer REST catalog metadata deletion until commit succeeds

[sfc-gh-rbachala]

Problem

InsertDeletionQueueRecord for writable REST catalog tables was called during
XACT_EVENT_PRE_COMMIT (inside ApplyIcebergMetadataChanges) — before the
REST catalog HTTP batch commit was sent (which happens in XACT_EVENT_COMMIT).

On a 429 rate-limit retry, PRE_COMMIT fires again with the same metadata
file path already in the queue:

ERROR: duplicate key value violates unique constraint "deletion_queue_pkey"

The previous approach (ON CONFLICT (path) DO NOTHING) masked the crash.
This PR fixes the root cause.

Approach

Defer the enqueue until after the REST commit returns HTTP 204:

1. ApplyIcebergMetadataChanges (metadata_operations.c) — replace the
   direct InsertDeletionQueueRecord call with
   AddRestCatalogMetadataForDeferredDeletion, which appends the old metadata
   path to a per-transaction pending list
   (PgLakeXactRestCatalog->metadataToEnqueueOnSuccess, in
   TopTransactionContext).

2. PostAllRestCatalogRequests (track_iceberg_metadata_changes.c) — on
   HTTP 204 only, copy pending entries to the process-global
   confirmedRestCatalogDeletions list (TopMemoryContext). On 429 or any
   other failure the paths are discarded with the transaction context — no
   duplicate key is possible.

3. IcebergXactCallback / XACT_EVENT_PRE_COMMIT (transaction_hooks.c)
   — call DrainConfirmedRestCatalogDeletions before
   ConsumeTrackedIcebergMetadataChanges. The drain runs inside the active
   snapshot brackets (required for SPI) and flushes confirmed paths from the
   previous successful transaction into deletion_queue.

Files changed

File	Change
pg_lake_iceberg/src/iceberg/metadata_operations.c	Replaced direct InsertDeletionQueueRecord with AddRestCatalogMetadataForDeferredDeletion via forward extern declaration (avoids circular build dependency — pg_lake_iceberg cannot include pg_lake_table headers)
pg_lake_table/include/pg_lake/transaction/track_iceberg_metadata_changes.h	Added datatype/timestamp.h; exported two new functions
pg_lake_table/src/transaction/track_iceberg_metadata_changes.c	New RestCatalogPendingMetadataDeletion struct, metadataToEnqueueOnSuccess field, confirmedRestCatalogDeletions global, AddRestCatalogMetadataForDeferredDeletion, DrainConfirmedRestCatalogDeletions, and 204-success promotion in PostAllRestCatalogRequests
pg_lake_table/src/transaction/transaction_hooks.c	DrainConfirmedRestCatalogDeletions() before ConsumeTrackedIcebergMetadataChanges in PRE_COMMIT
pg_lake_table/tests/pytests/test_deletion_queue_idempotency.py	Rewritten with behavioral tests that document the fix contract

Memory model

• Pending paths live in TopTransactionContext — freed automatically at transaction end if REST commit fails.
• Confirmed paths are copied to TopMemoryContext on 204 — survive across transaction boundaries until drained.
• The drain clears the global list before any SQL work, so a mid-drain error does not retry the same paths in the next transaction.

Edge cases verified

• Vacuum / snapshot expiration with no prior DML: AddRestCatalogMetadataForDeferredDeletion calls InitRestCatalogRequestsHashIfNeeded(), ensuring PgLakeXactRestCatalog is non-NULL even when BindRelationToXactRestCatalog was never called.
• CREATE TABLE: metadataPath is NULL for new REST tables; the if (metadataPath) guard prevents any enqueue.
• No actual changes: the early-return at !createNewSnapshot && !createNewTable fires before the writableRestCatalogTable block, so AddRestCatalogMetadataForDeferredDeletion is never reached.
• Process crash between COMMIT and next PRE_COMMIT: confirmed paths are lost; orphaned S3 files are not cleaned up. Acceptable — deletion queue is best-effort.

Fixes #226

Signed-off-by: Richie Bachala richie.bachala@snowflake.com

[sfc-gh-rbachala]

@sfc-gh-npuka @sfc-gh-okalaci — would appreciate a review when you have a moment. This is a targeted fix for the deletion_queue duplicate key error you filed in #226.

[sfc-gh-mslot]

I'm not entirely against it, but it masks the underlying issue that we inserted something into the deletion queue that is apparently still in use. It would be preferable to solve this by avoiding the deletion of files that might still be in use by an external REST catalog, after a failure.

[sfc-gh-rbachala]

Hi @sfc-gh-mslot — thanks for the feedback. I've rewritten the approach to address exactly that concern.

The original ON CONFLICT DO NOTHING fix was a band-aid that masked the crash without fixing the root cause. The new implementation takes a different approach: the metadata file path is only inserted into deletion_queue after the REST catalog commit returns HTTP 204.

Here's how it works now:

1. During XACT_EVENT_PRE_COMMIT, instead of calling InsertDeletionQueueRecord directly, ApplyIcebergMetadataChanges calls AddRestCatalogMetadataForDeferredDeletion. This accumulates the old metadata path in a per-transaction pending list (metadataToEnqueueOnSuccess) without touching the DB.

2. During XACT_EVENT_COMMIT, PostAllRestCatalogRequests sends the HTTP batch commit. Only if the response is 204 does it promote the pending paths to a process-global confirmed list (confirmedRestCatalogDeletions). On 429 or any other failure, the paths are silently discarded with the transaction context — so nothing that's still in use by the REST catalog ever reaches deletion_queue.

3. The confirmed paths are drained into deletion_queue at the start of the next transaction's PRE_COMMIT (inside an active snapshot, which SPI requires). By that point, the REST catalog has committed the replacement metadata, making the old path a true orphan.

The ON CONFLICT DO NOTHING has been reverted — the primary key is strict again.

[sfc-gh-mslot]

If the process ends we lose this list, so this doesn't seem like a good solution.