Skip to content

Conversation

@HaoYang670
Copy link
Contributor

Signed-off-by: remzi 13716567376yh@gmail.com

Which issue does this PR close?

None.
Related to #4685.

Rationale for this change

We should test the type errors in a query could be detected from either the logical plan (skip_failed_rules = false) or the physical plan (skip_failed_rules = true).

What changes are included in this PR?

Only update the test.

Are these changes tested?

Are there any user-facing changes?

Signed-off-by: remzi <13716567376yh@gmail.com>
@github-actions github-actions bot added the core Core DataFusion crate label Feb 12, 2023
@HaoYang670
Copy link
Contributor Author

@jackwener @alamb could you please help to review? Thank you!

Copy link
Member

@jackwener jackwener left a comment

Choose a reason for hiding this comment

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

Look great to me. Thank you @HaoYang670.
This PR is same with #5224.
I prepare polish them like this PR.

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thanks @HaoYang670 and @jackwener

I think @mustafasrepo and @metesynnada may be interested in these changes (to improve error checking / move the type checks out of physical plan creation and into logical plan creation)

Signed-off-by: remzi <13716567376yh@gmail.com>
Signed-off-by: remzi <13716567376yh@gmail.com>
@mustafasrepo
Copy link
Contributor

mustafasrepo commented Feb 13, 2023

LGTM!. This is a nice example to add tests for skipped errors. Thank you @HaoYang670.

@alamb alamb merged commit 3da7902 into apache:master Feb 13, 2023
@ursabot
Copy link

ursabot commented Feb 13, 2023

Benchmark runs are scheduled for baseline = 016ad8e and contender = 3da7902. 3da7902 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] ec2-t3-xlarge-us-east-2
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] test-mac-arm
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] ursa-i9-9960x
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] ursa-thinkcentre-m75q
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@HaoYang670 HaoYang670 deleted the window_frame_creation_dont_skip_failing_rule branch February 13, 2023 12:17
jiangzhx pushed a commit to jiangzhx/arrow-datafusion that referenced this pull request Feb 24, 2023
…n` (apache#5257)

* enhance the test

Signed-off-by: remzi <13716567376yh@gmail.com>

* update comment

Signed-off-by: remzi <13716567376yh@gmail.com>

---------

Signed-off-by: remzi <13716567376yh@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants