Skip to content

Disable skip_failed_rules optimizer config by default #6265

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
May 10, 2023
Merged

Disable skip_failed_rules optimizer config by default #6265

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d2def3b
Disable skip_failed_rules default
jackwener May 6, 2023
f0022b6
fix slt
jackwener May 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion datafusion/common/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ config_namespace! {
/// When set to true, the logical plan optimizer will produce warning
/// messages if any optimization rules produce errors and then proceed to the next
/// rule. When set to false, any rules that produce errors will cause the query to fail
pub skip_failed_rules: bool, default = true
pub skip_failed_rules: bool, default = false

/// Number of times that the optimizer will attempt to optimize the plan
pub max_passes: usize, default = 3
Expand Down
2 changes: 2 additions & 0 deletions datafusion/core/tests/sql/subqueries.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,8 @@ async fn support_order_by_correlated_columns() -> Result<()> {
Ok(())
}

// TODO: issue https://github.com/apache/arrow-datafusion/issues/6263
Copy link
Contributor

@comphead comphead May 8, 2023

Choose a reason for hiding this comment

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

👍 so only 1 test gets failed on optimization rules?

Copy link
Member Author

@jackwener jackwener May 8, 2023

Choose a reason for hiding this comment

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

Yes

Copy link
Contributor

@mingmwang mingmwang May 10, 2023

Choose a reason for hiding this comment

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

Which rule fail this test ?

Copy link
Member Author

@jackwener jackwener May 10, 2023

Choose a reason for hiding this comment

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

Which rule fail this test ?

decorrelate_where_in, you can see issue #6263

Copy link
Contributor

@mingmwang mingmwang May 10, 2023

Choose a reason for hiding this comment

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

I will fix this rule later.

#[ignore]
#[tokio::test]
async fn support_limit_subquery() -> Result<()> {
let ctx = create_join_context("t1_id", "t2_id", true)?;
Expand Down
2 changes: 1 addition & 1 deletion datafusion/core/tests/sqllogictests/test_files/arrow_typeof.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ select arrow_cast('30 minutes', 'Interval(MonthDayNano)');

## Duration

query error DataFusion error: This feature is not implemented: Can't create a scalar from array of type "Duration\(Second\)"
query error DataFusion error: Optimizer rule 'simplify_expressions' failed\ncaused by\nThis feature is not implemented: Can't create a scalar from array of type "Duration\(Second\)"
---
select arrow_cast(interval '30 minutes', 'Duration(Second)');

Expand Down
2 changes: 1 addition & 1 deletion datafusion/core/tests/sqllogictests/test_files/information_schema.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ datafusion.optimizer.repartition_file_scans true
datafusion.optimizer.repartition_joins true
datafusion.optimizer.repartition_sorts true
datafusion.optimizer.repartition_windows true
datafusion.optimizer.skip_failed_rules true
datafusion.optimizer.skip_failed_rules false
datafusion.optimizer.top_down_join_key_reordering true
datafusion.sql_parser.dialect generic
datafusion.sql_parser.enable_ident_normalization true
Expand Down
2 changes: 1 addition & 1 deletion datafusion/core/tests/sqllogictests/test_files/scalar.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -839,7 +839,7 @@ SELECT null, not(null)
----
NULL NULL

query error DataFusion error: Error during planning: NOT or BITWISE_NOT not supported for datatype: 'Utf8'
query error DataFusion error: Optimizer rule 'simplify_expressions' failed\ncaused by\nInternal error: NOT/BITWISE_NOT 'Literal \{ value: Utf8\("hi"\) \}' can't be evaluated because the expression's type is Utf8, not boolean or NULL or Integer\. This was likely caused by a bug in DataFusion's code and we would welcome that you file an bug report in our issue tracker
SELECT NOT('hi')

# test_negative_expressions()
Expand Down
2 changes: 1 addition & 1 deletion docs/source/user-guide/configs.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ Environment variables are read during `SessionConfig` initialisation so they mus
| datafusion.optimizer.repartition_file_scans | true | When set to true, file groups will be repartitioned to achieve maximum parallelism. Currently supported only for Parquet format in which case multiple row groups from the same file may be read concurrently. If false then each row group is read serially, though different files may be read in parallel. |
| datafusion.optimizer.repartition_windows | true | Should DataFusion repartition data using the partitions keys to execute window functions in parallel using the provided `target_partitions` level |
| datafusion.optimizer.repartition_sorts | true | Should DataFusion execute sorts in a per-partition fashion and merge afterwards instead of coalescing first and sorting globally. With this flag is enabled, plans in the form below `text "SortExec: [a@0 ASC]", " CoalescePartitionsExec", " RepartitionExec: partitioning=RoundRobinBatch(8), input_partitions=1", ` would turn into the plan below which performs better in multithreaded environments `text "SortPreservingMergeExec: [a@0 ASC]", " SortExec: [a@0 ASC]", " RepartitionExec: partitioning=RoundRobinBatch(8), input_partitions=1", ` |
| datafusion.optimizer.skip_failed_rules | true | When set to true, the logical plan optimizer will produce warning messages if any optimization rules produce errors and then proceed to the next rule. When set to false, any rules that produce errors will cause the query to fail |
| datafusion.optimizer.skip_failed_rules | false | When set to true, the logical plan optimizer will produce warning messages if any optimization rules produce errors and then proceed to the next rule. When set to false, any rules that produce errors will cause the query to fail |
| datafusion.optimizer.max_passes | 3 | Number of times that the optimizer will attempt to optimize the plan |
| datafusion.optimizer.top_down_join_key_reordering | true | When set to true, the physical plan optimizer will run a top down process to reorder the join keys |
| datafusion.optimizer.prefer_hash_join | true | When set to true, the physical plan optimizer will prefer HashJoin over SortMergeJoin. HashJoin can work more efficiently than SortMergeJoin but consumes more memory |
Expand Down