Skip to content

[CALCITE-6095] Arithmetic expression with VARBINARY value causes AssertionFailure#3527

Merged
mihaibudiu merged 1 commit intoapache:mainfrom
mihaibudiu:issue6095
Dec 6, 2023
Merged

[CALCITE-6095] Arithmetic expression with VARBINARY value causes AssertionFailure#3527
mihaibudiu merged 1 commit intoapache:mainfrom
mihaibudiu:issue6095

Conversation

@mihaibudiu
Copy link
Contributor

No description provided.

* [CALCITE-6095] Arithmetic expression with VARBINARY value causes AssertionFailure</a>.
*/
@Test public void testVarbitArithmetic() {
SqlOperatorFixture f = fixture();
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this belong better in TypeCoercionTest? I get that CAST is an operator but maybe that class is more tailored to the purpose of this ticket. Let me know what you think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually see that TypeCoercionTest has negative tests for binary in the method testImplicitCasts. So these tests are not sufficient.
The problem is in the type inference for the binary operator. The only negative tests in this file are using shouldNotCast, which does not seem to exercise the inference for binary operators.

There is an additional benefit having operator tests: we are also checking the error message. But that is less important.

@JiajunBernoulli
Copy link
Contributor

LGTM.

Clear error message is an improvement compared to AssertionFailure.

@mihaibudiu
Copy link
Contributor Author

What is the conclusion with respect to the Druid tests?
Can I merge this PR even though the CI has failed for these?

…rtionFailure

Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

90.9% 90.9% Coverage
0.0% 0.0% Duplication

@mihaibudiu mihaibudiu merged commit 1f6022b into apache:main Dec 6, 2023
@mihaibudiu mihaibudiu deleted the issue6095 branch December 6, 2023 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants