Fix create statement when using default minmax indicies by Algunenano · Pull Request #91429 · ClickHouse/ClickHouse

Algunenano · 2025-12-03T15:20:06Z

Changelog category (leave one):

Backward Incompatible Change

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

Multiple fixes around implicit indices. The schema shown or stored (keeper metadata) won't include implicit indices, such as the ones created by settings add_minmax_index_for_numeric_columns or add_minmax_index_for_string_columns. This might cause metadata errors when a ReplicatedMergeTree table is created or updated in a newer version, while there is a replica in an older release. For those cases send DDLs to the old replica until the cluster is upgraded completely.

This PR:

Extends and slightly simplifies Fix create statement when using default minmax indicies #87723.
Fixes implicit indices when the column type changes (related to Add a setting to control what to do with indices during mutations #89335)
Fixes implicit indices with ephemeral or alias columns
Fixes ReplicatedMergeTree metadata sharing with implicit indices, so it does not include them (as they are not included in disk metadata). This is necessary to treat them properly as implicitly. It creates a minor incompatibility between versions.

Closes #87286.
Closes #87723

Documentation entry for user-facing changes

Documentation is written (mandatory for new features)

clickhouse-gh · 2025-12-03T15:20:37Z

Workflow [PR], commit [3a2c52f]

Summary: ❌

job_name	test_name	status	info
Stateless tests (amd_binary, old analyzer, s3 storage, DatabaseReplicated, parallel)		failure
	01287_max_execution_speed	FAIL	cidb
Stateless tests (amd_msan, parallel)		failure
	00411_long_accurate_number_comparison_int2	FAIL	cidb
	02992_all_columns_should_have_comment	FAIL	cidb
	00816_long_concurrent_alter_column	FAIL	cidb
	03581_parallel_replicas_read_empty_ranges	FAIL	cidb
	01070_exception_code_in_query_log_table	FAIL	cidb
	01661_referer	FAIL	cidb
Stateless tests (amd_ubsan, parallel)		failure
	02844_max_backup_bandwidth_s3	FAIL	cidb
Stateless tests (amd_tsan, s3 storage, sequential, 1/2)		failure
	01014_lazy_database_basic	FAIL	cidb
BuzzHouse (amd_debug)		failure
	Logical error: 'Inconsistent AST formatting: the query: (STID: 2508-11b5)	FAIL	cidb

Copilot

Pull request overview

This PR fixes issues with implicit minmax indices created by settings add_minmax_index_for_numeric_columns and add_minmax_index_for_string_columns. The key change is that these implicit indices are now excluded from CREATE statements and ReplicatedMergeTree metadata, preventing version incompatibility issues. The PR also fixes implicit index handling when column types change and for ephemeral/alias columns.

Key Changes

Implicit indices are now tracked with an is_implicitly_created flag instead of relying on name prefix detection
CREATE statements and keeper metadata no longer include implicit indices (using new explicitToString() method)
Type changes on columns now properly update their implicit indices

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/Storages/IndicesDescription.h	Added `is_implicitly_created` flag and split `toString()` into `explicitToString()` and `allToString()` methods
src/Storages/IndicesDescription.cpp	Implemented explicit/all string conversion and updated index parsing to accept `is_implicitly_created` parameter
src/Storages/StorageInMemoryMetadata.h	Added implicit index settings flags and helper methods for managing implicit indices
src/Storages/StorageInMemoryMetadata.cpp	Implemented `addImplicitIndicesForColumn()` and `dropImplicitIndicesForColumn()` with validation
src/Storages/MergeTree/registerStorageMergeTree.cpp	Refactored implicit index creation to use new helper methods
src/Storages/MergeTree/ReplicatedMergeTreeTableMetadata.cpp	Updated to use `explicitToString()` for metadata comparison with backward compatibility check
src/Storages/StorageReplicatedMergeTree.cpp	Changed to use `explicitToString()` when comparing indices for alter operations
src/Storages/AlterCommands.cpp	Simplified ALTER logic to use new helper methods and handle type changes
src/Storages/MergeTree/MergeTreeIndices.h	Exposed `implicitValidation()` method
src/Storages/MergeTree/MergeTreeIndices.cpp	Extracted validation logic to be reusable for implicit index creation
src/Storages/MergeTree/PatchParts/RangesInPatchParts.cpp	Updated to check `isImplicitlyCreated()` flag instead of name prefix
src/Interpreters/InterpreterCreateQuery.cpp	Filtered out implicit indices when formatting CREATE statements
src/Databases/DatabasesCommon.cpp	Updated validation to pass `is_implicitly_created` parameter
tests/queries/0_stateless/03632_default_minmax_indices_alter.sql	Test for basic ALTER with implicit indices
tests/queries/0_stateless/03632_default_minmax_indices_alter.reference	Expected output for basic ALTER test
tests/queries/0_stateless/03748_default_minmax_indices_alter.sql	Comprehensive ALTER test covering type changes and renames
tests/queries/0_stateless/03748_default_minmax_indices_alter.reference	Expected output for comprehensive ALTER test
tests/queries/0_stateless/03749_implicit_index_ephemeral_alias.sql	Test for implicit indices with ephemeral and alias columns
tests/queries/0_stateless/03749_implicit_index_ephemeral_alias.reference	Expected output for ephemeral/alias column test

src/Storages/StorageInMemoryMetadata.cpp

Copilot · 2025-12-04T10:53:21Z

src/Storages/IndicesDescription.cpp

+        if (!index.isImplicitlyCreated())
+            list.children.push_back(index.definition_ast);


Add a comment explaining that only explicit indices are included in the output string to maintain backward compatibility and prevent metadata mismatches between versions.

src/Storages/MergeTree/ReplicatedMergeTreeTableMetadata.cpp

Co-authored-by: Copilot <[email protected]>

src/Storages/MergeTree/ReplicatedMergeTreeTableMetadata.cpp

SmitaRKulkarni

Rest all LGTM

SmitaRKulkarni and others added 6 commits December 2, 2025 18:25

Fix create statement when using default minmax indicies

12794da

Fix is_implicitly_created flag usage

f00a916

Address review comments

52272df

Fix after rebase and bugs around column type changes

aa9b953

Fix implicit indices with ephemeral and alias columns

91c9f15

Fix: Only store explicit indices for RMT metadata

0ac925d

clickhouse-gh bot added the pr-backward-incompatible Pull request with backwards incompatible changes label Dec 3, 2025

Algunenano marked this pull request as ready for review December 3, 2025 19:40

Algunenano requested a review from Copilot December 4, 2025 10:52

Copilot AI reviewed Dec 4, 2025

View reviewed changes

Apply suggestions from code review

3a2c52f

Co-authored-by: Copilot <[email protected]>

SmitaRKulkarni self-assigned this Dec 4, 2025

SmitaRKulkarni mentioned this pull request Dec 4, 2025

Fix create statement when using default minmax indicies #87723

Closed

SmitaRKulkarni reviewed Dec 4, 2025

View reviewed changes

src/Storages/MergeTree/ReplicatedMergeTreeTableMetadata.cpp Show resolved Hide resolved

SmitaRKulkarni approved these changes Dec 4, 2025

View reviewed changes

Algunenano enabled auto-merge December 4, 2025 17:10

Algunenano added this pull request to the merge queue Dec 4, 2025

Merged via the queue into ClickHouse:master with commit 5f1325f Dec 4, 2025
124 of 130 checks passed

Algunenano deleted the fix_default_minmax branch December 4, 2025 17:27

Algunenano mentioned this pull request Dec 4, 2025

Minmax indices by default #76867

Draft

robot-clickhouse-ci-1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Dec 4, 2025

EmeraldShift mentioned this pull request Jan 4, 2026

Implicit minmax indexes for date/time types #93354

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix create statement when using default minmax indicies#91429

Fix create statement when using default minmax indicies#91429
Algunenano merged 7 commits intoClickHouse:masterfrom
Algunenano:fix_default_minmax

Algunenano commented Dec 3, 2025 •

edited

Loading

Uh oh!

clickhouse-gh bot commented Dec 3, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Dec 4, 2025

Uh oh!

Uh oh!

Uh oh!

SmitaRKulkarni left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		if (!index.isImplicitlyCreated())
		list.children.push_back(index.definition_ast);

Conversation

Algunenano commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog category (leave one):

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

Documentation entry for user-facing changes

Uh oh!

clickhouse-gh bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

SmitaRKulkarni left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Algunenano commented Dec 3, 2025 •

edited

Loading

clickhouse-gh bot commented Dec 3, 2025 •

edited

Loading