Resolve hive partitioning sample path lazily

[evillique]

Object storage tables with an explicit schema and format no longer list the endpoint on CREATE/ATTACH — the hive partitioning sample path is resolved on the first use of the table. An unreachable endpoint used to block CREATE and server startup for hours.

Changelog category (leave one):

• Improvement

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

The hive partitioning sample path for object storage tables (e.g. S3) is resolved on the first use of the table instead of CREATE/ATTACH, so an unreachable endpoint no longer blocks table creation and server startup.

Version info

• Merged into: 26.8.1.682 (included in 26.8 and later)
• Backported to: 26.7.4.54, 26.6.3.57, 26.5.7.61, 26.3.20.2

[clickhouse-gh]

Workflow [PR], commit [021ec01]

Summary: ✅

• Performance Comparison: Performance dashboard

---

AI Review

Summary

This PR defers hive-partition sample-path resolution for explicit-schema object-storage tables from CREATE/ATTACH to first use and adds throw_on_hive_partitioning_resolution_failure for the query-time failure mode. CI is green, but I still see one correctness problem in the cached-resolution context and one first-query regression in path-filtered reads.

Findings

❌ Blockers

• [src/Storages/ObjectStorage/StorageObjectStorage.cpp:454-468] The cached hive metadata still depends on whichever session first resolves the table. resolveHivePartitioningSamplePathIfDeferred rebuilds the storage client from query_context before calling getPathSample, so per-query S3 credential restrictions / extra_credentials can choose a different sample file than another session would. Because hive_partitioning_sample_path_resolved then freezes the derived partition columns globally, the first successful session can make later sessions see the wrong hive columns or types until restart. Suggested fix: capture a stable attach-time resolution context, or disable table-global caching when configuration->update depends on query-scoped credentials and recompute per query.

⚠️ Majors

• [src/Storages/ObjectStorage/StorageObjectStorage.cpp:517-518] updateExternalDynamicMetadataIfExists now resolves the sample path unconditionally on the first query, before the optimizer knows whether the query actually needs hive columns. That forces an extra unfiltered listing and bypasses _path pushdown on the first selective read; the change in tests/queries/0_stateless/03741_s3_glob_table_path_pushdown.reference from 1 4 to 2 4 shows the added S3ListObjects. Suggested fix: only resolve when the query actually touches hive virtuals / needs hive-derived metadata, or reuse the first read-side iterator instead of issuing a separate getPathSample listing.

Tests

• ⚠️ Add a focused regression that exercises two sessions with different S3 credential restrictions or extra_credentials against the same table and proves the cached hive columns do not depend on which session touches it first.

Final Verdict

Request changes.

[clickhouse-gh]

Deferring the resolution to a copy of query_context makes the table's cached hive metadata depend on whichever query happens to touch the table first. This path rebuilds the client with the triggering session and then infers the sample path / hive virtual columns from that session's settings, but hive_partitioning_sample_path_resolved prevents later queries from recomputing it. For non-static object-storage configs that allow per-query credential changes, or even different cast_string_to_date_time_mode / list settings, two sessions can now produce different partition columns or types, yet the first successful one becomes global until restart. This needs a stable construction-time resolution context (or a per-query resolution path) rather than snapshotting the first query's session.

[kssenii]

We could potentially try to avoid double configuration->update call (it will also be called in read() a bit later), but since path sample is resolved only once, we can ignore that.

[kssenii]

Seems there can be a race with StorageObjectStorage::alter which also sets metadata

[evillique]

Now both of them run under hive_partitioning_resolution_mutex.

[kssenii]

May be makes sense to introduce a setting to make this throw instead of a warning, because it could change query result when user does not expect it. WDYT?

[evillique]

Good idea, I added a separate throw_on_hive_partitioning_resolution_failure setting enabled by default and disabled with older compatibility.

[kssenii]

Not sure how useful is this, since I'd expect a different error code for object storage related access error, or it is for a different kind of access?

[evillique]

Indeed, it was incorrect in the first place and not needed now anyway. I removed it.

[antaljanosbenjamin]

Suggested change

	bool hive_partitioning_sample_path_resolved = false;
	bool hive_partitioning_sample_path_resolved = false TSA_GUARDED_BY(hive_partitioning_resolution_mutex);

[clickhouse-gh]

The new eager-on-first-query call here makes the first selective read pay an unconditional bucket listing before _path pushdown has a chance to narrow anything. The 03741_s3_glob_table_path_pushdown reference change from 1 4 to 2 4 is exactly this extra S3ListObjects. For large globbed tables that means SELECT ... WHERE _path = ... now does one full list just to discover hive metadata, even when the query never touches hive columns. Can we defer resolution until a query actually needs hive virtuals, or reuse the first read-side iterator instead of issuing a separate getPathSample list?

[evillique]

The same listing previously happened at CREATE/ATTACH — every server start, every globbed table — so moving it to the first query costs one ListObjectsV2 per storage instance and fewer overall; deferring further isn't possible because the hive columns must exist during identifier resolution, before read().

[clickhouse-gh]

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	86.50%	86.50%	+0.00%
Functions	91.90%	91.90%	+0.00%
Branches	78.60%	78.60%	+0.00%

Changed lines: Changed C/C++ lines covered: 83/86 (96.51%) · Uncovered code

Full report · Diff report