adriangb commented on code in PR #25456:
URL: https://github.com/apache/datafusion/pull/25456#discussion_r4419702125
##########
datafusion/optimizer/src/projection_inliner.rs:
##########
@@ -0,0 +1,540 @@
+// SPDX-License-Identifier: Apache-2.0
+// SPDX-FileCopyrightText: See the NOTICE file distributed with this work for
additional information regarding copyright ownership
+// SPDX-FileContributor: Licensed to the Apache Software Foundation (ASF)
under one or more contributor license agreements
+
+//! A single, cost-aware check for rewriting expressions through a projection.
+//!
+//! Many rules move an expression that is written over the output of a
+//! projection below that projection, or merge it into the projection. To do
+//! this, they replace each column reference with the expression that defines
+//! the column. This is called *inlining*.
+//!
+//! Inlining can make the plan evaluate a definition more times than before:
+//!
+//! - The consumer can reference the column more than one time.
+//! - The projection can stay in the plan and still compute the definition.
+//!
+//! When the definition is volatile, this changes the query result. When the
+//! definition is expensive, this makes the query slower. A typical example is
a
+//! column that [`CommonSubexprEliminate`] created on purpose, so that an
+//! expensive expression is computed one time only.
+//!
+//! [`ProjectionInliner`] is the one place that makes this decision. Rules call
+//! it instead of doing the substitution themselves.
+//!
+//! [`CommonSubexprEliminate`]:
crate::common_subexpr_eliminate::CommonSubexprEliminate
+
+use std::collections::{HashMap, HashSet};
+
+use datafusion_common::tree_node::{
+ Transformed, TransformedResult, TreeNode, TreeNodeRecursion,
+};
+use datafusion_common::{Column, Result};
+use datafusion_expr::{Expr, ExpressionPlacement, Projection};
+
+/// The cost of one more evaluation of an expression.
+///
+/// The variants are in increasing order of cost.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
+pub(crate) enum DuplicationCost {
+ /// Column references and expressions that report
+ /// [`ExpressionPlacement::MoveTowardsLeafNodes`] (for example struct field
+ /// access). These are free, or almost free, to evaluate again.
+ Free,
+ /// Built-in operators over free or cheap operands: arithmetic,
+ /// comparisons, `CAST`, `CASE`, `IS NULL`, and similar. They are
+ /// vectorized kernels, so an extra evaluation costs little.
+ ///
+ /// A bare literal is also in this class. This keeps the merge guard of
+ /// `OptimizeProjections` (issue #8296) unchanged.
+ Cheap,
+ /// Contains a scalar function that reports
+ /// [`ExpressionPlacement::KeepInPlace`] for column arguments, or a
+ /// higher-order function. This is the default for a user-defined
+ /// function, so the optimizer must assume that it can be arbitrarily
+ /// expensive.
+ Expensive,
Review Comment:
This is again creating a mess of `interpret KeepInPlace as expensive` which
is not clear we want to do, as already pointed out in
https://github.com/apache/datafusion/pull/25456/changes#r4045318003
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
