alamb commented on code in PR #3890:
URL: https://github.com/apache/arrow-datafusion/pull/3890#discussion_r999826384
##########
datafusion/core/src/config.rs:
##########
@@ -237,6 +237,15 @@ impl BuiltInConfigs {
to reduce the number of rows decoded.",
false,
),
+ #[cfg(test)]
+ ConfigDefinition::new_bool(
Review Comment:
```suggestion
// skip_failing_rules default to true in tests so that
regressions are caught much sooner
ConfigDefinition::new_bool(
```
##########
datafusion/core/src/config.rs:
##########
@@ -237,6 +237,15 @@ 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.",
Review Comment:
```suggestion
"OVERRIDDEN FOR TEST",
```
##########
datafusion/core/src/physical_plan/planner.rs:
##########
@@ -1972,6 +1972,7 @@ mod tests {
Ok(())
}
+ #[ignore]
Review Comment:
We should have a ticket to track this failure. How does it fail with the
change in default?
##########
datafusion/core/src/config.rs:
##########
@@ -237,6 +237,15 @@ 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.",
Review Comment:
I think this could be very confusing to people (that tests and defaults are
different) so I suggest some more comments to make it clearer
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
