[
https://issues.apache.org/jira/browse/SPARK-60329?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Max Gekk updated SPARK-60329:
-----------------------------
Description:
With whole-stage codegen enabled, {{FilterExec}} (and the {{FILTER}} clause of
an aggregate function, through
{{GeneratePredicateHelper.generatePredicateCode}}) emits a plain-column
{{IsNotNull}} check lazily: just before the first conjunct that references its
column, or after all conjuncts if none does. With whole-stage codegen disabled,
{{And.eval}} evaluates the conjuncts in the order of the condition, where
{{InferFiltersFromConstraints}} puts the inferred {{isnotnull(...)}} checks
first and a user-written {{IS NOT NULL}} stays where it was written.
So when a plain-column {{IS NOT NULL}} check is FALSE on a row and a conjunct
that comes *later* in the condition but does not reference that column raises
an error on the same row, codegen reaches the error that the interpreted path
never evaluates. The query fails only with whole-stage codegen on.
Reproducer (ANSI mode):
{code:sql}
CREATE TABLE t USING parquet AS
SELECT * FROM VALUES (NULL, 1, 2147483647, NULL), (0, 1, 0, 1) AS v(a, b, y,
x);
SET spark.sql.parquet.filterPushdown=false;
SET spark.sql.codegen.wholeStage=true;
SELECT a FROM t WHERE x IS NOT NULL AND y + 1 > 0 AND x > 0;
-- [ARITHMETIC_OVERFLOW] overflow. ... SQLSTATE: 22003
SET spark.sql.codegen.wholeStage=false;
SELECT a FROM t WHERE x IS NOT NULL AND y + 1 > 0 AND x > 0;
-- 0
{code}
The optimized condition is {{isnotnull(y) AND isnotnull(x) AND (y + 1) > 0 AND
x > 0}}. With codegen off, {{isnotnull(x)}} is FALSE on the first row and the
row is dropped. With codegen on, {{isnotnull(x)}} is emitted just before {{x >
0}}, after {{(y + 1) > 0}}, so the overflow is raised.
This is independent of SPARK-60315, which is about a NULL conjunct stopping the
evaluation with codegen on. The fix for SPARK-60315 keeps the lazy placement
when no conjunct can be NULL, and only preserves the position of the pending
checks once a NULL conjunct flag is generated, so this shape still differs
between the two modes after it.
A fix should emit each plain-column {{IsNotNull}} check at its position in the
condition, in both {{generatePredicateCode}} and the CSE branch of
{{FilterExec.doConsume}}, which mirror each other by hand. The lazy placement
exists so that columns are not loaded until a conjunct needs them, so the
performance effect of the change on filters with many inferred checks should be
measured. Complex {{IsNotNull(expr)}} checks, whose inner expression can itself
raise, need the same care.
A small cleanup can ride along with this fix. In both code paths, when a
conjunct references a column whose direct {{IsNotNull}} check was already
generated, the {{else if}} branch of the reference loop (it is not guarded by
{{idx == -1}}) generates a redundant synthetic {{IsNotNull(r)}} for the same
column, once per column. It is a harmless duplicate null check, pre-existing,
and after SPARK-60315 it is routine for the first conjunct that references a
column whose check was generated before it. The fix is to guard the synthetic
check with {{idx == -1}}; it is left out of SPARK-60315 because it changes the
generated code when no conjunct can be NULL.
was:
With whole-stage codegen enabled, {{FilterExec}} (and the {{FILTER}} clause of
an aggregate function, through
{{GeneratePredicateHelper.generatePredicateCode}}) emits a plain-column
{{IsNotNull}} check lazily: just before the first conjunct that references its
column, or after all conjuncts if none does. With whole-stage codegen disabled,
{{And.eval}} evaluates the conjuncts in the order of the condition, where
{{InferFiltersFromConstraints}} puts the inferred {{isnotnull(...)}} checks
first and a user-written {{IS NOT NULL}} stays where it was written.
So when a plain-column {{IS NOT NULL}} check is FALSE on a row and a conjunct
that comes *later* in the condition but does not reference that column raises
an error on the same row, codegen reaches the error that the interpreted path
never evaluates. The query fails only with whole-stage codegen on.
Reproducer (ANSI mode):
{code:sql}
CREATE TABLE t USING parquet AS
SELECT * FROM VALUES (NULL, 1, 2147483647, NULL), (0, 1, 0, 1) AS v(a, b, y,
x);
SET spark.sql.parquet.filterPushdown=false;
SET spark.sql.codegen.wholeStage=true;
SELECT a FROM t WHERE x IS NOT NULL AND y + 1 > 0 AND x > 0;
-- [ARITHMETIC_OVERFLOW] overflow. ... SQLSTATE: 22003
SET spark.sql.codegen.wholeStage=false;
SELECT a FROM t WHERE x IS NOT NULL AND y + 1 > 0 AND x > 0;
-- 0
{code}
The optimized condition is {{isnotnull(y) AND isnotnull(x) AND (y + 1) > 0 AND
x > 0}}. With codegen off, {{isnotnull(x)}} is FALSE on the first row and the
row is dropped. With codegen on, {{isnotnull(x)}} is emitted just before {{x >
0}}, after {{(y + 1) > 0}}, so the overflow is raised.
This is independent of SPARK-60315, which is about a NULL conjunct stopping the
evaluation with codegen on. The fix for SPARK-60315 keeps the lazy placement
when no conjunct can be NULL, and only preserves the position of the pending
checks once a NULL conjunct flag is generated, so this shape still differs
between the two modes after it.
A fix should emit each plain-column {{IsNotNull}} check at its position in the
condition, in both {{generatePredicateCode}} and the CSE branch of
{{FilterExec.doConsume}}, which mirror each other by hand. The lazy placement
exists so that columns are not loaded until a conjunct needs them, so the
performance effect of the change on filters with many inferred checks should be
measured. Complex {{IsNotNull(expr)}} checks, whose inner expression can itself
raise, need the same care.
> Whole-stage codegen of a filter checks a plain-column IS NOT NULL later than
> its position in the condition
> ----------------------------------------------------------------------------------------------------------
>
> Key: SPARK-60329
> URL: https://issues.apache.org/jira/browse/SPARK-60329
> Project: Spark
> Issue Type: Bug
> Components: SQL
> Affects Versions: 4.2.0, 4.3.0, 5.0.0, 4.1.3, 4.0.4, 4.4.0
> Reporter: Max Gekk
> Priority: Major
>
> With whole-stage codegen enabled, {{FilterExec}} (and the {{FILTER}} clause
> of an aggregate function, through
> {{GeneratePredicateHelper.generatePredicateCode}}) emits a plain-column
> {{IsNotNull}} check lazily: just before the first conjunct that references
> its column, or after all conjuncts if none does. With whole-stage codegen
> disabled, {{And.eval}} evaluates the conjuncts in the order of the condition,
> where {{InferFiltersFromConstraints}} puts the inferred {{isnotnull(...)}}
> checks first and a user-written {{IS NOT NULL}} stays where it was written.
> So when a plain-column {{IS NOT NULL}} check is FALSE on a row and a conjunct
> that comes *later* in the condition but does not reference that column raises
> an error on the same row, codegen reaches the error that the interpreted path
> never evaluates. The query fails only with whole-stage codegen on.
> Reproducer (ANSI mode):
> {code:sql}
> CREATE TABLE t USING parquet AS
> SELECT * FROM VALUES (NULL, 1, 2147483647, NULL), (0, 1, 0, 1) AS v(a, b,
> y, x);
> SET spark.sql.parquet.filterPushdown=false;
> SET spark.sql.codegen.wholeStage=true;
> SELECT a FROM t WHERE x IS NOT NULL AND y + 1 > 0 AND x > 0;
> -- [ARITHMETIC_OVERFLOW] overflow. ... SQLSTATE: 22003
> SET spark.sql.codegen.wholeStage=false;
> SELECT a FROM t WHERE x IS NOT NULL AND y + 1 > 0 AND x > 0;
> -- 0
> {code}
> The optimized condition is {{isnotnull(y) AND isnotnull(x) AND (y + 1) > 0
> AND x > 0}}. With codegen off, {{isnotnull(x)}} is FALSE on the first row and
> the row is dropped. With codegen on, {{isnotnull(x)}} is emitted just before
> {{x > 0}}, after {{(y + 1) > 0}}, so the overflow is raised.
> This is independent of SPARK-60315, which is about a NULL conjunct stopping
> the evaluation with codegen on. The fix for SPARK-60315 keeps the lazy
> placement when no conjunct can be NULL, and only preserves the position of
> the pending checks once a NULL conjunct flag is generated, so this shape
> still differs between the two modes after it.
> A fix should emit each plain-column {{IsNotNull}} check at its position in
> the condition, in both {{generatePredicateCode}} and the CSE branch of
> {{FilterExec.doConsume}}, which mirror each other by hand. The lazy placement
> exists so that columns are not loaded until a conjunct needs them, so the
> performance effect of the change on filters with many inferred checks should
> be measured. Complex {{IsNotNull(expr)}} checks, whose inner expression can
> itself raise, need the same care.
> A small cleanup can ride along with this fix. In both code paths, when a
> conjunct references a column whose direct {{IsNotNull}} check was already
> generated, the {{else if}} branch of the reference loop (it is not guarded by
> {{idx == -1}}) generates a redundant synthetic {{IsNotNull(r)}} for the same
> column, once per column. It is a harmless duplicate null check, pre-existing,
> and after SPARK-60315 it is routine for the first conjunct that references a
> column whose check was generated before it. The fix is to guard the synthetic
> check with {{idx == -1}}; it is left out of SPARK-60315 because it changes
> the generated code when no conjunct can be NULL.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
