Skip to content

Commit 08a90d5

Browse files
ggevayclaude
andcommitted
repr: follow-ups for float canonicalization
Make the remaining sign-of-zero-sensitive scalar functions treat -0.0 as +0.0, so that results do not depend on whether a value crossed a packing boundary (e.g. a view vs a materialized view of the same query): cot(-0.0) now returns +Infinity (PostgreSQL returns -Infinity here), power no longer errors for a -0.0 base with a fractional exponent or a -0.0 exponent with a zero base, and ln/log10 (float and numeric) report the zero error rather than the negative error for -0.0. The ln/log10/power changes also match PostgreSQL, which uses zero-first checks and strict comparisons, and they let NaN inputs propagate to NaN results regardless of the NaN's sign bit. Regenerate the SourceData serialization stability snapshot: old encodings containing non-canonical floats now decode to canonical rows, so re-encoding them produces different bytes. Update the stale comment that pointed at MINIMUM_CONSOLIDATED_VERSION, which has not been consulted since codec-order consolidation was removed. Add platform checks covering the cross-version story for tables, Postgres sources, and Kafka upsert sources: rows written by an old version with raw float bits must cancel against retractions written by a new version, and must land in the same DISTINCT/GROUP BY/index groups. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent f13fd0c commit 08a90d5

7 files changed

Lines changed: 300 additions & 17 deletions

File tree

Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,220 @@
1+
# Copyright Materialize, Inc. and contributors. All rights reserved.
2+
#
3+
# Use of this software is governed by the Business Source License
4+
# included in the LICENSE file at the root of this repository.
5+
#
6+
# As of the Change Date specified in that file, in accordance with
7+
# the Business Source License, use of this software will be governed
8+
# by the Apache License, Version 2.0.
9+
from textwrap import dedent
10+
11+
from materialize.checks.actions import Testdrive
12+
from materialize.checks.checks import Check, externally_idempotent
13+
14+
# Row packing canonicalizes floats (-0.0 packs as +0.0, every NaN packs as one
15+
# bit pattern) so that byte equality of packed rows agrees with SQL float
16+
# equality. Data written by versions without that canonicalization carries the
17+
# raw bit patterns, so these checks exercise the cross-version story: rows
18+
# written by an older version must cancel against retractions written by a
19+
# newer one, and must land in the same DISTINCT/GROUP BY/index groups.
20+
#
21+
# In multi-version scenarios validate() also runs on versions without the
22+
# canonicalization, where -0.0 is a distinct arrangement key, so the
23+
# assertions that depend on it are gated on version 26.33 (which is when the
24+
# canonicalization was introduced). Row counts and upsert results hold on all
25+
# versions and are asserted unconditionally.
26+
#
27+
# NOTE: a genuine -0.0 needs a text->float cast ('-0'), a -0.0 literal goes
28+
# through numeric (no signed zero) and arrives as +0.0.
29+
30+
31+
class FloatCanonicalizationTable(Check):
32+
"""-0.0 and NaN in a table across versions: retraction of old-encoding
33+
rows, DISTINCT/GROUP BY collapse, and index point lookups."""
34+
35+
def initialize(self) -> Testdrive:
36+
return Testdrive(dedent("""
37+
> CREATE TABLE float_canon_table (id INT, f DOUBLE PRECISION);
38+
> INSERT INTO float_canon_table VALUES
39+
(1, '-0'), (2, '0'), (3, 'NaN'), (4, '-0'), (5, 1.5);
40+
"""))
41+
42+
def manipulate(self) -> list[Testdrive]:
43+
return [
44+
Testdrive(dedent(s))
45+
for s in [
46+
"""
47+
> CREATE MATERIALIZED VIEW float_canon_table_mv AS
48+
SELECT f, COUNT(*) AS c FROM float_canon_table GROUP BY f;
49+
> CREATE DEFAULT INDEX ON float_canon_table;
50+
> INSERT INTO float_canon_table VALUES (6, '-0');
51+
""",
52+
"""
53+
> DELETE FROM float_canon_table WHERE id = 4;
54+
> INSERT INTO float_canon_table VALUES (7, '0'), (8, 'NaN');
55+
""",
56+
]
57+
]
58+
59+
def validate(self) -> Testdrive:
60+
return Testdrive(dedent("""
61+
> SELECT count(*) FROM float_canon_table;
62+
7
63+
64+
>[version>=2603300] SELECT count(*) FROM (SELECT DISTINCT f FROM float_canon_table);
65+
3
66+
67+
>[version>=2603300] SELECT f::text, c FROM float_canon_table_mv;
68+
0 4
69+
1.5 1
70+
NaN 2
71+
72+
>[version>=2603300] SELECT id FROM float_canon_table WHERE f = 0;
73+
1
74+
2
75+
6
76+
7
77+
78+
>[version>=2603300] SELECT id FROM float_canon_table WHERE f = 'NaN';
79+
3
80+
8
81+
"""))
82+
83+
84+
@externally_idempotent(False)
85+
class FloatCanonicalizationPgCdc(Check):
86+
"""-0.0 and NaN ingested from a Postgres source across versions: an
87+
upstream DELETE/UPDATE after an upgrade must retract rows whose additions
88+
were written with the old float encoding."""
89+
90+
def initialize(self) -> Testdrive:
91+
return Testdrive(dedent("""
92+
$ postgres-execute connection=postgres://postgres:postgres@postgres
93+
CREATE USER postgres_float_canon WITH SUPERUSER PASSWORD 'postgres';
94+
ALTER USER postgres_float_canon WITH replication;
95+
DROP PUBLICATION IF EXISTS float_canon_publication;
96+
DROP TABLE IF EXISTS float_canon_pg_table;
97+
CREATE TABLE float_canon_pg_table (id INT PRIMARY KEY, f DOUBLE PRECISION);
98+
ALTER TABLE float_canon_pg_table REPLICA IDENTITY FULL;
99+
INSERT INTO float_canon_pg_table VALUES (1, '-0'), (2, '0'), (3, 'NaN'), (4, '-0');
100+
CREATE PUBLICATION float_canon_publication FOR ALL TABLES;
101+
102+
> CREATE SECRET float_canon_pgpass AS 'postgres';
103+
104+
> CREATE CONNECTION float_canon_pg_conn FOR POSTGRES
105+
HOST 'postgres',
106+
DATABASE postgres,
107+
USER postgres_float_canon,
108+
PASSWORD SECRET float_canon_pgpass;
109+
110+
> CREATE SOURCE float_canon_pg_source
111+
FROM POSTGRES CONNECTION float_canon_pg_conn
112+
(PUBLICATION 'float_canon_publication');
113+
> CREATE TABLE float_canon_pg FROM SOURCE float_canon_pg_source
114+
(REFERENCE float_canon_pg_table);
115+
116+
# Wait for the snapshot so the initial rows are ingested (and
117+
# thus encoded) by the version running this phase.
118+
> SELECT count(*) FROM float_canon_pg;
119+
4
120+
"""))
121+
122+
def manipulate(self) -> list[Testdrive]:
123+
return [
124+
Testdrive(dedent(s))
125+
for s in [
126+
"""
127+
$ postgres-execute connection=postgres://postgres:postgres@postgres
128+
INSERT INTO float_canon_pg_table VALUES (5, '-0'), (6, 'NaN');
129+
""",
130+
"""
131+
$ postgres-execute connection=postgres://postgres:postgres@postgres
132+
DELETE FROM float_canon_pg_table WHERE id IN (1, 6);
133+
UPDATE float_canon_pg_table SET f = '0' WHERE id = 4;
134+
""",
135+
]
136+
]
137+
138+
def validate(self) -> Testdrive:
139+
return Testdrive(dedent("""
140+
> SELECT count(*) FROM float_canon_pg;
141+
4
142+
143+
>[version>=2603300] SELECT count(*) FROM (SELECT DISTINCT f FROM float_canon_pg);
144+
2
145+
146+
>[version>=2603300] SELECT id FROM float_canon_pg WHERE f = 0;
147+
2
148+
4
149+
5
150+
151+
>[version>=2603300] SELECT id FROM float_canon_pg WHERE f = 'NaN';
152+
3
153+
"""))
154+
155+
156+
def float_canon_schemas() -> str:
157+
return dedent("""
158+
$ set float-canon-keyschema={
159+
"type": "record",
160+
"name": "Key",
161+
"fields": [ {"name": "key1", "type": "double"} ]
162+
}
163+
164+
$ set float-canon-schema={
165+
"type" : "record",
166+
"name" : "test",
167+
"fields" : [ {"name": "f1", "type": "double"} ]
168+
}
169+
""")
170+
171+
172+
class FloatCanonicalizationUpsert(Check):
173+
"""-0.0 in a Kafka upsert source's key and value across versions: a -0.0
174+
and a +0.0 key are the same key, and a post-upgrade tombstone must retract
175+
a value row written with the old float encoding."""
176+
177+
def initialize(self) -> Testdrive:
178+
return Testdrive(float_canon_schemas() + dedent("""
179+
$ kafka-create-topic topic=float-canon-upsert
180+
181+
$ kafka-ingest format=avro key-format=avro topic=float-canon-upsert key-schema=${float-canon-keyschema} schema=${float-canon-schema}
182+
{"key1": -0.0} {"f1": 1.0}
183+
{"key1": 2.0} {"f1": -0.0}
184+
185+
> CREATE SOURCE float_canon_upsert_src
186+
FROM KAFKA CONNECTION kafka_conn (TOPIC 'testdrive-float-canon-upsert-${testdrive.seed}')
187+
> CREATE TABLE float_canon_upsert FROM SOURCE float_canon_upsert_src (REFERENCE "testdrive-float-canon-upsert-${testdrive.seed}")
188+
FORMAT AVRO USING CONFLUENT SCHEMA REGISTRY CONNECTION csr_conn
189+
ENVELOPE UPSERT
190+
191+
# Wait for the snapshot so the initial rows are ingested (and
192+
# thus encoded) by the version running this phase.
193+
> SELECT count(*) FROM float_canon_upsert;
194+
2
195+
"""))
196+
197+
def manipulate(self) -> list[Testdrive]:
198+
return [
199+
Testdrive(float_canon_schemas() + dedent(s))
200+
for s in [
201+
"""
202+
# The +0.0 key is the same key as the -0.0 key, so this
203+
# replaces the (0, 1) row rather than adding a third row.
204+
$ kafka-ingest format=avro key-format=avro topic=float-canon-upsert key-schema=${float-canon-keyschema} schema=${float-canon-schema}
205+
{"key1": 0.0} {"f1": 3.0}
206+
""",
207+
"""
208+
# Tombstone the key whose value row (f1 = -0.0) may have been
209+
# written with the old float encoding.
210+
$ kafka-ingest format=avro key-format=avro topic=float-canon-upsert key-schema=${float-canon-keyschema} schema=${float-canon-schema}
211+
{"key1": 2.0}
212+
""",
213+
]
214+
]
215+
216+
def validate(self) -> Testdrive:
217+
return Testdrive(dedent("""
218+
> SELECT key1::text, f1::text FROM float_canon_upsert;
219+
0 3
220+
"""))

‎src/expr/src/scalar/func.rs‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1305,12 +1305,14 @@ fn neg_interval_inner(a: Interval) -> Result<Interval, EvalError> {
13051305
}
13061306

13071307
fn log_guard_numeric(val: &Numeric, function_name: &str) -> Result<(), EvalError> {
1308-
if val.is_negative() {
1309-
return Err(EvalError::NegativeOutOfDomain(function_name.into()));
1310-
}
1308+
// Check zero before the sign, like PostgreSQL, so that a negative zero
1309+
// (which the dec crate considers negative) reports the same error as +0.
13111310
if val.is_zero() {
13121311
return Err(EvalError::ZeroOutOfDomain(function_name.into()));
13131312
}
1313+
if val.is_negative() {
1314+
return Err(EvalError::NegativeOutOfDomain(function_name.into()));
1315+
}
13141316
Ok(())
13151317
}
13161318

@@ -1353,12 +1355,17 @@ fn log_base_numeric(mut a: Numeric, mut b: Numeric) -> Result<Numeric, EvalError
13531355

13541356
#[sqlfunc(propagates_nulls = true)]
13551357
fn power(a: f64, b: f64) -> Result<f64, EvalError> {
1356-
if a == 0.0 && b.is_sign_negative() {
1358+
// Strict less-than, so that a -0.0 exponent counts as zero (x^0 = 1)
1359+
// rather than as negative, like PostgreSQL.
1360+
if a == 0.0 && b < 0.0 {
13571361
return Err(EvalError::Undefined(
13581362
"zero raised to a negative power".into(),
13591363
));
13601364
}
1361-
if a.is_sign_negative() && b.fract() != 0.0 {
1365+
// Strict less-than, like PostgreSQL, so that -0.0 does not count as
1366+
// negative (row packing canonicalizes -0.0 to +0.0, so the sign of a zero
1367+
// must not be observable) and NaN propagates to a NaN result.
1368+
if a < 0.0 && b.fract() != 0.0 {
13621369
// Equivalent to PG error:
13631370
// > a negative number raised to a non-integer power yields a complex result
13641371
return Err(EvalError::ComplexOutOfRange("pow".into()));

‎src/expr/src/scalar/func/impls/float64.rs‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,12 @@ fn cot(a: f64) -> Result<f64, EvalError> {
371371
if a.is_infinite() {
372372
return Err(EvalError::InfinityOutOfDomain("cot".into()));
373373
}
374+
// -0.0 behaves as +0.0, so cot(-0.0) is +Infinity rather than
375+
// PostgreSQL's -Infinity. Row packing canonicalizes -0.0 to +0.0, so
376+
// honoring the sign here would make the result depend on whether the
377+
// input crossed a packing boundary (e.g. a view vs a materialized view
378+
// of the same query).
379+
let a = if a == 0.0 { 0.0 } else { a };
374380
Ok(1.0 / a.tan())
375381
}
376382

@@ -384,25 +390,30 @@ fn degrees(a: f64) -> f64 {
384390
a.to_degrees()
385391
}
386392

393+
// The guards in `log10` and `ln` use `== 0.0` and `< 0.0` rather than the
394+
// sign bit, so that -0.0 errors like +0.0 and NaN propagates to a NaN result,
395+
// like PostgreSQL. The sign of a zero must not be observable, since row
396+
// packing canonicalizes -0.0 to +0.0.
397+
387398
#[sqlfunc(sqlname = "log10f64")]
388399
fn log10(a: f64) -> Result<f64, EvalError> {
389-
if a.is_sign_negative() {
390-
return Err(EvalError::NegativeOutOfDomain("log10".into()));
391-
}
392400
if a == 0.0 {
393401
return Err(EvalError::ZeroOutOfDomain("log10".into()));
394402
}
403+
if a < 0.0 {
404+
return Err(EvalError::NegativeOutOfDomain("log10".into()));
405+
}
395406
Ok(a.log10())
396407
}
397408

398409
#[sqlfunc(sqlname = "lnf64")]
399410
fn ln(a: f64) -> Result<f64, EvalError> {
400-
if a.is_sign_negative() {
401-
return Err(EvalError::NegativeOutOfDomain("ln".into()));
402-
}
403411
if a == 0.0 {
404412
return Err(EvalError::ZeroOutOfDomain("ln".into()));
405413
}
414+
if a < 0.0 {
415+
return Err(EvalError::NegativeOutOfDomain("ln".into()));
416+
}
406417
Ok(a.ln())
407418
}
408419

‎src/expr/src/scalar/func/impls/numeric.rs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,14 @@ fn floor_numeric(mut a: Numeric) -> Numeric {
7979
}
8080

8181
fn log_guard_numeric(val: &Numeric, function_name: &str) -> Result<(), EvalError> {
82-
if val.is_negative() {
83-
return Err(EvalError::NegativeOutOfDomain(function_name.into()));
84-
}
82+
// Check zero before the sign, like PostgreSQL, so that a negative zero
83+
// (which the dec crate considers negative) reports the same error as +0.
8584
if val.is_zero() {
8685
return Err(EvalError::ZeroOutOfDomain(function_name.into()));
8786
}
87+
if val.is_negative() {
88+
return Err(EvalError::NegativeOutOfDomain(function_name.into()));
89+
}
8890
Ok(())
8991
}
9092

‎src/storage-types/src/snapshots/source-datas.txt‎

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

‎src/storage-types/src/sources.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2183,7 +2183,11 @@ mod tests {
21832183
// If you need to change how SourceDatas are encoded, that can be
21842184
// okay, but think through the consequences: a record whose old and
21852185
// new encodings differ never consolidates away inside existing
2186-
// persist shards. Loop in the persist team.
2186+
// persist shards, so an addition written by an old version and its
2187+
// retraction written by a new version both stay in the shard
2188+
// forever. Readers stay correct because they consolidate rows after
2189+
// decoding, where the two encodings become identical, but every
2190+
// reader must tolerate such pairs. Loop in the persist team.
21872191
assert_eq!(
21882192
encoded,
21892193
reencoded.as_str(),

‎test/sqllogictest/funcs.slt‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1240,10 +1240,13 @@ SELECT cot(0::double)
12401240
----
12411241
inf
12421242

1243+
# cot treats -0.0 as +0.0 so that the result does not depend on whether the
1244+
# input crossed a packing boundary (packing canonicalizes -0.0 to +0.0).
1245+
# PostgreSQL returns -Infinity here.
12431246
query R
12441247
SELECT cot(-0::double)
12451248
----
1246-
-inf
1249+
inf
12471250

12481251
query R
12491252
SELECT sin(1::double)
@@ -1477,6 +1480,42 @@ SELECT ln(-1)
14771480
query error function ln is not defined for zero
14781481
SELECT ln(0)
14791482

1483+
# A float -0.0 input behaves exactly like +0.0 in ln, log10, and power, so
1484+
# that results (and error messages) do not depend on whether the input
1485+
# crossed a packing boundary (packing canonicalizes -0.0 to +0.0).
1486+
1487+
query error function ln is not defined for zero
1488+
SELECT ln('-0'::double)
1489+
1490+
query error function log10 is not defined for zero
1491+
SELECT log10('-0'::double)
1492+
1493+
query R
1494+
SELECT power('-0'::double, 0.5)
1495+
----
1496+
0
1497+
1498+
query R
1499+
SELECT power(0::double, '-0'::double)
1500+
----
1501+
1
1502+
1503+
# NaN inputs propagate in ln and power regardless of the NaN's sign bit,
1504+
# matching PostgreSQL.
1505+
query R
1506+
SELECT ln('-NaN'::double)
1507+
----
1508+
NaN
1509+
1510+
query R
1511+
SELECT power('-NaN'::double, 0.5)
1512+
----
1513+
NaN
1514+
1515+
# A numeric negative zero (reachable via rounding) also errors like zero.
1516+
query error function log10 is not defined for zero
1517+
SELECT log10((-0.1)::decimal(10,0))
1518+
14801519
query R
14811520
SELECT ln(13.0000::decimal(15, 5))
14821521
----

0 commit comments

Comments
 (0)