Skip to content

test: Add tests for Scalar and Inverval values for UnaryMinus#538

Merged
andygrove merged 10 commits intoapache:mainfrom
vaibhawvipul:issue-508
Jun 11, 2024
Merged

test: Add tests for Scalar and Inverval values for UnaryMinus#538
andygrove merged 10 commits intoapache:mainfrom
vaibhawvipul:issue-508

Conversation

@vaibhawvipul
Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #508 .

Rationale for this change

Improves test coverage for various unary minus.

What changes are included in this PR?

Tests for scalar values and intervals.

How are these changes tested?

All tests pass.

@vaibhawvipul vaibhawvipul marked this pull request as ready for review June 7, 2024 07:11
Copy link
Copy Markdown
Contributor

@kazuyukitanimura kazuyukitanimura left a comment

Choose a reason for hiding this comment

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

Mostly looks good.
Somehow Github failed launching CI...

Comment thread spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala Outdated
Comment thread spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala
Comment thread spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala
@kazuyukitanimura kazuyukitanimura changed the title bug: Add tests for Scalar and Inverval values for UnaryMinus test: Add tests for Scalar and Inverval values for UnaryMinus Jun 7, 2024
Comment thread spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala Outdated
@kazuyukitanimura
Copy link
Copy Markdown
Contributor

pending ci

@andygrove
Copy link
Copy Markdown
Member

Thanks @vaibhawvipul and @kazuyukitanimura

assert(sparkException.getMessage.contains(dtype + " overflow"))
assert(cometException.getMessage.contains(dtype + " overflow"))
case (None, None) => assert(true) // got same outputs
case (None, None) => checkSparkAnswerAndOperator(sql(query))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: we call checkSparkMaybeThrows,, and if that doesn't fail, then we call checkSparkAnswerAndOperator. I think this could all be streamlined, but this is beyond the scope of this PR

@andygrove andygrove merged commit a4e268c into apache:main Jun 11, 2024
himadripal pushed a commit to himadripal/datafusion-comet that referenced this pull request Sep 7, 2024
…#538)

* adding scalar tests

* refactor and test for interval

* ci checks fixed

* running operator checks when no error

* removing redundant sqlconf

* fix ci errorsg

* moving interval test to array section

* ci fixes
coderfender pushed a commit to coderfender/datafusion-comet that referenced this pull request Dec 13, 2025
…iles with Scala 2.13 (apache#538)

Scala 2.13 builds were generating Java 17 class files.

We switched to using Java 17 due to Spark test dependencies that were
compiled for Java 17. Despite setting the target to be Java 11, the
`scala-maven-plugin` generated Java 17 class files. Updating the config
for the `maven-compiler-plugin` also had no impact.

Debug logs showed the Java 17 setting was being forced.

- Use a jdk11 builder when compiling Java to force Java 11 class files.
- Fixes jdk11 builder
@vaibhawvipul vaibhawvipul deleted the issue-508 branch March 20, 2026 17:14
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.

Add tests for Scalar and Inverval values for UnaryMinus

3 participants