Skip to content

[GLUTEN-10524] Remove unnecessary outputAttributes from BasicScanExecTransformer#10525

Merged
philo-he merged 2 commits intoapache:mainfrom
beliefer:10524
Sep 2, 2025
Merged

[GLUTEN-10524] Remove unnecessary outputAttributes from BasicScanExecTransformer#10525
philo-he merged 2 commits intoapache:mainfrom
beliefer:10524

Conversation

@beliefer
Copy link
Copy Markdown
Contributor

What changes are proposed in this pull request?

This PR proposes to remove unnecessary outputAttributes for BasicScanExecTransformer.
Fixes #10524

How was this patch tested?

GA tests.

@github-actions github-actions bot added the CORE works for Gluten Core label Aug 25, 2025
@github-actions
Copy link
Copy Markdown

#10524

@github-actions
Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@github-actions
Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@github-actions
Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@github-actions
Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@github-actions
Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@github-actions
Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@github-actions
Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

} else {
// Retrieve the original attributes based on expression ID so that capitalization matches.
val originalAttributes = AttributeMap(relation.output.map(a => a -> a))
requestedAttributes.map(attr => originalAttributes.getOrElse(attr, attr)).distinct
Copy link
Copy Markdown
Contributor

@zml1206 zml1206 Aug 26, 2025

Choose a reason for hiding this comment

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

How about add prunedOutput to AbstractHiveTableScanExec and update output function for AbstractHiveTableScanExec?

Copy link
Copy Markdown
Contributor Author

@beliefer beliefer Aug 26, 2025

Choose a reason for hiding this comment

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

It sounds we can override the output for AbstractHiveTableScanExec directly.
But it seems the other subclass of AbstractHiveTableScanExec will be added.
What about your opinion? @zhztheplayer

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.

@beliefer, AbstractHiveTableScanExec was introduced to fix code compatibility across different Spark versions. It resides in the shim layer to hold version-specific code for the corresponding Spark version. Therefore, I assume only HiveTableScanExecTransformer extends it, and no other classes will extend it in the future.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@philo-he Thank you. Let's override the output for AbstractHiveTableScanExec directly.

@github-actions
Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@github-actions
Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@beliefer
Copy link
Copy Markdown
Contributor Author

ping @FelixYBW @zhouyuan cc @zzcclp

@beliefer
Copy link
Copy Markdown
Contributor Author

beliefer commented Sep 1, 2025

@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 1, 2025

Run Gluten Clickhouse CI on x86

@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 1, 2025

Run Gluten Clickhouse CI on x86

@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 1, 2025

Run Gluten Clickhouse CI on x86

1 similar comment
@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 1, 2025

Run Gluten Clickhouse CI on x86

@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 1, 2025

Run Gluten Clickhouse CI on x86

@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 1, 2025

Run Gluten Clickhouse CI on x86

@beliefer beliefer requested review from philo-he and zml1206 September 2, 2025 02:05
Copy link
Copy Markdown
Contributor

@zml1206 zml1206 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

Copy link
Copy Markdown
Member

@philo-he philo-he left a comment

Choose a reason for hiding this comment

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

Thanks.

@philo-he philo-he changed the title [GLUTEN-10524] Remove unnecessary outputAttributes for BasicScanExecTransformer [GLUTEN-10524] Remove unnecessary outputAttributes from BasicScanExecTransformer Sep 2, 2025
@philo-he philo-he merged commit 18449a2 into apache:main Sep 2, 2025
56 checks passed
@beliefer
Copy link
Copy Markdown
Contributor Author

beliefer commented Sep 2, 2025

@philo-he @zml1206 Thank you all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLICKHOUSE CORE works for Gluten Core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove unnecessary outputAttributes for BasicScanExecTransformer

3 participants