Skip to content

Fix checkpoint with SET-based credentials#1224

Open
pckilgore wants to merge 1 commit into
duckdb:v1.5-variegatafrom
pckilgore:fix/checkpoint-set-credentials
Open

Fix checkpoint with SET-based credentials#1224
pckilgore wants to merge 1 commit into
duckdb:v1.5-variegatafrom
pckilgore:fix/checkpoint-set-credentials

Conversation

@pckilgore

@pckilgore pckilgore commented Jun 5, 2026

Copy link
Copy Markdown

fixes #1222

Copies users context through some new connections created when doing Checkpoint to ensure settings are available when needed:

User's session (ClientContext A)
  has: SET s3_access_key_id = 'admin', SET s3_secret_access_key = 'password', etc.
  runs: CHECKPOINT;

then

DuckLakeTransactionManager::Checkpoint(context=A)
  creates: Connection #1 (new ClientContext B — empty settings)
  ───── FIX #1: B.user_settings = A.user_settings ─────
  runs: conn->Query("CALL ducklake_delete_orphaned_files('ducklake')")

then

ducklake_delete_orphaned_files executes
  needs to query DuckLake metadata + S3
  calls: DuckLakeTransaction::Get(context=B, catalog)
         gets/creates a DuckLakeTransaction where weak_ptr context == B

then

DuckLakeTransaction::GetConnection()
  creates: Connection #2 (new ClientContext C — empty settings)
  ───── FIX #2: C.user_settings = B.user_settings ─────
  runs metadata query including: read_blob 

then
       
httpfs resolves S3 credentials
  calls ClientContext C → TryGetCurrentSetting("s3_access_key_id")
  looks in: C.config.user_settings
  WITHOUT fixes: empty == 403 Forbidden
  WITH fixes: finds 'admin' (copied A to B to C) == success

Tests all pass locally.

@pckilgore pckilgore marked this pull request as ready for review June 5, 2026 21:38
@pckilgore pckilgore changed the base branch from main to v1.5-variegata June 9, 2026 01:03
@pckilgore pckilgore force-pushed the fix/checkpoint-set-credentials branch from d266480 to 721bb43 Compare June 9, 2026 01:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CHECKPOINT handles missing S3 files inconsistently depending on credential method

1 participant