Skip to content

HIVE-29765: HiveBetween#unparse ignores negated flag - #6637

Merged
zabetak merged 1 commit into
apache:masterfrom
rubenada:HIVE-29765
Aug 6, 2026
Merged

HIVE-29765: HiveBetween#unparse ignores negated flag#6637
zabetak merged 1 commit into
apache:masterfrom
rubenada:HIVE-29765

Conversation

@rubenada

@rubenada rubenada commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

HiveBetween#unparse will consider the negated flag.

Why are the changes needed?

Fix the unparse of NOT BETWEEN.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

New unit test added.

@ayushtkn ayushtkn left a comment

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.

Does it lead to any query level failure or so? If so can we extend a test?

@rubenada

Copy link
Copy Markdown
Contributor Author

@ayushtkn it seems this was untested, I will prepare a test

@rubenada

Copy link
Copy Markdown
Contributor Author

@ayushtkn I included a new unit test, could you please take another look?

@ayushtkn

Copy link
Copy Markdown
Member

@rubenada why are we going with Junit style test, can't we we have a q file test?

@rubenada

Copy link
Copy Markdown
Contributor Author

@ayushtkn it was simpler. It seems in practice the "NOT BETWEEN" operator is currently not generated in the intermediate planner conversions (that's probably why this bug went under the radar).
There's a PR to "enable" it (which unveiled the current issue): #6634 ; but there are some discussions on that Jira whether or not we should really apply that change, so that one may not land in master.
Nevertheless, we would need to fix this bug.

@sonarqubecloud

sonarqubecloud Bot commented Aug 3, 2026

Copy link
Copy Markdown

@soumyakanti3578 soumyakanti3578 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. +1 pending tests.

@zabetak zabetak left a comment

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.

LGTM!

There is one test failure on the CI run but has to do with container startup so unrelated to the changes here.

Testing / split-08 / PostProcess / testCliDriver[iceberg_rest_catalog_gravitino] – org.apache.hadoop.hive.cli.TestIcebergRESTCatalogGravitinoLlapLocalCliDriver

Error
Container startup failed for image apache/ozone:2.1.0
Stacktrace
org.testcontainers.containers.ContainerLaunchException: Container startup failed for image apache/ozone:2.1.0
	at org.testcontainers.containers.GenericContainer.doStart(GenericContainer.java:351)
	at org.testcontainers.containers.GenericContainer.start(GenericContainer.java:322)
	at org.apache.hadoop.hive.cli.OzoneS3GatewayContainers.startS3g(OzoneS3GatewayContainers.java:194)
	at org.apache.hadoop.hive.cli.OzoneS3GatewayContainers.start(OzoneS3GatewayContainers.java:77)
	at org.apache.hadoop.hive.cli.TestIcebergRESTCatalogGravitinoLlapLocalCliDriver.setup(TestIcebergRESTCatalogGravitinoLlapLocalCliDriver.java:130)

@zabetak
zabetak merged commit 964d706 into apache:master Aug 6, 2026
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants