Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
12 changes: 7 additions & 5 deletions docs/source/contributor-guide/floating_point.md
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,13 @@ and only for a type that contains a `FLOAT` or `DOUBLE`.
`MaxSparkVersion`, and add the `run-all-spark-profiles` label to the pull request. CI pins one
patch release per Spark line, so when a patch release changed the rule, also test the routing in
Scala.
- Add the expression to `CometFloatSemanticsSuite`, which crosses the edge values with operator
contexts and with every expression and aggregate that accepts floating-point input. New
expressions go in its `expressions` or `aggregates` list. If one does not match Spark yet, add a
`KnownGap` that links the issue. Known gaps are strict: a case that starts matching Spark fails,
so the fix also removes the entry.
- Add the expression to the sweep in `float_semantics/`, which crosses the edge values with
operator contexts and with every expression and aggregate that accepts floating-point input.
A new expression goes in `expressions.sql`, or in `aggregates.sql` for an aggregate, with one
query for `DOUBLE` and one for `FLOAT`. If one does not match Spark yet, mark its queries
`ignore(<issue>)`. A SQL file test cannot expect a difference, so a gap that should fail once it
is fixed goes in `CometFloatSemanticsSuite`, whose test for `collect_set` before Spark 4.2 fails
when the answers start to match.
Comment on lines +207 to +210

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could a known mismatch go in a strict CometFloatSemanticsSuite test instead of ignore, so the test fails once it's fixed?

Suggested change
query for `DOUBLE` and one for `FLOAT`. If one does not match Spark yet, mark its queries
`ignore(<issue>)`. A SQL file test cannot expect a difference, so a gap that should fail once it
is fixed goes in `CometFloatSemanticsSuite`, whose test for `collect_set` before Spark 4.2 fails
when the answers start to match.
query for `DOUBLE` and one for `FLOAT`. If one does not match Spark yet, add a strict test in
`CometFloatSemanticsSuite` that links the issue and fails once the answers match, like its
`collect_set` test before Spark 4.2.


Fixtures to start from include `expressions/conditional/float_comparisons.sql`,
`expressions/math/nan_divisor.sql`, `expressions/math/greatest_least_floating_point.sql`,
Expand Down
193 changes: 193 additions & 0 deletions spark/src/test/resources/sql-tests/float_semantics/aggregates.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
-- Licensed to the Apache Software Foundation (ASF) under one
-- or more contributor license agreements. See the NOTICE file
-- distributed with this work for additional information
-- regarding copyright ownership. The ASF licenses this file
-- to you under the Apache License, Version 2.0 (the
-- "License"); you may not use this file except in compliance
-- with the License. You may obtain a copy of the License at
--
-- http://www.apache.org/licenses/LICENSE-2.0
--
-- Unless required by applicable law or agreed to in writing,
-- software distributed under the License is distributed on an
-- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-- KIND, either express or implied. See the License for the
-- specific language governing permissions and limitations
-- under the License.

-- Aggregate functions over float input. Each pair of edge values is expanded from one
-- input row, so its two values reach the same partial aggregate in array order. Window
-- tests also order by pos to preserve signed-zero ties after a shuffle. mode gets each pair
-- as (l, r, r), so the mode is unique under every Spark equality rule.
--
-- The e and p CTEs negate the flagged stored values to make the sign-bit NaN at query time
-- (see comparison.sql). The sweep tracks #6385.
--
-- collect_set is in aggregates_collect_set_4_2.sql.


statement
CREATE TABLE fs_edge_double(id INT, c DOUBLE, neg BOOLEAN) USING parquet

statement
INSERT INTO fs_edge_double VALUES
(1, 0.0D, false),
(2, double('-0.0'), false),
(3, double('NaN'), false),
(4, double('NaN'), true),
(5, 1.0D, false),
(6, -1.0D, false),
(7, double('Infinity'), false),
(8, double('-Infinity'), false),
(9, NULL, false)

statement
CREATE TABLE fs_pair_double USING parquet AS
SELECT a.id * 10 + b.id AS id, a.c AS l, b.c AS r, a.neg AS lneg, b.neg AS rneg
FROM fs_edge_double a CROSS JOIN fs_edge_double b

statement
CREATE TABLE fs_edge_float(id INT, c FLOAT, neg BOOLEAN) USING parquet

statement
INSERT INTO fs_edge_float VALUES
(1, float(0.0), false),
(2, float('-0.0'), false),
(3, float('NaN'), false),
(4, float('NaN'), true),
(5, float(1.0), false),
(6, float(-1.0), false),
(7, float('Infinity'), false),
(8, float('-Infinity'), false),
(9, NULL, false)

statement
CREATE TABLE fs_pair_float USING parquet AS
SELECT a.id * 10 + b.id AS id, a.c AS l, b.c AS r, a.neg AS lneg, b.neg AS rneg
FROM fs_edge_float a CROSS JOIN fs_edge_float b

-- max, DOUBLE
query
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_double)
SELECT id, max(x) FROM (SELECT id, pos, x FROM p LATERAL VIEW posexplode(array(l, r)) t AS pos, x) GROUP BY id

-- max, FLOAT
query
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_float)
SELECT id, max(x) FROM (SELECT id, pos, x FROM p LATERAL VIEW posexplode(array(l, r)) t AS pos, x) GROUP BY id

-- min, DOUBLE
query
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_double)
SELECT id, min(x) FROM (SELECT id, pos, x FROM p LATERAL VIEW posexplode(array(l, r)) t AS pos, x) GROUP BY id

-- min, FLOAT
query
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_float)
SELECT id, min(x) FROM (SELECT id, pos, x FROM p LATERAL VIEW posexplode(array(l, r)) t AS pos, x) GROUP BY id

-- max_by, DOUBLE
query
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_double)
SELECT id, max_by(pos, x) FROM (SELECT id, pos, x FROM p LATERAL VIEW posexplode(array(l, r)) t AS pos, x) GROUP BY id

-- max_by, FLOAT
query
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_float)
SELECT id, max_by(pos, x) FROM (SELECT id, pos, x FROM p LATERAL VIEW posexplode(array(l, r)) t AS pos, x) GROUP BY id

-- min_by, DOUBLE
query
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_double)
SELECT id, min_by(pos, x) FROM (SELECT id, pos, x FROM p LATERAL VIEW posexplode(array(l, r)) t AS pos, x) GROUP BY id

-- min_by, FLOAT
query
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_float)
SELECT id, min_by(pos, x) FROM (SELECT id, pos, x FROM p LATERAL VIEW posexplode(array(l, r)) t AS pos, x) GROUP BY id

-- sum, DOUBLE
query
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_double)
SELECT id, sum(x) FROM (SELECT id, pos, x FROM p LATERAL VIEW posexplode(array(l, r)) t AS pos, x) GROUP BY id

-- sum, FLOAT
query
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_float)
SELECT id, sum(x) FROM (SELECT id, pos, x FROM p LATERAL VIEW posexplode(array(l, r)) t AS pos, x) GROUP BY id

-- avg, DOUBLE
query
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_double)
SELECT id, avg(x) FROM (SELECT id, pos, x FROM p LATERAL VIEW posexplode(array(l, r)) t AS pos, x) GROUP BY id

-- avg, FLOAT
query
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_float)
SELECT id, avg(x) FROM (SELECT id, pos, x FROM p LATERAL VIEW posexplode(array(l, r)) t AS pos, x) GROUP BY id

-- count(DISTINCT), DOUBLE
query
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_double)
SELECT id, count(DISTINCT x) FROM (SELECT id, pos, x FROM p LATERAL VIEW posexplode(array(l, r)) t AS pos, x) GROUP BY id

-- count(DISTINCT), FLOAT
query
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_float)
SELECT id, count(DISTINCT x) FROM (SELECT id, pos, x FROM p LATERAL VIEW posexplode(array(l, r)) t AS pos, x) GROUP BY id

-- approx_count_distinct, DOUBLE
query
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_double)
SELECT id, approx_count_distinct(x) FROM (SELECT id, pos, x FROM p LATERAL VIEW posexplode(array(l, r)) t AS pos, x) GROUP BY id

-- approx_count_distinct, FLOAT
query
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_float)
SELECT id, approx_count_distinct(x) FROM (SELECT id, pos, x FROM p LATERAL VIEW posexplode(array(l, r)) t AS pos, x) GROUP BY id

-- mode, DOUBLE
-- mode falls back to Spark unless Mode.allowIncompatible is set, so only the answer is checked
-- here. expressions/aggregate/mode.sql runs it natively.
query spark_answer_only
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_double)
SELECT id, mode(x) FROM (SELECT id, x FROM p LATERAL VIEW explode(array(l, r, r)) t AS x) GROUP BY id

-- mode, FLOAT
query spark_answer_only
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_float)
SELECT id, mode(x) FROM (SELECT id, x FROM p LATERAL VIEW explode(array(l, r, r)) t AS x) GROUP BY id
Comment on lines +149 to +159

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These fall back in the default config, so they compare Spark with Spark. The (l, r, r) triples give a unique winner per pair, so they look suited to a file with -- Config: spark.comet.expression.Mode.allowIncompatible=true, which mode.sql only exercises for DOUBLE with a canonical NaN. #7115 makes float mode fall back before Spark 3.5.2, so that file would likely need MinSparkVersion: 3.5. I have not run it.


