Merged
Conversation
The issue was a logical error in `FunctionBinaryArithmetic` where `minus` operation threw "Arguments of 'minus' have incorrect data types" when operating on `Int64` and `Int32` types during header computation. The bug was fixed by commit 3c264f8 which corrected recursive calls to `getReturnTypeImplStatic` for special type cases. This test verifies the fix by testing the exact query pattern from the issue: `if(CAST(equals(), 'Nullable(Bool)'), sign(Int64), Int64) - Int32` Closes #70015 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
|
Workflow [PR], commit [c85e0c9] Summary: ❌
|
Algunenano
reviewed
Feb 5, 2026
Member
Algunenano
left a comment
There was a problem hiding this comment.
I can't reproduce the original issue with this reproducer:
$ for dir in *24\.*; do echo $dir; ./$dir/usr/bin/clickhouse local --queries-file /tmp/repro.sql; done
clickhouse-common-static-24.10.1.2812
-2
-1
5
5
5
5
clickhouse-common-static-24.11.1.2557
-2
-1
5
5
5
5
clickhouse-common-static-24.12.2.29
-2
-1
5
5
5
5
clickhouse-common-static-24.1.3.31
-2
-1
5
5
5
5
clickhouse-common-static-24.2.1.2248
-2
-1
5
5
5
5
clickhouse-common-static-24.3.2.23
-2
-1
5
5
5
5
clickhouse-common-static-24.4.1.2088
-2
-1
5
5
5
5
clickhouse-common-static-24.5.4.49
-2
-1
5
5
5
5
clickhouse-common-static-24.6.2.17
-2
-1
5
5
5
5
clickhouse-common-static-24.7.1.2915
-2
-1
5
5
5
5
clickhouse-common-static-24.8.2.3
-2
-1
5
5
5
5
clickhouse-common-static-24.9.1.3278
-2
-1
5
5
5
5
Are you sure it's not too simplified?
Member
Author
|
That's interesting. I've found another problem with JIT while doing this PR (see the sibling PR). But after factoring it away, it looks like the test serves only as a proof of an attempt to reproduce the old issue. |
Member
Author
|
I'll remove the misleading comment from the test, and then I think we can have it - it will give some marginal advantage to AST Fuzzer and, potentially, the test still has some unique code paths. (It will be nice when @alexbakharew finish the differential coverage) |
Algunenano
approved these changes
Feb 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The issue was a logical error in
FunctionBinaryArithmeticwhereminusoperation threw "Arguments of 'minus' have incorrect data types" when operating onInt64andInt32types during header computation.The bug was fixed by commit 3c264f8 which corrected recursive calls to
getReturnTypeImplStaticfor special type cases.This test verifies the fix by testing the exact query pattern from the issue:
if(CAST(equals(), 'Nullable(Bool)'), sign(Int64), Int64) - Int32Closes #70015
Changelog category (leave one):