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

change pre_cast_lit_in_comparison to unwrap_cast_in_comparison #3662

Merged
merged 2 commits into from
Sep 30, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
4 changes: 2 additions & 2 deletions datafusion/core/src/execution/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,10 @@ use datafusion_expr::{TableSource, TableType};
use datafusion_optimizer::decorrelate_where_exists::DecorrelateWhereExists;
use datafusion_optimizer::decorrelate_where_in::DecorrelateWhereIn;
use datafusion_optimizer::filter_null_join_keys::FilterNullJoinKeys;
use datafusion_optimizer::pre_cast_lit_in_comparison::PreCastLitInComparisonExpressions;
use datafusion_optimizer::rewrite_disjunctive_predicate::RewriteDisjunctivePredicate;
use datafusion_optimizer::scalar_subquery_to_join::ScalarSubqueryToJoin;
use datafusion_optimizer::type_coercion::TypeCoercion;
use datafusion_optimizer::unwrap_cast_in_comparison::UnwrapCastInComparison;
use datafusion_sql::{
parser::DFParser,
planner::{ContextProvider, SqlToRel},
Expand Down Expand Up @@ -1466,9 +1466,9 @@ impl SessionState {
}

let mut rules: Vec<Arc<dyn OptimizerRule + Sync + Send>> = vec![
Arc::new(PreCastLitInComparisonExpressions::new()),
Arc::new(TypeCoercion::new()),
Arc::new(SimplifyExpressions::new()),
Arc::new(UnwrapCastInComparison::new()),
Copy link
Contributor

Choose a reason for hiding this comment

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

this name is much easier to understand for me -- thank you

Arc::new(DecorrelateWhereExists::new()),
Arc::new(DecorrelateWhereIn::new()),
Arc::new(ScalarSubqueryToJoin::new()),
Expand Down
20 changes: 1 addition & 19 deletions datafusion/core/tests/sql/explain_analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -767,8 +767,6 @@ async fn test_physical_plan_display_indent_multi_children() {
#[tokio::test]
#[cfg_attr(tarpaulin, ignore)]
async fn csv_explain() {
// TODO: https://github.com/apache/arrow-datafusion/issues/3622 refactor the `PreCastLitInComparisonExpressions`

// This test uses the execute function that create full plan cycle: logical, optimized logical, and physical,
// then execute the physical plan and return the final explain results
let ctx = SessionContext::new();
Expand All @@ -779,23 +777,6 @@ async fn csv_explain() {

// Note can't use `assert_batches_eq` as the plan needs to be
// normalized for filenames and number of cores
let expected = vec![
vec![
"logical_plan",
"Projection: #aggregate_test_100.c1\
\n Filter: CAST(#aggregate_test_100.c2 AS Int32) > Int32(10)\
Copy link
Contributor Author

Choose a reason for hiding this comment

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

CAST(#aggregate_test_100.c2 AS Int32) will be converted to #aggregate_test_100.c2, and the Int32(10) will be converted to Int8(10)

\n TableScan: aggregate_test_100 projection=[c1, c2], partial_filters=[CAST(#aggregate_test_100.c2 AS Int32) > Int32(10)]"
Copy link
Contributor

Choose a reason for hiding this comment

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

The key difference for anyone else following along is that

partial_filters=[CAST(#aggregate_test_100.c2 AS Int32) > Int32(10)]

Has become

partial_filters=[partial_filters=[#aggregate_test_100.c2 > Int8(10)]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, we remove the cast for CAST(#aggregate_test_100.c2 AS Int32), and add cast for Int32(10)

],
vec!["physical_plan",
"ProjectionExec: expr=[c1@0 as c1]\
\n CoalesceBatchesExec: target_batch_size=4096\
\n FilterExec: CAST(c2@1 AS Int32) > 10\
\n RepartitionExec: partitioning=RoundRobinBatch(NUM_CORES)\
\n CsvExec: files=[ARROW_TEST_DATA/csv/aggregate_test_100.csv], has_header=true, limit=None, projection=[c1, c2]\
\n"
]];
assert_eq!(expected, actual);

let expected = vec![
vec![
"logical_plan",
Expand All @@ -811,6 +792,7 @@ async fn csv_explain() {
\n CsvExec: files=[ARROW_TEST_DATA/csv/aggregate_test_100.csv], has_header=true, limit=None, projection=[c1, c2]\
\n"
]];
assert_eq!(expected, actual);

let sql = "explain SELECT c1 FROM aggregate_test_100 where c2 > 10";
let actual = execute(&ctx, sql).await;
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ pub mod subquery_filter_to_join;
pub mod type_coercion;
pub mod utils;

pub mod pre_cast_lit_in_comparison;
pub mod rewrite_disjunctive_predicate;
#[cfg(test)]
pub mod test;
pub mod unwrap_cast_in_comparison;

pub use optimizer::{OptimizerConfig, OptimizerRule};
Loading