Skip to content

make_date under a codegen-dispatched parent falls back to Spark since #7261 #7284

Open

Description

@dwsmith1983

Describe the bug

Since #7261, a make_date under an expression that Comet runs through the codegen dispatcher falls back to Spark, and the whole operator holding it leaves Comet. Before #7261 the same expressions stayed in Comet.

CometMakeDate.unsupportedParentReason walks the parents of every make_date, and any multi-child parent it does not list ends in the default arm ("Other native multi-child expressions may evaluate operands that Spark skips"):

private[serde] def unsupportedParentReason(expr: Expression): Option[String] = {
var pending: List[(Expression, Option[String])] = (expr, None) :: Nil
while (pending.nonEmpty) {
val (current, parent) = pending.head
pending = pending.tail
if (current.isInstanceOf[MakeDate] && parent.isDefined) {
return Some(s"make_date requires Spark evaluation under ${parent.get}")
}
def pushChildren(mask: Option[String]): Unit = {
current.children.reverseIterator.foreach(child => pending = (child, mask) :: pending)
}
if (parent.isDefined) {
pushChildren(parent)
} else {
current match {
// These native expressions already select the rows passed to their children.
case _: If | _: CaseWhen | _: MakeDate => pushChildren(None)
// COALESCE lowers to CASE and evaluates an earlier child again in its result branch.
case coalesce: Coalesce if coalesce.deterministic => pushChildren(None)
// These parents evaluate every child in Spark, including NULL values.
case _: EqualNullSafe | _: CreateArray | _: CreateNamedStruct | _: Greatest | _: Least |
_: HashExpression[_] =>
pushChildren(None)
// Arithmetic always requires left, but a NULL left can hide a throwing right operand.
// Admit the date only if native evaluation cannot expose that skipped sibling.
case _: Add | _: Subtract | _: Multiply =>
val left = current.children.head
val right = current.children(1)
val rightMask = if (left.nullable) Some(current.nodeName) else None
val leftMask = if (left.nullable && !canEvaluateEagerly(right)) rightMask else None
pending = (left, leftMask) :: (right, rightMask) :: pending
// These parents evaluate left first and skip right only when left is NULL.
case _: BinaryComparison | _: DateAdd | _: DateSub | _: DateDiff | _: DateFormatClass =>
val left = current.children.head
val right = current.children(1)
val rightMask = if (left.nullable) Some(current.nodeName) else None
pending = (left, None) :: (right, rightMask) :: pending
case _: SortOrder => pushChildren(Some(current.nodeName))
// Empty membership can skip its sole operand in Spark.
case in: In if in.list.isEmpty => pushChildren(Some(current.nodeName))
case inSet: InSet if inSet.hset.isEmpty => pushChildren(Some(current.nodeName))
// Nonempty IN always evaluates its probe, but may skip candidates on individual rows.
case in: In =>
in.list.reverseIterator.foreach { child =>
pending = (child, Some(in.nodeName)) :: pending
}
val probeParent = if (in.list.forall(canEvaluateEagerly)) {
None
} else {
Some("In with computed candidates")
}
pending = (in.value, probeParent) :: pending
case _ if current.children.size <= 1 => pushChildren(None)
// Other native multi-child expressions may evaluate operands that Spark skips.
case _ => pushChildren(Some(current.nodeName))

That covers dispatched parents such as AddMonths and MonthsBetween. A dispatched parent never runs the native make_date, though. CometCodegenDispatch binds the whole subtree and runs Spark's own generated code for it, so the make_date under it is evaluated by Spark with Spark's short-circuiting:

// Bind against only the AttributeReferences the tree actually reads, so ordinals align with
// the data args we ship.
val attrs = target.collect { case a: AttributeReference => a }.distinct
val boundExpr = BindReferences.bindReference(target, AttributeSeq(attrs))

Results are the same either way; only the operator moves back to Spark.

Steps to reproduce

On main (967d09b), Spark 4.1 and 3.5, default configuration:

CREATE TABLE md_parent(y INT, m INT, d INT, dt DATE) USING parquet;
INSERT INTO md_parent VALUES
  (2024, 1, 31, DATE '2024-03-15'), (NULL, 2, 1, NULL), (2024, NULL, 1, DATE '2023-12-01');

SELECT add_months(dt, 1) FROM md_parent;                     -- stays in Comet (dispatched)
SELECT add_months(make_date(y, m, d), 1) FROM md_parent;     -- falls back
SELECT months_between(make_date(y, m, d), dt) FROM md_parent; -- falls back

The second query plans as:

Project [COMET: make_date requires Spark evaluation under AddMonths]
+- CometColumnarToRow
   +- CometNativeScan parquet spark_catalog.default.md_parent

and the third falls back with make_date requires Spark evaluation under MonthsBetween. On f1121ca, the commit before #7261, all three queries pass expect_dispatch in CometSqlFileTestSuite.

Expected behavior

A make_date under a parent that the codegen dispatcher will take stays in Comet, as it did before #7261. One way: have the walk stop at a parent whose serde is a CometCodegenDispatch while spark.comet.exec.scalaUDF.codegen.enabled is on, since that subtree runs as Spark code. Natively evaluated parents keep their current checks.

Additional context

Found while merging main into #5864, which adds more dispatched datetime parents (DateAddInterval, DateAddYMInterval, SubtractDates, SubtractTimestamps, TimestampAddYMInterval); a make_date under any of them falls back the same way.

@pingzh, this follows from #7261. Would you like to take it?

Activity

  1. pingzh commented on Oct 28, 2026

    @pingzh
    Contributor

    take

  2. pingzh commented on Oct 28, 2026

    @pingzh
    Contributor

    @dwsmith1983 happy to take it 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions