Skip to content

Commit 9833ce1

Browse files
ggevayclaude
ggevay
and
claude
committed
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 9833ce1

7 files changed

Lines changed: 321 additions & 17 deletions

File tree

‎misc/python/materialize/checks/all_checks/float_canonicalization.py‎

Lines changed: 241 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,241 @@
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+
# NOTE: a genuine -0.0 needs a text->float cast ('-0'), a -0.0 literal goes
22+
# through numeric (no signed zero) and arrives as +0.0.
23+
24+
25+
class FloatCanonicalizationTable(Check):
26+
"""-0.0 and NaN in a table across versions: retraction of old-encoding
27+
rows, DISTINCT/GROUP BY collapse, and index point lookups."""
28+
29+
def initialize(self) -> Testdrive:
30+
return Testdrive(
31+
dedent(
32+
"""
33+
> CREATE TABLE float_canon_table (id INT, f DOUBLE PRECISION);
34+
> INSERT INTO float_canon_table VALUES
35+
(1, '-0'), (2, '0'), (3, 'NaN'), (4, '-0'), (5, 1.5);
36+
"""
37+
)
38+
)
39+
40+
def manipulate(self) -> list[Testdrive]:
41+
return [
42+
Testdrive(dedent(s))
43+
for s in [
44+
"""
45+
> CREATE MATERIALIZED VIEW float_canon_table_mv AS
46+
SELECT f, COUNT(*) AS c FROM float_canon_table GROUP BY f;
47+
> CREATE DEFAULT INDEX ON float_canon_table;
48+
> INSERT INTO float_canon_table VALUES (6, '-0');
49+
""",
50+
"""
51+
> DELETE FROM float_canon_table WHERE id = 4;
52+
> INSERT INTO float_canon_table VALUES (7, '0'), (8, 'NaN');
53+
""",
54+
]
55+
]
56+
57+
def validate(self) -> Testdrive:
58+
return Testdrive(
59+
dedent(
60+
"""
61+
> SELECT count(*) FROM float_canon_table;
62+
7
63+
64+
> SELECT count(*) FROM (SELECT DISTINCT f FROM float_canon_table);
65+
3
66+
67+
> SELECT f::text, c FROM float_canon_table_mv;
68+
0 4
69+
1.5 1
70+
NaN 2
71+
72+
> SELECT id FROM float_canon_table WHERE f = 0;
73+
1
74+
2
75+
6
76+
7
77+
78+
> SELECT id FROM float_canon_table WHERE f = 'NaN';
79+
3
80+
8
81+
"""
82+
)
83+
)
84+
85+
86+
@externally_idempotent(False)
87+
class FloatCanonicalizationPgCdc(Check):
88+
"""-0.0 and NaN ingested from a Postgres source across versions: an
89+
upstream DELETE/UPDATE after an upgrade must retract rows whose additions
90+
were written with the old float encoding."""
91+
92+
def initialize(self) -> Testdrive:
93+
return Testdrive(
94+
dedent(
95+
"""
96+
$ postgres-execute connection=postgres://postgres:postgres@postgres
97+
CREATE USER postgres_float_canon WITH SUPERUSER PASSWORD 'postgres';
98+
ALTER USER postgres_float_canon WITH replication;
99+
DROP PUBLICATION IF EXISTS float_canon_publication;
100+
DROP TABLE IF EXISTS float_canon_pg_table;
101+
CREATE TABLE float_canon_pg_table (id INT PRIMARY KEY, f DOUBLE PRECISION);
102+
ALTER TABLE float_canon_pg_table REPLICA IDENTITY FULL;
103+
INSERT INTO float_canon_pg_table VALUES (1, '-0'), (2, '0'), (3, 'NaN'), (4, '-0');
104+
CREATE PUBLICATION float_canon_publication FOR ALL TABLES;
105+
106+
> CREATE SECRET float_canon_pgpass AS 'postgres';
107+
108+
> CREATE CONNECTION float_canon_pg_conn FOR POSTGRES
109+
HOST 'postgres',
110+
DATABASE postgres,
111+
USER postgres_float_canon,
112+
PASSWORD SECRET float_canon_pgpass;
113+
114+
> CREATE SOURCE float_canon_pg_source
115+
FROM POSTGRES CONNECTION float_canon_pg_conn
116+
(PUBLICATION 'float_canon_publication');
117+
> CREATE TABLE float_canon_pg FROM SOURCE float_canon_pg_source
118+
(REFERENCE float_canon_pg_table);
119+
120+
# Wait for the snapshot so the initial rows are ingested (and
121+
# thus encoded) by the version running this phase.
122+
> SELECT count(*) FROM float_canon_pg;
123+
4
124+
"""
125+
)
126+
)
127+
128+
def manipulate(self) -> list[Testdrive]:
129+
return [
130+
Testdrive(dedent(s))
131+
for s in [
132+
"""
133+
$ postgres-execute connection=postgres://postgres:postgres@postgres
134+
INSERT INTO float_canon_pg_table VALUES (5, '-0'), (6, 'NaN');
135+
""",
136+
"""
137+
$ postgres-execute connection=postgres://postgres:postgres@postgres
138+
DELETE FROM float_canon_pg_table WHERE id IN (1, 6);
139+
UPDATE float_canon_pg_table SET f = '0' WHERE id = 4;
140+
""",
141+
]
142+
]
143+
144+
def validate(self) -> Testdrive:
145+
return Testdrive(
146+
dedent(
147+
"""
148+
> SELECT count(*) FROM float_canon_pg;
149+
4
150+
151+
> SELECT count(*) FROM (SELECT DISTINCT f FROM float_canon_pg);
152+
2
153+
154+
> SELECT id FROM float_canon_pg WHERE f = 0;
155+
2
156+
4
157+
5
158+
159+
> SELECT id FROM float_canon_pg WHERE f = 'NaN';
160+
3
161+
"""
162+
)
163+
)
164+
165+
166+
def float_canon_schemas() -> str:
167+
return dedent(
168+
"""
169+
$ set float-canon-keyschema={
170+
"type": "record",
171+
"name": "Key",
172+
"fields": [ {"name": "key1", "type": "double"} ]
173+
}
174+
175+
$ set float-canon-schema={
176+
"type" : "record",
177+
"name" : "test",
178+
"fields" : [ {"name": "f1", "type": "double"} ]
179+
}
180+
"""
181+
)
182+
183+
184+
class FloatCanonicalizationUpsert(Check):
185+
"""-0.0 in a Kafka upsert source's key and value across versions: a -0.0
186+
and a +0.0 key are the same key, and a post-upgrade tombstone must retract
187+
a value row written with the old float encoding."""
188+
189+
def initialize(self) -> Testdrive:
190+
return Testdrive(
191+
float_canon_schemas()
192+
+ dedent(
193+
"""
194+
$ kafka-create-topic topic=float-canon-upsert
195+
196+
$ kafka-ingest format=avro key-format=avro topic=float-canon-upsert key-schema=${float-canon-keyschema} schema=${float-canon-schema}
197+
{"key1": -0.0} {"f1": 1.0}
198+
{"key1": 2.0} {"f1": -0.0}
199+
200+
> CREATE SOURCE float_canon_upsert_src
201+
FROM KAFKA CONNECTION kafka_conn (TOPIC 'testdrive-float-canon-upsert-${testdrive.seed}')
202+
> CREATE TABLE float_canon_upsert FROM SOURCE float_canon_upsert_src (REFERENCE "testdrive-float-canon-upsert-${testdrive.seed}")
203+
FORMAT AVRO USING CONFLUENT SCHEMA REGISTRY CONNECTION csr_conn
204+
ENVELOPE UPSERT
205+
206+
# Wait for the snapshot so the initial rows are ingested (and
207+
# thus encoded) by the version running this phase.
208+
> SELECT count(*) FROM float_canon_upsert;
209+
2
210+
"""
211+
)
212+
)
213+
214+
def manipulate(self) -> list[Testdrive]:
215+
return [
216+
Testdrive(float_canon_schemas() + dedent(s))
217+
for s in [
218+
"""
219+
# The +0.0 key is the same key as the -0.0 key, so this
220+
# replaces the (0, 1) row rather than adding a third row.
221+
$ kafka-ingest format=avro key-format=avro topic=float-canon-upsert key-schema=${float-canon-keyschema} schema=${float-canon-schema}
222+
{"key1": 0.0} {"f1": 3.0}
223+
""",
224+
"""
225+
# Tombstone the key whose value row (f1 = -0.0) may have been
226+
# written with the old float encoding.
227+
$ kafka-ingest format=avro key-format=avro topic=float-canon-upsert key-schema=${float-canon-keyschema} schema=${float-canon-schema}
228+
{"key1": 2.0}
229+
""",
230+
]
231+
]
232+
233+
def validate(self) -> Testdrive:
234+
return Testdrive(
235+
dedent(
236+
"""
237+
> SELECT key1::text, f1::text FROM float_canon_upsert;
238+
0 3
239+
"""
240+
)
241+
)

‎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(),

0 commit comments

Comments
 (0)