Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SNOW-1950183 Resolve Iceberg ingestion filename mismatch between upload and registration #958

Merged

Conversation

sfc-gh-alhuang
Copy link
Contributor

@sfc-gh-alhuang sfc-gh-alhuang commented Mar 5, 2025

The SDK has a bug when uploading Iceberg Parquet files at the moment of token expiration. It correctly uploads the file to S3 using a new token and file name but mistakenly registers it with the old file name. This PR ensure the SDK registers the file with the refreshed file name.

Server side fix for previous SDK versions.

@sfc-gh-alhuang sfc-gh-alhuang marked this pull request as ready for review March 6, 2025 07:40
@sfc-gh-alhuang sfc-gh-alhuang requested review from sfc-gh-tzhang and a team as code owners March 6, 2025 07:40
Copy link

@sfc-gh-dorlovsky sfc-gh-dorlovsky left a comment

Choose a reason for hiding this comment

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

Looks great, but I have some comments, PTAL

@sfc-gh-alhuang sfc-gh-alhuang force-pushed the alhuang-SNOW-1950183-iceberg-filename-mismatch branch from 15b60c3 to 7ac7194 Compare March 7, 2025 00:26
@@ -17,8 +17,8 @@ interface IStorage {
*
* @param blobPath
* @param blob
* @return The String ETag returned by the upload. Can be null in situations where the underlying
* layer does not have an ETag to return.
* @return The IcebergPostUploadMetadata returned by the upload. Empty if and only if it is a

Choose a reason for hiding this comment

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

Nit: change to positive 'Present iff and only if the implementation is creating Iceberg blobs'.
Note that I prefer not to mention iceberg tables as long as we can abstract the storage interface away from them.

@sfc-gh-alhuang sfc-gh-alhuang enabled auto-merge (squash) March 7, 2025 19:21
@sfc-gh-alhuang sfc-gh-alhuang merged commit 393f302 into master Mar 7, 2025
49 checks passed
@sfc-gh-alhuang sfc-gh-alhuang deleted the alhuang-SNOW-1950183-iceberg-filename-mismatch branch March 7, 2025 20:38
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.

2 participants