Skip to content

Conversation

@cxzl25
Copy link
Contributor

@cxzl25 cxzl25 commented Nov 3, 2022

What changes were proposed in this pull request?

BHJ LeftAnti does not update numOutputRows when codegen is disabled

Why are the changes needed?

PR #29104 Only update numOutputRows when codegen is enabled, but there is no numOutputRows when codegen is disabled, and numOutputRows is equal to 0.

Does this PR introduce any user-facing change?

No

How was this patch tested?

add UT

@github-actions github-actions bot added the SQL label Nov 3, 2022
@cxzl25
Copy link
Contributor Author

cxzl25 commented Nov 3, 2022

Current

enable codegen

image

disable codegen

image

Fix

image

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@cxzl25
Copy link
Contributor Author

cxzl25 commented Nov 7, 2022

Help review. Thanks.
@leanken-zz @cloud-fan @wangyum

@cxzl25
Copy link
Contributor Author

cxzl25 commented Nov 24, 2022

Help review. Thanks. @HyukjinKwon @yaooqinn

@cloud-fan
Copy link
Contributor

Can we add a test in SQLMetricsSuite? If it's hard we can skip the test.

@yaooqinn
Copy link
Member

Can we add a test in SQLMetricsSuite? If it's hard we can skip the test.

+1

@cxzl25
Copy link
Contributor Author

cxzl25 commented Nov 24, 2022

Add UT.

Current

[info] - SPARK-41003: BHJ LeftAnti does not update numOutputRows when codegen is disabled *** FAILED *** (10 seconds, 124 milliseconds)
[info]   0 did not equal 2 The query plan metric numOutputRows did not match, expected:2, actual:0 (SQLMetricsTestUtils.scala:270)
[info]   org.scalatest.exceptions.TestFailedException:

Fix

[info] - SPARK-41003: BHJ LeftAnti does not update numOutputRows when codegen is disabled (9 seconds, 123 milliseconds)

Copy link
Member

@yaooqinn yaooqinn left a comment

Choose a reason for hiding this comment

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

LGTM

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in c1df6a0 Nov 28, 2022
beliefer pushed a commit to beliefer/spark that referenced this pull request Dec 15, 2022
…degen is disabled

### What changes were proposed in this pull request?
BHJ LeftAnti does not update numOutputRows when codegen is disabled

### Why are the changes needed?
PR apache#29104 Only update numOutputRows when codegen is enabled, but there is no numOutputRows when codegen is disabled, and numOutputRows is equal to 0.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
add UT

Closes apache#38489 from cxzl25/SPARK-41003.

Authored-by: sychen <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
beliefer pushed a commit to beliefer/spark that referenced this pull request Dec 18, 2022
…degen is disabled

### What changes were proposed in this pull request?
BHJ LeftAnti does not update numOutputRows when codegen is disabled

### Why are the changes needed?
PR apache#29104 Only update numOutputRows when codegen is enabled, but there is no numOutputRows when codegen is disabled, and numOutputRows is equal to 0.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
add UT

Closes apache#38489 from cxzl25/SPARK-41003.

Authored-by: sychen <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants