Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GLUTEN-3854][CORE][FOLLOWUP] Add ColumnarInputAdapter back to recover UI graph #3933

Merged
merged 3 commits into from
Dec 6, 2023

Conversation

ulysses-you
Copy link
Contributor

What changes were proposed in this pull request?

This pr is followup fix of #3854. We can not remove ColumnarInputAdapter, otherwise the UI graph would be broken.

How was this patch tested?

before:
image

after:
image

Copy link

github-actions bot commented Dec 5, 2023

#3854

Copy link

github-actions bot commented Dec 5, 2023

Run Gluten Clickhouse CI

@ulysses-you
Copy link
Contributor Author

cc @zhztheplayer thank you

Comment on lines 80 to 113
// This is not strictly needed because the codegen transformation happens after the columnar
// transformation but just for consistency
override def supportsColumnar: Boolean = child.supportsColumnar

// this is the most important effect of this class
override def supportCodegen: Boolean = false

override def doExecuteColumnar(): RDD[ColumnarBatch] = {
child.executeColumnar()
}

override def nodeName: String = s"InputAdapter"

override def generateTreeString(
depth: Int,
lastChildren: Seq[Boolean],
append: String => Unit,
verbose: Boolean,
prefix: String = "",
addSuffix: Boolean = false,
maxFields: Int,
printNodeId: Boolean,
indent: Int = 0): Unit = {
child.generateTreeString(
depth,
lastChildren,
append,
verbose,
prefix = "",
addSuffix = false,
maxFields,
printNodeId,
indent)
}
Copy link
Member

Choose a reason for hiding this comment

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

It seems that we only need to override #supportCodegen ? Can we remove others to simply code? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, addressed

Copy link

github-actions bot commented Dec 6, 2023

Run Gluten Clickhouse CI

Copy link

github-actions bot commented Dec 6, 2023

Run Gluten Clickhouse CI

Copy link
Member

@zhztheplayer zhztheplayer left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@zhztheplayer zhztheplayer merged commit f389b11 into apache:main Dec 6, 2023
17 checks passed
@ulysses-you ulysses-you deleted the input-adapter branch December 6, 2023 07:49
@GlutenPerfBot
Copy link
Contributor

===== Performance report for TPCH SF2000 with Velox backend, for reference only ====

query log/native_3933_time.csv log/native_master_12_05_2023_a46243420_time.csv difference percentage
q1 33.27 34.72 1.449 104.36%
q2 25.01 25.00 -0.010 99.96%
q3 38.23 37.98 -0.255 99.33%
q4 37.33 38.25 0.921 102.47%
q5 71.59 72.06 0.475 100.66%
q6 6.99 5.37 -1.626 76.75%
q7 84.67 82.30 -2.375 97.20%
q8 87.33 86.96 -0.375 99.57%
q9 121.69 126.92 5.237 104.30%
q10 45.60 45.43 -0.170 99.63%
q11 20.29 20.31 0.014 100.07%
q12 23.03 27.02 3.993 117.34%
q13 46.50 47.15 0.652 101.40%
q14 18.72 19.01 0.294 101.57%
q15 28.49 29.59 1.097 103.85%
q16 15.34 15.81 0.471 103.07%
q17 103.19 103.87 0.686 100.66%
q18 152.02 150.70 -1.317 99.13%
q19 14.73 14.53 -0.206 98.60%
q20 28.19 28.18 -0.014 99.95%
q21 223.66 223.72 0.058 100.03%
q22 14.03 13.17 -0.862 93.85%
total 1239.91 1248.05 8.137 100.66%

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.

3 participants