Skip to content

Commit a14198c

Browse files
committed
Fix Dolt fail-closed preflight
1 parent b5d0d67 commit a14198c

5 files changed

Lines changed: 180 additions & 15 deletions

File tree

‎src/openstatspec/sql/capabilities.py‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,10 @@ def active_connection(database_url: str) -> dict[str, Any]:
115115
raw_product_version = _required_text_probe(
116116
connection, "select DOLT_VERSION()", "DOLT_VERSION()",
117117
)
118+
if raw_product_version.strip() != "2.2.2":
119+
raise UnsupportedOperationError(
120+
"The active Dolt product version must be exactly 2.2.2."
121+
)
118122
identity_source = "SELECT @@version, @@version_comment, DOLT_VERSION()"
119123
elif "mariadb" in identity_text:
120124
profile_name = "mariadb"
@@ -236,13 +240,14 @@ def _profile(
236240
"maximum_row_bytes": profile.max_row_bytes,
237241
}
238242
theoretical = (
239-
{"maximum_value_bytes": profile.max_text_value_bytes}
243+
{"maximum_value_bytes": 4_294_967_295}
240244
if name == "dolt" else declared
241245
)
242246
proposed = (
243247
{
244248
"maximum_physical_columns": declared["maximum_physical_columns"],
245249
"maximum_source_variables": declared["maximum_source_variables"],
250+
"maximum_value_bytes": declared["maximum_value_bytes"],
246251
"maximum_row_bytes": declared["maximum_row_bytes"],
247252
}
248253
if name == "dolt" else None
@@ -264,7 +269,7 @@ def _profile(
264269
"maximum_source_variables": "proposed Dolt adapter envelope",
265270
"maximum_physical_columns": "proposed Dolt adapter envelope",
266271
"identifier_limit": "observed on exact Dolt 2.2.2",
267-
"maximum_value_bytes": "Dolt LONGTEXT theoretical type ceiling",
272+
"maximum_value_bytes": "observed Dolt adapter value envelope",
268273
"maximum_row_bytes": "proposed Dolt adapter envelope",
269274
}
270275
if name == "dolt"
@@ -302,7 +307,11 @@ def _profile(
302307
packet = int(observed["max_allowed_packet"])
303308
payload = max(0, (packet - 131_072) // 2)
304309
effective["maximum_value_bytes"] = min(
305-
theoretical["maximum_value_bytes"], payload,
310+
(
311+
declared["maximum_value_bytes"]
312+
if name == "dolt" else theoretical["maximum_value_bytes"]
313+
),
314+
payload,
306315
)
307316
effective["maximum_statement_bytes"] = payload
308317
sources["maximum_value_bytes"] = "active @@max_allowed_packet worst-case payload"
@@ -329,6 +338,12 @@ def _profile(
329338
"effective_limits": effective,
330339
"effective_limits_status": status,
331340
"numeric_type": "DOUBLE PRECISION" if name == "postgresql" else "DOUBLE" if name in {"mysql", "mariadb", "dolt"} else "REAL",
341+
"numeric_value_policy": {
342+
"finite_binary64": "supported",
343+
"nan": "rejected_before_ddl",
344+
"positive_infinity": "rejected_before_ddl",
345+
"negative_infinity": "rejected_before_ddl",
346+
},
332347
"text_type": "LONGTEXT" if name in {"mysql", "mariadb", "dolt"} else "TEXT",
333348
"ddl_atomic": name not in {"mysql", "mariadb", "dolt"},
334349
"failure_cleanup": "compensating_cleanup" if name in {"mysql", "mariadb", "dolt"} else "transaction_rollback",
@@ -340,7 +355,9 @@ def _profile(
340355
"proposed_adapter_envelope" if name == "dolt" else "theoretical_engine_limit"
341356
),
342357
"identifier_limit": "observed_exact_version" if name == "dolt" else "theoretical_engine_limit",
343-
"maximum_value_bytes": "theoretical_engine_limit",
358+
"maximum_value_bytes": (
359+
"observed_exact_version" if name == "dolt" else "theoretical_engine_limit"
360+
),
344361
"maximum_row_bytes": (
345362
"proposed_adapter_envelope" if name == "dolt" else "theoretical_engine_limit"
346363
),
@@ -373,11 +390,11 @@ def _profile(
373390

374391

375392
def server_version_supported(profile: str, raw_version: str) -> bool:
393+
if profile == "dolt":
394+
return raw_version.strip() == "2.2.2"
376395
version = _version_tuple(raw_version)
377396
if profile == "sqlite":
378397
return (3, 24) <= version[:2] < (4, 0)
379-
if profile == "dolt":
380-
return version == (2, 2, 2)
381398
allowed = {
382399
"postgresql": {(17,), (18,)},
383400
"mysql": {(8, 4), (9, 7)},

‎src/openstatspec/sql/profiles.py‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
Importers use this information for preflight checks before creating a dataset.
55
"""
66

7+
import math
8+
79
from collections.abc import Iterable, Mapping
810
from dataclasses import dataclass
911
from typing import Any
@@ -55,7 +57,7 @@ def as_dict(self) -> dict[str, object]:
5557
)
5658
DOLT = SqlProfile(
5759
"dolt", (), 305, 64, True, True,
58-
4_294_967_295, 65_504, True, ("PyMySQL",),
60+
65_504, 65_504, True, ("PyMySQL",),
5961
)
6062
# Dolt deliberately has no URL scheme. It is selected only after the server
6163
# reached through the MySQL wire family positively identifies itself as Dolt.
@@ -176,6 +178,15 @@ def preflight(
176178
row_bytes = 8
177179
for variable in variables:
178180
if variable.get("storage_kind") != "string":
181+
value = row.get(str(variable["physical_name"]))
182+
if value is not None and not math.isfinite(float(value)):
183+
raise _exceeded(
184+
"nonfinite_numeric_value",
185+
f"row {row_ordinal} value for {variable['source_name']!r} "
186+
"is not a finite binary64 value.",
187+
row_ordinal=row_ordinal,
188+
source_name=variable["source_name"],
189+
)
179190
row_bytes += 8
180191
continue
181192
value = row.get(str(variable["physical_name"]), "")
@@ -189,7 +200,7 @@ def preflight(
189200
row_ordinal=row_ordinal, source_name=variable["source_name"],
190201
encoded_bytes=encoded_bytes, maximum=profile.max_text_value_bytes,
191202
)
192-
row_bytes += 20 if profile.name in {"mysql", "mariadb"} else encoded_bytes
203+
row_bytes += 20 if profile.name in {"mysql", "mariadb", "dolt"} else encoded_bytes
193204
if row_bytes > profile.max_row_bytes:
194205
raise _exceeded(
195206
"row_size_limit",
@@ -203,7 +214,7 @@ def preflight(
203214
def _row_storage_bytes(profile: SqlProfile, variable: Mapping[str, Any]) -> int:
204215
if variable.get("storage_kind") == "numeric":
205216
return 8
206-
if profile.name in {"mysql", "mariadb"}:
217+
if profile.name in {"mysql", "mariadb", "dolt"}:
207218
return 20
208219
return int(variable.get("string_width") or 0)
209220

‎tests/test_atomic_import.py‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,43 @@ def test_occupied_foreign_namespace_fails_without_modification(tmp_path) -> None
209209
assert connection.execute("select value from foreign_data").fetchall() == [("keep",)]
210210

211211

212+
@pytest.mark.parametrize("value", [float("nan"), float("inf"), float("-inf")])
213+
def test_dolt_nonfinite_preflight_creates_no_dataset_or_physical_table(
214+
tmp_path, monkeypatch, value,
215+
) -> None:
216+
database_path = tmp_path / "dolt-nonfinite.sqlite"
217+
database = f"sqlite:///{database_path}"
218+
monkeypatch.setattr(wide, "effective_profile", lambda _url: (DOLT, {}))
219+
variables = [{
220+
"ordinal": 1, "source_name": "value", "physical_name": "value",
221+
"storage_kind": "numeric", "string_width": None, "label": "",
222+
"format": "F8.2", "measure": "scale", "alignment": "right",
223+
"display_width": 8, "value_labels": "{}", "missing_ranges": "[]",
224+
}]
225+
226+
with pytest.raises(Exception, match="Target capability exceeded"):
227+
create_wide_dataset(
228+
database_url=database, dataset_id="nonfinite",
229+
source_name="nonfinite.sav", source_format="SAV",
230+
rows=[{"value": value}], variables=variables,
231+
)
232+
233+
connection = sqlite3.connect(database_path)
234+
assert "data_nonfinite" not in {
235+
row[0] for row in connection.execute(
236+
"select name from sqlite_master where type = 'table'"
237+
)
238+
}
239+
assert connection.execute("select count(*) from dataset").fetchone() == (0,)
240+
assert connection.execute("select count(*) from variable").fetchone() == (0,)
241+
assert connection.execute(
242+
"select status from operation"
243+
).fetchall() == [("failed",)]
244+
assert connection.execute(
245+
"select dataset_id, event_code from fidelity_event"
246+
).fetchall() == [(None, "target_capability_exceeded")]
247+
248+
212249
def test_empty_namespace_dolt_width_failure_initializes_identity_and_one_audit(
213250
tmp_path, monkeypatch,
214251
) -> None:

‎tests/test_sql_profiles.py‎

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,9 @@ def test_normative_catalog_compiles_for_every_sql_family() -> None:
8383
[
8484
("mysql", "8.4.6", True), ("mysql", "9.7.0", True),
8585
("mysql", "8.0.44", False),
86-
("dolt", "2.2.2", True), ("dolt", "2.2.3", False),
86+
("dolt", "2.2.2", True), ("dolt", " 2.2.2 ", True),
87+
("dolt", "2.2.3", False), ("dolt", "2.2.2-rc1", False),
88+
("dolt", "2.2.2+build.1", False), ("dolt", "garbage", False),
8789
("mariadb", "11.4.8-MariaDB", True),
8890
("mariadb", "11.8.3-MariaDB", True),
8991
("mariadb", "12.3.1-MariaDB", True),
@@ -223,6 +225,25 @@ def test_dolt_comment_requires_a_nonempty_product_version(monkeypatch) -> None:
223225
active_connection("mysql+pymysql://user@host/database")
224226

225227

228+
@pytest.mark.parametrize(
229+
"product_version",
230+
["2.2.2-rc1", "2.2.2+build.1", "v2.2.2", "garbage"],
231+
)
232+
def test_dolt_identity_rejects_nonexact_product_versions(
233+
monkeypatch, product_version,
234+
) -> None:
235+
connection = _mock_mysql_probes(
236+
monkeypatch, **{"select DOLT_VERSION()": product_version},
237+
)
238+
239+
with pytest.raises(UnsupportedOperationError, match="exactly 2.2.2"):
240+
active_connection("mysql+pymysql://user@host/database")
241+
242+
assert connection.calls == [
243+
"select @@version", "select @@version_comment", "select DOLT_VERSION()",
244+
]
245+
246+
226247
def test_unknown_mysql_wire_product_fails_closed_without_dolt_probe(monkeypatch) -> None:
227248
connection = _mock_mysql_probes(
228249
monkeypatch,
@@ -268,6 +289,7 @@ def test_effective_profile_selects_dolt_without_changing_url_profile(monkeypatch
268289
assert profile.name == "dolt"
269290
assert profile.url_schemes == ()
270291
assert profile.max_physical_variables == 305
292+
assert profile.max_text_value_bytes == 65_504
271293
assert profile.max_row_bytes == 65_504
272294
assert observed is active
273295

@@ -281,22 +303,32 @@ def test_dolt_declaration_labels_conservative_envelopes() -> None:
281303
assert declaration["ci_tested_server_versions"] == ["Dolt 2.2.2"]
282304
assert declaration["proposed_adapter_limits"]["maximum_physical_columns"] == 306
283305
assert declaration["proposed_adapter_limits"]["maximum_source_variables"] == 305
306+
assert declaration["proposed_adapter_limits"]["maximum_value_bytes"] == 65_504
284307
assert declaration["proposed_adapter_limits"]["maximum_row_bytes"] == 65_504
308+
assert declaration["theoretical_limits"]["maximum_value_bytes"] == 4_294_967_295
285309
assert declaration["observed_limits"]["minimum_observed_physical_columns"] == 307
286310
assert declaration["observed_limits"]["identifier_limit"]["value"] == 64
287311
assert declaration["observed_limits"]["rejected_identifier_bytes"] == 65
288312
assert set(declaration["proposed_adapter_limits"]) == {
289-
"maximum_physical_columns", "maximum_source_variables", "maximum_row_bytes",
313+
"maximum_physical_columns", "maximum_source_variables",
314+
"maximum_value_bytes", "maximum_row_bytes",
290315
}
291316
assert declaration["limit_bases"]["maximum_physical_columns"] == "proposed_adapter_envelope"
292317
assert declaration["limit_bases"]["identifier_limit"] == "observed_exact_version"
318+
assert declaration["limit_bases"]["maximum_value_bytes"] == "observed_exact_version"
293319
assert declaration["limit_bases"]["maximum_statement_bytes"] == "active_connection_observation"
294320
assert declaration["effective_limits"] is None
295321
assert declaration["text_type"] == "LONGTEXT"
296322
assert capabilities.profile_declarations()["mysql"]["text_type"] == "LONGTEXT"
297323
assert capabilities.profile_declarations()["mariadb"]["text_type"] == "LONGTEXT"
298324
assert declaration["ddl_atomic"] is False
299325
assert declaration["failure_cleanup"] == "compensating_cleanup"
326+
assert declaration["numeric_value_policy"] == {
327+
"finite_binary64": "supported",
328+
"nan": "rejected_before_ddl",
329+
"positive_infinity": "rejected_before_ddl",
330+
"negative_infinity": "rejected_before_ddl",
331+
}
300332
assert declaration["storage_evidence"]["binary64"]["maximum_finite_round_trip_exact"] is True
301333
assert declaration["storage_evidence"]["binary64"]["source"]
302334
assert declaration["storage_evidence"]["binary64"]["version"] == "2.2.2"
@@ -320,13 +352,30 @@ def test_dolt_uses_longtext_without_changing_mysql_storage() -> None:
320352
def test_dolt_row_preflight_counts_utf8_values_against_adapter_envelope() -> None:
321353
variables = [{
322354
"ordinal": 1, "source_name": "value", "physical_name": "value",
323-
"storage_kind": "string", "string_width": 65_496,
355+
"storage_kind": "string", "string_width": 65_504,
324356
}]
325-
preflight(DOLT, variables, rows=[{"value": "x" * 65_496}])
357+
preflight(DOLT, variables, rows=[{"value": "x" * 65_504}])
326358

327359
with pytest.raises(UnsupportedOperationError) as error:
328-
preflight(DOLT, variables, rows=[{"value": "x" * 65_497}])
329-
assert error.value.details["reason"] == "row_size_limit"
360+
preflight(DOLT, variables, rows=[{"value": "x" * 65_505}])
361+
assert error.value.details["reason"] == "text_value_limit"
362+
363+
364+
@pytest.mark.parametrize("value", [float("nan"), float("inf"), float("-inf")])
365+
def test_dolt_numeric_preflight_rejects_nonfinite_values(value) -> None:
366+
variables = [{
367+
"ordinal": 1, "source_name": "value", "physical_name": "value",
368+
"storage_kind": "numeric", "string_width": None,
369+
}]
370+
371+
with pytest.raises(UnsupportedOperationError) as error:
372+
preflight(DOLT, variables, rows=[{"value": value}])
373+
374+
assert error.value.details == {
375+
"reason": "nonfinite_numeric_value",
376+
"row_ordinal": 1,
377+
"source_name": "value",
378+
}
330379

331380

332381
def test_validate_identity_failure_happens_before_catalog_access(monkeypatch) -> None:

‎tests/test_sql_services.py‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,57 @@ def fail_after_normative_write(*args, **kwargs):
200200
assert f"data_{dataset_id}" not in inspect_database(engine).get_table_names()
201201

202202

203+
def test_live_dolt_adapter_value_boundary_is_atomic() -> None:
204+
database_url = os.environ.get("OPENSTATSPEC_DOLT_URL")
205+
if not database_url:
206+
pytest.skip("OPENSTATSPEC_DOLT_URL is not configured")
207+
token = uuid4().hex[:8]
208+
accepted_id = f"dolt_value_accepted_{token}"
209+
rejected_id = f"dolt_value_rejected_{token}"
210+
accepted_value = "é" * 32_752
211+
rejected_value = accepted_value + "x"
212+
assert len(accepted_value.encode("utf-8")) == 65_504
213+
assert len(rejected_value.encode("utf-8")) == 65_505
214+
variables = [{
215+
"ordinal": 1, "source_name": "value", "physical_name": "value",
216+
"storage_kind": "string", "string_width": 65_504, "label": "",
217+
"format": "A65504", "measure": "nominal", "alignment": "left",
218+
"display_width": 8, "value_labels": "{}", "missing_ranges": "[]",
219+
}]
220+
221+
imported = wide.create_wide_dataset(
222+
database_url=database_url, dataset_id=accepted_id,
223+
source_name="accepted.sav", source_format="SAV",
224+
rows=[{"value": accepted_value}], variables=variables,
225+
)
226+
assert imported["case_count"] == 1
227+
228+
with pytest.raises(UnsupportedOperationError) as caught:
229+
wide.create_wide_dataset(
230+
database_url=database_url, dataset_id=rejected_id,
231+
source_name="rejected.sav", source_format="SAV",
232+
rows=[{"value": rejected_value}], variables=variables,
233+
)
234+
assert caught.value.details["reason"] == "text_value_limit"
235+
236+
engine = create_engine(database_url)
237+
accepted_table = wide.data_table_name(accepted_id)
238+
rejected_table = wide.data_table_name(rejected_id)
239+
quote = engine.dialect.identifier_preparer.quote
240+
with engine.connect() as connection:
241+
assert connection.execute(text(
242+
f"SELECT OCTET_LENGTH(value) FROM {quote(accepted_table)}"
243+
)).scalar_one() == 65_504
244+
assert connection.execute(text(
245+
"SELECT COUNT(*) FROM dataset_catalog WHERE dataset_id = :dataset_id"
246+
), {"dataset_id": rejected_id}).scalar_one() == 0
247+
assert connection.execute(text(
248+
"SELECT COUNT(*) FROM dataset WHERE dataset_name = :dataset_id"
249+
), {"dataset_id": rejected_id}).scalar_one() == 0
250+
assert rejected_table not in inspect_database(engine).get_table_names()
251+
engine.dispose()
252+
253+
203254
def test_live_dolt_published_storage_and_identifier_evidence() -> None:
204255
database_url = os.environ.get("OPENSTATSPEC_DOLT_URL")
205256
if not database_url:

0 commit comments

Comments
 (0)