-- global max and min, DOUBLE
query
WITH e AS (SELECT id, IF(neg, -c, c) AS c FROM fs_edge_double)
SELECT max(c), min(c) FROM e

-- global max and min, FLOAT
query
WITH e AS (SELECT id, IF(neg, -c, c) AS c FROM fs_edge_float)
SELECT max(c), min(c) FROM e

-- global max_by and min_by, DOUBLE
query
WITH e AS (SELECT id, IF(neg, -c, c) AS c FROM fs_edge_double)
SELECT max_by(id, c), min_by(id, c) FROM e

-- global max_by and min_by, FLOAT
query
WITH e AS (SELECT id, IF(neg, -c, c) AS c FROM fs_edge_float)
SELECT max_by(id, c), min_by(id, c) FROM e

-- window max and min, DOUBLE
query
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_double)
SELECT id, pos, max(x) OVER w, min(x) OVER w FROM (SELECT id, pos, x FROM p LATERAL VIEW posexplode(array(l, r)) t AS pos, x)
WINDOW w AS (PARTITION BY id ORDER BY pos
ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING)

-- window max and min, FLOAT
query
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_float)
SELECT id, pos, max(x) OVER w, min(x) OVER w FROM (SELECT id, pos, x FROM p LATERAL VIEW posexplode(array(l, r)) t AS pos, x)
WINDOW w AS (PARTITION BY id ORDER BY pos
ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING)
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
-- Licensed to the Apache Software Foundation (ASF) under one
-- or more contributor license agreements. See the NOTICE file
-- distributed with this work for additional information
-- regarding copyright ownership. The ASF licenses this file
-- to you under the Apache License, Version 2.0 (the
-- "License"); you may not use this file except in compliance
-- with the License. You may obtain a copy of the License at
--
-- http://www.apache.org/licenses/LICENSE-2.0
--
-- Unless required by applicable law or agreed to in writing,
-- software distributed under the License is distributed on an
-- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-- KIND, either express or implied. See the License for the
-- specific language governing permissions and limitations
-- under the License.

-- collect_set over float input, from Spark 4.2 on. Spark 4.2 normalizes the input of
-- collect_set: -0.0 and 0.0 are one value, and so are all NaNs. Before 4.2
-- Spark's collect_set treats -0.0 and 0.0 as one value and NaNs as distinct, which Comet does
-- not match (#5312). CometFloatSemanticsSuite keeps those versions as a known gap, so that
-- it starts failing when a fix lands.
--
-- Each pair of edge values is expanded from one input row (see aggregates.sql).

-- MinSparkVersion: 4.2

statement
CREATE TABLE fs_edge_double(id INT, c DOUBLE, neg BOOLEAN) USING parquet

statement
INSERT INTO fs_edge_double VALUES
(1, 0.0D, false),
(2, double('-0.0'), false),
(3, double('NaN'), false),
(4, double('NaN'), true),
(5, 1.0D, false),
(6, -1.0D, false),
(7, double('Infinity'), false),
(8, double('-Infinity'), false),
(9, NULL, false)

statement
CREATE TABLE fs_pair_double USING parquet AS
SELECT a.id * 10 + b.id AS id, a.c AS l, b.c AS r, a.neg AS lneg, b.neg AS rneg
FROM fs_edge_double a CROSS JOIN fs_edge_double b

statement
CREATE TABLE fs_edge_float(id INT, c FLOAT, neg BOOLEAN) USING parquet

statement
INSERT INTO fs_edge_float VALUES
(1, float(0.0), false),
(2, float('-0.0'), false),
(3, float('NaN'), false),
(4, float('NaN'), true),
(5, float(1.0), false),
(6, float(-1.0), false),
(7, float('Infinity'), false),
(8, float('-Infinity'), false),
(9, NULL, false)

statement
CREATE TABLE fs_pair_float USING parquet AS
SELECT a.id * 10 + b.id AS id, a.c AS l, b.c AS r, a.neg AS lneg, b.neg AS rneg
FROM fs_edge_float a CROSS JOIN fs_edge_float b

-- DOUBLE
query
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_double)
SELECT id, size(collect_set(x)) FROM (SELECT id, x FROM p LATERAL VIEW explode(array(l, r)) t AS x) GROUP BY id

-- FLOAT
query
WITH p AS (SELECT id, IF(lneg, -l, l) AS l, IF(rneg, -r, r) AS r FROM fs_pair_float)
SELECT id, size(collect_set(x)) FROM (SELECT id, x FROM p LATERAL VIEW explode(array(l, r)) t AS x) GROUP BY id
Loading
Loading