pan3793 commented on code in PR #58661:
URL: https://github.com/apache/spark/pull/58661#discussion_r3986454202
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/InlineCTE.scala:
##########
@@ -77,15 +83,18 @@ case class InlineCTE(
private def shouldInline(cteDef: CTERelationDef, refCount: Int): Boolean = {
// A CTE definition that requests to skip inlining is never inlined, even
in `alwaysInline`
// mode, so that a producer can guarantee the CTE is materialized rather
than duplicated.
- !cteDef.forceSkipInline && (alwaysInline || {
- // We do not need to check enclosed `CTERelationRef`s for
`deterministic` or
- // `OuterReference`, because:
- // 1) It is fine to inline a CTE if it references another CTE that is
non-deterministic;
- // 2) Any `CTERelationRef` that contains `OuterReference` would have
been inlined first.
- refCount == 1 ||
- cteDef.deterministic ||
-
cteDef.child.exists(_.expressions.exists(_.isInstanceOf[OuterReference]))
- })
+ !cteDef.forceSkipInline && (alwaysInline || (cteDef.materialized match {
Review Comment:
Follow-up in 371d28e61ec: the `materialized` clause is optimizer-only.
`CheckAnalysis` runs `InlineCTE(isAnalysis = true)` before
`MaterializedCTECheck`, so as written it preempted the analysis error with
`INTERNAL_ERROR` and broke `SQLQueryTestSuite.cte.sql`. The backstop you asked
for is unchanged for programmatic plans (`Optimize.execute` on a `materialized
= Some(true)` def with an outer reference still fails with `INTERNAL_ERROR`).
--
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]
