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

Fix PREWHERE for Merge with different default types #46454

Merged
merged 2 commits into from Feb 17, 2023

Conversation

azat
Copy link
Collaborator

@azat azat commented Feb 15, 2023

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in official stable or prestable release)

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

Fix PREWHERE for Merge with different default types (fixes some NOT_FOUND_COLUMN_IN_BLOCK when the default type for the column differs, also allow PREWHERE when the type of column is the same across tables, and prohibit it, only if it differs)

In case of underlying table has an ALIAS for this column, while in Merge table it is not marked as an alias, there will NOT_FOUND_COLUMN_IN_BLOCK error.

Further more, when underlying tables has different default type for the column, i.e. one has ALIAS and another has real column, then you will also get NOT_FOUND_COLUMN_IN_BLOCK, because Merge engine should take care of this.

Also this patch reworks how PREWHERE is handled for Merge table, and now if you use PREWHERE on the column that has the same type and default type (ALIAS, ...) then it will be possible, and only if the type differs, it will be prohibited and throw ILLEGAL_PREWHERE error.

And last, but not least, also respect this restrictions for optimize_move_to_prewhere.

Cc: @vdimir
Fixes: #46286

@robot-clickhouse-ci-2 robot-clickhouse-ci-2 added the pr-bugfix Pull request with bugfix, not backported by default label Feb 15, 2023
@azat azat force-pushed the merge-alias-prewhere branch 2 times, most recently from e066165 to fe6f95c Compare February 15, 2023 21:31
@vdimir vdimir self-assigned this Feb 16, 2023
@azat
Copy link
Collaborator Author

azat commented Feb 16, 2023

ClickHouse build check — 9/20 artifact groups are OK (7 of 8 builds are OK)

Feb 16 14:47:02 + DESTDIR=/build/packages/root
Feb 16 14:47:02 + ninja programs/install
...
Feb 16 14:54:17 matching "./root/usr/bin/clickhouse-report": file does not exist

vs

Feb 16 00:08:06 + DESTDIR=/build/packages/root
Feb 16 00:08:06 + ninja install
...
Feb 16 00:14:58 -- Installing: /build/packages/root/usr/bin/clickhouse-report

Conflict with #46367, rebasing.

In case of underlying table has an ALIAS for this column, while in Merge
table it is not marked as an alias, there will NOT_FOUND_COLUMN_IN_BLOCK
error.

Further more, when underlying tables has different default type for the
column, i.e. one has ALIAS and another has real column, then you will
also get NOT_FOUND_COLUMN_IN_BLOCK, because Merge engine should take
care of this.

Also this patch reworks how PREWHERE is handled for Merge table, and now
if you use PREWHERE on the column that has the same type and default
type (ALIAS, ...) then it will be possible, and only if the type
differs, it will be prohibited and throw ILLEGAL_PREWHERE error.

And last, but not least, also respect this restrictions for
optimize_move_to_prewhere.

v2: introduce IStorage::supportedPrewhereColumns()
v3: Remove excessive condition for PREWHERE in StorageMerge::read()
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
@azat
Copy link
Collaborator Author

azat commented Feb 17, 2023

Test failures does not looks related:

Integration tests (asan) [4/6] — fail: 1, passed: 258, flaky: 0

Stateless tests (tsan) [2/5] — Tests are not finished, fail: 1, passed: 347, skipped: 2

  • 01682_gather_utils_ubsan
2023-02-16 23:04:51 01682_gather_utils_ubsan:                                               [ FAIL ] 0.00 sec. - server died

The server was working, but apparently was too slow?

2023.02.16 23:04:54.317579 [ 3396 ] {baa64b54-0628-4b02-8f40-b46bc80cdc09} <Error> executeQuery: Code: 210. DB::NetException: Connection reset by peer, while reading from socket ([::1]:46250). (NETWORK_ERROR) (version 23.2.1.1) (from [::1]:46250) (comment: 01700_deltasum.sql) (in query: select deltaSumMerge(rows) as delta_sum from ( select * from ( select deltaSumState(arrayJoin([3, 5])) as rows union all select deltaSumState(arrayJoin([1, 2])) as rows union all select deltaSumState(arrayJoin([4, 6])) as rows ) order by rows ) order by delta_sum;), Stack trace (when copying this message, always include the lines below):
...
2023.02.16 23:05:01.858861 [ 668 ] {} <Information> Application: Child process exited normally with code 0.

Stress test (ubsan) — Killed by signal (in clickhouse-server.log)

It was still trying to read after the query finished, hence the logical error:

2023.02.17 00:50:37.081501 [ 2089 ] {87495cdd-4221-43ae-97ce-431b2866c939} <Test> CachedOnDiskReadBufferFromFile: Current count: 0, position: 0, read range: [0:None], file segment: File segment: [1168, 1910], key: aed7dbe66431615aa782a40463732410, state: DOWNLOADED, downloaded size: 743, reserved size: 743, downloader id: None, current write offset: 1911, first non-downloaded offset: 1911, caller id: 87495cdd-4221-43ae-97ce-431b2866c939:2089, detached: 0, kind: Regular
2023.02.17 00:50:37.081571 [ 2089 ] {87495cdd-4221-43ae-97ce-431b2866c939} <Test> CachedOnDiskReadBufferFromFile: Read 743 bytes, read type CACHED, position: 0, offset: 743, remaining read range: [743:None]
2023.02.17 00:50:37.081656 [ 2089 ] {87495cdd-4221-43ae-97ce-431b2866c939} <Test> CachedOnDiskReadBufferFromFile: Key: aed7dbe66431615aa782a40463732410. Returning with 743 bytes, buffer position: 1168 (offset: 0, predownloaded: 0), buffer available: 743, current range: [1168, 1910], current offset: 1911, file segment state: DOWNLOADED, current write offset: 1911, read_type: CACHED, reading until position: 1911, started with offset: 1168, remaining ranges: [1168, 1910]
2023.02.17 00:50:37.082580 [ 2723 ] {87495cdd-4221-43ae-97ce-431b2866c939} <Test> CachedOnDiskReadBufferFromFile: Having 1 file segments to read: [0, 466499], current offset: 90241
2023.02.17 00:50:37.082948 [ 2597 ] {} <Error> virtual DB::AsynchronousReadIndirectBufferFromRemoteFS::~AsynchronousReadIndirectBufferFromRemoteFS(): Code: 241. DB::Exception: Memory limit (total) exceeded: would use 34.66 GiB (attempt to allocate chunk of 1059181 bytes), maximum: 34.29 GiB. OvercommitTracker decision: Query was selected to stop by OvercommitTracker.: Cache info: Buffer path: 00170_test/msy/xqbtkkzdbgrteewnhqbxbccnpkkvv, hash key: d48555dc33b06a2a514f8f717619cf96, file_offset_of_buffer_end: 90241, internal buffer remaining read range: None, read_type: CACHED, last caller: 87495cdd-4221-43ae-97ce-431b2866c939:2723, file segment info: None. (MEMORY_LIMIT_EXCEEDED), Stack trace (when copying this message, always include the lines below):
2023.02.17 00:50:37.084012 [ 27211 ] {87495cdd-4221-43ae-97ce-431b2866c939} <Error> TCPHandler: Code: 241. DB::Exception: Memory limit (total) exceeded: would use 34.65 GiB (attempt to allocate chunk of 1637907 bytes), maximum: 34.29 GiB. OvercommitTracker decision: Query was selected to stop by OvercommitTracker.: (while reading column Title): While executing MergeTreeThread. (MEMORY_LIMIT_EXCEEDED), Stack trace (when copying this message, always include the lines below):
2023.02.17 00:50:37.095254 [ 27211 ] {87495cdd-4221-43ae-97ce-431b2866c939} <Debug> TCPHandler: Processed in 0.373981984 sec.
2023.02.17 00:50:37.095406 [ 27211 ] {87495cdd-4221-43ae-97ce-431b2866c939} <Debug> MemoryTracker: Peak memory usage (for query): 397.67 MiB.
2023.02.17 00:50:37.096458 [ 2651 ] {87495cdd-4221-43ae-97ce-431b2866c939} <Fatal> : Logical error: 'Cache usage is not allowed (query_id: 87495cdd-4221-43ae-97ce-431b2866c939)'.

This is an interesting issue, will take a look.

@vdimir vdimir merged commit 0f182f6 into ClickHouse:master Feb 17, 2023
@azat azat deleted the merge-alias-prewhere branch February 17, 2023 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-bugfix Pull request with bugfix, not backported by default
Projects
None yet
Development

Successfully merging this pull request may close these issues.

23.1 PREWHERE is disabled for non-identical Merge tables
3 participants