Skip to content

Infer wildcard partition strategy from a {_partition_id} path on CREATE - #111279

Merged
fm4v merged 7 commits into
masterfrom
nik/fix-implicit-wildcard-partition-strategy-compat
Jul 25, 2026
Merged

Infer wildcard partition strategy from a {_partition_id} path on CREATE#111279
fm4v merged 7 commits into
masterfrom
nik/fix-implicit-wildcard-partition-strategy-compat

Conversation

@fm4v

@fm4v fm4v commented Jul 21, 2026

Copy link
Copy Markdown
Member

Related: #107437
Related: #86746
Related: #106465

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 into CHANGELOG.md):

A {_partition_id} placeholder in the path of a file-like engine (S3, AzureBlobStorage, URL, etc.) with no explicit partition_strategy implies the wildcard strategy again, regardless of file_like_engine_default_partition_strategy. This restores backward compatibility for pre-26.6 DDL that started failing with BAD_ARGUMENTS ("Partition strategy hive can not be used with a '_partition_id' wildcard in the path") after #107437.


Motivation

#86746 made hive the default partition strategy for file-like engines from compatibility >= 26.6, but an unconditional compatibility shim kept {_partition_id} paths working. #107437 removed that shim to fix the validation inconsistency from #106465 — and thereby broke every pre-26.6 CREATE statement (and DDL generator) that uses a {_partition_id} path with no explicit partition_strategy: under the hive default they now fail with BAD_ARGUMENTS.

The key observation: the two strategies are mutually exclusive on path shape — wildcard requires {_partition_id} in the path, hive forbids it. So when the user writes {_partition_id} and does not specify a strategy, the path alone determines the only strategy that can possibly work. Rejecting it forces the user to type partition_strategy = 'wildcard' — information the path already carries unambiguously.

This PR makes initPartitionStrategy infer wildcard from the path shape on CREATE, exactly the way it already does on ATTACH / server startup / RESTORE since #107437. The file_like_engine_default_partition_strategy default now only applies to paths without the placeholder. The actual validation fix of #107437 is fully preserved: an explicit partition_strategy = 'hive' with a {_partition_id} path still raises BAD_ARGUMENTS, and ATTACH path-shape inference is unchanged.

Full behavior matrix

CREATE (and table functions), with PARTITION BY:

Path has {_partition_id} Explicit partition_strategy Effective default (via compatibility) ≤ 26.5 26.6.1 (after #86746) 26.6.2 / 26.7 (after #107437) This fix
yes wildcard (compat < 26.6) ✅ wildcard ✅ wildcard ✅ wildcard ✅ wildcard
yes hive (compat ≥ 26.6) n/a ✅ wildcard BAD_ARGUMENTS (the break) ✅ wildcard (restored)
yes wildcard any ✅ wildcard ✅ wildcard ✅ wildcard ✅ wildcard
yes hive any BAD_ARGUMENTS BAD_ARGUMENTS BAD_ARGUMENTS BAD_ARGUMENTS
no wildcard (compat < 26.6) BAD_ARGUMENTS (wildcard requires the placeholder) BAD_ARGUMENTS BAD_ARGUMENTS BAD_ARGUMENTS
no hive (compat ≥ 26.6) n/a ✅ hive ✅ hive ✅ hive
no wildcard any BAD_ARGUMENTS BAD_ARGUMENTS BAD_ARGUMENTS BAD_ARGUMENTS
no hive any ✅ hive ✅ hive ✅ hive ✅ hive

ATTACH / server startup / RESTORE / replicated-DDL replay, implicit strategy (strategy is derived from path shape, never from the mutable default — introduced by #107437, unchanged here):

Path has {_partition_id} 26.6.1 (after #86746) 26.6.2 / 26.7 (after #107437) This fix
yes ✅ wildcard (via the shim) ✅ wildcard ✅ wildcard
no ✅/❌ consulted the current default ✅ hive ✅ hive

The only behavior change of this PR is the single bold cell: implicit strategy + {_partition_id} path on CREATE under the hive default works again, as it did in every release before 26.6.2 / 26.7. Every rejection introduced or kept by #107437 for explicit strategies remains in force, so the inconsistency from #106465 does not come back: the implicit case is not "silently accepted as hive", it deterministically resolves to wildcard — the same table you would get on any pre-26.6 server.

Why this matters

CREATE TABLE ... ENGINE = S3('.../{_partition_id}.native', ...) PARTITION BY ... is a long-standing documented pattern; it is emitted by existing DDL generators (for example, orchestration tooling that provisions system log tables with object storage). After #107437 (26.7.1.1258, backported to 26.6.2.92) such CREATE statements fail with BAD_ARGUMENTS within a stable release series. Since only CREATE is affected (ATTACH/startup infer from path shape), running tables survive the upgrade but any re-provisioning breaks — the worst kind of surprise. This PR should be backported to 26.6.

Tests

  • 04337_s3_hive_partition_id_compatibility_validation.sql (from Fix hive partition validation when compatibility is 26.6 #107437) updated: the implicit + {_partition_id} + compatibility = '26.6' case now expects success, and an explicit-hive rejection case was added; the ATTACH regression cases are unchanged.
  • New 04614_implicit_wildcard_partition_strategy_from_path.sql: write/read round-trip through an implicit-wildcard table under the hive default, DETACH/ATTACH survival, explicit-hive rejection on the same path, and the table-function INSERT INTO FUNCTION s3(...) PARTITION BY case.

Version info

  • Merged into: 26.8.1.120 (included in 26.8 and later)
  • Backported to: 26.7.2.19, 26.6.2.116

After #107437, a CREATE
statement with a `{_partition_id}` placeholder in the path and no
explicit `partition_strategy` was rejected with `BAD_ARGUMENTS` whenever
the effective `file_like_engine_default_partition_strategy` default was
`hive` (the default since 26.6). This broke pre-26.6 DDL and DDL
generators that rely on the implicit wildcard strategy.

A `{_partition_id}` placeholder is valid only under the `wildcard`
strategy, so when no explicit strategy is given the path alone
determines the only strategy that can work. `initPartitionStrategy` now
infers `wildcard` from the path shape on CREATE, the same way it already
does on ATTACH / server startup / RESTORE. The default setting only
applies to paths without the placeholder, and an explicit
`partition_strategy = 'hive'` still rejects such paths.

Related: #106465
Related: #86746
@fm4v fm4v added pr-must-backport Pull request should be backported intentionally. Use this label with great care! v26.6-must-backport and removed pr-must-backport Pull request should be backported intentionally. Use this label with great care! labels Jul 21, 2026
@clickhouse-gh

clickhouse-gh Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Workflow [PR], commit [82d543e]

Summary:

job_name test_name status info comment
Stateless tests (amd_tsan, s3 storage, parallel, 2/3) FAIL
04611_join_runtime_filters_index_analysis_skip_index FAIL cidb IGNORED
Stress test (amd_debug) FAIL
Logical error: Inconsistent KeyCondition behavior (STID: 5182-2f27) FAIL cidb IGNORED
Upgrade check (amd_release) FAIL
Error message in clickhouse-server.log (see upgrade_error_messages.txt) FAIL cidb IGNORED

AI Review

Summary

This PR restores the implicit wildcard partition strategy for object-storage writes when the path already contains {_partition_id}, fixing the 26.6/26.7 regression for pre-26.6 DDL. The runtime change and regression coverage look reasonable, but one docs/en alias page still documents the old behavior and will mislead users of gcs.

Findings

⚠️ Majors

  • [docs/en/sql-reference/table-functions/gcs.md:40,193] The gcs docs page still omits the new defaulting rule in its partition_strategy argument description and still goes straight from the Partitioned Write intro into examples that spell out partition_strategy='wildcard' as if it were required. That now contradicts both the implementation and the updated s3/in-source docs, even though gcs is just an alias of s3, so users following the gcs page will still be told the old compatibility story.
    Suggested fix: mirror the updated wording from the gcs source docs in src/TableFunctions/TableFunctionObjectStorage.cpp into the docs/en alias page, including the short note above the Partitioned Write examples that a path containing {_partition_id} already implies wildcard.
Final Verdict

Status: ⚠️ Request changes

@clickhouse-gh clickhouse-gh Bot added the pr-bugfix Pull request with bugfix, not backported by default label Jul 21, 2026
Comment thread src/Storages/ObjectStorage/StorageObjectStorageConfiguration.cpp
The flaky check runs the test several times concurrently against the
same S3 bucket, which exposed three test bugs: S3 keys were shared
between runs, the main read glob `test_04614_implicit_wildcard_*` also
matched the table-function output `..._fn_55`, and the untyped literal
`SELECT 55 AS a` wrote a `UInt8` parquet column - when schema inference
picked that file's schema, every value was cast to `UInt8` (`333` ->
`77`, `4444` -> `92`).

Prefix all keys with `currentDatabase()`, use disjoint `tbl_`/`fn_`
prefixes, and type the literal as `UInt64`.

CI: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=111279&sha=e25642ae15185036b93ee94dc6c2c3c4c0fe52d9&name_0=PR&name_1=Stateless%20tests%20%28amd_asan_ubsan%2C%20flaky%20check%29
PR: #111279
@fm4v
fm4v enabled auto-merge July 22, 2026 08:23
The `partition_strategy` defaulting description in the in-source docs
and the S3/AzureBlobStorage doc pages said the default comes solely
from `file_like_engine_default_partition_strategy`. Since the implicit
strategy is now derived from the path shape first, state that a
`{_partition_id}` placeholder implies `wildcard` and the setting only
applies to paths without the placeholder. Also clarify the setting
description itself.

PR: #111279
@fm4v

fm4v commented Jul 22, 2026

Copy link
Copy Markdown
Member Author

Good catch — the defaulting contract description was indeed stale after this change. Updated in b242110:

  • All 24 occurrences of the two defaulting sentences (in-source docs in registerStorageObjectStorage.cpp / TableFunctionObjectStorage.cpp, plus the mirrored docs/en and docs/reference pages for S3 / AzureBlobStorage engines and table functions) now state that a {_partition_id} placeholder in the path implies wildcard, and that file_like_engine_default_partition_strategy only applies to paths without the placeholder.
  • The description of file_like_engine_default_partition_strategy itself in Settings.cpp now carries the same caveat.

@mintlify

mintlify Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
ClickHouse-docs 🟢 Ready View Preview Jul 22, 2026, 8:51 AM

Master reworded the partition_strategy defaulting docs (setting-only
contract, "paths containing {_partition_id} must explicitly set
partition_strategy='wildcard'"), which conflicts with the path-shape
inference this PR introduces. Kept this branch's wording and applied
the path-shape rule to the reworded gcs variants that merged cleanly.
Comment thread docs/reference/functions/table-functions/gcs.mdx Outdated
Comment thread src/Core/Settings.cpp
The "Partitioned Write" sections of the gcs and azureBlobStorage table
function docs still said that paths containing `{_partition_id}`
require an explicit `wildcard` strategy under the 26.6 `hive` default,
contradicting the argument tables updated earlier in this PR. Also
refresh the generated `session-settings.mdx` entry for
`file_like_engine_default_partition_strategy` to match the new
description in `Settings.cpp`.

PR: #111279
@fm4v

fm4v commented Jul 22, 2026

Copy link
Copy Markdown
Member Author

Both follow-ups addressed in df01945:

  • The "Partitioned Write" paragraphs in the gcs and azureBlobStorage table function docs (in-source at TableFunctionObjectStorage.cpp:937 / :1124 and the rendered docs/reference pages) no longer claim that {_partition_id} paths require an explicit partition_strategy='wildcard' — they now state the placeholder implies wildcard, and the hive default applies only to paths without it.
  • The generated docs/reference/settings/session-settings.mdx entry for file_like_engine_default_partition_strategy is refreshed to match the updated description in Settings.cpp.

A repo-wide sweep for the stale wording ("require an explicit", "is the default partition strategy", "must explicitly set partition_strategy") comes back clean.

Master replaced the static autogenerated session-settings page with the
SessionSettingsExplorer component, deleting the text this branch had
edited. Took master's version: the explorer renders setting
descriptions from source, so the updated
`file_like_engine_default_partition_strategy` description in
`Settings.cpp` flows through without a checked-in page edit.
| `structure` | Structure of the table. Format `'column1_name column1_type, column2_name column2_type, ...'`. |
| `compression_method` | Parameter is optional. Supported values: `none`, `gzip` or `gz`, `brotli` or `br`, `xz` or `LZMA`, `zstd` or `zst`. By default, it will autodetect compression method by file extension. |
| `partition_strategy` | Optional. Supported values: `wildcard` or `hive`. `wildcard` requires `{_partition_id}` in the path. It is the default only when `compatibility` is older than `26.6`; otherwise, including when current defaults apply, the default is `hive`. |
| `partition_strategy` | Optional. Supported values: `wildcard` or `hive`. `wildcard` requires `{_partition_id}` in the path. It is the default when the path contains `{_partition_id}` (the only strategy compatible with such a path), or when `compatibility` is older than `26.6`; otherwise, including when current defaults apply, the default is `hive`. |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

gcs still has one stale public-docs surface in the current tree: docs/en/sql-reference/table-functions/gcs.md was not updated alongside this source/Mintlify change. Its argument table at line 40 still omits that a {_partition_id} path implicitly selects wildcard, and its ## Partitioned Write section at line 191 still lacks the new note that the explicit partition_strategy='wildcard' is optional. Since docs/en is the current Docusaurus source, the published gcs docs stay inconsistent unless that page is updated too.

The "Docs check (Mintlify)" job forbids direct edits inside
AUTOGENERATED regions of docs/reference. The partition strategy
defaulting fixes remain in the generator sources: the in-source docs in
registerStorageObjectStorage.cpp / TableFunctionObjectStorage.cpp, the
docs/en pages, and the Settings.cpp setting description. The generated
mirrors will be refreshed by the docs pipeline.

CI: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=111279&sha=866a3ecbe1b58da3f25485dab0f99931956a5a23&name_0=PR&name_1=Docs%20check%20%28Mintlify%29
PR: #111279
@kssenii kssenii self-assigned this Jul 24, 2026
@clickhouse-gh

clickhouse-gh Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

LLVM Coverage Report

Metric Baseline Current Δ
Lines 86.20% 86.30% +0.10%
Functions 92.00% 92.00% +0.00%
Branches 78.40% 78.40% +0.00%

Changed lines: Changed C/C++ lines covered: 67/67 (100.00%) · Uncovered code

Full report · Diff report

@fm4v
fm4v added this pull request to the merge queue Jul 25, 2026
Merged via the queue into master with commit 2fe24af Jul 25, 2026
175 of 179 checks passed
@fm4v
fm4v deleted the nik/fix-implicit-wildcard-partition-strategy-compat branch July 25, 2026 13:42
@robot-ch-test-poll robot-ch-test-poll added the pr-synced-to-cloud The PR is synced to the cloud repo label Jul 25, 2026
@robot-clickhouse-ci-1 robot-clickhouse-ci-1 added the pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR label Jul 25, 2026
fm4v added a commit that referenced this pull request Jul 26, 2026
Backport #111279 to 26.7: Infer wildcard partition strategy from a `{_partition_id}` path on CREATE
robot-ch-test-poll3 added a commit that referenced this pull request Jul 26, 2026
Cherry pick #111279 to 26.6: Infer wildcard partition strategy from a `{_partition_id}` path on CREATE
robot-clickhouse added a commit that referenced this pull request Jul 26, 2026
@robot-ch-test-poll4 robot-ch-test-poll4 added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Jul 26, 2026
clickhouse-gh Bot added a commit that referenced this pull request Jul 26, 2026
Backport #111279 to 26.6: Infer wildcard partition strategy from a `{_partition_id}` path on CREATE
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-bugfix Pull request with bugfix, not backported by default pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR pr-synced-to-cloud The PR is synced to the cloud repo v26.6-must-backport

Projects

None yet

Development

Successfully merging this pull request may close these issues.

S3 engine is accepted with partition_strategy='hive' and {_partition_id} placeholder

5 participants