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

Test input_file_name, input_file_block_start & input_file_block_length when scan falls back #6318

Merged

Conversation

gaoyangxiaozhu
Copy link
Contributor

@gaoyangxiaozhu gaoyangxiaozhu commented Jul 3, 2024

What changes were proposed in this pull request?

We need covert all input_file-* expression when scan is fallback other than input_file_name, this a small follow up of PR #6200

The PR mainly do

  1. leverage scan fallback config to make sure scan is fallback when do the test instead of using array nested complex data type, since CH backend actually support it.
  2. also cover expr - input_file_block_start, input_file_block_length since it should also be supported

(Fixes: #ISSUE-ID)

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)

Copy link

github-actions bot commented Jul 3, 2024

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:

Copy link

github-actions bot commented Jul 3, 2024

Run Gluten Clickhouse CI

Copy link

github-actions bot commented Jul 3, 2024

Run Gluten Clickhouse CI

Copy link

github-actions bot commented Jul 3, 2024

Run Gluten Clickhouse CI

2 similar comments
@gaoyangxiaozhu
Copy link
Contributor Author

Run Gluten Clickhouse CI

@gaoyangxiaozhu
Copy link
Contributor Author

Run Gluten Clickhouse CI

Copy link

github-actions bot commented Jul 4, 2024

Run Gluten Clickhouse CI

2 similar comments
@gaoyangxiaozhu
Copy link
Contributor Author

Run Gluten Clickhouse CI

@gaoyangxiaozhu
Copy link
Contributor Author

Run Gluten Clickhouse CI

@gaoyangxiaozhu
Copy link
Contributor Author

@PHILO-HE could we check why CH ci fail as i re-run multiple times it still can't pass the CI.

@PHILO-HE
Copy link
Contributor

PHILO-HE commented Jul 4, 2024

@PHILO-HE could we check why CH ci fail as i re-run multiple times it still can't pass the CI.

Did you check the console log of CH CI? See https://github.com/apache/incubator-gluten/blob/main/docs/get-started/ClickHouse.md#new-ci-system.

@gaoyangxiaozhu
Copy link
Contributor Author

gaoyangxiaozhu commented Jul 4, 2024

@PHILO-HE could we check why CH ci fail as i re-run multiple times it still can't pass the CI.

Did you check the console log of CH CI? See https://github.com/apache/incubator-gluten/blob/main/docs/get-started/ClickHouse.md#new-ci-system.

looks surely have UT issue, will double check and fix

@gaoyangxiaozhu gaoyangxiaozhu changed the title Refactor UT of input_file_name with scan is fallback [WIP] Refactor UT of input_file_name with scan is fallback Jul 5, 2024
Copy link

github-actions bot commented Jul 9, 2024

Run Gluten Clickhouse CI

1 similar comment
Copy link

github-actions bot commented Jul 9, 2024

Run Gluten Clickhouse CI

@gaoyangxiaozhu gaoyangxiaozhu force-pushed the gayangya/input_file_name_ut_refactor branch from 9f6b0c5 to c1e21d2 Compare July 9, 2024 11:52
Copy link

github-actions bot commented Jul 9, 2024

Run Gluten Clickhouse CI

@gaoyangxiaozhu gaoyangxiaozhu changed the title [WIP] Refactor UT of input_file_name with scan is fallback Refactor UT of input_file_name with scan is fallback Jul 9, 2024
@gaoyangxiaozhu
Copy link
Contributor Author

@PHILO-HE / @rui-mo help review

@gaoyangxiaozhu
Copy link
Contributor Author

ping @PHILO-HE / @rui-mo again

Copy link
Contributor

@rui-mo rui-mo left a comment

Choose a reason for hiding this comment

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

Could we use a more proper name for this PR as it touches all input_file-* expression as you mentioned? Have we supported the other input_file* expression like input_file_block_start? Thanks.

@gaoyangxiaozhu
Copy link
Contributor Author

Could we use a more proper name for this PR as it touches all input_file-* expression as you mentioned? Have we supported the other input_file* expression like input_file_block_start? Thanks.

sure, all input_file_* is supported. updated the title.

@gaoyangxiaozhu gaoyangxiaozhu changed the title Refactor UT of input_file_name with scan is fallback Refactor UT of input_file_name with scan is fallback more robust and cover all supported exprs Jul 10, 2024
@gaoyangxiaozhu gaoyangxiaozhu requested a review from rui-mo July 10, 2024 09:31
Copy link
Contributor

@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.

Some questions need clarification. Thanks!

@PHILO-HE PHILO-HE changed the title Refactor UT of input_file_name with scan is fallback more robust and cover all supported exprs Test input_file_name, input_file_block_start & input_file_block_length when scan falls back Jul 10, 2024
Copy link

Run Gluten Clickhouse CI

@gaoyangxiaozhu gaoyangxiaozhu requested a review from PHILO-HE July 10, 2024 10:43
Copy link

Run Gluten Clickhouse CI

Copy link
Contributor

@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.

Looks nice!

@gaoyangxiaozhu
Copy link
Contributor Author

hello @PHILO-HE could you help merge ?

@PHILO-HE PHILO-HE merged commit 8e0d56e into apache:main Jul 11, 2024
41 checks passed
weiting-chen pushed a commit to weiting-chen/gluten that referenced this pull request Aug 10, 2024
weiting-chen added a commit that referenced this pull request Aug 12, 2024
* [GLUTEN-6612] Fix ParquetFileFormat issue caused by the setting of local property isNativeApplicable (#6627)

* [CORE] Fix schema mismatch between ReadRelNode and LocalFilesNode (#6746)

Co-authored-by: 蒋添 <[email protected]>

* [UT] Test input_file_name, input_file_block_start & input_file_block_length when scan falls back (#6318)

* [VL] Fix E function fallback issue (#6397)

* [VL] Add Scala 2.13 support (#6326)

* [VL] Add Scala 2.13 support

* Fix scalaStyle issues

* Fix Scala Style issues

* Add Spark 3.5.1 and Scala 2.13 test in workflow

* Add run-spark-test-spark35-scala213 job

* Add Spark 3.5.1 and Scala 2.13 test in workflow

* Fix tests failures

* Fix tests failures

* ScalaStyle fix

* Fix SoftAffinitySuite

* Fix ArrowUtil error

* Fix backend-velox scala issues

* Fix ColumnarArrowEvalPythonExec issues

* Fix ColumnarArrowEvalPythonExec issues

* Fix TestOperator.scala for style issues

* Fix TestOperator.scala for style issues

* Fix issues in DeltaRewriteTransformerRules.scala

* DeltaRewriteTransformerRules fix

* Fix style issues

* Fix issues

* Fix issues

* Fix issues

* Fix issues

* Fix issues

* Fix issues

* Fix issues

---------

Co-authored-by: Hongze Zhang <[email protected]>

* [VL] Fix Alinux3 arrow build issue (#6363)

* update velox docker and port PR #6363 for get_velox.sh update

---------

Co-authored-by: PHILO-HE <[email protected]>
Co-authored-by: jiangjiangtian <[email protected]>
Co-authored-by: 蒋添 <[email protected]>
Co-authored-by: 高阳阳 <[email protected]>
Co-authored-by: Preetesh2110 <[email protected]>
Co-authored-by: Hongze Zhang <[email protected]>
Co-authored-by: Joey <[email protected]>
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