Skip to content

[GLUTEN-8851][VL] Fix the TakeOrderedAndProjectExecTransformer does not respect cudf tag#10821

Merged
jinchengchenghh merged 2 commits intoapache:mainfrom
jinchengchenghh:fixorderby
Sep 29, 2025
Merged

[GLUTEN-8851][VL] Fix the TakeOrderedAndProjectExecTransformer does not respect cudf tag#10821
jinchengchenghh merged 2 commits intoapache:mainfrom
jinchengchenghh:fixorderby

Conversation

@jinchengchenghh
Copy link
Contributor

A follow up of #10754

@github-actions github-actions bot added the CORE works for Gluten Core label Sep 29, 2025
@github-actions
Copy link

Run Gluten Clickhouse CI on x86

@zhztheplayer zhztheplayer changed the title [GLUTEN_8851][VL] Fix the TakeOrderedAndProjectExecTransformer does not respect cudf tag [GLUTEN-8851][VL] Fix the TakeOrderedAndProjectExecTransformer does not respect cudf tag Sep 29, 2025
@github-actions
Copy link

#8851

} else {
val limitStagePlan =
WholeStageTransformer(limitBeforeShuffle)(transformStageCounter.incrementAndGet())
limitStagePlan.copyTagsFrom(child)
Copy link
Member

@zhztheplayer zhztheplayer Sep 29, 2025

Choose a reason for hiding this comment

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

Doing this may bring some subtle shortcomings because limitStagePlan is not logically equal to child (they don't output the same data from user's view). Thus, the new code may work for cuDF but may technically break other tags.

Can we invoke the code in CudfNodeValidationRule here to specifically add the cuDF tags?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is in function doExecuteColumnar, so we cannot change it in Plan level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do I change it to only apply for Cudf tag?

Copy link
Member

Choose a reason for hiding this comment

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

Do I change it to only apply for Cudf tag?

For sure. And it's reasonable to keep calling copyTagsFrom on the WholeStageTransformer but let's only add cuDF tag for limitStagePlan, if it works for you.

@github-actions
Copy link

Run Gluten Clickhouse CI on x86

@jinchengchenghh jinchengchenghh merged commit 27d6c22 into apache:main Sep 29, 2025
57 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CORE works for Gluten Core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants