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

allow_experimental_join_condition has a bug #71693

Closed
alexey-milovidov opened this issue Nov 9, 2024 · 4 comments · Fixed by #71857
Closed

allow_experimental_join_condition has a bug #71693

alexey-milovidov opened this issue Nov 9, 2024 · 4 comments · Fixed by #71857
Assignees
Labels
bug Confirmed user-visible misbehaviour in official release experimental feature Bug in the feature that should not be used in production

Comments

@alexey-milovidov
Copy link
Member

alexey-milovidov commented Nov 9, 2024

build_debug/programs/clickhouse -q "

CREATE TABLE t1 (key String, attr String, a UInt64, b UInt64, c Nullable(UInt64)) ENGINE = MergeTree ORDER BY key;
CREATE TABLE t2 (key String, attr String, a UInt64, b UInt64, c Nullable(UInt64)) ENGINE = MergeTree ORDER BY key;

INSERT INTO t1 VALUES ('key1', 'a', 1, 1, 2), ('key1', 'b', 2, 3, 2), ('key1', 'c', 3, 2, 1), ('key1', 'd', 4, 7, 2), ('key1', 'e', 5, 5, 5), ('key2', 'a2', 1, 1, 1), ('key4', 'f', 2, 3, 4);

INSERT INTO t2 VALUES ('key1', 'A', 1, 2, 1), ('key1', 'B', 2, 1, 2), ('key1', 'C', 3, 4, 5), ('key1', 'D', 4, 1, 6), ('key3', 'a3', 1, 1, 1), ('key4', 'F', 1,1,1);

SET receive_timeout = 10., receive_data_timeout_ms = 10000, allow_suspicious_low_cardinality_types = true, log_queries = true, table_function_remote_max_addresses = 200, join_use_nulls = false, max_execution_time = 10., join_algorithm = 'grace_hash', max_memory_usage = 10000000000, send_logs_level = 'fatal', allow_introspection_functions = true, allow_experimental_analyzer = true, allow_experimental_join_condition = true;

SELECT t1.* FROM t1 FULL OUTER JOIN t2 ON (t1.key = t2.key) AND ((t1.a = 2) OR indexHint(t2.b > 0, toNullable(toUInt256(28)), 2, 20, toUInt128(toUInt256(12)), 4, 13, 31, materialize(toLowCardinality(8)), 45, 9, 17, 9, 4) OR indexHint(28, materialize(2), 20, 12, toLowCardinality(4), 13, toLowCardinality(toNullable(31)), 8, 45, 9, 17, 9, 4)) WHERE t2.b > 0 ORDER BY ALL DESC NULLS LAST;

"
clickhouse: /home/milovidov/work/ClickHouse/src/Common/PODArray.h:395: const T &DB::PODArray<char8_t, 4096, Allocator<false, false>, 63, 64>::operator[](ssize_t) const [T = char8_t, initial_bytes = 4096, TAllocator = Allocator<false, false>, pad_right_ = 63, pad_left_ = 64]: Assertion `(n >= (static_cast<ssize_t>(pad_left_) ? -1 : 0)) && (n <= static_cast<ssize_t>(this->size()))' failed.
@alexey-milovidov alexey-milovidov added the potential bug To be reviewed by developers and confirmed/rejected. label Nov 9, 2024
@alexey-milovidov alexey-milovidov changed the title Grace hash join has a bug allow_experimental_join_condition has a bug Nov 9, 2024
@alexey-milovidov
Copy link
Member Author

Minimized example:

build_debug/programs/clickhouse -q "

CREATE TABLE t1 (key String, attr String, a UInt64, b UInt64, c Nullable(UInt64)) ENGINE = MergeTree ORDER BY key;
CREATE TABLE t2 (key String, attr String, a UInt64, b UInt64, c Nullable(UInt64)) ENGINE = MergeTree ORDER BY key;

INSERT INTO t1 VALUES ('key1', 'a', 1, 1, 2), ('key1', 'b', 2, 3, 2), ('key1', 'c', 3, 2, 1), ('key1', 'd', 4, 7, 2), ('key1', 'e', 5, 5, 5), ('key2', 'a2', 1, 1, 1), ('key4', 'f', 2, 3, 4);

INSERT INTO t2 VALUES ('key1', 'A', 1, 2, 1), ('key1', 'B', 2, 1, 2), ('key1', 'C', 3, 4, 5), ('key1', 'D', 4, 1, 6), ('key3', 'a3', 1, 1, 1), ('key4', 'F', 1,1,1);

SET allow_experimental_join_condition = true;

SELECT t1.* FROM t1 FULL OUTER JOIN t2 ON t1.key = t2.key AND (t1.a = 2 OR indexHint(t2.a = 2))"

@alexey-milovidov alexey-milovidov added the experimental feature Bug in the feature that should not be used in production label Nov 11, 2024
@lgbo-ustc
Copy link
Contributor

lgbo-ustc commented Nov 11, 2024

May remove this check

@Algunenano Algunenano added bug Confirmed user-visible misbehaviour in official release and removed potential bug To be reviewed by developers and confirmed/rejected. labels Nov 11, 2024
@vdimir vdimir self-assigned this Nov 13, 2024
@vdimir
Copy link
Member

vdimir commented Nov 13, 2024

@lgbo-ustc This is helpful, thanks!
However, what was the initial intent behind adding this check? Why does everything else work without it?

@lgbo-ustc
Copy link
Contributor

lgbo-ustc commented Nov 14, 2024

@lgbo-ustc This is helpful, thanks! However, what was the initial intent behind adding this check? Why does everything else work without it?

I think removing this should be OK. An inequal condition should contain both columns from left and right table, I think add this check should be OK and could help to find some unexpected cases. If there are only columns from one table, use the normal join algorithm should be more efficient. indexHint is an unexpected case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed user-visible misbehaviour in official release experimental feature Bug in the feature that should not be used in production
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants