[CALCITE-7206] Avoid duplicate compare(Object, Object) bridge method in the Enumerable merge join comparator#5080
Open
anxkhn wants to merge 1 commit into
Open
[CALCITE-7206] Avoid duplicate compare(Object, Object) bridge method in the Enumerable merge join comparator#5080anxkhn wants to merge 1 commit into
anxkhn wants to merge 1 commit into
Conversation
…in the Enumerable merge join comparator PhysTypeImpl.generateComparator always appends a bridge compare(Object, Object) method when EnumerableRules.BRIDGE_METHODS is enabled. When the boxed row Java class is already Object (for example a merge join key of type ANY, represented as a bare Object), the primary compare method already has the signature compare(Object, Object), so the bridge collapses to the same signature and the generated Comparator declares two identical methods, which fails to compile with "Error while compiling generated Java code". Skip the bridge method when javaRowClass is Object, since the primary method already satisfies the Comparator contract. The normal Object[] row case is unchanged.
|
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
1 participant
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.
Jira Link
CALCITE-7206
Changes Proposed
PhysTypeImpl.generateComparatoralways appends a bridgecompare(Object, Object)method to the generatedComparatorwhenEnumerableRules.BRIDGE_METHODSis enabled (the default). The bridge exists sothe erased
Comparator.compare(Object, Object)interface method delegates to thestrongly typed primary
compare(rowClass, rowClass)method.When the row is a single column whose Java class is already
Object(forexample a merge join whose key has type
ANY, materialized as a bareObject),the primary method's boxed signature is
compare(Object, Object), which isidentical to the bridge. The emitted
Comparatorthen declares twocompare(Object, Object)methods and Janino fails to compile it withError while compiling generated Java code. This reproduces on anEnumerableMergeJoinwith an
Object-typed join key, as reported on the Jira.This change guards the bridge block with
javaRowClass != Object.class: when theboxed row class is already
Object, the primarycomparemethod alreadysatisfies the
Comparatorcontract, so no bridge is needed and none is emitted.For every other row class (the common
Object[]row and all primitive-boxedrows) the boxed class differs from
Object, so the bridge is still generated andbehavior is unchanged.
The same private
generateComparatoralso backs the comparators used byEnumerableWindow,EnumerableAsofJoin, andEnumerableSortedAggregate, soeach of those paths would have hit the identical duplicate-method failure on an
Objectrow and is fixed by the same guard.Added a focused unit test in
PhysTypeTest,testMergeJoinComparatorWithObjectRowHasNoDuplicateBridge, that builds thereported shape (a single-column
ANYstruct withJavaRowFormat.SCALAR, so therow Java class is
Object), callsgenerateMergeJoinComparator, and asserts thegenerated source keeps the primary
compare(Object v0, Object v1)method but nolonger contains the colliding bridge parameters. The test fails on
mainwiththe duplicate bridge method and passes with this change.
./gradlew :core:test --tests "org.apache.calcite.adapter.enumerable.PhysTypeTest"and./gradlew :core:test --tests "org.apache.calcite.test.enumerable.EnumerableJoinTest"pass, andautostyleCheck/checkstyleMain/checkstyleTestare clean.