Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add experimental support for descending sorting keys in MergeTree tables #71095

Merged
merged 19 commits into from
Dec 12, 2024

Conversation

amosbird
Copy link
Collaborator

@amosbird amosbird commented Oct 27, 2024

Changelog category (leave one):

  • New Feature

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

This PR introduces a new setting allow_experimental_reverse_key that enables support for descending sort order in MergeTree sorting keys. This is useful for time series analysis, especially TopN queries. Example usage:

CREATE TABLE example
(
    time DateTime,
    key Int32,
    value String
) ENGINE = MergeTree
ORDER BY (time DESC, key);  -- Descending order for 'time' field
SETTINGS allow_experimental_reverse_key = 1;

SELECT * from example WHERE key = 'xxx' ORDER BY time DESC LIMIT 10;  -- This will apply more efficient ReadInOrder optimization instead of ReadInReverseOrder

Some preliminary perf test:

create table event_log_asc (dt DateTime, key UInt16) engine MergeTree order by dt;

insert into event_log_asc select now() - number, rand() from numbers(400000000);

optimize table event_log_asc final;

create table event_log_desc (dt DateTime, key UInt16) engine MergeTree order by dt desc settings allow_experimental_reverse_key = 1;

insert into event_log_desc select * from event_log_asc;

optimize table event_log_desc final;

SELECT *
FROM event_log_asc
WHERE key = 123
ORDER BY dt DESC
LIMIT 10

10 rows in set. Elapsed: 0.109 sec. Processed 27.53 million rows, 61.34 MB (253.50 million rows/s., 564.70 MB/s.)

SELECT *
FROM event_log_desc
WHERE key = 123
ORDER BY dt DESC
LIMIT 10

10 rows in set. Elapsed: 0.024 sec. Processed 25.44 million rows, 56.27 MB (1.06 billion rows/s., 2.35 GB/s.)

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

Information about CI checks: https://clickhouse.com/docs/en/development/continuous-integration/

CI Settings (Only check the boxes if you know what you are doing):

  • Allow: All Required Checks
  • Allow: Stateless tests
  • Allow: Stateful tests
  • Allow: Integration Tests
  • Allow: Performance tests
  • Allow: All Builds
  • Allow: batch 1, 2 for multi-batch jobs
  • Allow: batch 3, 4, 5, 6 for multi-batch jobs

  • Exclude: Style check
  • Exclude: Fast test
  • Exclude: All with ASAN
  • Exclude: All with TSAN, MSAN, UBSAN, Coverage
  • Exclude: All with aarch64, release, debug

  • Run only fuzzers related jobs (libFuzzer fuzzers, AST fuzzers, etc.)
  • Exclude: AST fuzzers

  • Do not test
  • Woolen Wolfdog
  • Upload binaries for special builds
  • Disable merge-commit
  • Disable CI cache

@robot-ch-test-poll3 robot-ch-test-poll3 added the pr-feature Pull request with new product feature label Oct 27, 2024
@robot-ch-test-poll2
Copy link
Contributor

robot-ch-test-poll2 commented Oct 27, 2024

This is an automated comment for commit 663d26e with description of existing statuses. It's updated for the latest CI running

❌ Click here to open a full report in a separate page

Check nameDescriptionStatus
Stateless testsRuns stateless functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc❌ failure
Successful checks
Check nameDescriptionStatus
AST fuzzerRuns randomly generated queries to catch program errors. The build type is optionally given in parenthesis. If it fails, ask a maintainer for help✅ success
BuildsThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS✅ success
ClickBenchRuns [ClickBench](https://github.com/ClickHouse/ClickBench/) with instant-attach table✅ success
Compatibility checkChecks that clickhouse binary runs on distributions with old libc versions. If it fails, ask a maintainer for help✅ success
Docker keeper imageThe check to build and optionally push the mentioned image to docker hub✅ success
Docker server imageThe check to build and optionally push the mentioned image to docker hub✅ success
Docs checkBuilds and tests the documentation✅ success
Fast testNormally this is the first check that is ran for a PR. It builds ClickHouse and runs most of stateless functional tests, omitting some. If it fails, further checks are not started until it is fixed. Look at the report to see which tests fail, then reproduce the failure locally as described here✅ success
Flaky testsChecks if new added or modified tests are flaky by running them repeatedly, in parallel, with more randomization. Functional tests are run 100 times with address sanitizer, and additional randomization of thread scheduling. Integration tests are run up to 10 times. If at least once a new test has failed, or was too long, this check will be red. We don't allow flaky tests, read the doc✅ success
Install packagesChecks that the built packages are installable in a clear environment✅ success
Integration testsThe integration tests report. In parenthesis the package type is given, and in square brackets are the optional part/total tests✅ success
Performance ComparisonMeasure changes in query performance. The performance test report is described in detail here. In square brackets are the optional part/total tests✅ success
Stateful testsRuns stateful functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc✅ success
Stress testRuns stateless functional tests concurrently from several clients to detect concurrency-related errors✅ success
Style checkRuns a set of checks to keep the code style clean. If some of tests failed, see the related log from the report✅ success
Unit testsRuns the unit tests for different release types✅ success
Upgrade checkRuns stress tests on server version from last release and then tries to upgrade it to the version from the PR. It checks if the new server can successfully startup without any errors, crashes or sanitizer asserts✅ success

@alexey-milovidov alexey-milovidov added the can be tested Allows running workflows for external contributors label Oct 27, 2024
@amosbird amosbird marked this pull request as ready for review October 28, 2024 02:19
@KochetovNicolai KochetovNicolai self-assigned this Oct 28, 2024
@KochetovNicolai KochetovNicolai force-pushed the implement-23210 branch 2 times, most recently from 52d1398 to aea4036 Compare November 1, 2024 10:41
@KochetovNicolai KochetovNicolai added this pull request to the merge queue Dec 12, 2024
Merged via the queue into ClickHouse:master with commit 204fd5b Dec 12, 2024
215 of 218 checks passed
@robot-ch-test-poll1 robot-ch-test-poll1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Dec 12, 2024
@@ -202,6 +202,7 @@ namespace ErrorCodes
DECLARE(String, storage_policy, "default", "Name of storage disk policy", 0) \
DECLARE(String, disk, "", "Name of storage disk. Can be specified instead of storage policy.", 0) \
DECLARE(Bool, allow_nullable_key, false, "Allow Nullable types as primary keys.", 0) \
DECLARE(Bool, allow_experimental_reverse_key, false, "Allow descending sorting key in MergeTree tables (experimental feature).", 0) \
Copy link
Member

Choose a reason for hiding this comment

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

Please mark the setting as EXPERIMENTAL in the flags (the 0 at the end). Otherwise it's not experimental, just it has a different name

Copy link
Contributor

Choose a reason for hiding this comment

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

seems to need validation in the code

baibaichen added a commit to Kyligence/gluten that referenced this pull request Dec 13, 2024
baibaichen added a commit to Kyligence/gluten that referenced this pull request Dec 13, 2024
baibaichen added a commit to apache/incubator-gluten that referenced this pull request Dec 13, 2024
* [GLUTEN-1632][CH]Daily Update Clickhouse Version (20241213)

* Fix build due to ClickHouse/ClickHouse#71095

---------

Co-authored-by: kyligence-git <[email protected]>
Co-authored-by: Chang Chen <[email protected]>
@alexey-milovidov
Copy link
Member

@alexey-milovidov
Copy link
Member

Although, there is good speedup when I read a small number of columns.

@amosbird
Copy link
Collaborator Author

It does not give any speedup:

Are you using this query to verify?

SELECT *
FROM hits_desc
ORDER BY EventTime DESC
LIMIT 10
FORMAT Vertical

You can try adding non-primary key filters that cannot be satisfied by the last few granules alone (e.g., filters that result in fewer than 10 matches).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
can be tested Allows running workflows for external contributors pr-feature Pull request with new product feature pr-synced-to-cloud The PR is synced to the cloud repo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants