From e4875898d29103966de6014167338a0ffb165954 Mon Sep 17 00:00:00 2001 From: comphead Date: Tue, 18 Oct 2022 17:22:04 -0700 Subject: [PATCH 1/4] skip failing tests default to false --- datafusion/core/src/config.rs | 9 +++++++++ datafusion/core/src/execution/context.rs | 14 ++++++++++++++ datafusion/core/src/physical_plan/planner.rs | 1 + 3 files changed, 24 insertions(+) diff --git a/datafusion/core/src/config.rs b/datafusion/core/src/config.rs index 2a2139fb255c..cdceb4c45022 100644 --- a/datafusion/core/src/config.rs +++ b/datafusion/core/src/config.rs @@ -194,6 +194,15 @@ impl BuiltInConfigs { configuration setting '{}'.", OPT_COALESCE_BATCHES), 4096, ), + #[cfg(test)] + ConfigDefinition::new_bool( + OPT_OPTIMIZER_SKIP_FAILED_RULES, + "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.", + false + ), + #[cfg(not(test))] ConfigDefinition::new_bool( OPT_OPTIMIZER_SKIP_FAILED_RULES, "When set to true, the logical plan optimizer will produce warning \ diff --git a/datafusion/core/src/execution/context.rs b/datafusion/core/src/execution/context.rs index f7fb0eb90194..1a29bf70340b 100644 --- a/datafusion/core/src/execution/context.rs +++ b/datafusion/core/src/execution/context.rs @@ -2389,6 +2389,20 @@ mod tests { Ok(()) } + #[tokio::test] + async fn check_skip_failing_rules_true() -> Result<()> { + let ctx = SessionContext::new(); + + assert!(!ctx + .state() + .config + .config_options + .read() + .get_bool(OPT_OPTIMIZER_SKIP_FAILED_RULES) + .unwrap_or_default()); + Ok(()) + } + struct MyPhysicalPlanner {} #[async_trait] diff --git a/datafusion/core/src/physical_plan/planner.rs b/datafusion/core/src/physical_plan/planner.rs index 1995a6196eed..bfdad5ce1fbc 100644 --- a/datafusion/core/src/physical_plan/planner.rs +++ b/datafusion/core/src/physical_plan/planner.rs @@ -1972,6 +1972,7 @@ mod tests { Ok(()) } + #[ignore] #[tokio::test] async fn in_list_types_struct_literal() -> Result<()> { // expression: "a in (struct::null, 'a')" From 39ecfd3c3abe5f63c0cf2dc22c886985e6006b56 Mon Sep 17 00:00:00 2001 From: comphead Date: Tue, 18 Oct 2022 17:26:00 -0700 Subject: [PATCH 2/4] fix test name --- datafusion/core/src/execution/context.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datafusion/core/src/execution/context.rs b/datafusion/core/src/execution/context.rs index 1a29bf70340b..1446cb3da04b 100644 --- a/datafusion/core/src/execution/context.rs +++ b/datafusion/core/src/execution/context.rs @@ -2390,7 +2390,7 @@ mod tests { } #[tokio::test] - async fn check_skip_failing_rules_true() -> Result<()> { + async fn check_skip_failing_rules_false() -> Result<()> { let ctx = SessionContext::new(); assert!(!ctx From 21712d0a8bfb9a15f453228cb6e8f47255d3c420 Mon Sep 17 00:00:00 2001 From: comphead Date: Fri, 21 Oct 2022 17:48:53 -0700 Subject: [PATCH 3/4] make regression tests to run as cfg --- datafusion/common/Cargo.toml | 1 + datafusion/core/Cargo.toml | 1 + datafusion/core/src/config.rs | 15 ++++++--------- datafusion/core/src/execution/context.rs | 14 -------------- datafusion/core/src/physical_plan/planner.rs | 1 - datafusion/expr/Cargo.toml | 1 + datafusion/optimizer/Cargo.toml | 1 + datafusion/physical-expr/Cargo.toml | 1 + datafusion/row/Cargo.toml | 1 + datafusion/sql/Cargo.toml | 1 + 10 files changed, 13 insertions(+), 24 deletions(-) diff --git a/datafusion/common/Cargo.toml b/datafusion/common/Cargo.toml index c8ed7a034ccf..4d1a39709088 100644 --- a/datafusion/common/Cargo.toml +++ b/datafusion/common/Cargo.toml @@ -37,6 +37,7 @@ avro = ["apache-avro"] default = [] jit = ["cranelift-module"] pyarrow = ["pyo3", "arrow/pyarrow"] +optimizer_failed_rules = [] [dependencies] apache-avro = { version = "0.14", default-features = false, features = ["snappy"], optional = true } diff --git a/datafusion/core/Cargo.toml b/datafusion/core/Cargo.toml index 6512db0ae922..b4431d5025b9 100644 --- a/datafusion/core/Cargo.toml +++ b/datafusion/core/Cargo.toml @@ -52,6 +52,7 @@ regex_expressions = ["datafusion-physical-expr/regex_expressions"] scheduler = ["rayon"] simd = ["arrow/simd"] unicode_expressions = ["datafusion-physical-expr/regex_expressions", "datafusion-sql/unicode_expressions"] +optimizer_failed_rules = [] [dependencies] ahash = { version = "0.8", default-features = false, features = ["runtime-rng"] } diff --git a/datafusion/core/src/config.rs b/datafusion/core/src/config.rs index f928376f7969..0f9cfce342a5 100644 --- a/datafusion/core/src/config.rs +++ b/datafusion/core/src/config.rs @@ -237,20 +237,17 @@ impl BuiltInConfigs { to reduce the number of rows decoded.", false, ), - #[cfg(test)] - ConfigDefinition::new_bool( - OPT_OPTIMIZER_SKIP_FAILED_RULES, - "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.", - false - ), - #[cfg(not(test))] + // By default tests run with ignoring optimizer errors. To make errors visible need to run the project + // with enabled 'optimizer_failed_rules' configuration + // cargo test --features "optimizer_failed_rules" ConfigDefinition::new_bool( OPT_OPTIMIZER_SKIP_FAILED_RULES, "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.", + #[cfg(feature = "optimizer_failed_rules")] + false, + #[cfg(not(feature = "optimizer_failed_rules"))] true ), ConfigDefinition::new_u64( diff --git a/datafusion/core/src/execution/context.rs b/datafusion/core/src/execution/context.rs index 1566aaaaba2e..f140ce1c3b98 100644 --- a/datafusion/core/src/execution/context.rs +++ b/datafusion/core/src/execution/context.rs @@ -2403,20 +2403,6 @@ mod tests { Ok(()) } - #[tokio::test] - async fn check_skip_failing_rules_false() -> Result<()> { - let ctx = SessionContext::new(); - - assert!(!ctx - .state() - .config - .config_options - .read() - .get_bool(OPT_OPTIMIZER_SKIP_FAILED_RULES) - .unwrap_or_default()); - Ok(()) - } - struct MyPhysicalPlanner {} #[async_trait] diff --git a/datafusion/core/src/physical_plan/planner.rs b/datafusion/core/src/physical_plan/planner.rs index bfdad5ce1fbc..1995a6196eed 100644 --- a/datafusion/core/src/physical_plan/planner.rs +++ b/datafusion/core/src/physical_plan/planner.rs @@ -1972,7 +1972,6 @@ mod tests { Ok(()) } - #[ignore] #[tokio::test] async fn in_list_types_struct_literal() -> Result<()> { // expression: "a in (struct::null, 'a')" diff --git a/datafusion/expr/Cargo.toml b/datafusion/expr/Cargo.toml index d3b348c4bcb3..7c790f3f4a49 100644 --- a/datafusion/expr/Cargo.toml +++ b/datafusion/expr/Cargo.toml @@ -33,6 +33,7 @@ name = "datafusion_expr" path = "src/lib.rs" [features] +optimizer_failed_rules = [] [dependencies] ahash = { version = "0.8", default-features = false, features = ["runtime-rng"] } diff --git a/datafusion/optimizer/Cargo.toml b/datafusion/optimizer/Cargo.toml index e07d798caf8d..f8742172570d 100644 --- a/datafusion/optimizer/Cargo.toml +++ b/datafusion/optimizer/Cargo.toml @@ -35,6 +35,7 @@ path = "src/lib.rs" [features] default = ["unicode_expressions"] unicode_expressions = [] +optimizer_failed_rules = [] [dependencies] arrow = { version = "25.0.0", features = ["prettyprint"] } diff --git a/datafusion/physical-expr/Cargo.toml b/datafusion/physical-expr/Cargo.toml index ba4dc0b036a9..f0b718ee257d 100644 --- a/datafusion/physical-expr/Cargo.toml +++ b/datafusion/physical-expr/Cargo.toml @@ -37,6 +37,7 @@ crypto_expressions = ["md-5", "sha2", "blake2", "blake3"] default = ["crypto_expressions", "regex_expressions", "unicode_expressions"] regex_expressions = ["regex"] unicode_expressions = ["unicode-segmentation"] +optimizer_failed_rules = [] [dependencies] ahash = { version = "0.8", default-features = false, features = ["runtime-rng"] } diff --git a/datafusion/row/Cargo.toml b/datafusion/row/Cargo.toml index 52e6e50c7e85..cb5d0b91a79a 100644 --- a/datafusion/row/Cargo.toml +++ b/datafusion/row/Cargo.toml @@ -35,6 +35,7 @@ path = "src/lib.rs" [features] # Used to enable JIT code generation jit = ["datafusion-jit"] +optimizer_failed_rules = [] [dependencies] arrow = "25.0.0" diff --git a/datafusion/sql/Cargo.toml b/datafusion/sql/Cargo.toml index 4a88a01e31b0..3db707dfab70 100644 --- a/datafusion/sql/Cargo.toml +++ b/datafusion/sql/Cargo.toml @@ -35,6 +35,7 @@ path = "src/lib.rs" [features] default = ["unicode_expressions"] unicode_expressions = [] +optimizer_failed_rules = [] [dependencies] arrow = { version = "25.0.0", default-features = false } From 45e8c0cf809375d90a800996cd0e2efe41be4ec3 Mon Sep 17 00:00:00 2001 From: comphead Date: Sun, 23 Oct 2022 11:04:53 -0700 Subject: [PATCH 4/4] cargo toml fmt --- datafusion/common/Cargo.toml | 2 +- datafusion/core/Cargo.toml | 2 +- datafusion/optimizer/Cargo.toml | 2 +- datafusion/physical-expr/Cargo.toml | 2 +- datafusion/sql/Cargo.toml | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/datafusion/common/Cargo.toml b/datafusion/common/Cargo.toml index 4d1a39709088..8842894b745f 100644 --- a/datafusion/common/Cargo.toml +++ b/datafusion/common/Cargo.toml @@ -36,8 +36,8 @@ path = "src/lib.rs" avro = ["apache-avro"] default = [] jit = ["cranelift-module"] -pyarrow = ["pyo3", "arrow/pyarrow"] optimizer_failed_rules = [] +pyarrow = ["pyo3", "arrow/pyarrow"] [dependencies] apache-avro = { version = "0.14", default-features = false, features = ["snappy"], optional = true } diff --git a/datafusion/core/Cargo.toml b/datafusion/core/Cargo.toml index b4431d5025b9..8eebb9adfd51 100644 --- a/datafusion/core/Cargo.toml +++ b/datafusion/core/Cargo.toml @@ -46,13 +46,13 @@ default = ["crypto_expressions", "regex_expressions", "unicode_expressions"] force_hash_collisions = [] # Used to enable JIT code generation jit = ["datafusion-jit", "datafusion-row/jit"] +optimizer_failed_rules = [] pyarrow = ["pyo3", "arrow/pyarrow", "datafusion-common/pyarrow"] regex_expressions = ["datafusion-physical-expr/regex_expressions"] # Used to enable scheduler scheduler = ["rayon"] simd = ["arrow/simd"] unicode_expressions = ["datafusion-physical-expr/regex_expressions", "datafusion-sql/unicode_expressions"] -optimizer_failed_rules = [] [dependencies] ahash = { version = "0.8", default-features = false, features = ["runtime-rng"] } diff --git a/datafusion/optimizer/Cargo.toml b/datafusion/optimizer/Cargo.toml index f8742172570d..699ab7273abb 100644 --- a/datafusion/optimizer/Cargo.toml +++ b/datafusion/optimizer/Cargo.toml @@ -34,8 +34,8 @@ path = "src/lib.rs" [features] default = ["unicode_expressions"] -unicode_expressions = [] optimizer_failed_rules = [] +unicode_expressions = [] [dependencies] arrow = { version = "25.0.0", features = ["prettyprint"] } diff --git a/datafusion/physical-expr/Cargo.toml b/datafusion/physical-expr/Cargo.toml index f0b718ee257d..ec5d70460938 100644 --- a/datafusion/physical-expr/Cargo.toml +++ b/datafusion/physical-expr/Cargo.toml @@ -35,9 +35,9 @@ path = "src/lib.rs" [features] crypto_expressions = ["md-5", "sha2", "blake2", "blake3"] default = ["crypto_expressions", "regex_expressions", "unicode_expressions"] +optimizer_failed_rules = [] regex_expressions = ["regex"] unicode_expressions = ["unicode-segmentation"] -optimizer_failed_rules = [] [dependencies] ahash = { version = "0.8", default-features = false, features = ["runtime-rng"] } diff --git a/datafusion/sql/Cargo.toml b/datafusion/sql/Cargo.toml index 3db707dfab70..9f6ad14cf41d 100644 --- a/datafusion/sql/Cargo.toml +++ b/datafusion/sql/Cargo.toml @@ -34,8 +34,8 @@ path = "src/lib.rs" [features] default = ["unicode_expressions"] -unicode_expressions = [] optimizer_failed_rules = [] +unicode_expressions = [] [dependencies] arrow = { version = "25.0.0", default-features = false }