[GLUTEN-8616] [VL] Add support for Existence Join for broadcast nested loop join#9588
Conversation
|
Run Gluten Clickhouse CI on x86 |
|
cc @zhztheplayer @philo-he could you please take a look thanks! |
ArnavBalyan
force-pushed
the
arnavb/existence-join
branch
from
09b402f to
b66a40c
Compare
|
Run Gluten Clickhouse CI on x86 |
1 similar comment
|
Run Gluten Clickhouse CI on x86 |
ArnavBalyan
force-pushed
the
arnavb/existence-join
branch
from
cb2d355 to
305e011
Compare
|
Run Gluten Clickhouse CI on x86 |
ArnavBalyan
force-pushed
the
arnavb/existence-join
branch
from
305e011 to
d29f843
Compare
|
Run Gluten Clickhouse CI on x86 |
ArnavBalyan
force-pushed
the
arnavb/existence-join
branch
from
d29f843 to
2479d1f
Compare
|
Run Gluten Clickhouse CI on x86 |
| class GlutenExistenceJoinSuite extends ExistenceJoinSuite with GlutenSQLTestsBaseTrait {} | ||
| class GlutenExistenceJoinSuite extends ExistenceJoinSuite with GlutenSQLTestsBaseTrait { | ||
|
|
||
| test("test existence join with broadcast nested loop join") { |
There was a problem hiding this comment.
Can we create a new test class under backends-velox/ to include this test? Then, it will be tested on all supported spark versions.
BTW, please add a check to ensure the operator is really offloaded to Gluten/Velox.
ArnavBalyan
force-pushed
the
arnavb/existence-join
branch
from
2479d1f to
07cd172
Compare
|
Run Gluten Clickhouse CI on x86 |
|
cc @philo-he @zhztheplayer gentle reminder thanks! |
philo-he
left a comment
philo-he
left a comment
There was a problem hiding this comment.
Looks good! Some minor comments.
| } | ||
| } | ||
|
|
||
| protected def backendSpecificJoinValidation(): Option[ValidationResult] = None |
There was a problem hiding this comment.
Is this new method being used anywhere? If not, please remove it.
|
|
||
| class GlutenExistenceJoinSuite extends VeloxWholeStageTransformerSuite with SQLTestUtils { | ||
|
|
||
| test("test existence join with broadcast nested loop join") { |
There was a problem hiding this comment.
Nit: remove "test" from test name.
| assert(count == 1, s"Expected 1 VeloxBroadcastNestedLoopJoinExecTransformer, but found $count") | ||
| } | ||
| override protected val resourcePath: String = "N/A" | ||
| override protected val fileFormat: String = "N/A" |
There was a problem hiding this comment.
Nit: move the above overriding val at the class's beginning part.
ArnavBalyan
force-pushed
the
arnavb/existence-join
branch
from
07cd172 to
4dd4db8
Compare
|
Run Gluten Clickhouse CI on x86 |
3 participants
What changes were proposed in this pull request?
How was this patch tested?