[VL][Core] Add InputFileReplaceFallback Rule

[gaoyangxiaozhu]

What changes were proposed in this pull request?

The is a following up PR for input_file_name native support, it

1. Add a inputfileNameReplaceFallbackRule to handle the case if scan fallback due to complex data type
2. Add a feature flag to control if enable the rules in case any corner issues happen we can disable the rule

(Fixes: facebookincubator/velox#9957)

How was this patch tested?

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)

(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

[github-actions]

Thanks for opening a pull request!

Could you open an issue for this pull request on Github Issues?

https://github.com/apache/incubator-gluten/issues

Then could you also rename commit message and pull request title in the following format?

[GLUTEN-${ISSUES_ID}][COMPONENT]feat/fix: ${detailed message}

See also:

• Other pull requests

[github-actions]

Run Gluten Clickhouse CI

[zhztheplayer]

I think we'd follow a criteria when rewriting an expression / operator before entering actual offloading phase (e.g., using genExtendedColumnarValidationRules to do the replacement): to make it compatible with both native lib and vanilla Spark as long as possible and to avoid converting it back when the computation doesn't offload.

Can you take a look at some current rules that already follow the criteria, e.g., the ones to offload collet_list, collect_set, hll, bloom-filter, dpp subquery, to see if we can do the same on the expressions used by input_file_name ?

[zhztheplayer]

A basic question for #6021 : Why didn't we do the replacement during scan offloading?

If it's about the co-offloading for project and scan, what I can imagine is that we can add a individual offloading rule. E.g., OffloadInputFileAwareProjectAndScan or something to match against project + scan pattern then offload them together. Once not offload-able, the rule just does nothing. Thus we don't have to use another rule to do fallback.

[github-actions]

Run Gluten Clickhouse CI

[gaoyangxiaozhu]

I think we'd follow a criteria when rewriting an expression / operator before entering actual offloading phase (e.g., using genExtendedColumnarValidationRules to do the replacement): to make it compatible with both native lib and vanilla Spark as long as possible and to avoid converting it back when the computation doesn't offload.

Can you take a look at some current rules that already follow the criteria, e.g., the ones to offload collet_list, collect_set, hll, bloom-filter, dpp subquery, to see if we can do the same on the expressions used by input_file_name ?

@zhztheplayer , thank you! actually i have checked this part but didn't figure out a similar way for input_file_name support. The current implement for collect_list, collect_set etc is based on truth that velox already has a related function available only need some data type compatible change. But velox right now doesn't have a input_file_name available.

And you can check this discussion for why we pick current solution for input_file_name - facebookincubator/velox#9957

[gaoyangxiaozhu]

A basic question for #6021 : Why didn't we do the replacement during scan offloading?

If it's about the co-offloading for project and scan, what I can imagine is that we can add a individual offloading rule. E.g., OffloadInputFileAwareProjectAndScan or something to match against project + scan pattern then offload them together. Once not offload-able, the rule just does nothing. Thus we don't have to use another rule to do fallback.

A basic question for #6021 : Why didn't we do the replacement during scan offloading?

If it's about the co-offloading for project and scan, what I can imagine is that we can add a individual offloading rule. E.g., OffloadInputFileAwareProjectAndScan or something to match against project + scan pattern then offload them together. Once not offload-able, the rule just does nothing. Thus we don't have to use another rule to do fallback.

For this question - Why didn't we do the replacement during scan offloading?
hello, Hongze, because we only want to do the replacement action when needed as the related project node does has input_file_name expression otherwise do nothing.

And for this part - If it's about the co-offloading for project and scan, what I can imagine is that we can add a individual offloading rule
Actually, Hongze, I thought before if it is possible to leverage just one rule do the both thing replacement or nothing. But didn't figure out how to do that since one project may has multiple scan child nodes which some of them is fallback, some not, and honestly i don't make it clear about how to accurately obtain which projects + scans need replacement and which do not in just one rule.

So if you have a good idea to share, I'd like do a following PR after this PR to refactor this part to merge 2 rules together. :)

[github-actions]

Run Gluten Clickhouse CI

[zhztheplayer]

I'd like do a following PR after this PR to refactor this part to merge 2 rules together. :)

Which works for me but let's turn off the option by default and mark it as experimental? Until we got a more robust design. Thanks.

[zhztheplayer]

BTW I'll take a closer look at the code asap.

[github-actions]

Run Gluten Clickhouse CI

[github-actions]

Run Gluten Clickhouse CI

[gaoyangxiaozhu]

I'd like do a following PR after this PR to refactor this part to merge 2 rules together. :)

Which works for me but let's turn off the option by default and mark it as experimental? Until we got a more robust design. Thanks.

thanks, done to mark it experimental and also turn off by default @FelixYBW FYI

[gaoyangxiaozhu]

BTW I'll take a closer look at the code asap.

thanks, since without this change, user may encounter the failure issue if scan fallback with complex data type and project has input_file_name.

[gaoyangxiaozhu]

don't think the 3.4 fail related to my change and it is randomly, meanwhile the CH CI fail also looks a CI issue ? @zhouyuan / @PHILO-HE could you help double check ?

[PHILO-HE]

Run Gluten Clickhouse CI

[PHILO-HE]

@gaoyangxiaozhu, Spark 3.4 tests passed after re-triggered. Let's wait for CH CI.

[zhztheplayer]

@gaoyangxiaozhu I opened #6157 for further work to improve this. I am assign that issue to you. And I suggest to have a clear plan for the new implementation before merging this.

[gaoyangxiaozhu]

@gaoyangxiaozhu I opened #6157 for further work to improve this. I am assign that issue to you. And I suggest to have a clear plan for the new implementation before merging this.

@gaoyangxiaozhu I opened #6157 for further work to improve this. I am assign that issue to you. And I suggest to have a clear plan for the new implementation before merging this.

got! So I send seperate PR #6161 to turn off inputFileNameReplaceRule with feature flag, let make sure that PR merged in case any issue caused due to scan fallback.

@jinchengchenghh / @zhztheplayer / @FelixYBW please help review.

[lwz9103]

Run Gluten Clickhouse CI

[lwz9103]

Run Gluten Clickhouse CI