Skip to content

Allow any deterministic or injective functions in PK to prune granules in KeyCondition#92952

Merged
alexey-milovidov merged 80 commits intoClickHouse:masterfrom
nihalzp:allow-det-functions-in-pk
Feb 22, 2026
Merged

Allow any deterministic or injective functions in PK to prune granules in KeyCondition#92952
alexey-milovidov merged 80 commits intoClickHouse:masterfrom
nihalzp:allow-det-functions-in-pk

Conversation

@nihalzp
Copy link
Copy Markdown
Member

@nihalzp nihalzp commented Dec 23, 2025

Only deterministic expression in PK:

Example of a deterministic (but non-injective) primary key:

ENGINE = MergeTree()
ORDER BY left(pod, length(pod) - length(substringIndex(pod, '-', -1)) - 1);

Example predicates that can use the index:

SELECT * FROM table WHERE pod = 'alice';
SELECT * FROM table WHERE pod IN ('alice', 'bob');
SELECT * FROM table WHERE has(['alice', 'bob'], pod);

Deterministic + Injective expression in PK:

Example of an injective primary key:

ENGINE = MergeTree()
ORDER BY reverse(tuple(reverse(p), hex(p)))

Example predicates that can effectively use the index:

SELECT * FROM table WHERE p != 'abc';
SELECT * FROM table WHERE p NOT IN ('abc', '12345');
SELECT * FROM table WHERE NOT has(['abc', '12345'], p);

See tests/queries/0_stateless/03778_deterministic_injective_functions_key_condition.sql for concrete examples.

Changelog category (leave one):

  • Performance Improvement

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

Allow any deterministic expression in Primary Key to be used for data skipping (e.g. ORDER BY cityHash64(user_id)/ ORDER BY length(user_id)). For deterministic expressions, ClickHouse can apply the expression to query constants and use the result in the primary key index for predicates like =, IN, and has. If the expression is also injective (e.g. ORDER BY hex(p) or ORDER BY reverse(tuple(reverse(p), hex(p)))), we can effectively use the index for the negated forms: !=, NOT IN, and NOT has. Closes #10685. Closes #82161.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Dec 23, 2025

Workflow [PR], commit [818a29a]

Summary:

@clickhouse-gh clickhouse-gh bot added the pr-performance Pull request with some performance improvements label Dec 23, 2025
Copy link
Copy Markdown
Collaborator

@amosbird amosbird left a comment

Choose a reason for hiding this comment

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

Overall LGTM. The idea is clear.

/// however, there could be other functions that can throw on some inputs.
try
{
dag.actions->execute(block, false, true);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

IIRC, we don't allow using exceptions as part of normal control flow.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I am not very proud of the try-catch block, but I do not see a good alternative: we cannot know in advance whether a given constant can be transformed safely. ClickHouse has too many deterministic functions and many of them throw on invalid input. We also cannot let those errors propagate, because no pruning is better than the query failing to execute due to an exception during index analysis.

Let me know if you have any ideas on how to avoid this.

if (out_column->isNullable())
{
const auto & n = assert_cast<const ColumnNullable &>(*out_column);
for (char8_t b : n.getNullMapData())
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't understand the null-handling logic here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This null-handling is to check if the dag-transformed column contains any NULL or not. If it contains any NULL, then we reject and do not create the atom.

I have refactored the function to make it more readable. Let me know what you think.


/// Looking for possible transformation of `column = constant` into `partition_expr = function(constant)`
bool KeyCondition::canConstantBeWrappedByFunctions(
bool KeyCondition::extractDeterministicFunctionsDagFromKey(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A comment with an example would help clarify what is being extracted.

}


bool applyDeterministicDagToColumn(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A comment with an example would help explain the transformation.

@nihalzp nihalzp requested a review from amosbird January 30, 2026 10:40
Example of a deterministic (but non-injective) primary key:
```sql
ENGINE = MergeTree()
ORDER BY cityHash64(user_id)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe length(user_id) will make a more intuitive example (not everyone might know why cityHash isn't injective, l. 308).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good point. I will update it.

@nihalzp
Copy link
Copy Markdown
Member Author

nihalzp commented Feb 10, 2026

This query:

SET use_statistics=1;
SET optimize_move_to_prewhere=1;

DROP TABLE IF EXISTS test;
CREATE TABLE test (x Float64, y UInt64)
ENGINE = MergeTree
ORDER BY y
SETTINGS index_granularity=1, auto_statistics_types='minmax';

INSERT INTO test
SELECT if(number % 10 = 0, toFloat64('nan'), toFloat64(number)), number
FROM numbers(100000);

SELECT count() FROM test WHERE x < toFloat64('nan') AND y > 99990;

has the following UBSAN error (found by Stateless tests (amd_ubsan) by mutating settings of 03779_float_nan_key_condition.sql):

/src/Storages/Statistics/ConditionSelectivityEstimator.cpp:132:39: runtime error: nan is outside the range of representable values of type 'unsigned long'

This bug is also reproducible in master. This PR also contains the fix.

@nihalzp nihalzp linked an issue Feb 18, 2026 that may be closed by this pull request
@alexey-milovidov alexey-milovidov merged commit 7c57219 into ClickHouse:master Feb 22, 2026
262 of 264 checks passed
@robot-ch-test-poll robot-ch-test-poll added the pr-synced-to-cloud The PR is synced to the cloud repo label Feb 22, 2026
@PedroTadim
Copy link
Copy Markdown
Member

Hello! Could you add a performance test showing data being skipped, then the query latency is improved?

@nihalzp
Copy link
Copy Markdown
Member Author

nihalzp commented Feb 23, 2026

@PedroTadim The EXPLAIN queries show the data being skipped. There are some existing performance that have sped up:

image image

@PedroTadim
Copy link
Copy Markdown
Member

@PedroTadim The EXPLAIN queries show the data being skipped. There are some existing performance that have sped up:
image image

Ok, maybe next time you could add a snapshot like that one to show the performance increase.

Algunenano pushed a commit to Algunenano/ClickHouse that referenced this pull request Feb 24, 2026
…in-pk

Allow any deterministic or injective functions in PK to prune granules in KeyCondition
@rschu1ze rschu1ze added the post-approved Approved, but after the PR is merged. label Mar 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

post-approved Approved, but after the PR is merged. pr-performance Pull request with some performance improvements pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

7 participants