Skip to content

Conversation

@hackintoshrao
Copy link

Enhances append_partition_data_file_test.rs to verify the correct partition
layout in MinIO:

  • Add URL parsing to extract bucket and prefix from table location
  • Implement S3 ListObjectsV2 check for partition directory
  • Add debug logging for table location and data file paths
  • Verify objects exist in the expected partition directory (id=100/)

This helps diagnose the partition layout issue by confirming whether files are being written to the correct partitioned directory structure.

Karthic Rao added 4 commits January 13, 2025 06:47
- Updates test fixture to use localhost instead of container IPs
- Adds S3 client configuration for direct MinIO access
- Comments out unused container IP resolution
Changes:
- Switch MinIO image to 'latest' tag for better stability
- Add port 9000 mapping for MinIO (9000:9000)
- Add 'mc' as dependency for REST service to fix the race condition
Enhances append_partition_data_file_test.rs to verify correct partition
layout in MinIO:

- Add URL parsing to extract bucket and prefix from table location
- Implement S3 ListObjectsV2 check for partition directory
- Add debug logging for table location and data file paths
- Verify objects exist in expected partition directory (id=100/)

This helps diagnose the partition layout issue by confirming whether
files are being written to the correct partitioned directory structure.
@liurenjie1024
Copy link
Contributor

Thanks @hackintoshrao for this pr. I'll convert it to draft first as it's only used for showing the expected path, but there is no guarantee in iceberg spec that it should generate such a directory.

@Xuanwo
Copy link
Member

Xuanwo commented Mar 6, 2025

Thanks @hackintoshrao for this pr. I'll convert it to draft first as it's only used for showing the expected path, but there is no guarantee in iceberg spec that it should generate such a directory.

Yes, I feel the same as @liurenjie1024. The physical layout is not part of the iceberg spec, and the writer has the ultimate authority to decide how to place them. It's better not to enforce this here.

Also, cc @Fokko—what do you think?

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.

3 participants