Skip to content

Antalya 26.6: Antalya 26.3 Backport - Added test cases for s3 encoding fix - #2126

Open
zvonand wants to merge 5 commits into
antalya-26.6from
feature/antalya-26.6/pr-1781
Open

Antalya 26.6: Antalya 26.3 Backport - Added test cases for s3 encoding fix#2126
zvonand wants to merge 5 commits into
antalya-26.6from
feature/antalya-26.6/pr-1781

Conversation

@zvonand

@zvonand zvonand commented Jul 29, 2026

Copy link
Copy Markdown
Member

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Backport of #1516, unit/integration tests added. The original fix was in the resolveObjectStorageForPath function(Utils.cpp) which was renamed/rewritten to getProperFilePathFromMetadataInfo. The tests are to make sure % encoded URLs still work in iceberg. closes #1348 (#1781 by @subkanthi).

CI/CD Options

Exclude tests:

  • Fast test
  • Integration Tests
  • Stateless tests
  • Stateful tests
  • Performance tests
  • All with ASAN
  • All with TSAN
  • All with MSAN
  • All with UBSAN
  • All with Coverage
  • All with Aarch64
  • All Regression
  • Disable CI Cache

Regression jobs to run:

  • Fast suites (mostly <1h)
  • Aggregate Functions (2h)
  • Alter (1.5h)
  • Benchmark (30m)
  • ClickHouse Keeper (1h)
  • Iceberg (2h)
  • LDAP (1h)
  • Parquet (1.5h)
  • RBAC (1.5h)
  • SSL Server (1h)
  • S3 (2h)
  • S3 Export (2h)
  • Swarms (30m)
  • Tiered Storage (2h)

Cherry-picked from #1781.


Documentation entry for user-facing changes

Backport of #1516, unit tests added.

Antalya 26.3 Backport - Added test cases for s3 encoding fix
@zvonand zvonand added releasy Created/managed by RelEasy antalya-26.6 labels Jul 29, 2026
@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown

Workflow [PR], commit [2f346bc]

@subkanthi subkanthi self-assigned this Aug 3, 2026
@subkanthi

Copy link
Copy Markdown
Collaborator

Local build does not have compiler errors


 Finished `dev` profile [unoptimized + debuginfo] target(s) in 1m 49s
[12717/16478] cd /root/Documents/ClickHouse/rust/workspace/polyglot &...cmake/config.toml --offline --locked -- -Cdefault-linker-libraries=no
   Compiling serde_core v1.0.228
   Compiling serde_json v1.0.150
   Compiling thiserror v1.0.69
   Compiling serde v1.0.228
   Compiling syn v2.0.118
   Compiling serde_derive v1.0.228
   Compiling thiserror-impl v1.0.69
   Compiling polyglot-sql v0.1.15
   Compiling _ch_rust_polyglot v0.1.0 (/root/Documents/ClickHouse/rust/workspace/polyglot)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 1m 47s
[16478/16478] Linking CXX executable programs/clickhouse

@zvonand

zvonand commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

@blau-ai fix compiler errors directly in this branch

The gtest added by the backport called getProperFilePathFromMetadataInfo,
a free function that exists in antalya-26.3 but was refactored into
IcebergPathResolver::resolve() in 26.6, so the test failed to compile
(use of undeclared identifier) and broke Build (amd_debug)/(arm_debug).

Rewrite the four test cases against the current IcebergPathResolver API.
The resolver applies the same prefix-stripping string logic, so the
percent-encoding-preservation expectations are unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@blau-ai

blau-ai commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Fixed the build failure and pushed directly to feature/antalya-26.6/pr-1781 (as requested) — commit cab296ae0ba.

What was broken

Build (amd_debug) and Build (arm_debug) failed compiling the new unit test:

src/Storages/ObjectStorage/DataLakes/Iceberg/tests/gtest_iceberg_path_resolver.cpp:9:19:
  error: use of undeclared identifier 'getProperFilePathFromMetadataInfo'

(same error at lines 18, 27, 36 → ninja: build stopped)

Root cause

The backported test (from 1338_fix_3_decoding_antaly_26_3) calls the free function
DB::Iceberg::getProperFilePathFromMetadataInfo(data_path, common_path, table_location).
That function exists in antalya-26.3, but in 26.6 the same logic was refactored
into IcebergPathResolver::resolve() (.../Iceberg/IcebergPath.h / IcebergPath.cpp).
So the symbol simply does not exist on this branch — only the tests were backported, not the old API.

Fix

Rewrote the four test cases against the current IcebergPathResolver API. The mapping is 1:1:

getProperFilePathFromMetadataInfo(data_path, common_path, table_location)
  == IcebergPathResolver(table_location, /*table_root=*/common_path)
         .resolve(IcebergPathFromMetadata::deserialize(data_path))

resolve() performs the identical prefix-stripping via pure string ops, so the
percent-encoding-preservation expectations are unchanged (%2F, %20, multi-segment,
and the http:// scheme case all keep their asserted results). No production code was touched.

Note / verification

I can't build or run tests in this environment, so the compile fix is validated by CI on this
PR — please watch the re-run of Build (amd_debug) / Build (arm_debug).

The failing Regression … / Stateless tests checks are separate from this build error and were
not addressed here; let me know if you'd like me to triage those next.

@zvonand zvonand added the forwardport This is a frontport of code that existed in previous Antalya versions label Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

antalya antalya-26.6 forwardport This is a frontport of code that existed in previous Antalya versions releasy Created/managed by RelEasy

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants