From f053c29edb11be2adbfd6ade8734440fa226ac7e Mon Sep 17 00:00:00 2001 From: "exe.dev user" Date: Sat, 3 Oct 2026 00:21:34 +0000 Subject: [PATCH 1/6] fix(hedgerow): reject unsupported formats before replication Co-authored-by: Shelley --- hedgerow/README.md | 9 ++++- hedgerow/src/hedgerow/__init__.py | 2 ++ hedgerow/src/hedgerow/daemon.py | 15 +++++++- hedgerow/src/hedgerow/discovery.py | 3 ++ hedgerow/src/hedgerow/formats.py | 9 +++++ hedgerow/src/hedgerow/halts.py | 4 +++ hedgerow/src/hedgerow/ingestion.py | 9 +++++ hedgerow/tests/fakes.py | 3 ++ hedgerow/tests/test_daemon_unit.py | 29 +++++++++++++-- hedgerow/tests/test_events_discovery.py | 48 ++++++++++++++++++++++--- 10 files changed, 122 insertions(+), 9 deletions(-) create mode 100644 hedgerow/src/hedgerow/formats.py diff --git a/hedgerow/README.md b/hedgerow/README.md index 97b1eb73..abe2e77f 100644 --- a/hedgerow/README.md +++ b/hedgerow/README.md @@ -152,7 +152,7 @@ for this table. A nonzero lag can be entirely other tables' commits (the next cycle drains it as an empty window); use it as a staleness/liveness signal, not a volume estimate. -## Runbook: the seven halt conditions +## Runbook: the eight halt conditions hedgerow HALTS (exits nonzero, no retry) when continuing would be wrong. A supervisor must NOT blindly restart these — the same condition will @@ -260,6 +260,13 @@ the last error), expect up to `1 + max_window_replays` copies of the window's rows in the destination worst-case, and restart — the window replays once more from the committed offset. +### 8. Unsupported data format (exit 10) + +Both replication modes require Parquet source and destination tables. +Packed MergeTree tables (`write.format.default=clickhouse-mergetree-packed`) are refused, including empty tables. +Each change window is checked before any file is read, rows are appended, or offsets are advanced. +Use a Parquet destination or the Python ClickHouse packed adapter instead. + ## Development ```sh diff --git a/hedgerow/src/hedgerow/__init__.py b/hedgerow/src/hedgerow/__init__.py index 3d90d0cb..3ab66a81 100644 --- a/hedgerow/src/hedgerow/__init__.py +++ b/hedgerow/src/hedgerow/__init__.py @@ -28,6 +28,7 @@ PersistentFailureError, SchemaMismatchError, SplitBrainError, + UnsupportedFormatError, ) from .projection import ProjectionPlan, validate_projection from .window import Window, plan_window @@ -55,6 +56,7 @@ "SchemaMismatchError", "SourceConfig", "SplitBrainError", + "UnsupportedFormatError", "Window", "__version__", "load_config", diff --git a/hedgerow/src/hedgerow/daemon.py b/hedgerow/src/hedgerow/daemon.py index e353bdd3..14877231 100644 --- a/hedgerow/src/hedgerow/daemon.py +++ b/hedgerow/src/hedgerow/daemon.py @@ -57,6 +57,8 @@ import pyarrow as pa import pyarrow.parquet as pq +from pyhoglake.types import columns_to_arrow_schema + from pyhoglake import ( AlreadyExistsError, CommitConflictError, @@ -68,10 +70,10 @@ ValidationError, ) from pyhoglake import IncarnationChangedError as ClientIncarnationChangedError -from pyhoglake.types import columns_to_arrow_schema from .config import HedgerowConfig from .filtering import RowFilter, build_filter +from .formats import require_parquet from .halts import ( DataIntegrityError, DeletesPresentError, @@ -193,11 +195,17 @@ def start(self) -> None: self._source_catalog = self._source_client.catalog(cfg.source.catalog) self._source_ns = self._source_catalog.namespace(cfg.source.namespace) source_table = self._source_ns.table(cfg.source.table) + require_parquet( + (source_table.properties or {}).get("write.format.default", "parquet") + ) self.source_uuid = source_table.table_uuid dest_catalog = self._dest_client.catalog(cfg.destination.catalog) self._dest_ns = dest_catalog.namespace(cfg.destination.namespace) dest_table = self._dest_ns.table(cfg.destination.table) + require_parquet( + (dest_table.properties or {}).get("write.format.default", "parquet") + ) self.dest_uuid = dest_table.table_uuid # Destination columns define the projected set; source must cover @@ -251,6 +259,7 @@ def _resolve_source_table(self): f"(pinned uuid {self.source_uuid}) no longer exists. " "HALT: refusing to continue against a dropped table." ) from None + require_parquet((table.properties or {}).get("write.format.default", "parquet")) if table.table_uuid != self.source_uuid: raise IncarnationChangedError( f"source table {cfg.catalog}/{cfg.namespace}.{cfg.table} was " @@ -270,6 +279,7 @@ def _resolve_dest_table(self): f"destination table {cfg.catalog}/{cfg.namespace}.{cfg.table} " f"(pinned uuid {self.dest_uuid}) no longer exists. HALT." ) from None + require_parquet((table.properties or {}).get("write.format.default", "parquet")) if table.table_uuid != self.dest_uuid: raise IncarnationChangedError( f"destination table {cfg.catalog}/{cfg.namespace}.{cfg.table} " @@ -356,6 +366,9 @@ def run_once(self) -> CycleResult: "from the source)." ) + for file in plan.files: + require_parquet(file.file_format) + rows_read = rows_appended = appends = 0 max_rows = cfg.replication.max_rows_per_append max_append_retries = cfg.replication.max_append_retries diff --git a/hedgerow/src/hedgerow/discovery.py b/hedgerow/src/hedgerow/discovery.py index 32023d9a..7bef173f 100644 --- a/hedgerow/src/hedgerow/discovery.py +++ b/hedgerow/src/hedgerow/discovery.py @@ -21,6 +21,7 @@ from pyhoglake.models import ChangesPlan from .events import EventTransform +from .formats import require_parquet from .halts import DataIntegrityError, DeletesPresentError, IncarnationChangedError from .pending import Fragment, PendingStore, SourceFile @@ -54,6 +55,8 @@ def discover_window( raise DeletesPresentError( "raw source has deletion vectors; refusing to skip or reinterpret pending input" ) + for file in plan.files: + require_parquet(file.file_format) fragments = [] source_files = [] source_bytes = selected_bytes = 0 diff --git a/hedgerow/src/hedgerow/formats.py b/hedgerow/src/hedgerow/formats.py new file mode 100644 index 00000000..fab67bde --- /dev/null +++ b/hedgerow/src/hedgerow/formats.py @@ -0,0 +1,9 @@ +from .halts import UnsupportedFormatError + + +def require_parquet(file_format: str) -> None: + if file_format != "parquet": + raise UnsupportedFormatError( + f"Hedgerow cannot replicate data format {file_format!r}. " + "Use Parquet source and destination tables." + ) diff --git a/hedgerow/src/hedgerow/halts.py b/hedgerow/src/hedgerow/halts.py index a44a69fd..e3c6c5c5 100644 --- a/hedgerow/src/hedgerow/halts.py +++ b/hedgerow/src/hedgerow/halts.py @@ -79,3 +79,7 @@ class PersistentFailureError(HaltError): halting instead of retrying forever.""" exit_code = 9 + + +class UnsupportedFormatError(HaltError): + exit_code = 10 diff --git a/hedgerow/src/hedgerow/ingestion.py b/hedgerow/src/hedgerow/ingestion.py index 3af98ac4..ba22de5a 100644 --- a/hedgerow/src/hedgerow/ingestion.py +++ b/hedgerow/src/hedgerow/ingestion.py @@ -12,6 +12,7 @@ from tempfile import TemporaryDirectory import pyarrow.parquet as pq + from pyhoglake import ReconciliationRequiredError from .buffering import BufferPolicy @@ -22,6 +23,7 @@ write_duckdb_event_partition, ) from .events import EventTransform +from .formats import require_parquet from .halts import ( DataIntegrityError, DeletesPresentError, @@ -146,6 +148,10 @@ def __init__( self.max_snapshot_window = max_snapshot_window self.source_info = source.info() self.destination_info = destination.info() + for info in (self.source_info, self.destination_info): + require_parquet( + (info.properties or {}).get("write.format.default", "parquet") + ) self.transform = EventTransform( self.source_info.columns, self.destination_info, @@ -183,6 +189,9 @@ def _guard(self): (self.destination, self.destination_info), ): current = table.info() + require_parquet( + (current.properties or {}).get("write.format.default", "parquet") + ) if current.table_uuid != pinned.table_uuid: raise IncarnationChangedError("buffered ingestion table was recreated") if (current.columns, current.partition_spec, current.sort_spec) != ( diff --git a/hedgerow/tests/fakes.py b/hedgerow/tests/fakes.py index 2b6feb3f..48141940 100644 --- a/hedgerow/tests/fakes.py +++ b/hedgerow/tests/fakes.py @@ -18,6 +18,7 @@ from typing import Any import pyarrow as pa + from pyhoglake import ( ChangesPlan, Column, @@ -94,6 +95,7 @@ class FakeSourceTable: expired_below: int = 0 # changes(from < expired_below) -> 410 # if set, changes() reports this uuid in the plan (recreate-mid-flight) plan_uuid_override: str | None = None + properties: dict[str, str] = dc_field(default_factory=dict) def changes( self, from_snapshot: int, to_snapshot: int | None = None @@ -145,6 +147,7 @@ class FakeDestTable: conflict_first_n_appends: int = 0 _append_calls: int = 0 _next_snapshot: int = 100 + properties: dict[str, str] = dc_field(default_factory=dict) def append(self, data: pa.Table, **kwargs: Any) -> CommitResult: self._append_calls += 1 diff --git a/hedgerow/tests/test_daemon_unit.py b/hedgerow/tests/test_daemon_unit.py index 2a87bd07..0330a2f3 100644 --- a/hedgerow/tests/test_daemon_unit.py +++ b/hedgerow/tests/test_daemon_unit.py @@ -1,7 +1,7 @@ """Daemon cycle logic against scripted fakes: offset-commit ordering, halt paths, at-least-once replay, batching/bounded memory, pacing.""" -from dataclasses import dataclass +from dataclasses import dataclass, replace from dataclasses import field as dc_field import pyarrow as pa @@ -18,7 +18,6 @@ delete_file, table_batch_reader, ) -from pyhoglake import NotFoundError from hedgerow import ( DeletesPresentError, @@ -32,7 +31,9 @@ ReplicationConfig, SchemaMismatchError, SourceConfig, + UnsupportedFormatError, ) +from pyhoglake import NotFoundError SRC_COLS = ( col("id", "long", 1, 0, nullable=False), @@ -738,3 +739,27 @@ def truncated(*args): env.daemon.run_once() assert env.offset() == before assert env.dest_table.total_rows == 1 + + +@pytest.mark.parametrize("side", ["source_table", "dest_table"]) +def test_non_parquet_table_refused_even_when_empty(side: str) -> None: + env = build_env() + getattr(env, side).properties = { + "write.format.default": "clickhouse-mergetree-packed" + } + with pytest.raises(UnsupportedFormatError, match="clickhouse-mergetree-packed"): + env.daemon.run_once() + assert env.offset() is None + assert env.dest_table.total_rows == 0 + + +def test_non_parquet_file_refused_before_partial_window_append() -> None: + env = build_env( + files={1: src_data([1]), 2: src_data([2])}, config=make_config(max_rows=1) + ) + files = env.source_table.files_by_snapshot[2] + files[0] = replace(files[0], file_format="clickhouse-mergetree-packed") + with pytest.raises(UnsupportedFormatError, match="clickhouse-mergetree-packed"): + env.daemon.run_once() + assert env.offset() is None + assert env.dest_table.total_rows == 0 diff --git a/hedgerow/tests/test_events_discovery.py b/hedgerow/tests/test_events_discovery.py index 3d626e3b..9ae9df89 100644 --- a/hedgerow/tests/test_events_discovery.py +++ b/hedgerow/tests/test_events_discovery.py @@ -3,6 +3,14 @@ import pyarrow as pa import pyarrow.parquet as pq import pytest +from hedgerow.discovery import discover_window +from hedgerow.events import EventTransform +from hedgerow.halts import ( + DataIntegrityError, + SchemaMismatchError, + UnsupportedFormatError, +) +from hedgerow.pending import PendingStore from pyhoglake.models import ( ChangesPlan, Column, @@ -14,11 +22,6 @@ TableInfo, ) -from hedgerow.discovery import discover_window -from hedgerow.events import EventTransform -from hedgerow.halts import DataIntegrityError, SchemaMismatchError -from hedgerow.pending import PendingStore - def layout(): columns = tuple( @@ -239,3 +242,38 @@ def test_json_conversion_must_be_explicit_and_type_checked(): EventTransform(source, dest, json_columns=("properties",)) with pytest.raises(SchemaMismatchError, match="JSON mapping"): EventTransform(source, dest, json_columns=("event",)) + + +def test_discovery_refuses_non_parquet_before_reading_window(tmp_path) -> None: + files = ( + DataFile(1, "s3://example/raw.parquet", "parquet", 3, 100, 0, "provided", 1), + DataFile( + 2, + "s3://example/data.packed", + "clickhouse-mergetree-packed", + 3, + 100, + 3, + "provided", + 1, + ), + ) + plan = ChangesPlan("source", 0, 1, files) + store = PendingStore(str(tmp_path / "pending.sqlite"), {}) + + def unexpected_open(path: str) -> pq.ParquetFile: + pytest.fail(f"Opened {path} before validating all formats") + + try: + with pytest.raises(UnsupportedFormatError, match="clickhouse-mergetree-packed"): + discover_window( + store, + plan, + "source", + {1: datetime(2026, 1, 1, tzinfo=UTC)}, + layout(), + unexpected_open, + ) + assert store.discovered == 0 + finally: + store.close() From 4107cea660596ceab88008b3162f897aec5189cf Mon Sep 17 00:00:00 2001 From: fuziontech Date: Sat, 3 Oct 2026 03:41:14 +0000 Subject: [PATCH 2/6] feat: add packed MergeTree format support Co-authored-by: Shelley --- duckdb-client/README.md | 7 + .../src/include/common/hoglake_wire.hpp | 1 + duckdb-client/src/rest/hoglake_api_client.cpp | 7 + duckdb-client/src/storage/hoglake_insert.cpp | 4 + .../src/storage/hoglake_multi_file_list.cpp | 6 + .../src/storage/hoglake_table_entry.cpp | 4 + .../test/fixtures/packed_format_fixture.py | 73 ++ duckdb-client/test/run-live-tests.sh | 2 + .../test/sql/hoglake_packed_format.test | 38 + hedgerow/src/hedgerow/daemon.py | 3 +- hedgerow/src/hedgerow/ingestion.py | 1 - hedgerow/tests/fakes.py | 1 - hedgerow/tests/test_daemon_unit.py | 2 +- hedgerow/tests/test_events_discovery.py | 17 +- pyhoglake/README.md | 57 +- pyhoglake/src/pyhoglake/__init__.py | 10 + pyhoglake/src/pyhoglake/client.py | 42 +- pyhoglake/src/pyhoglake/formats.py | 22 + pyhoglake/src/pyhoglake/models.py | 3 +- pyhoglake/src/pyhoglake/packed.py | 690 ++++++++++++++++++ pyhoglake/tests/test_metadata_parity.py | 22 + pyhoglake/tests/test_packed.py | 574 +++++++++++++++ server/README.md | 31 + server/schema.sql | 118 ++- .../kotlin/com/posthog/hoglake/api/Dto.kt | 9 +- .../kotlin/com/posthog/hoglake/api/Routes.kt | 1 + .../com/posthog/hoglake/api/UploadRoutes.kt | 18 +- .../posthog/hoglake/commit/CommitService.kt | 129 +++- .../hoglake/compaction/CompactionService.kt | 52 +- .../com/posthog/hoglake/hydrator/Hydrator.kt | 1 + .../com/posthog/hoglake/model/FileFormats.kt | 48 ++ .../kotlin/com/posthog/hoglake/model/Model.kt | 6 + .../hoglake/persistence/HogSchemaColumns.kt | 4 +- .../posthog/hoglake/persistence/TableRepo.kt | 32 +- .../posthog/hoglake/service/AlterService.kt | 18 + .../posthog/hoglake/service/CatalogService.kt | 21 +- .../hoglake/service/TableCreationService.kt | 17 +- .../posthog/hoglake/service/TableMetadata.kt | 51 +- .../posthog/hoglake/service/UploadService.kt | 86 ++- .../V25__packed_mergetree_format.sql | 196 +++++ .../src/main/resources/openapi/hoglake.yaml | 37 +- .../com/posthog/hoglake/WireJsonTest.kt | 24 + .../posthog/hoglake/api/ApiIntegrationTest.kt | 56 ++ .../hoglake/commit/CommitServiceTest.kt | 137 ++++ .../CompactionPlanningIntegrationTest.kt | 61 +- .../hydrator/HydratorIntegrationTest.kt | 30 +- ...17FilePathIndexMigrationIntegrationTest.kt | 12 +- ...MergetreeFormatMigrationIntegrationTest.kt | 205 ++++++ .../hoglake/service/TableMetadataTest.kt | 70 ++ .../service/UploadServiceIntegrationTest.kt | 46 ++ .../commit_full | 2 +- 51 files changed, 3012 insertions(+), 92 deletions(-) create mode 100644 duckdb-client/test/fixtures/packed_format_fixture.py create mode 100644 duckdb-client/test/sql/hoglake_packed_format.test create mode 100644 pyhoglake/src/pyhoglake/formats.py create mode 100644 pyhoglake/src/pyhoglake/packed.py create mode 100644 pyhoglake/tests/test_packed.py create mode 100644 server/src/main/kotlin/com/posthog/hoglake/model/FileFormats.kt create mode 100644 server/src/main/resources/db/migration/V25__packed_mergetree_format.sql create mode 100644 server/src/test/kotlin/com/posthog/hoglake/persistence/V25PackedMergetreeFormatMigrationIntegrationTest.kt diff --git a/duckdb-client/README.md b/duckdb-client/README.md index dbe1c33b..e2d8e2eb 100644 --- a/duckdb-client/README.md +++ b/duckdb-client/README.md @@ -98,3 +98,10 @@ the test reads the `_hog_row_id` carrier), extension-written and pyhoglake-written partition values land in byte-identical partition strings, and both directions of the reserved-field-id contract refuse typed instead of killing the instance. + +## Data formats + +The extension reads and writes Parquet tables only. +It refuses tables with `write.format.default=clickhouse-mergetree-packed`, including empty tables, before scanning or writing objects. +Scan plans also validate each file format before passing paths to the Parquet reader. +Use the Python ClickHouse packed adapter for these tables. diff --git a/duckdb-client/src/include/common/hoglake_wire.hpp b/duckdb-client/src/include/common/hoglake_wire.hpp index b6f43848..c5a1db34 100644 --- a/duckdb-client/src/include/common/hoglake_wire.hpp +++ b/duckdb-client/src/include/common/hoglake_wire.hpp @@ -64,6 +64,7 @@ struct HoglakeSortSpec { struct HoglakeTableInfo { string name; + string file_format = "parquet"; string namespace_name; string table_uuid; vector columns; diff --git a/duckdb-client/src/rest/hoglake_api_client.cpp b/duckdb-client/src/rest/hoglake_api_client.cpp index 5a95f7dd..8dba05a3 100644 --- a/duckdb-client/src/rest/hoglake_api_client.cpp +++ b/duckdb-client/src/rest/hoglake_api_client.cpp @@ -187,6 +187,13 @@ HoglakeTableInfo ParseTableInfo(yyjson_val *obj) { info.name = GetString(obj, "name"); info.namespace_name = GetString(obj, "namespace"); info.table_uuid = GetString(obj, "table_uuid"); + auto properties = yyjson_obj_get(obj, "properties"); + if (properties && !yyjson_is_null(properties)) { + if (!yyjson_is_obj(properties)) { + throw InvalidInputException("hoglake: expected object for table properties"); + } + info.file_format = GetString(properties, "write.format.default", "parquet"); + } // Table-schema numerics were the one struct the R3 sweep missed: // record_count feeds NumericCast in GetStorageInfo. // diff --git a/duckdb-client/src/storage/hoglake_insert.cpp b/duckdb-client/src/storage/hoglake_insert.cpp index 45a2241f..66637526 100644 --- a/duckdb-client/src/storage/hoglake_insert.cpp +++ b/duckdb-client/src/storage/hoglake_insert.cpp @@ -223,6 +223,10 @@ PhysicalOperator &HoglakeInsert::PlanInsert(ClientContext &context, PhysicalPlan plan_transaction.RequireDMLAllowed(table.ParentSchema().name.GetIdentifierName(), table.GetWireInfo().name, false /* is_delete */); auto &wire = table.GetWireInfo(); + if (wire.file_format != "parquet") { + throw NotImplementedException("hoglake: DuckDB cannot write data format '%s'; use a ClickHouse writer", + wire.file_format); + } auto columns = wire.columns; std::sort(columns.begin(), columns.end(), diff --git a/duckdb-client/src/storage/hoglake_multi_file_list.cpp b/duckdb-client/src/storage/hoglake_multi_file_list.cpp index dc7a0572..f89dd5ca 100644 --- a/duckdb-client/src/storage/hoglake_multi_file_list.cpp +++ b/duckdb-client/src/storage/hoglake_multi_file_list.cpp @@ -189,6 +189,12 @@ void HoglakeMultiFileList::LoadFileList() const { auto &table = read_info.table; auto ns = table.ParentSchema().name.GetIdentifierName(); files = transaction->Api().PlanScan(ns, read_info.table_name, read_info.travel); + for (const auto &file : files) { + if (file.data_file.file_format != "parquet") { + throw NotImplementedException("hoglake: DuckDB cannot read data format '%s'; use a ClickHouse reader", + file.data_file.file_format); + } + } read_file_list = true; } diff --git a/duckdb-client/src/storage/hoglake_table_entry.cpp b/duckdb-client/src/storage/hoglake_table_entry.cpp index fe8f6adf..9c995334 100644 --- a/duckdb-client/src/storage/hoglake_table_entry.cpp +++ b/duckdb-client/src/storage/hoglake_table_entry.cpp @@ -29,6 +29,10 @@ unique_ptr HoglakeTableEntry::GetStatistics(ClientContext &conte } TableFunction HoglakeTableEntry::GetScanFunction(ClientContext &context, unique_ptr &bind_data) { + if (table_info.file_format != "parquet") { + throw NotImplementedException("hoglake: DuckDB cannot read data format '%s'; use a ClickHouse reader", + table_info.file_format); + } auto function = HoglakeFunctions::GetHoglakeScanFunction(*context.db); auto &transaction = HoglakeTransaction::Get(context, ParentCatalog()); function.function_info = HoglakeFunctionInfo::Create(*this, transaction); diff --git a/duckdb-client/test/fixtures/packed_format_fixture.py b/duckdb-client/test/fixtures/packed_format_fixture.py new file mode 100644 index 00000000..d4e6647c --- /dev/null +++ b/duckdb-client/test/fixtures/packed_format_fixture.py @@ -0,0 +1,73 @@ +"""Create the two packed-format tables used by hoglake_packed_format.test.""" + +import os +import uuid + +import pyarrow as pa + +from pyhoglake import AlreadyExistsError, HoglakeClient, NotFoundError, S3Config + +HOGLAKE_URL = os.environ.get("HOGLAKE_URL", "http://localhost:8080") +S3_ENDPOINT = os.environ.get("HOGLAKE_S3_ENDPOINT", "http://localhost:19000") +S3_ACCESS_KEY = os.environ.get("HOGLAKE_S3_ACCESS_KEY", "hoglake") +S3_SECRET_KEY = os.environ.get("HOGLAKE_S3_SECRET_KEY", "hoglake123") +CATALOG = "duckext-packed" +DATA_PATH = "s3://duckext-itest/packed/" +FORMAT = "clickhouse-mergetree-packed" + + +def main() -> None: + s3 = S3Config( + access_key=S3_ACCESS_KEY, + secret_key=S3_SECRET_KEY, + endpoint_override=S3_ENDPOINT, + region="us-east-1", + allow_bucket_creation=True, + ) + with HoglakeClient(HOGLAKE_URL, s3=s3) as client: + s3.filesystem().create_dir("duckext-itest") + try: + catalog = client.catalog(CATALOG) + except NotFoundError: + catalog = client.create_catalog(CATALOG, DATA_PATH) + try: + namespace = catalog.create_namespace("ns1") + except AlreadyExistsError: + namespace = catalog.namespace("ns1") + for name in ("packed_empty", "packed_registered"): + try: + namespace.table(name).drop() + except NotFoundError: + pass + table = namespace.create_table( + name, + pa.schema([pa.field("id", pa.int64(), nullable=False)]), + properties={"write.format.default": FORMAT}, + ) + if name == "packed_registered": + catalog.commit_prepared( + { + "idempotency_key": str(uuid.uuid4()), + "read_snapshot": catalog.refresh().head_snapshot_id, + "appends": [ + { + "namespace": "ns1", + "table": name, + "expected_table_uuid": table.table_uuid, + "files": [ + { + "path": DATA_PATH + "data.packed", + "file_format": FORMAT, + "record_count": 1, + "file_size_bytes": 1, + "column_stats": [], + } + ], + } + ], + } + ) + + +if __name__ == "__main__": + main() diff --git a/duckdb-client/test/run-live-tests.sh b/duckdb-client/test/run-live-tests.sh index e43b62f9..90207d1c 100755 --- a/duckdb-client/test/run-live-tests.sh +++ b/duckdb-client/test/run-live-tests.sh @@ -14,6 +14,8 @@ export DUCKEXT_S3_ENDPOINT="${DUCKEXT_S3_ENDPOINT:-localhost:9000}" PYHOGLAKE_DIR="${PYHOGLAKE_DIR:-$HOME/src/hoglake/pyhoglake}" echo "== fixtures (pyhoglake) ==" +(cd "$PYHOGLAKE_DIR" && HOGLAKE_S3_ENDPOINT="http://$DUCKEXT_S3_ENDPOINT" \ + uv run python "$HERE/test/fixtures/packed_format_fixture.py") (cd "$PYHOGLAKE_DIR" && HOGLAKE_S3_ENDPOINT="http://$DUCKEXT_S3_ENDPOINT" \ uv run python "$HERE/test/fixtures/read_fixture.py") diff --git a/duckdb-client/test/sql/hoglake_packed_format.test b/duckdb-client/test/sql/hoglake_packed_format.test new file mode 100644 index 00000000..81d2149d --- /dev/null +++ b/duckdb-client/test/sql/hoglake_packed_format.test @@ -0,0 +1,38 @@ +# Packed MergeTree is explicit unsupported input for this Parquet-only extension. +require-env HOGLAKE_URL + +require hoglake + +require parquet + +require httpfs + +require-env DUCKEXT_S3_ENDPOINT + +statement ok +CREATE SECRET duckext_minio (TYPE s3, KEY_ID 'hoglake', SECRET 'hoglake123', + ENDPOINT '{DUCKEXT_S3_ENDPOINT}', USE_SSL false, + URL_STYLE 'path') + +statement ok +ATTACH 'hoglake:duckext-packed' AS lake (ENDPOINT '{HOGLAKE_URL}') + +statement error +SELECT * FROM lake.ns1.packed_empty +---- +:.*DuckDB cannot read data format 'clickhouse-mergetree-packed'.* + +statement error +INSERT INTO lake.ns1.packed_empty VALUES (1) +---- +:.*DuckDB cannot write data format 'clickhouse-mergetree-packed'.* + +statement error +SELECT count(*) FROM lake.ns1.packed_registered +---- +:.*DuckDB cannot read data format 'clickhouse-mergetree-packed'.* + +query I +SELECT 42 +---- +42 diff --git a/hedgerow/src/hedgerow/daemon.py b/hedgerow/src/hedgerow/daemon.py index 14877231..34fb3a74 100644 --- a/hedgerow/src/hedgerow/daemon.py +++ b/hedgerow/src/hedgerow/daemon.py @@ -57,8 +57,6 @@ import pyarrow as pa import pyarrow.parquet as pq -from pyhoglake.types import columns_to_arrow_schema - from pyhoglake import ( AlreadyExistsError, CommitConflictError, @@ -70,6 +68,7 @@ ValidationError, ) from pyhoglake import IncarnationChangedError as ClientIncarnationChangedError +from pyhoglake.types import columns_to_arrow_schema from .config import HedgerowConfig from .filtering import RowFilter, build_filter diff --git a/hedgerow/src/hedgerow/ingestion.py b/hedgerow/src/hedgerow/ingestion.py index ba22de5a..b11365d8 100644 --- a/hedgerow/src/hedgerow/ingestion.py +++ b/hedgerow/src/hedgerow/ingestion.py @@ -12,7 +12,6 @@ from tempfile import TemporaryDirectory import pyarrow.parquet as pq - from pyhoglake import ReconciliationRequiredError from .buffering import BufferPolicy diff --git a/hedgerow/tests/fakes.py b/hedgerow/tests/fakes.py index 48141940..91e5be48 100644 --- a/hedgerow/tests/fakes.py +++ b/hedgerow/tests/fakes.py @@ -18,7 +18,6 @@ from typing import Any import pyarrow as pa - from pyhoglake import ( ChangesPlan, Column, diff --git a/hedgerow/tests/test_daemon_unit.py b/hedgerow/tests/test_daemon_unit.py index 0330a2f3..37d0ae87 100644 --- a/hedgerow/tests/test_daemon_unit.py +++ b/hedgerow/tests/test_daemon_unit.py @@ -18,6 +18,7 @@ delete_file, table_batch_reader, ) +from pyhoglake import NotFoundError from hedgerow import ( DeletesPresentError, @@ -33,7 +34,6 @@ SourceConfig, UnsupportedFormatError, ) -from pyhoglake import NotFoundError SRC_COLS = ( col("id", "long", 1, 0, nullable=False), diff --git a/hedgerow/tests/test_events_discovery.py b/hedgerow/tests/test_events_discovery.py index 9ae9df89..061fe7be 100644 --- a/hedgerow/tests/test_events_discovery.py +++ b/hedgerow/tests/test_events_discovery.py @@ -3,14 +3,6 @@ import pyarrow as pa import pyarrow.parquet as pq import pytest -from hedgerow.discovery import discover_window -from hedgerow.events import EventTransform -from hedgerow.halts import ( - DataIntegrityError, - SchemaMismatchError, - UnsupportedFormatError, -) -from hedgerow.pending import PendingStore from pyhoglake.models import ( ChangesPlan, Column, @@ -22,6 +14,15 @@ TableInfo, ) +from hedgerow.discovery import discover_window +from hedgerow.events import EventTransform +from hedgerow.halts import ( + DataIntegrityError, + SchemaMismatchError, + UnsupportedFormatError, +) +from hedgerow.pending import PendingStore + def layout(): columns = tuple( diff --git a/pyhoglake/README.md b/pyhoglake/README.md index 21d9d881..754479d9 100644 --- a/pyhoglake/README.md +++ b/pyhoglake/README.md @@ -1,10 +1,11 @@ # pyhoglake Python client for [hoglake](https://github.com/PostHog/hoglake#readme), the Postgres-native -lakehouse-catalog control plane. A **thin API wrapper**: no embedded -engine, no SQL, no direct catalog-database access — ever. The client -writes parquet to object storage itself and registers it with the -control plane via footer-shipping commits. +lakehouse-catalog control plane. A **thin API wrapper**: no direct +catalog-database access. The standard writer path writes Parquet to +object storage and registers it with the control plane via footer-shipping +commits. The optional packed MergeTree adapter invokes a configured +`clickhouse local` executable; ClickHouse remains outside the library. ## Install @@ -106,6 +107,54 @@ for s in catalog.snapshots(before=head + 1): # with non-zero after) ``` +## Packed MergeTree adapter + +`ClickHousePackedAdapter` is the explicit writer and reader for tables created with +`write.format.default=clickhouse-mergetree-packed`: + +```python +import pyarrow as pa +from pyhoglake import ClickHousePackedAdapter + +packed = ns.create_table( + "packed_events", + pa.schema( + [ + pa.field("id", pa.int64(), nullable=False), + pa.field("name", pa.string()), + ] + ), + properties={"write.format.default": "clickhouse-mergetree-packed"}, +) +adapter = ClickHousePackedAdapter("/usr/local/bin/clickhouse") +adapter.append(packed, pa.table({"id": [1, 2], "name": ["a", "b"]})) +rows = adapter.read(packed, snapshot=catalog.refresh().head_snapshot_id) +``` + +For durable publication, call `prepare_append`, persist the returned JSON payload, then call +`commit_prepared` with that exact payload. Do not call `prepare_append` again with the same +idempotency key; replay `commit_prepared` with the persisted payload. Preparation creates one +MergeTree part in an isolated local directory, refuses any layout except one `data.packed` file, +claims a fresh `.packed` object +path from Hoglake, uploads it, and includes Arrow-derived Iceberg bounds in the registration. +`abandon_prepared` fences a payload that is known not to have committed. Do not abandon after an +unknown commit outcome; replay the exact payload instead. + +Reads request one exact Hoglake scan plan, download only those registered objects, attach them to an +isolated local table, enable ClickHouse `table_readonly`, and return an Arrow table. They never list +or scan an object-storage prefix. The adapter currently supports boolean, signed and unsigned integer, +float, double, string, binary, date, and second/millisecond/microsecond/nanosecond timestamp columns. +Date and timestamp values must also fit the configured ClickHouse version's `Date32`/`DateTime64` +ranges; the adapter does not widen those engine domains. Schemas are fixed. Partition specs, sort +orders, deletion vectors, explicit row IDs, packed-part +compaction, and mixed-format tables are refused. The writer and reader should use the same ClickHouse +version; no cross-version compatibility or remote-read performance claim is made. + +The adapter also bounds each part, part count and total registered bytes in a read, ClickHouse +memory, result bytes, worker threads, and process time. Constructor arguments can lower or raise those limits for a +known workload. The ordinary `Table.append` and `prepare_append_*` methods remain Parquet-only and +reject packed tables before writing an object. + ## Configuration | What | How | diff --git a/pyhoglake/src/pyhoglake/__init__.py b/pyhoglake/src/pyhoglake/__init__.py index be5a91cd..df81bdb0 100644 --- a/pyhoglake/src/pyhoglake/__init__.py +++ b/pyhoglake/src/pyhoglake/__init__.py @@ -29,6 +29,11 @@ UnsupportedTypeError, ValidationError, ) +from .formats import ( + CLICKHOUSE_MERGETREE_PACKED_FORMAT, + PARQUET_FORMAT, + TABLE_FORMAT_PROPERTY, +) from .models import ( AppendedFile, AppendResult, @@ -53,6 +58,7 @@ ViewInfo, ) from .ops import AlterOp +from .packed import ClickHousePackedAdapter from .types import arrow_type_to_coltype, coltype_to_arrow # Read from the installed metadata so it cannot drift from pyproject.toml. @@ -68,6 +74,9 @@ logging.getLogger("pyhoglake").addHandler(logging.NullHandler()) __all__ = [ + "CLICKHOUSE_MERGETREE_PACKED_FORMAT", + "PARQUET_FORMAT", + "TABLE_FORMAT_PROPERTY", "UNGUARDED", "AlreadyExistsError", "AlterOp", @@ -78,6 +87,7 @@ "CatalogOptions", "ChangesPlan", "CleanupResult", + "ClickHousePackedAdapter", "Column", "ColumnStats", "CommitConflictError", diff --git a/pyhoglake/src/pyhoglake/client.py b/pyhoglake/src/pyhoglake/client.py index 83975cda..0216473e 100644 --- a/pyhoglake/src/pyhoglake/client.py +++ b/pyhoglake/src/pyhoglake/client.py @@ -1,8 +1,8 @@ """The hoglake client: a thin wrapper over the control-plane REST API. -Data never flows through the server: ``Table.append`` writes parquet to -object storage itself (pyarrow S3FileSystem) and registers the file with -footer-derived stats via the commit endpoint (footer-shipping commits). +Data never flows through the server: ``Table.append`` writes Parquet to +object storage itself and registers footer-derived stats. Packed MergeTree +tables use the separate :mod:`pyhoglake.packed` adapter. """ from __future__ import annotations @@ -38,6 +38,7 @@ ReconciliationRequiredError, ValidationError, ) +from .formats import require_parquet from .models import ( AppendedFile, AppendResult, @@ -847,17 +848,29 @@ def commit_prepared( _uuid.UUID(payload["idempotency_key"]) return self._commit(payload, prepared=True, table=table) + def _commit_uploads( + self, payload: dict[str, Any], *, table: Table | None = None + ) -> CommitResult: + """Publish an exact request whose object paths have durable upload claims.""" + if not payload.get("idempotency_key") or payload.get("read_snapshot") is None: + raise ValueError( + "claimed upload commits require idempotency_key and read_snapshot" + ) + _uuid.UUID(payload["idempotency_key"]) + return self._commit(payload, table=table, endpoint="/commit/uploads") + def _commit( self, payload: dict[str, Any], *, prepared: bool = False, table: Table | None = None, + endpoint: str | None = None, ) -> CommitResult: try: body = self._client._request( "POST", - self._path("/commit/prepared" if prepared else "/commit"), + self._path(endpoint or ("/commit/prepared" if prepared else "/commit")), json=payload, conflict=CommitConflictError, ) @@ -969,12 +982,24 @@ def __repr__(self) -> str: # pragma: no cover # -- tables ------------------------------------------------------------ - def create_table(self, name: str, schema: pa.Schema) -> Table: + def create_table( + self, + name: str, + schema: pa.Schema, + *, + properties: Mapping[str, str] | None = None, + ) -> Table: _check_reserved_columns(schema) + request: dict[str, Any] = { + "name": name, + "columns": schema_to_column_defs(schema), + } + if properties is not None: + request["properties"] = dict(properties) body = self._catalog._client._request( "POST", self._path("/tables"), - json={"name": name, "columns": schema_to_column_defs(schema)}, + json=request, ) return Table(self, TableInfo.from_wire(body)) @@ -1110,7 +1135,7 @@ def comment(self) -> str | None: @property def properties(self) -> dict[str, str] | None: - """The table's user properties; None when none are set.""" + """The table's versioned properties; None when none are set.""" return self._info.properties def _path(self, suffix: str = "") -> str: @@ -1362,6 +1387,7 @@ def append( """ catalog = self._namespace._catalog client = catalog._client + require_parquet(self._info.properties, "Table.append") # Reserved-prefix fast-fail before ANY request or upload: a user # `_hog*` field could otherwise reach parquet on a pre-reservation @@ -1676,6 +1702,7 @@ def _prepare_append( loop this replaced interleaved them), so a refusal orphans nothing and the fan-out is handed work already known to be good. """ + require_parquet(self._info.properties, "Table.prepare_append_files") uploaded: list[str] = [] try: _uuid.UUID(idempotency_key) @@ -1696,6 +1723,7 @@ def _prepare_append( # _prepared_read). read_snapshot = catalog.refresh().head_snapshot_id info = self._check_incarnation(expected) + require_parquet(info.properties, "Table.prepare_append_files") if expected_table_info is not None and ( info.columns, info.partition_spec, diff --git a/pyhoglake/src/pyhoglake/formats.py b/pyhoglake/src/pyhoglake/formats.py new file mode 100644 index 00000000..ffd0a034 --- /dev/null +++ b/pyhoglake/src/pyhoglake/formats.py @@ -0,0 +1,22 @@ +from __future__ import annotations + +from collections.abc import Mapping + +from .errors import ValidationError + +TABLE_FORMAT_PROPERTY = "write.format.default" +PARQUET_FORMAT = "parquet" +CLICKHOUSE_MERGETREE_PACKED_FORMAT = "clickhouse-mergetree-packed" + + +def table_format(properties: Mapping[str, str] | None) -> str: + return (properties or {}).get(TABLE_FORMAT_PROPERTY, PARQUET_FORMAT) + + +def require_parquet(properties: Mapping[str, str] | None, operation: str) -> None: + file_format = table_format(properties) + if file_format != PARQUET_FORMAT: + raise ValidationError( + f"{operation} supports Parquet tables only; table format is {file_format!r}", + status_code=None, + ) diff --git a/pyhoglake/src/pyhoglake/models.py b/pyhoglake/src/pyhoglake/models.py index ecf2f29b..8e4022b2 100644 --- a/pyhoglake/src/pyhoglake/models.py +++ b/pyhoglake/src/pyhoglake/models.py @@ -346,7 +346,7 @@ class TableInfo: snapshot_id: int | None = None #: Versioned table comment; None when the table has none. comment: str | None = None - #: Inert user metadata; None when no properties are set. + #: Versioned table properties; None when no properties are set. properties: dict[str, str] | None = None #: The snapshot the three totals above are exact AS OF, when they are #: a SAMPLE; None when they are exact (a time-travel read, a DDL @@ -460,6 +460,7 @@ class DataFile: footer_size: int | None = None spec_id: int | None = None partition_values: tuple[str | None, ...] | None = None + explicit_row_ids: bool = False @classmethod def from_wire(cls, d: dict[str, Any]) -> DataFile: diff --git a/pyhoglake/src/pyhoglake/packed.py b/pyhoglake/src/pyhoglake/packed.py new file mode 100644 index 00000000..f2e59425 --- /dev/null +++ b/pyhoglake/src/pyhoglake/packed.py @@ -0,0 +1,690 @@ +"""ClickHouse-local adapter for restricted single-object packed MergeTree parts.""" + +from __future__ import annotations + +import contextlib +import json +import re +import subprocess +import tempfile +import uuid +from collections.abc import Mapping, Sequence +from datetime import datetime +from pathlib import Path +from typing import Any + +import pyarrow as pa +import pyarrow.compute as pc + +from .bounds import encode_bound, normalize_bound +from .client import Table, _align_table +from .errors import HoglakeError, ValidationError +from .formats import CLICKHOUSE_MERGETREE_PACKED_FORMAT, table_format +from .models import ( + AppendedFile, + AppendResult, + Column, + ColumnStats, + CommitResult, + TableInfo, +) +from .upload import Upload, perform_upload, s3_key + +_SAFE_IDENTIFIER = re.compile(r"^[A-Za-z_][A-Za-z0-9_-]{0,127}$") + +_SUPPORTED_TYPES = frozenset( + { + "boolean", + "int8", + "int16", + "int", + "long", + "uint8", + "uint16", + "uint32", + "uint64", + "float", + "double", + "date", + "timestamp_s", + "timestamp_ms", + "timestamp", + "timestamp_ns", + "timestamptz", + "string", + "binary", + } +) + +_CLICKHOUSE_TYPES = { + "boolean": "Bool", + "int8": "Int8", + "int16": "Int16", + "int": "Int32", + "long": "Int64", + "uint8": "UInt8", + "uint16": "UInt16", + "uint32": "UInt32", + "uint64": "UInt64", + "float": "Float32", + "double": "Float64", + "date": "Date32", + "timestamp_s": "DateTime64(0, 'UTC')", + "timestamp_ms": "DateTime64(3, 'UTC')", + "timestamp": "DateTime64(6, 'UTC')", + "timestamp_ns": "DateTime64(9, 'UTC')", + "timestamptz": "DateTime64(6, 'UTC')", + "string": "String", + "binary": "String", +} + +_DEFAULT_MAX_PART_BYTES = 8 * 1024**3 +_DEFAULT_MAX_SNAPSHOT_BYTES = 64 * 1024**3 +_DEFAULT_MAX_SNAPSHOT_PARTS = 10_000 +_DEFAULT_MAX_MEMORY_BYTES = 2 * 1024**3 +_DEFAULT_MAX_RESULT_BYTES = 8 * 1024**3 + +_ARROW_TYPES: dict[str, pa.DataType] = { + "boolean": pa.bool_(), + "int8": pa.int8(), + "int16": pa.int16(), + "int": pa.int32(), + "long": pa.int64(), + "uint8": pa.uint8(), + "uint16": pa.uint16(), + "uint32": pa.uint32(), + "uint64": pa.uint64(), + "float": pa.float32(), + "double": pa.float64(), + "date": pa.date32(), + "timestamp_s": pa.timestamp("s"), + "timestamp_ms": pa.timestamp("ms"), + "timestamp": pa.timestamp("us"), + "timestamp_ns": pa.timestamp("ns"), + "timestamptz": pa.timestamp("us", tz="UTC"), + "string": pa.string(), + "binary": pa.binary(), +} + + +def _quote_identifier(value: str) -> str: + return "`" + value.replace("`", "``") + "`" + + +def _quote_string(value: str) -> str: + return "'" + value.replace("\\", "\\\\").replace("'", "\\'") + "'" + + +def _schema(info: TableInfo) -> pa.Schema: + fields: list[pa.Field] = [] + for column in info.columns: + if not _SAFE_IDENTIFIER.fullmatch(column.name): + raise ValidationError( + f"packed MergeTree requires a safe catalog column identifier, got {column.name!r}", + status_code=None, + ) + if column.children or column.type not in _SUPPORTED_TYPES: + raise ValidationError( + f"packed MergeTree does not support column {column.name!r} " + f"of type {column.type!r}", + status_code=None, + ) + fields.append( + pa.field(column.name, _ARROW_TYPES[column.type], nullable=column.nullable) + ) + return pa.schema(fields) + + +def _require_packed(info: TableInfo) -> None: + actual = table_format(info.properties) + if actual != CLICKHOUSE_MERGETREE_PACKED_FORMAT: + raise ValidationError( + "ClickHousePackedAdapter requires " + f"{CLICKHOUSE_MERGETREE_PACKED_FORMAT!r}, got {actual!r}", + status_code=None, + ) + if info.partition_spec is not None and info.partition_spec.fields: + raise ValidationError( + "packed MergeTree tables do not support partition specs", + status_code=None, + ) + if info.sort_spec is not None and info.sort_spec.fields: + raise ValidationError( + "packed MergeTree tables do not support sort orders", + status_code=None, + ) + _schema(info) + + +def _arrow_stream(table: pa.Table) -> bytes: + sink = pa.BufferOutputStream() + with pa.ipc.new_stream(sink, table.schema) as writer: + writer.write_table(table) + return sink.getvalue().to_pybytes() + + +def _min_max(array: pa.Array, column: Column) -> tuple[Any, Any] | None: + values = pc.drop_null(array) + if len(values) == 0: + return None + if column.type in {"float", "double"}: + values = pc.filter(values, pc.invert(pc.is_nan(values))) + if len(values) == 0: + return None + if column.type == "boolean": + any_true = bool(pc.any(values).as_py()) + all_true = bool(pc.all(values).as_py()) + return all_true, any_true + result = pc.min_max(values) + lower = result["min"] + upper = result["max"] + if column.type == "timestamp_ns": + return lower.value, upper.value + return lower.as_py(), upper.as_py() + + +def _column_stats(table: pa.Table, columns: tuple[Column, ...]) -> list[dict[str, Any]]: + stats: list[dict[str, Any]] = [] + for column in columns: + array = table.column(column.name).combine_chunks() + nan_count: int | None = None + if column.type in {"float", "double"}: + non_null = pc.drop_null(array) + nan_count = int(pc.sum(pc.is_nan(non_null)).as_py() or 0) + bounds = _min_max(array, column) + lower = upper = None + if bounds is not None: + lower = normalize_bound( + column.type, + encode_bound(column.type, bounds[0], column.type_params), + lower=True, + ) + upper = normalize_bound( + column.type, + encode_bound(column.type, bounds[1], column.type_params), + lower=False, + ) + stats.append( + ColumnStats( + field_id=column.field_id, + value_count=len(array), + null_count=array.null_count, + nan_count=nan_count, + lower_bound=lower, + upper_bound=upper, + ).to_wire() + ) + return stats + + +class ClickHousePackedAdapter: + """Produce and read packed parts with a trusted ``clickhouse local`` executable. + + The adapter downloads only paths returned by the requested Hoglake scan plan. + Each operation uses an isolated temporary ClickHouse data directory. + """ + + def __init__( + self, + executable: str | Path | Sequence[str] = "clickhouse", + *, + timeout: float = 120.0, + max_threads: int = 2, + max_part_bytes: int = _DEFAULT_MAX_PART_BYTES, + max_snapshot_bytes: int = _DEFAULT_MAX_SNAPSHOT_BYTES, + max_snapshot_parts: int = _DEFAULT_MAX_SNAPSHOT_PARTS, + max_memory_bytes: int = _DEFAULT_MAX_MEMORY_BYTES, + max_result_bytes: int = _DEFAULT_MAX_RESULT_BYTES, + ) -> None: + command: tuple[str, ...] + if isinstance(executable, (str, Path)): + command = (str(executable),) + else: + command = tuple(executable) + if not command or any(not part for part in command): + raise ValueError("executable must contain at least one non-empty argument") + if timeout <= 0: + raise ValueError("timeout must be positive") + if max_threads <= 0: + raise ValueError("max_threads must be positive") + limits = { + "max_part_bytes": max_part_bytes, + "max_snapshot_bytes": max_snapshot_bytes, + "max_snapshot_parts": max_snapshot_parts, + "max_memory_bytes": max_memory_bytes, + "max_result_bytes": max_result_bytes, + } + invalid = [name for name, value in limits.items() if value <= 0] + if invalid: + raise ValueError(f"{', '.join(invalid)} must be positive") + self._command = command + self._timeout = timeout + self._max_threads = max_threads + self._max_part_bytes = max_part_bytes + self._max_snapshot_bytes = max_snapshot_bytes + self._max_snapshot_parts = max_snapshot_parts + self._max_memory_bytes = max_memory_bytes + self._max_result_bytes = max_result_bytes + + def prepare_append( + self, + table: Table, + data: pa.Table, + *, + idempotency_key: str | None = None, + ) -> dict[str, Any]: + """Create, claim and upload one packed part, returning its exact commit payload.""" + if data.num_rows == 0: + raise ValidationError( + "packed MergeTree append requires at least one row", + status_code=None, + ) + operation = uuid.UUID(idempotency_key) if idempotency_key else uuid.uuid4() + catalog = table._namespace._catalog + expected_uuid = table.table_uuid + info = table._check_incarnation(expected_uuid) + _require_packed(info) + if info.read_snapshot_id is None: + raise HoglakeError( + "packed appends require a server that reports Table.read_snapshot_id" + ) + read_snapshot = info.read_snapshot_id + aligned = _align_table(data, _schema(info)) + claim_path: str | None = None + with tempfile.TemporaryDirectory(prefix="pyhoglake-packed-write-") as directory: + root = Path(directory) + self._run(root, self._create_sql(info, "packed_write")) + self._run( + root, + f"INSERT INTO {_quote_identifier('packed_write')} FORMAT ArrowStream", + input_bytes=_arrow_stream(aligned), + ) + part = self._single_part(root, "packed_write", aligned.num_rows) + packed_file = part / "data.packed" + packed_size = packed_file.stat().st_size + if packed_size > self._max_part_bytes: + raise ValidationError( + f"packed part is {packed_size} bytes, over max_part_bytes " + f"{self._max_part_bytes}", + status_code=None, + ) + stats = _column_stats(aligned, info.columns) + body = { + "owner": str(operation), + "prefix": ( + f"{catalog.data_path.rstrip('/')}/data/{info.namespace}/" + f"{info.name}/{operation}" + ), + "file_kind": "data", + "file_format": CLICKHOUSE_MERGETREE_PACKED_FORMAT, + } + claim = catalog._client._request( + "PUT", + catalog._path(f"/uploads/{uuid.uuid4()}"), + json=body, + ) + claim_path = str(claim["path"]) + try: + self._upload(table, packed_file, claim_path) + except BaseException: + self._abandon(table, operation, [claim_path], best_effort=True) + raise + registration = { + "path": claim_path, + "file_format": CLICKHOUSE_MERGETREE_PACKED_FORMAT, + "record_count": aligned.num_rows, + "file_size_bytes": packed_size, + "column_stats": stats, + } + return { + "idempotency_key": str(operation), + "read_snapshot": read_snapshot, + "appends": [ + { + "namespace": table.namespace, + "table": table.name, + "expected_table_uuid": expected_uuid, + "files": [registration], + } + ], + } + + def commit_prepared(self, table: Table, payload: dict[str, Any]) -> CommitResult: + """Publish the exact payload returned by :meth:`prepare_append`.""" + self._validate_payload(table, payload) + return table._namespace._catalog._commit_uploads(payload, table=table) + + def append( + self, + table: Table, + data: pa.Table, + *, + idempotency_key: str | None = None, + ) -> AppendResult: + """Prepare, upload and publish one packed part.""" + payload = self.prepare_append(table, data, idempotency_key=idempotency_key) + try: + result = self.commit_prepared(table, payload) + except BaseException as error: + with contextlib.suppress(AttributeError, TypeError): + error.prepared_payload = payload # type: ignore[attr-defined] + raise + file = payload["appends"][0]["files"][0] + return AppendResult( + snapshot_id=result.snapshot_id, + schema_version=result.schema_version, + files=( + AppendedFile( + path=file["path"], + record_count=file["record_count"], + partition_values=None, + ), + ), + ) + + def abandon_prepared(self, table: Table, payload: Mapping[str, Any]) -> int: + """Fence an uncommitted prepared upload so normal cleanup can reclaim it.""" + self._validate_payload(table, payload) + owner = uuid.UUID(str(payload["idempotency_key"])) + paths = [ + str(file["path"]) + for append in payload["appends"] + for file in append["files"] + ] + return self._abandon(table, owner, paths) + + def read( + self, + table: Table, + *, + snapshot: int | None = None, + at_timestamp: datetime | str | None = None, + ) -> pa.Table: + """Read exactly the packed parts in one Hoglake scan plan.""" + info = table.info(snapshot=snapshot, at_timestamp=at_timestamp, totals=False) + _require_packed(info) + schema = _schema(info) + if info.read_snapshot_id is None: + raise HoglakeError( + "packed reads require a server that reports Table.read_snapshot_id" + ) + plan = table.scan_plan(snapshot=info.read_snapshot_id) + if len(plan) > self._max_snapshot_parts: + raise ValidationError( + f"packed snapshot has {len(plan)} parts, over max_snapshot_parts " + f"{self._max_snapshot_parts}", + status_code=None, + ) + for item in plan: + file = item.data_file + if file.file_format != CLICKHOUSE_MERGETREE_PACKED_FORMAT: + raise ValidationError( + f"snapshot contains unsupported data format {file.file_format!r}", + status_code=None, + ) + if item.delete_file is not None: + raise ValidationError( + "packed MergeTree reads do not support deletion vectors", + status_code=None, + ) + if file.explicit_row_ids: + raise ValidationError( + "packed MergeTree reads do not support explicit row-id files", + status_code=None, + ) + if file.file_size_bytes > self._max_part_bytes: + raise ValidationError( + f"packed part {file.path} is {file.file_size_bytes} bytes, over " + f"max_part_bytes {self._max_part_bytes}", + status_code=None, + ) + snapshot_bytes = sum(item.data_file.file_size_bytes for item in plan) + if snapshot_bytes > self._max_snapshot_bytes: + raise ValidationError( + f"packed snapshot is {snapshot_bytes} bytes, over max_snapshot_bytes " + f"{self._max_snapshot_bytes}", + status_code=None, + ) + if not plan: + return pa.Table.from_batches([], schema=schema) + + with tempfile.TemporaryDirectory(prefix="pyhoglake-packed-read-") as directory: + root = Path(directory) + self._run(root, self._create_sql(info, "packed_read")) + data_path = ( + self._run( + root, + "SELECT arrayJoin(data_paths) FROM system.tables " + "WHERE name='packed_read' FORMAT TSVRaw", + ) + .decode("utf-8") + .strip() + ) + table_path = Path(data_path).resolve() + try: + table_path.relative_to(root.resolve()) + except ValueError as error: + raise HoglakeError( + "ClickHouse table path escaped its isolated directory" + ) from error + detached = table_path / "detached" + detached.mkdir(parents=True, exist_ok=True) + for index, item in enumerate(plan, start=1): + destination = detached / f"all_{index}_{index}_0" + destination.mkdir() + self._download( + table, + item.data_file.path, + destination / "data.packed", + item.data_file.file_size_bytes, + ) + actual = (destination / "data.packed").stat().st_size + if actual != item.data_file.file_size_bytes: + raise HoglakeError( + f"downloaded packed part size {actual} does not match registered " + f"size {item.data_file.file_size_bytes} for {item.data_file.path}" + ) + attach = ";".join( + f"ALTER TABLE {_quote_identifier('packed_read')} ATTACH PART " + f"'all_{index}_{index}_0'" + for index in range(1, len(plan) + 1) + ) + self._run( + root, + attach + + ";ALTER TABLE " + + _quote_identifier("packed_read") + + " MODIFY SETTING table_readonly=1", + ) + columns = ", ".join(_quote_identifier(field.name) for field in schema) + raw = self._run( + root, + f"SELECT {columns} FROM {_quote_identifier('packed_read')} " + "ORDER BY _part, _part_offset FORMAT ArrowStream", + ) + with pa.ipc.open_stream(raw) as reader: + result = reader.read_all() + try: + result = result.cast(schema) + except (pa.ArrowInvalid, pa.ArrowNotImplementedError) as error: + raise HoglakeError( + f"ClickHouse returned an Arrow schema incompatible with the packed table: {error}" + ) from error + expected_rows = sum(item.data_file.record_count for item in plan) + if result.num_rows != expected_rows: + raise HoglakeError( + f"ClickHouse returned {result.num_rows} rows for parts registered with " + f"{expected_rows} rows" + ) + return result + + def _create_sql(self, info: TableInfo, name: str) -> str: + _require_packed(info) + columns = [] + for column in info.columns: + clickhouse_type = _CLICKHOUSE_TYPES[column.type] + if column.nullable: + clickhouse_type = f"Nullable({clickhouse_type})" + columns.append(f"{_quote_identifier(column.name)} {clickhouse_type}") + return ( + f"CREATE TABLE {_quote_identifier(name)} ({', '.join(columns)}) " + "ENGINE=MergeTree ORDER BY tuple() SETTINGS " + "min_bytes_for_wide_part=0, min_rows_for_wide_part=0, " + "min_bytes_for_full_part_storage=1000000000000000000, " + "min_rows_for_full_part_storage=1000000000000000000, " + "max_bytes_to_merge_at_max_space_in_pool=0" + ) + + def _single_part(self, root: Path, table: str, expected_rows: int) -> Path: + raw = self._run( + root, + "SELECT name, path, rows FROM system.parts " + f"WHERE active AND table={_quote_string(table)} FORMAT JSONEachRow", + ) + rows = [json.loads(line) for line in raw.splitlines() if line] + if len(rows) != 1 or int(rows[0]["rows"]) != expected_rows: + raise HoglakeError( + f"ClickHouse produced {len(rows)} active parts for {expected_rows} rows" + ) + part = Path(rows[0]["path"]).resolve() + try: + part.relative_to(root.resolve()) + except ValueError as error: + raise HoglakeError( + "ClickHouse part path escaped its isolated directory" + ) from error + entries = list(part.iterdir()) + if ( + len(entries) != 1 + or entries[0].name != "data.packed" + or not entries[0].is_file() + or entries[0].is_symlink() + ): + names = sorted(entry.name for entry in entries) + raise HoglakeError( + "ClickHouse did not produce the restricted single-file packed layout; " + f"part entries were {names}" + ) + return part + + def _run(self, root: Path, sql: str, *, input_bytes: bytes | None = None) -> bytes: + command = [ + *self._command, + "local", + "--path", + str(root), + "--background_schedule_pool_size", + str(self._max_threads), + "--max_threads", + str(self._max_threads), + "--max_memory_usage", + str(self._max_memory_bytes), + "--max_result_bytes", + str(self._max_result_bytes), + "--result_overflow_mode", + "throw", + "--output_format_arrow_string_as_string", + "0", + "--query", + sql, + ] + try: + completed = subprocess.run( + command, + input=input_bytes, + capture_output=True, + check=False, + timeout=self._timeout, + ) + except subprocess.TimeoutExpired as error: + raise HoglakeError( + f"clickhouse local exceeded the {self._timeout:g}s timeout" + ) from error + except OSError as error: + raise HoglakeError( + f"could not execute clickhouse local: {error}" + ) from error + if completed.returncode != 0: + detail = completed.stderr.decode("utf-8", errors="replace").strip() + raise HoglakeError( + "clickhouse local failed" + (f": {detail[:2000]}" if detail else "") + ) + return completed.stdout + + @staticmethod + def _upload(table: Table, source: Path, uri: str) -> None: + client = table._namespace._catalog._client + perform_upload( + client._filesystem(), + client._put_client(1), + Upload(uri=uri, size=source.stat().st_size, path=str(source)), + ) + + @staticmethod + def _download( + table: Table, + uri: str, + destination: Path, + expected_size: int, + ) -> None: + filesystem = table._namespace._catalog._client._filesystem() + copied = 0 + with ( + filesystem.open_input_stream(s3_key(uri)) as source, + destination.open("wb") as sink, + ): + while copied <= expected_size: + chunk = source.read(min(8 * 1024 * 1024, expected_size + 1 - copied)) + if not chunk: + break + sink.write(chunk) + copied += len(chunk) + if copied > expected_size: + destination.unlink(missing_ok=True) + raise HoglakeError( + f"packed object {uri} exceeds its registered size {expected_size}" + ) + + @staticmethod + def _abandon( + table: Table, + owner: uuid.UUID, + paths: list[str], + *, + best_effort: bool = False, + ) -> int: + if not paths: + return 0 + catalog = table._namespace._catalog + try: + body = catalog._client._request( + "POST", + catalog._path("/uploads/abandon"), + json={"owner": str(owner), "paths": paths}, + ) + except HoglakeError: + if best_effort: + return 0 + raise + return int(body["abandoned"]) + + @staticmethod + def _validate_payload(table: Table, payload: Mapping[str, Any]) -> None: + try: + operation = uuid.UUID(str(payload["idempotency_key"])) + appends = payload["appends"] + append = appends[0] + files = append["files"] + except (KeyError, IndexError, TypeError, ValueError) as error: + raise ValueError("invalid packed prepared payload") from error + if ( + len(appends) != 1 + or len(files) != 1 + or append.get("namespace") != table.namespace + or append.get("table") != table.name + or append.get("expected_table_uuid") != table.table_uuid + or files[0].get("file_format") != CLICKHOUSE_MERGETREE_PACKED_FORMAT + or payload.get("read_snapshot") is None + or str(operation) != str(payload["idempotency_key"]) + ): + raise ValueError("invalid packed prepared payload") diff --git a/pyhoglake/tests/test_metadata_parity.py b/pyhoglake/tests/test_metadata_parity.py index b3e850b9..2867f877 100644 --- a/pyhoglake/tests/test_metadata_parity.py +++ b/pyhoglake/tests/test_metadata_parity.py @@ -192,3 +192,25 @@ def test_totals_false_is_a_documented_parameter(): "the writer path sends ?totals=false on every table read; the spec " "does not document the parameter on GET /tables/{table}" ) + + +_FILE_FORMATS_REL = Path( + "server/src/main/kotlin/com/posthog/hoglake/model/FileFormats.kt" +) + + +def test_packed_type_subset_matches_server(): + from pyhoglake.packed import _SUPPORTED_TYPES + + source = _server_file(_FILE_FORMATS_REL).read_text() + block = re.search( + r"val packedColumnTypes: Set =\s*setOf\((.*?)\n\s*\)", + source, + re.DOTALL, + ) + assert block, "packedColumnTypes block not found in FileFormats.kt" + server = { + name.lower() if name != "UUID_T" else "uuid" + for name in re.findall(r"ColType\.([A-Z0-9_]+)", block.group(1)) + } + assert _SUPPORTED_TYPES == server diff --git a/pyhoglake/tests/test_packed.py b/pyhoglake/tests/test_packed.py new file mode 100644 index 00000000..ebae4b5d --- /dev/null +++ b/pyhoglake/tests/test_packed.py @@ -0,0 +1,574 @@ +import io +import json +import os +import re +from datetime import UTC, date, datetime +from pathlib import Path + +import httpx +import pyarrow as pa +import pytest + +from pyhoglake import ( + CLICKHOUSE_MERGETREE_PACKED_FORMAT, + CatalogInfo, + ClickHousePackedAdapter, + HoglakeClient, + TableInfo, + ValidationError, +) +from pyhoglake.client import Catalog, Namespace, Table +from pyhoglake.models import Column +from pyhoglake.packed import _column_stats + +BASE = "http://hog.test" +CATALOG_WIRE = { + "name": "cat", + "data_path": "s3://bkt/lake", + "head_snapshot_id": 5, + "schema_version": 1, +} + + +class _Output(io.BytesIO): + def __init__(self, files: dict[str, bytes], key: str): + super().__init__() + self._files = files + self._key = key + + def close(self) -> None: + if not self.closed: + self._files[self._key] = self.getvalue() + super().close() + + +class FakeS3: + def __init__(self, *, fail_upload: bool = False) -> None: + self.files: dict[str, bytes] = {} + self.fail_upload = fail_upload + + def filesystem(self): + return self + + def open_output_stream(self, key: str): + if self.fail_upload: + raise OSError(f"object store refused {key}") + return _Output(self.files, key) + + def open_input_stream(self, key: str): + return io.BytesIO(self.files[key]) + + +def _columns() -> tuple[Column, ...]: + definitions = [ + ("flag", "boolean", True), + ("i8", "int8", False), + ("i16", "int16", True), + ("i32", "int", True), + ("i64", "long", True), + ("u8", "uint8", True), + ("u16", "uint16", True), + ("u32", "uint32", True), + ("u64", "uint64", True), + ("f32", "float", True), + ("f64", "double", True), + ("text", "string", True), + ("payload", "binary", True), + ("day", "date", True), + ("ts_s", "timestamp_s", True), + ("ts_ms", "timestamp_ms", True), + ("ts_us", "timestamp", True), + ("ts_ns", "timestamp_ns", True), + ("ts_tz", "timestamptz", True), + ] + return tuple( + Column( + name=name, + type=type_, + field_id=index, + ordinal=index - 1, + nullable=nullable, + ) + for index, (name, type_, nullable) in enumerate(definitions, start=1) + ) + + +def _table_wire(columns: tuple[Column, ...] | None = None) -> dict: + cols = columns or _columns() + return { + "name": "events", + "namespace": "ns1", + "table_uuid": "0b8ee9ba-79a1-4f3e-b7e5-6a0b6ab6f012", + "columns": [ + { + "name": column.name, + "type": column.type, + "field_id": column.field_id, + "ordinal": column.ordinal, + "nullable": column.nullable, + } + for column in cols + ], + "properties": {"write.format.default": CLICKHOUSE_MERGETREE_PACKED_FORMAT}, + "record_count": 0, + "file_count": 0, + "file_size_bytes": 0, + "read_snapshot_id": 5, + } + + +def _table(fake_s3: FakeS3, columns: tuple[Column, ...] | None = None) -> Table: + client = HoglakeClient(BASE, s3=fake_s3) + catalog = Catalog(client, CatalogInfo.from_wire(CATALOG_WIRE)) + return Table(Namespace(catalog, "ns1"), TableInfo.from_wire(_table_wire(columns))) + + +def _data() -> pa.Table: + columns = _columns() + fields = [] + arrays = [] + values = { + "flag": [True, None], + "i8": [-8, 8], + "i16": [-16, None], + "i32": [-32, 32], + "i64": [-64, 64], + "u8": [8, None], + "u16": [16, 17], + "u32": [2**32 - 1, 1], + "u64": [2**64 - 1, 1], + "f32": [1.5, -2.5], + "f64": [2.5, -0.0], + "text": ["hé", None], + "payload": [b"\x00\xff", b"x"], + "day": [date(2026, 10, 3), None], + "ts_s": [datetime(2026, 10, 3, 1, 2, 3), None], + "ts_ms": [datetime(2026, 10, 3, 1, 2, 3, 456000), None], + "ts_us": [datetime(2026, 10, 3, 1, 2, 3, 456789), None], + "ts_tz": [datetime(2026, 10, 3, 1, 2, 3, 456789, tzinfo=UTC), None], + } + arrow_types = { + "boolean": pa.bool_(), + "int8": pa.int8(), + "int16": pa.int16(), + "int": pa.int32(), + "long": pa.int64(), + "uint8": pa.uint8(), + "uint16": pa.uint16(), + "uint32": pa.uint32(), + "uint64": pa.uint64(), + "float": pa.float32(), + "double": pa.float64(), + "string": pa.string(), + "binary": pa.binary(), + "date": pa.date32(), + "timestamp_s": pa.timestamp("s"), + "timestamp_ms": pa.timestamp("ms"), + "timestamp": pa.timestamp("us"), + "timestamp_ns": pa.timestamp("ns"), + "timestamptz": pa.timestamp("us", tz="UTC"), + } + for column in columns: + type_ = arrow_types[column.type] + fields.append(pa.field(column.name, type_, nullable=column.nullable)) + if column.name == "ts_ns": + arrays.append(pa.array([1790992923456789123, None], type=type_)) + else: + arrays.append(pa.array(values[column.name], type=type_)) + return pa.Table.from_arrays(arrays, schema=pa.schema(fields)) + + +def test_normal_parquet_append_refuses_a_packed_table_before_io() -> None: + fake_s3 = FakeS3() + table = _table(fake_s3, (_columns()[4],)) + with pytest.raises(ValidationError, match="Parquet tables only"): + table.append(pa.table({"i64": [1]})) + with pytest.raises(ValidationError, match="Parquet tables only"): + table.prepare_append_files( + [], idempotency_key="00000000-0000-0000-0000-000000000001" + ) + with pytest.raises(ValidationError, match="Parquet tables only"): + table.prepare_append_tables( + [], idempotency_key="00000000-0000-0000-0000-000000000002" + ) + assert fake_s3.files == {} + table._namespace._catalog._client.close() + + +def test_create_table_sends_storage_properties(httpx_mock) -> None: + client = HoglakeClient(BASE) + catalog = Catalog(client, CatalogInfo.from_wire(CATALOG_WIRE)) + httpx_mock.add_response( + method="POST", + url=f"{BASE}/v1/catalogs/cat/namespaces/ns1/tables", + json=_table_wire((_columns()[4],)), + ) + Namespace(catalog, "ns1").create_table( + "events", + pa.schema([pa.field("i64", pa.int64())]), + properties={"write.format.default": CLICKHOUSE_MERGETREE_PACKED_FORMAT}, + ) + body = json.loads(httpx_mock.get_requests()[-1].content) + assert body["properties"] == { + "write.format.default": CLICKHOUSE_MERGETREE_PACKED_FORMAT + } + client.close() + + +def test_head_read_pins_scan_to_the_table_info_snapshot(httpx_mock) -> None: + fake_s3 = FakeS3() + table = _table(fake_s3, (_columns()[4],)) + table_url = f"{BASE}/v1/catalogs/cat/namespaces/ns1/tables/events" + httpx_mock.add_response( + method="GET", + url=f"{table_url}?totals=false", + json=_table_wire((_columns()[4],)), + ) + httpx_mock.add_response( + method="GET", + url=f"{table_url}/scan?snapshot=5", + json=[], + ) + + result = ClickHousePackedAdapter("unused").read(table) + assert result.num_rows == 0 + assert result.schema == pa.schema([pa.field("i64", pa.int64())]) + table._namespace._catalog._client.close() + + +def test_explicit_row_id_file_is_refused_from_wire(httpx_mock) -> None: + fake_s3 = FakeS3() + table = _table(fake_s3, (_columns()[4],)) + table_url = f"{BASE}/v1/catalogs/cat/namespaces/ns1/tables/events" + httpx_mock.add_response( + method="GET", + url=f"{table_url}?totals=false", + json=_table_wire((_columns()[4],)), + ) + httpx_mock.add_response( + method="GET", + url=f"{table_url}/scan?snapshot=5", + json=[ + { + "data_file": { + "data_file_id": 1, + "path": "s3://bkt/lake/data.packed", + "file_format": CLICKHOUSE_MERGETREE_PACKED_FORMAT, + "record_count": 1, + "file_size_bytes": 1, + "row_id_start": 0, + "stats_state": "provided", + "begin_snapshot": 5, + "explicit_row_ids": True, + } + } + ], + ) + + with pytest.raises(ValidationError, match="explicit row-id"): + ClickHousePackedAdapter("unused").read(table) + table._namespace._catalog._client.close() + + +def test_snapshot_part_count_is_bounded_before_download(httpx_mock) -> None: + fake_s3 = FakeS3() + table = _table(fake_s3, (_columns()[4],)) + table_url = f"{BASE}/v1/catalogs/cat/namespaces/ns1/tables/events" + httpx_mock.add_response( + method="GET", + url=f"{table_url}?totals=false", + json=_table_wire((_columns()[4],)), + ) + files = [] + for file_id in (1, 2): + files.append( + { + "data_file": { + "data_file_id": file_id, + "path": f"s3://bkt/lake/{file_id}.packed", + "file_format": CLICKHOUSE_MERGETREE_PACKED_FORMAT, + "record_count": 1, + "file_size_bytes": 1, + "row_id_start": file_id - 1, + "stats_state": "provided", + "begin_snapshot": 5, + } + } + ) + httpx_mock.add_response( + method="GET", + url=f"{table_url}/scan?snapshot=5", + json=files, + ) + + with pytest.raises(ValidationError, match="max_snapshot_parts"): + ClickHousePackedAdapter("unused", max_snapshot_parts=1).read(table) + assert fake_s3.files == {} + table._namespace._catalog._client.close() + + +def test_failed_upload_is_abandoned(httpx_mock) -> None: + class LocalPartAdapter(ClickHousePackedAdapter): + def _run(self, root, sql, *, input_bytes=None): + if sql.startswith("INSERT INTO"): + part = root / "part" + part.mkdir() + (part / "data.packed").write_bytes(b"packed") + return b"" + + def _single_part(self, root, table, expected_rows): + return root / "part" + + fake_s3 = FakeS3(fail_upload=True) + table = _table(fake_s3, (_columns()[4],)) + table_url = f"{BASE}/v1/catalogs/cat/namespaces/ns1/tables/events" + httpx_mock.add_response( + method="GET", + url=f"{table_url}?totals=false", + json=_table_wire((_columns()[4],)), + ) + httpx_mock.add_response( + method="PUT", + url=re.compile(f"{re.escape(BASE)}/v1/catalogs/cat/uploads/.*"), + json={"path": "s3://bkt/lake/data/failed.packed"}, + ) + httpx_mock.add_response( + method="POST", + url=f"{BASE}/v1/catalogs/cat/uploads/abandon", + json={"abandoned": 1}, + ) + + with pytest.raises(OSError, match="object store refused"): + LocalPartAdapter("unused").prepare_append( + table, pa.table({"i64": pa.array([1], pa.int64())}) + ) + abandon = json.loads(httpx_mock.get_requests()[-1].content) + assert abandon["paths"] == ["s3://bkt/lake/data/failed.packed"] + table._namespace._catalog._client.close() + + +def test_prepared_payload_is_bound_to_its_table() -> None: + fake_s3 = FakeS3() + table = _table(fake_s3, (_columns()[4],)) + other_wire = _table_wire((_columns()[4],)) + other_wire["name"] = "other" + other_wire["table_uuid"] = "11111111-1111-4111-8111-111111111111" + other = Table(table._namespace, TableInfo.from_wire(other_wire)) + payload = { + "idempotency_key": "00000000-0000-4000-8000-000000000001", + "read_snapshot": 5, + "appends": [ + { + "namespace": table.namespace, + "table": table.name, + "expected_table_uuid": table.table_uuid, + "files": [ + { + "path": "s3://bkt/lake/a.packed", + "file_format": CLICKHOUSE_MERGETREE_PACKED_FORMAT, + } + ], + } + ], + } + + with pytest.raises(ValueError, match="invalid packed prepared payload"): + ClickHousePackedAdapter("unused").commit_prepared(other, payload) + table._namespace._catalog._client.close() + + +def test_arrow_stats_exclude_nans_and_normalize_signed_zero() -> None: + columns = ( + Column("f", "double", 1, 0, nullable=True), + Column("ts", "timestamp_ns", 2, 1, nullable=True), + ) + table = pa.table( + { + "f": pa.array([float("nan"), -0.0, None], pa.float64()), + "ts": pa.array([1234567891, None, 1234567890], pa.timestamp("ns")), + } + ) + stats = _column_stats(table, columns) + assert stats[0]["value_count"] == 3 + assert stats[0]["null_count"] == 1 + assert stats[0]["nan_count"] == 1 + assert stats[0]["lower_bound"] == "AAAAAAAAAIA=" + assert stats[0]["upper_bound"] == "AAAAAAAAAAA=" + assert stats[1]["lower_bound"] == "0gKWSQAAAAA=" + assert stats[1]["upper_bound"] == "0wKWSQAAAAA=" + + +def test_real_clickhouse_packed_append_and_snapshot_read(httpx_mock) -> None: + executable = os.environ.get("PYHOGLAKE_CLICKHOUSE") + if not executable: + pytest.skip("set PYHOGLAKE_CLICKHOUSE to a clickhouse executable or wrapper") + fake_s3 = FakeS3() + table = _table(fake_s3) + adapter = ClickHousePackedAdapter(Path(executable), timeout=180) + data = _data() + table_url = f"{BASE}/v1/catalogs/cat/namespaces/ns1/tables/events" + claimed = "s3://bkt/lake/data/ns1/events/test/claimed.packed" + + httpx_mock.add_response( + method="GET", url=f"{table_url}?totals=false", json=_table_wire() + ) + + def claim(request: httpx.Request) -> httpx.Response: + body = json.loads(request.content) + assert body["file_format"] == CLICKHOUSE_MERGETREE_PACKED_FORMAT + return httpx.Response(200, json={"path": claimed}) + + httpx_mock.add_callback( + claim, + method="PUT", + url=re.compile(f"{re.escape(BASE)}/v1/catalogs/cat/uploads/.*"), + ) + + payload = adapter.prepare_append(table, data) + registration = payload["appends"][0]["files"][0] + assert registration["path"] == claimed + assert registration["file_format"] == CLICKHOUSE_MERGETREE_PACKED_FORMAT + assert "footer_size" not in registration + assert "split_offsets" not in registration + assert len(registration["column_stats"]) == len(data.column_names) + key = claimed.removeprefix("s3://") + assert fake_s3.files[key] + + httpx_mock.add_response( + method="POST", + url=f"{BASE}/v1/catalogs/cat/commit/uploads", + json={"snapshot_id": 6, "schema_version": 1}, + ) + assert adapter.commit_prepared(table, payload).snapshot_id == 6 + commit_body = json.loads(httpx_mock.get_requests()[-1].content) + assert commit_body == payload + + second_claimed = "s3://bkt/lake/data/ns1/events/test/second.packed" + second_table = _table_wire() + second_table["read_snapshot_id"] = 6 + httpx_mock.add_response( + method="GET", url=f"{table_url}?totals=false", json=second_table + ) + httpx_mock.add_response( + method="PUT", + url=re.compile(f"{re.escape(BASE)}/v1/catalogs/cat/uploads/.*"), + json={"path": second_claimed}, + ) + second_payload = adapter.prepare_append(table, data) + httpx_mock.add_response( + method="POST", + url=f"{BASE}/v1/catalogs/cat/commit/uploads", + json={"snapshot_id": 7, "schema_version": 1}, + ) + assert adapter.commit_prepared(table, second_payload).snapshot_id == 7 + second_key = second_claimed.removeprefix("s3://") + + read_wire = _table_wire() + read_wire["read_snapshot_id"] = 7 + httpx_mock.add_response( + method="GET", + url=f"{table_url}?snapshot=7&totals=false", + json=read_wire, + ) + httpx_mock.add_response( + method="GET", + url=f"{table_url}/scan?snapshot=7", + json=[ + { + "data_file": { + "data_file_id": 1, + "path": claimed, + "file_format": CLICKHOUSE_MERGETREE_PACKED_FORMAT, + "record_count": data.num_rows, + "file_size_bytes": len(fake_s3.files[key]), + "row_id_start": 0, + "stats_state": "provided", + "begin_snapshot": 6, + } + }, + { + "data_file": { + "data_file_id": 2, + "path": second_claimed, + "file_format": CLICKHOUSE_MERGETREE_PACKED_FORMAT, + "record_count": data.num_rows, + "file_size_bytes": len(fake_s3.files[second_key]), + "row_id_start": data.num_rows, + "stats_state": "provided", + "begin_snapshot": 7, + } + }, + ], + ) + expected = pa.concat_tables([data, data]) + assert adapter.read(table, snapshot=7).equals(expected) + table._namespace._catalog._client.close() + + +@pytest.mark.integration +def test_live_server_claim_commit_and_multi_part_snapshot_read(live_server_url) -> None: + executable = os.environ.get("PYHOGLAKE_CLICKHOUSE") + if not executable: + pytest.skip("set PYHOGLAKE_CLICKHOUSE to a clickhouse executable or wrapper") + from pyhoglake import S3Config + + suffix = os.urandom(6).hex() + catalog_name = f"packed-it-{suffix}" + bucket = "pyhoglake-packed-itest" + s3 = S3Config( + access_key=os.environ.get("HOGLAKE_S3_ACCESS_KEY", "hoglake"), + secret_key=os.environ.get("HOGLAKE_S3_SECRET_KEY", "hoglake123"), + endpoint_override=os.environ.get( + "HOGLAKE_S3_ENDPOINT", "http://localhost:9000" + ), + region="us-east-1", + allow_bucket_creation=True, + ) + s3.filesystem().create_dir(bucket) + with HoglakeClient(live_server_url, s3=s3) as client: + catalog = client.create_catalog(catalog_name, f"s3://{bucket}/{catalog_name}/") + namespace = catalog.create_namespace("ns") + schema = pa.schema( + [ + pa.field("id", pa.int64(), nullable=False), + pa.field("name", pa.string()), + pa.field("payload", pa.binary()), + pa.field("at", pa.timestamp("ns")), + ] + ) + table = namespace.create_table( + "events", + schema, + properties={"write.format.default": CLICKHOUSE_MERGETREE_PACKED_FORMAT}, + ) + adapter = ClickHousePackedAdapter(Path(executable), timeout=180) + first = pa.table( + { + "id": pa.array([1, 2], pa.int64()), + "name": ["a", None], + "payload": pa.array([b"x", b"\x00\xff"], pa.binary()), + "at": pa.array([1000000001, 1000000002], pa.timestamp("ns")), + }, + schema=schema, + ) + second = pa.table( + { + "id": pa.array([3], pa.int64()), + "name": ["c"], + "payload": pa.array([b"z"], pa.binary()), + "at": pa.array([1000000003], pa.timestamp("ns")), + }, + schema=schema, + ) + snapshot_one = adapter.append(table, first).snapshot_id + snapshot_two = adapter.append(table, second).snapshot_id + + assert adapter.read(table, snapshot=snapshot_one).equals(first) + assert adapter.read(table, snapshot=snapshot_two).equals( + pa.concat_tables([first, second]) + ) + files = table.files(snapshot=snapshot_two) + assert [file.file_format for file in files] == [ + CLICKHOUSE_MERGETREE_PACKED_FORMAT, + CLICKHOUSE_MERGETREE_PACKED_FORMAT, + ] diff --git a/server/README.md b/server/README.md index d9dc4e67..0a2df65c 100644 --- a/server/README.md +++ b/server/README.md @@ -236,6 +236,37 @@ here scales with that number — the hydrator's sweep, the expiry and retirement arithmetic, and the commit receipt, whose stored request body is the one that grows fastest (hoglake#240). +### Packed MergeTree format + +A table may set `write.format.default=clickhouse-mergetree-packed` at creation. Absence means +`parquet`, and the effective value is immutable. Every registration must match its table format. +The packed contract is deliberately narrow: one registered object contains the bytes of one +ClickHouse `data.packed` part, `column_stats` must be present (an empty array means no bounds), and +`footer_size` plus Parquet `split_offsets` are forbidden. + +Packed tables are append-only, unpartitioned, unsorted, and fixed-schema. They admit only the +scalar types covered by the Python `ClickHousePackedAdapter`: boolean, signed and unsigned integers, +float, double, string, binary, date, and timestamp variants, within the configured ClickHouse +version's `Date32` and `DateTime64` value ranges. Column add, drop, rename, promotion, +partition or sort changes, truncate, and deletion-vector commits are refused. Drop remains valid. +The Python adapter exports only a part directory containing exactly `data.packed`; projections and +ClickHouse metadata that escape that file are outside this format. + +Upload claims accept `file_format=clickhouse-mergetree-packed`, persist that selection, and mint a +fresh `.packed` path. Publication verifies the claimed format and settles the claim in the same +transaction as the file row. The table identity row also stores the immutable format, and a database +foreign key prevents any server version from publishing a differently formatted data row. The +Parquet hydrator filters +to `file_format='parquet'`, and both compaction candidate selection and direct planned-group execution +refuse packed inputs. Expiry, retirement, removal-queue fencing, snapshots, row-range allocation, and +exact-path cleanup keep their existing one-row/one-object behavior. + +The DuckDB extension and Hedgerow reject packed tables and files. The Python adapter is the only +reader and writer in this repository. It materializes an exact snapshot's registered part list into +an isolated `clickhouse local` table and enables `table_readonly` after attachment. This is a local +correctness adapter, not evidence of remote-read performance, cross-version ClickHouse compatibility, +or support in Trino or the planned Iceberg facade. + ### Row lineage Every append gets a contiguous row-id range per file diff --git a/server/schema.sql b/server/schema.sql index b9b3d234..4853fb1b 100644 --- a/server/schema.sql +++ b/server/schema.sql @@ -125,6 +125,7 @@ CREATE TABLE hog_table ( created_snapshot bigint NOT NULL, dropped_snapshot bigint, next_field_id bigint NOT NULL DEFAULT 1, + file_format text NOT NULL DEFAULT 'parquet', -- The replacement edge (V14): the incarnation THIS row replaced, set -- by CatalogService.createTable when it publishes an atomic -- replacement. Recorded rather than derived from @@ -150,6 +151,10 @@ CREATE TABLE hog_table ( retirement_eligible_at timestamptz, PRIMARY KEY (catalog_id, table_id), UNIQUE (catalog_id, table_uuid), + CONSTRAINT hog_table_file_format_check + CHECK (file_format IN ('parquet', 'clickhouse-mergetree-packed')), + CONSTRAINT hog_table_format_identity + UNIQUE (catalog_id, table_id, file_format), -- The edge has no FK, so this is its only structural defence: a -- self-edge would be a 1-cycle for the recursive walk that follows it. CONSTRAINT hog_table_no_self_replacement CHECK (replaced_table_id <> table_id) @@ -285,7 +290,7 @@ CREATE TABLE hog_data_file ( end_snapshot bigint, path text NOT NULL, -- absolute object-store URI; no relative chains file_format text NOT NULL DEFAULT 'parquet' - CHECK (file_format IN ('parquet')), + CHECK (file_format IN ('parquet', 'clickhouse-mergetree-packed')), record_count bigint NOT NULL CHECK (record_count >= 0), file_size_bytes bigint NOT NULL CHECK (file_size_bytes >= 0), footer_size bigint, @@ -326,6 +331,13 @@ CREATE TABLE hog_data_file ( FOREIGN KEY (catalog_id, table_id) REFERENCES hog_table ON DELETE CASCADE, CHECK (end_snapshot IS NULL OR end_snapshot > begin_snapshot) ); +-- NOT VALID avoids a manifest-wide deployment scan. Existing rows predate +-- the alternate format and are therefore Parquet; new rows are checked. +ALTER TABLE hog_data_file + ADD CONSTRAINT hog_data_file_table_format_fk + FOREIGN KEY (catalog_id, table_id, file_format) + REFERENCES hog_table (catalog_id, table_id, file_format) + NOT VALID; CREATE INDEX hog_data_file_live ON hog_data_file (catalog_id, table_id, begin_snapshot) WHERE end_snapshot IS NULL; @@ -768,6 +780,9 @@ CREATE TABLE hog_upload ( prefix text NOT NULL, path text NOT NULL, file_kind text NOT NULL CHECK (file_kind IN ('data', 'delete')), + -- NULL on claims minted by replicas that predate format-aware claims. + -- Registration interprets a NULL data format as legacy Parquet only. + file_format text, state text NOT NULL DEFAULT 'active' CHECK (state IN ('active', 'registered', 'abandoned')), expires_at timestamptz NOT NULL DEFAULT now() + interval '24 hours', last_scheduled_at timestamptz, @@ -777,6 +792,107 @@ CREATE TABLE hog_upload ( CREATE INDEX hog_upload_owner ON hog_upload (catalog_id, owner) WHERE state = 'active'; CREATE INDEX hog_upload_cleanup ON hog_upload (catalog_id, last_scheduled_at, upload_id) WHERE state <> 'registered'; +CREATE FUNCTION hog_enforce_table_version_format() +RETURNS trigger LANGUAGE plpgsql AS ' +DECLARE + expected_format text; + actual_format text; +BEGIN + SELECT file_format INTO expected_format + FROM hog_table + WHERE catalog_id = NEW.catalog_id AND table_id = NEW.table_id; + actual_format := COALESCE(NEW.properties ->> ''write.format.default'', ''parquet''); + IF expected_format IS NOT NULL AND actual_format <> expected_format THEN + RAISE EXCEPTION ''table property format % does not match immutable table format %'', + actual_format, expected_format + USING ERRCODE = ''check_violation''; + END IF; + RETURN NEW; +END'; + +CREATE TRIGGER hog_table_version_format_guard +BEFORE INSERT OR UPDATE OF properties ON hog_table_version +FOR EACH ROW EXECUTE FUNCTION hog_enforce_table_version_format(); + +CREATE FUNCTION hog_reject_packed_table_mutation() +RETURNS trigger LANGUAGE plpgsql AS ' +DECLARE + row_catalog_id bigint; + row_table_id bigint; + table_format text; + table_created_snapshot bigint; + table_dropped_snapshot bigint; +BEGIN + IF TG_OP = ''DELETE'' THEN + row_catalog_id := OLD.catalog_id; + row_table_id := OLD.table_id; + ELSE + row_catalog_id := NEW.catalog_id; + row_table_id := NEW.table_id; + END IF; + + SELECT file_format, created_snapshot, hog_table.dropped_snapshot + INTO table_format, table_created_snapshot, table_dropped_snapshot + FROM hog_table + WHERE catalog_id = row_catalog_id AND table_id = row_table_id; + + IF table_format IS DISTINCT FROM ''clickhouse-mergetree-packed'' + OR table_dropped_snapshot IS NOT NULL THEN + IF TG_OP = ''DELETE'' THEN + RETURN OLD; + END IF; + RETURN NEW; + END IF; + + IF TG_TABLE_NAME = ''hog_column'' AND TG_OP = ''INSERT'' + AND NEW.begin_snapshot = table_created_snapshot THEN + RETURN NEW; + END IF; + + RAISE EXCEPTION ''packed MergeTree table does not support mutation through %'', TG_TABLE_NAME + USING ERRCODE = ''check_violation''; +END'; + +CREATE TRIGGER hog_packed_column_guard +BEFORE INSERT OR UPDATE OR DELETE ON hog_column +FOR EACH ROW EXECUTE FUNCTION hog_reject_packed_table_mutation(); +CREATE TRIGGER hog_packed_partition_guard +BEFORE INSERT OR UPDATE OR DELETE ON hog_partition_spec +FOR EACH ROW EXECUTE FUNCTION hog_reject_packed_table_mutation(); +CREATE TRIGGER hog_packed_sort_guard +BEFORE INSERT OR UPDATE OR DELETE ON hog_sort_spec +FOR EACH ROW EXECUTE FUNCTION hog_reject_packed_table_mutation(); +CREATE TRIGGER hog_packed_delete_file_guard +BEFORE INSERT OR UPDATE ON hog_delete_file +FOR EACH ROW EXECUTE FUNCTION hog_reject_packed_table_mutation(); +CREATE TRIGGER hog_packed_data_file_end_guard +BEFORE UPDATE OF end_snapshot ON hog_data_file +FOR EACH ROW EXECUTE FUNCTION hog_reject_packed_table_mutation(); + +CREATE FUNCTION hog_enforce_claimed_data_file_format() +RETURNS trigger LANGUAGE plpgsql AS ' +DECLARE + claimed_format text; +BEGIN + IF strpos(NEW.path, ''/trino-upload/'') = 0 THEN + RETURN NEW; + END IF; + + SELECT COALESCE(file_format, ''parquet'') INTO claimed_format + FROM hog_upload + WHERE catalog_id = NEW.catalog_id AND path = NEW.path AND file_kind = ''data''; + IF claimed_format IS NOT NULL AND claimed_format <> NEW.file_format THEN + RAISE EXCEPTION ''data file format % does not match upload claim format %'', + NEW.file_format, claimed_format + USING ERRCODE = ''check_violation''; + END IF; + RETURN NEW; +END'; + +CREATE TRIGGER hog_claimed_data_file_format_guard +BEFORE INSERT ON hog_data_file +FOR EACH ROW EXECUTE FUNCTION hog_enforce_claimed_data_file_format(); + -- Replacement lineage (V14): the access path for the walk -- OffsetRepo.releaseSupersededOffsets runs on every offset commit and -- inside the expiry sweep, following `replaced_table_id` forward from a diff --git a/server/src/main/kotlin/com/posthog/hoglake/api/Dto.kt b/server/src/main/kotlin/com/posthog/hoglake/api/Dto.kt index 6f1856e7..860d4bf0 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/api/Dto.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/api/Dto.kt @@ -178,7 +178,11 @@ data class ColumnDefDto( ) } -data class CreateTableRequestDto(val name: String, val columns: List) +data class CreateTableRequestDto( + val name: String, + val columns: List, + val properties: Map = emptyMap(), +) data class ColumnDto( val name: String, @@ -383,6 +387,8 @@ data class FileRegistrationDto( val partitionValues: List? = null, /** Optional row-group start offsets from a footer-shipping writer; validated at commit. */ val splitOffsets: List? = null, + /** Defaults to parquet so older writers and stored payloads keep their existing meaning. */ + val fileFormat: String = com.posthog.hoglake.model.FileFormats.PARQUET, ) { fun toModel() = FileRegistration( @@ -393,6 +399,7 @@ data class FileRegistrationDto( columnStats = columnStats?.map { it.toModel() }, partitionValues = partitionValues, splitOffsets = splitOffsets, + fileFormat = fileFormat, ) } diff --git a/server/src/main/kotlin/com/posthog/hoglake/api/Routes.kt b/server/src/main/kotlin/com/posthog/hoglake/api/Routes.kt index fa5b0f5f..0139ee01 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/api/Routes.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/api/Routes.kt @@ -129,6 +129,7 @@ fun Application.installApiRoutes( call.namespace(), req.name, req.columns.map { it.toModel() }, + req.properties, ).toDto(), ) } diff --git a/server/src/main/kotlin/com/posthog/hoglake/api/UploadRoutes.kt b/server/src/main/kotlin/com/posthog/hoglake/api/UploadRoutes.kt index eb436048..15f7a129 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/api/UploadRoutes.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/api/UploadRoutes.kt @@ -11,7 +11,12 @@ import io.ktor.server.routing.route import io.ktor.server.routing.routing import java.util.UUID -data class ClaimUploadDto(val owner: UUID, val prefix: String, val fileKind: String) +data class ClaimUploadDto( + val owner: UUID, + val prefix: String, + val fileKind: String, + val fileFormat: String? = null, +) data class UploadOwnerDto(val owner: UUID) @@ -28,7 +33,16 @@ fun Application.installUploadRoutes(uploads: UploadService) { } catch (_: IllegalArgumentException) { throw BadRequestException("invalid upload UUID") } - call.respond(uploads.claim(call.parameters["catalog"]!!, id, req.owner, req.prefix, req.fileKind)) + call.respond( + uploads.claim( + call.parameters["catalog"]!!, + id, + req.owner, + req.prefix, + req.fileKind, + req.fileFormat, + ), + ) } post("/renew") { val req = call.receive() diff --git a/server/src/main/kotlin/com/posthog/hoglake/commit/CommitService.kt b/server/src/main/kotlin/com/posthog/hoglake/commit/CommitService.kt index 4727744d..1e91bac7 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/commit/CommitService.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/commit/CommitService.kt @@ -5,6 +5,7 @@ import com.posthog.hoglake.model.ColType import com.posthog.hoglake.model.CommitRequest import com.posthog.hoglake.model.CommitResult import com.posthog.hoglake.model.DeleteFileRegistration +import com.posthog.hoglake.model.FileFormats import com.posthog.hoglake.model.FileRegistration import com.posthog.hoglake.model.HoglakeException import com.posthog.hoglake.model.StatsSanity @@ -15,6 +16,7 @@ import com.posthog.hoglake.observability.Audit import com.posthog.hoglake.observability.Metrics import com.posthog.hoglake.persistence.Locks import com.posthog.hoglake.persistence.bindBigintArrayOrNull +import com.posthog.hoglake.service.UploadRegistration import com.posthog.hoglake.storedPayloadObjectMapper import io.github.oshai.kotlinlogging.KotlinLogging import org.jdbi.v3.core.Handle @@ -324,9 +326,21 @@ class CommitService( validateFiles( h, catalogId, - ResolvedAppend(namespace, table, tableId, files, liveSpec(h, catalogId, tableId)), + ResolvedAppend( + namespace, + table, + tableId, + files, + liveTableFormat(h, catalogId, tableId), + liveSpec(h, catalogId, tableId), + ), ) - com.posthog.hoglake.service.UploadService.register(h, catalogId, uploadOwner, files.map { it.path to "data" }) + com.posthog.hoglake.service.UploadService.register( + h, + catalogId, + uploadOwner, + files.map { UploadRegistration(it.path, "data", it.fileFormat) }, + ) checkRemovalQueueCollisions(h, catalogId, listOf(append), emptyList()) if (files.isEmpty()) return val firstId = @@ -407,6 +421,8 @@ class CommitService( val table: String, val tableId: Long, val files: List, + /** Effective table storage format; absent property means parquet. */ + val tableFormat: String, /** Live spec with >= 1 field, or null for an unpartitioned table. */ val spec: LiveSpec?, ) @@ -424,6 +440,7 @@ class CommitService( val namespace: String, val table: String, val tableId: Long, + val tableFormat: String, val files: List, ) @@ -626,15 +643,27 @@ class CommitService( mergedAppends.map { (key, files) -> val (namespace, table) = key val live = resolveGuarded(key) - ResolvedAppend(namespace, table, live.tableId, files, liveSpec(h, catalogId, live.tableId)) + ResolvedAppend( + namespace, + table, + live.tableId, + files, + live.tableFormat, + liveSpec(h, catalogId, live.tableId), + ) } - val appendTableIdByName = - resolvedAppends.associate { (it.namespace to it.table) to it.tableId } + val appendTableByName = + resolvedAppends.associateBy { it.namespace to it.table } val resolvedDeletes = mergedDeletes.map { (key, files) -> val (namespace, table) = key - val tableId = appendTableIdByName[key] ?: resolveGuarded(key).tableId - ResolvedDeletes(namespace, table, tableId, files) + val appended = appendTableByName[key] + if (appended != null) { + ResolvedDeletes(namespace, table, appended.tableId, appended.tableFormat, files) + } else { + val live = resolveGuarded(key) + ResolvedDeletes(namespace, table, live.tableId, live.tableFormat, files) + } } // The second thing read_snapshot is REQUIRED for, and the reason @@ -797,8 +826,12 @@ class CommitService( h, catalogId, req.idempotencyKey, - resolvedAppends.flatMap { a -> a.files.map { it.path to "data" } } + - resolvedDeletes.flatMap { d -> d.files.map { it.path to "delete" } }, + resolvedAppends.flatMap { a -> + a.files.map { UploadRegistration(it.path, "data", it.fileFormat) } + } + + resolvedDeletes.flatMap { d -> + d.files.map { UploadRegistration(it.path, "delete") } + }, ) checkRemovalQueueCollisions(h, catalogId, resolvedAppends, resolvedDeletes) @@ -1015,10 +1048,10 @@ class CommitService( h.prepareBatch( """ INSERT INTO hog_data_file (catalog_id, data_file_id, table_id, begin_snapshot, - path, record_count, file_size_bytes, footer_size, + path, file_format, record_count, file_size_bytes, footer_size, row_id_start, stats_state, spec_id, split_offsets) VALUES (:catalogId, :dataFileId, :tableId, :beginSnapshot, - :path, :recordCount, :fileSizeBytes, :footerSize, + :path, :fileFormat, :recordCount, :fileSizeBytes, :footerSize, :rowIdStart, :statsState, :specId, :splitOffsets) """, ) @@ -1118,6 +1151,7 @@ class CommitService( .bind("tableId", append.tableId) .bind("beginSnapshot", snapshotId) .bind("path", file.path) + .bind("fileFormat", file.fileFormat) .bind("recordCount", file.recordCount) .bind("fileSizeBytes", file.fileSizeBytes) .bind("footerSize", file.footerSize) @@ -1351,8 +1385,12 @@ class CommitService( insertBatch.execute() } - /** A live table's id + identity uuid (the incarnation the name currently binds to). */ - private data class LiveTable(val tableId: Long, val tableUuid: UUID) + /** A live table's id, identity uuid and effective storage format. */ + private data class LiveTable( + val tableId: Long, + val tableUuid: UUID, + val tableFormat: String, + ) /** * The refusal for a commit whose table did not resolve live: a @@ -1419,7 +1457,7 @@ class CommitService( ): LiveTable? = h.createQuery( """ - SELECT tv.table_id, t.table_uuid + SELECT tv.table_id, t.table_uuid, t.file_format AS table_format FROM hog_table_version tv JOIN hog_namespace ns ON ns.catalog_id = tv.catalog_id AND ns.namespace_id = tv.namespace_id @@ -1436,10 +1474,27 @@ class CommitService( .bind("catalogId", catalogId) .bind("namespace", namespace) .bind("table", table) - .map { rs, _ -> LiveTable(rs.getLong(1), rs.getObject(2) as UUID) } + .map { rs, _ -> LiveTable(rs.getLong(1), rs.getObject(2) as UUID, rs.getString(3)) } .findOne() .orElse(null) + private fun liveTableFormat( + h: Handle, + catalogId: Long, + tableId: Long, + ): String = + h.createQuery( + """ + SELECT file_format + FROM hog_table + WHERE catalog_id = :catalogId AND table_id = :tableId + """, + ) + .bind("catalogId", catalogId) + .bind("tableId", tableId) + .mapTo(String::class.java) + .one() + /** * The table's live partition spec, or null when unpartitioned. A spec * with zero fields (SetPartitionSpec([])) is unpartitioned too. @@ -1503,10 +1558,26 @@ class CommitService( .toList() .toMap() } + if (append.tableFormat !in FileFormats.allowed) { + throw HoglakeException.Validation( + "$qualified has unsupported table format '${append.tableFormat}'", + ) + } for (file in append.files) { if (file.path.isBlank()) { throw HoglakeException.Validation("blank file path in append to $qualified") } + if (file.fileFormat !in FileFormats.allowed) { + throw HoglakeException.Validation( + "unsupported file_format '${file.fileFormat}' for ${file.path} in $qualified", + ) + } + if (file.fileFormat != append.tableFormat) { + throw HoglakeException.Validation( + "file_format '${file.fileFormat}' for ${file.path} does not match " + + "$qualified format '${append.tableFormat}'", + ) + } if (file.recordCount < 0) { throw HoglakeException.Validation( "negative record_count for ${file.path} in $qualified", @@ -1523,6 +1594,29 @@ class CommitService( "negative file_size_bytes for ${file.path} in $qualified", ) } + if (file.fileFormat == FileFormats.CLICKHOUSE_MERGETREE_PACKED) { + if (file.recordCount == 0L || file.fileSizeBytes == 0L) { + throw HoglakeException.Validation( + "packed MergeTree registration ${file.path} must contain at least one row and one byte", + ) + } + if (file.columnStats == null) { + throw HoglakeException.Validation( + "packed MergeTree registration ${file.path} must provide column_stats; " + + "use an empty array when no bounds are available", + ) + } + if (file.footerSize != null) { + throw HoglakeException.Validation( + "packed MergeTree registration ${file.path} must not provide footer_size", + ) + } + if (file.splitOffsets != null) { + throw HoglakeException.Validation( + "packed MergeTree registration ${file.path} must not provide split_offsets", + ) + } + } // Refused, not repaired: unlike a stats row there is no // partial list worth keeping — readers ignore a list that // breaks the contract, and a sorted-but-wrong one misplaces @@ -1639,6 +1733,11 @@ class CommitService( val seenTargets = HashSet() for (deletes in resolved) { val qualified = "${deletes.namespace}.${deletes.table}" + if (deletes.tableFormat != FileFormats.PARQUET) { + throw HoglakeException.Validation( + "$qualified uses '${deletes.tableFormat}' and does not support deletion vectors", + ) + } for (reg in deletes.files) { if (reg.path.isBlank()) { throw HoglakeException.Validation("blank delete file path in $qualified") diff --git a/server/src/main/kotlin/com/posthog/hoglake/compaction/CompactionService.kt b/server/src/main/kotlin/com/posthog/hoglake/compaction/CompactionService.kt index 58bb58da..f7443a37 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/compaction/CompactionService.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/compaction/CompactionService.kt @@ -9,6 +9,7 @@ import com.posthog.hoglake.model.ColType import com.posthog.hoglake.model.Column import com.posthog.hoglake.model.ColumnStats import com.posthog.hoglake.model.CompactionResult +import com.posthog.hoglake.model.FileFormats import com.posthog.hoglake.model.HoglakeException import com.posthog.hoglake.model.MaintenanceTask import com.posthog.hoglake.model.MaintenanceTrigger @@ -836,6 +837,8 @@ data class CompactionCandidate( */ val footerSize: Long?, val rowIdStart: Long, + /** Physical format. Parquet is the only format this rewriter can open. */ + val fileFormat: String = FileFormats.PARQUET, /** * `hog_data_file.explicit_row_ids` — true when this file is a * COMPACTION OUTPUT and carries its row ids in a physical @@ -1077,6 +1080,8 @@ class CompactionService( val namespace: String, val table: String, val tableId: Long, + /** Effective table file format. Only Parquet tables enter the candidate query. */ + val fileFormat: String, /** Live columns at the planning head — BINDING for the rewrite shape. */ val columns: List, /** Live sort order — BINDING for the rewrite. Empty = row-id order. */ @@ -1336,6 +1341,7 @@ class CompactionService( namespace = ns.name, table = t.name, tableId = t.tableId, + fileFormat = FileFormats.tableFormat(t.properties), columns = TableRepo.columnsAt(h, cat.catalogId, t.tableId, cat.headSnapshotId), sortFields = SortRepo.sortSpecAt(h, cat.catalogId, t.tableId, cat.headSnapshotId) @@ -1498,6 +1504,12 @@ class CompactionService( available: Long = 0, truncated: Boolean = false, ) = CandidateFetch(ctx, budget, rowCapacity, emptyMap(), 0, 0, available, truncated) + // A format predicate alone would scan every row of a homogeneous packed table + // looking for a Parquet match that cannot exist. Refuse from table metadata + // before any statement touches the manifest; the row predicate remains as a + // defense against malformed mixed-format state. + if (ctx.fileFormat != FileFormats.PARQUET) return empty() + // The scalar rewriter cannot preserve VARIANT groups yet. Do not enqueue // work that could drop payloads or repeatedly fail the maintenance loop. // allNodes, not the top level: a variant nested inside a struct @@ -1911,17 +1923,18 @@ class CompactionService( "" } return """ - SELECT f.data_file_id, f.path, f.record_count, f.file_size_bytes, + SELECT f.data_file_id, f.path, f.file_format, f.record_count, f.file_size_bytes, f.footer_size, f.row_id_start, f.explicit_row_ids, f.spec_id, dv.delete_file_id AS dv_id, dv.path AS dv_path, dv.delete_count AS dv_count$valuesProjection FROM ( - SELECT f0.catalog_id, f0.data_file_id, f0.path, f0.record_count, + SELECT f0.catalog_id, f0.data_file_id, f0.path, f0.file_format, f0.record_count, f0.file_size_bytes, f0.footer_size, f0.row_id_start, f0.explicit_row_ids, f0.spec_id FROM hog_data_file f0 WHERE f0.catalog_id = :catalogId AND f0.table_id = :tableId AND f0.end_snapshot IS NULL + AND f0.file_format = 'parquet' AND f0.file_size_bytes < :targetBytes$rowBound$armSql$innerOrder LIMIT :scanLimit ) f @@ -2076,6 +2089,7 @@ class CompactionService( fileSizeBytes = rs.getLong("file_size_bytes"), footerSize = rs.getObject("footer_size", java.lang.Long::class.java)?.toLong(), rowIdStart = rs.getLong("row_id_start"), + fileFormat = rs.getString("file_format"), explicitRowIds = rs.getBoolean("explicit_row_ids"), dv = rs.getObject("dv_id")?.let { @@ -3228,7 +3242,33 @@ class CompactionService( val ctx = jdbi.inTransactionUnchecked { h -> h.execute("SET TRANSACTION ISOLATION LEVEL REPEATABLE READ READ ONLY") - tableContext(h, catalog, namespace, table, defaults) + val resolved = tableContext(h, catalog, namespace, table, defaults) + if (group.files.isNotEmpty()) { + val actualFormats = + h.createQuery( + """ + SELECT data_file_id, file_format + FROM hog_data_file + WHERE catalog_id = :catalogId AND table_id = :tableId + AND data_file_id IN () + """, + ) + .bind("catalogId", resolved.catalogId) + .bind("tableId", resolved.tableId) + .bindList("ids", group.files.map { it.dataFileId }) + .map { rs, _ -> rs.getLong("data_file_id") to rs.getString("file_format") } + .list() + .toMap() + group.files.firstOrNull { + actualFormats[it.dataFileId] != FileFormats.PARQUET + }?.let { file -> + throw InvalidDataException( + "compaction supports parquet inputs only; data_file_id ${file.dataFileId} " + + "uses '${actualFormats[file.dataFileId] ?: "unknown"}'", + ) + } + } + resolved } return compactGroup(ctx, group) } @@ -3240,6 +3280,12 @@ class CompactionService( ctx: TableContext, group: CompactionGroup, ): GroupOutcome { + group.files.firstOrNull { it.fileFormat != FileFormats.PARQUET }?.let { file -> + throw InvalidDataException( + "compaction supports parquet inputs only; data_file_id ${file.dataFileId} " + + "uses '${file.fileFormat}'", + ) + } val inputs = group.files.map { f -> // Read the object IN PLACE. This used to fetch the diff --git a/server/src/main/kotlin/com/posthog/hoglake/hydrator/Hydrator.kt b/server/src/main/kotlin/com/posthog/hoglake/hydrator/Hydrator.kt index 47eaf8b4..4fcee69b 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/hydrator/Hydrator.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/hydrator/Hydrator.kt @@ -751,6 +751,7 @@ class Hydrator( JOIN hog_table t ON t.catalog_id = f.catalog_id AND t.table_id = f.table_id WHERE f.stats_state = 'pending' + AND f.file_format = 'parquet' AND t.dropped_snapshot IS NULL ORDER BY f.catalog_id, f.data_file_id LIMIT :limit diff --git a/server/src/main/kotlin/com/posthog/hoglake/model/FileFormats.kt b/server/src/main/kotlin/com/posthog/hoglake/model/FileFormats.kt new file mode 100644 index 00000000..bb3a407d --- /dev/null +++ b/server/src/main/kotlin/com/posthog/hoglake/model/FileFormats.kt @@ -0,0 +1,48 @@ +package com.posthog.hoglake.model + +/** Storage-format contract shared by table metadata, registration and maintenance paths. */ +object FileFormats { + const val TABLE_PROPERTY: String = "write.format.default" + const val PARQUET: String = "parquet" + const val CLICKHOUSE_MERGETREE_PACKED: String = "clickhouse-mergetree-packed" + + val allowed: Set = setOf(PARQUET, CLICKHOUSE_MERGETREE_PACKED) + + /** + * The types the first packed-part adapter maps losslessly between Hoglake, Arrow and ClickHouse. + * Adding a type requires an adapter round-trip test before the server admits it. + */ + val packedColumnTypes: Set = + setOf( + ColType.BOOLEAN, + ColType.INT8, + ColType.INT16, + ColType.INT, + ColType.LONG, + ColType.UINT8, + ColType.UINT16, + ColType.UINT32, + ColType.UINT64, + ColType.FLOAT, + ColType.DOUBLE, + ColType.DATE, + ColType.TIMESTAMP_S, + ColType.TIMESTAMP_MS, + ColType.TIMESTAMP, + ColType.TIMESTAMP_NS, + ColType.TIMESTAMPTZ, + ColType.STRING, + ColType.BINARY, + ) + + fun tableFormat(properties: Map): String = properties[TABLE_PROPERTY] ?: PARQUET + + fun isPacked(properties: Map): Boolean = tableFormat(properties) == CLICKHOUSE_MERGETREE_PACKED +} + +/** Jackson value filter that keeps legacy Parquet registrations absent on stored payloads. */ +class ParquetFileFormatFilter { + override fun equals(other: Any?): Boolean = other == FileFormats.PARQUET + + override fun hashCode(): Int = 0 +} diff --git a/server/src/main/kotlin/com/posthog/hoglake/model/Model.kt b/server/src/main/kotlin/com/posthog/hoglake/model/Model.kt index 22e5abc6..5337a2a0 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/model/Model.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/model/Model.kt @@ -1120,6 +1120,12 @@ data class FileRegistration( */ @get:JsonInclude(JsonInclude.Include.NON_NULL) val splitOffsets: List? = null, + /** Physical storage format. Omitted legacy payloads decode as parquet. */ + @get:JsonInclude( + value = JsonInclude.Include.CUSTOM, + valueFilter = ParquetFileFormatFilter::class, + ) + val fileFormat: String = FileFormats.PARQUET, ) data class TableAppend( diff --git a/server/src/main/kotlin/com/posthog/hoglake/persistence/HogSchemaColumns.kt b/server/src/main/kotlin/com/posthog/hoglake/persistence/HogSchemaColumns.kt index 6799092b..378c0f80 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/persistence/HogSchemaColumns.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/persistence/HogSchemaColumns.kt @@ -62,7 +62,7 @@ object HogSchemaColumns { "hog_table" to setOf( "catalog_id", "table_id", "table_uuid", "created_snapshot", - "dropped_snapshot", "next_field_id", + "dropped_snapshot", "next_field_id", "file_format", // V14: the recorded replacement edge, written by // TableRepo.insertTable and read by // OffsetRepo.releaseSupersededOffsets. @@ -156,7 +156,7 @@ object HogSchemaColumns { ), "hog_upload" to setOf( - "catalog_id", "upload_id", "owner", "prefix", "path", "file_kind", "state", + "catalog_id", "upload_id", "owner", "prefix", "path", "file_kind", "file_format", "state", "expires_at", "last_scheduled_at", ), // CleanupService (claim + drain + ledger), ExpiryService queue diff --git a/server/src/main/kotlin/com/posthog/hoglake/persistence/TableRepo.kt b/server/src/main/kotlin/com/posthog/hoglake/persistence/TableRepo.kt index 88dba7ba..de791e88 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/persistence/TableRepo.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/persistence/TableRepo.kt @@ -4,6 +4,7 @@ import com.posthog.hoglake.model.ChangeKind import com.posthog.hoglake.model.ColType import com.posthog.hoglake.model.Column import com.posthog.hoglake.model.ColumnDef +import com.posthog.hoglake.model.FileFormats import com.posthog.hoglake.model.HoglakeException import com.posthog.hoglake.model.TableSummaryInfo import org.jdbi.v3.core.Handle @@ -133,21 +134,36 @@ object TableRepo { createdSnapshot: Long, tableUuid: UUID = UUID.randomUUID(), replacedTableId: Long? = null, - ): UUID = - handle.createQuery( - """ - INSERT INTO hog_table (catalog_id, table_id, created_snapshot, table_uuid, replaced_table_id) - VALUES (:catalogId, :tableId, :createdSnapshot, :tableUuid, :replacedTableId) - RETURNING table_uuid - """, - ) + fileFormat: String = FileFormats.PARQUET, + ): UUID { + val sql = + if (fileFormat == FileFormats.PARQUET) { + """ + INSERT INTO hog_table + (catalog_id, table_id, created_snapshot, table_uuid, replaced_table_id) + VALUES + (:catalogId, :tableId, :createdSnapshot, :tableUuid, :replacedTableId) + RETURNING table_uuid + """ + } else { + """ + INSERT INTO hog_table + (catalog_id, table_id, created_snapshot, table_uuid, replaced_table_id, file_format) + VALUES + (:catalogId, :tableId, :createdSnapshot, :tableUuid, :replacedTableId, :fileFormat) + RETURNING table_uuid + """ + } + return handle.createQuery(sql) .bind("catalogId", catalogId) .bind("tableId", tableId) .bind("createdSnapshot", createdSnapshot) .bind("tableUuid", tableUuid) .bind("replacedTableId", replacedTableId) + .apply { if (fileFormat != FileFormats.PARQUET) bind("fileFormat", fileFormat) } .map { rs, _ -> rs.getObject("table_uuid") as UUID } .one() + } /** * Allocate [count] consecutive field ids from hog_table.next_field_id diff --git a/server/src/main/kotlin/com/posthog/hoglake/service/AlterService.kt b/server/src/main/kotlin/com/posthog/hoglake/service/AlterService.kt index 9e199819..14b96cb7 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/service/AlterService.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/service/AlterService.kt @@ -89,6 +89,24 @@ class AlterService(private val jdbi: Jdbi) { currentTableUuid = t.tableUuid, ) } + ops.filterIsInstance().forEach { + TableMetadata.validateProperties(it.properties) + TableMetadata.requireFormatUnchanged(t.properties, it.properties) + } + if (com.posthog.hoglake.model.FileFormats.isPacked(t.properties)) { + val unsupported = + ops.firstOrNull { + it is AlterOp.AddColumn || it is AlterOp.DropColumn || + it is AlterOp.RenameColumn || it is AlterOp.PromoteColumn || + it is AlterOp.SetPartitionSpec || it is AlterOp.SetSortOrder + } + if (unsupported != null) { + throw HoglakeException.Validation( + "packed MergeTree tables have a fixed schema and do not support " + + (unsupported::class.simpleName ?: "this alter operation"), + ) + } + } if (readSnapshot != null) { val head = CatalogRepo.findByName(h, catalog)!! if (readSnapshot < 0 || readSnapshot > head.headSnapshotId) { diff --git a/server/src/main/kotlin/com/posthog/hoglake/service/CatalogService.kt b/server/src/main/kotlin/com/posthog/hoglake/service/CatalogService.kt index a8b6498d..f38b21bd 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/service/CatalogService.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/service/CatalogService.kt @@ -8,6 +8,7 @@ import com.posthog.hoglake.model.CommitResult import com.posthog.hoglake.model.ConsumerOffset import com.posthog.hoglake.model.DataFile import com.posthog.hoglake.model.FileColumnStats +import com.posthog.hoglake.model.FileFormats import com.posthog.hoglake.model.FileOrderingBounds import com.posthog.hoglake.model.FileStats import com.posthog.hoglake.model.HoglakeException @@ -276,6 +277,7 @@ class CatalogService(private val jdbi: Jdbi) { namespace: String, name: String, columns: List, + properties: Map = emptyMap(), ): TableInfo = Audit.audited( "table_create", @@ -283,7 +285,10 @@ class CatalogService(private val jdbi: Jdbi) { "$namespace.$name", detail = { "columns=${columns.size}" }, ) { - jdbi.inTransactionUnchecked { h -> createTable(h, catalog, namespace, name, columns) } + jdbi.inTransactionUnchecked { + h -> + createTable(h, catalog, namespace, name, columns, properties = properties) + } } /** Caller may compose creation with file registration in the same transaction. */ @@ -302,6 +307,7 @@ class CatalogService(private val jdbi: Jdbi) { ): TableInfo { TableMetadata.validateComment(comment) TableMetadata.validateProperties(properties) + TableMetadata.validateDefinitionForFormat(properties, columns, partitionFields, sortFields) validateTableDefinition(name, columns) val cols = initialColumns(columns) // Publication catches definition validation and records a rejected receipt. @@ -347,7 +353,15 @@ class CatalogService(private val jdbi: Jdbi) { // consumer's offset release depends on must be a fact, not a // convention re-derived later (OffsetRepo.releaseSupersededOffsets). val createdUuid = - TableRepo.insertTable(h, cat.catalogId, tableId, alloc.snapshotId, tableUuid, replacementTableId) + TableRepo.insertTable( + h, + cat.catalogId, + tableId, + alloc.snapshotId, + tableUuid, + replacementTableId, + FileFormats.tableFormat(properties), + ) // nodeCount, not columns.size: a nested column needs one id per // NODE, not one per top-level column. Allocating by size would // hand back a range too short and every subtree after the first @@ -497,6 +511,9 @@ class CatalogService(private val jdbi: Jdbi) { currentTableUuid = t.tableUuid, ) } + if (com.posthog.hoglake.model.FileFormats.isPacked(t.properties)) { + throw HoglakeException.Validation("packed MergeTree tables do not support truncate") + } val alloc = CatalogRepo.allocateSnapshot(h, cat.catalogId) SnapshotRepo.insert(h, cat.catalogId, alloc.snapshotId, alloc.schemaVersion) // Reuse the DDL conflict barrier, without changing the table's versioned metadata. diff --git a/server/src/main/kotlin/com/posthog/hoglake/service/TableCreationService.kt b/server/src/main/kotlin/com/posthog/hoglake/service/TableCreationService.kt index dd99e34f..5eb67004 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/service/TableCreationService.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/service/TableCreationService.kt @@ -5,6 +5,7 @@ import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper import com.posthog.hoglake.commit.CommitService import com.posthog.hoglake.model.Column import com.posthog.hoglake.model.ColumnDef +import com.posthog.hoglake.model.FileFormats import com.posthog.hoglake.model.FileRegistration import com.posthog.hoglake.model.HoglakeException import com.posthog.hoglake.model.PartitionFieldDef @@ -92,6 +93,12 @@ class TableCreationService( // the forest prepare refused. TableMetadata.validateComment(definition.comment) TableMetadata.validateProperties(definition.properties) + TableMetadata.validateDefinitionForFormat( + definition.properties, + definition.columns, + definition.partitionFields, + definition.sortFields, + ) catalogs.validateTableDefinition(definition.name, definition.columns) AlterService( jdbi, @@ -180,6 +187,7 @@ class TableCreationService( "duplicate file paths", ) } + val tableFormat = FileFormats.tableFormat(operation.definition.properties) files.forEach { if (!it.path.startsWith( operation.writePath, @@ -187,7 +195,14 @@ class TableCreationService( ) { throw HoglakeException.Validation("file outside operation write_path") } - it.validateFooterSize(required = true) + if (it.fileFormat != tableFormat) { + throw HoglakeException.Validation( + "file_format '${it.fileFormat}' does not match table format '$tableFormat'", + ) + } + if (tableFormat == FileFormats.PARQUET) { + it.validateFooterSize(required = true) + } } h.createUpdate( """ diff --git a/server/src/main/kotlin/com/posthog/hoglake/service/TableMetadata.kt b/server/src/main/kotlin/com/posthog/hoglake/service/TableMetadata.kt index 0c7fca16..226f7916 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/service/TableMetadata.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/service/TableMetadata.kt @@ -1,8 +1,12 @@ package com.posthog.hoglake.service +import com.posthog.hoglake.model.ColumnDef +import com.posthog.hoglake.model.FileFormats import com.posthog.hoglake.model.HoglakeException +import com.posthog.hoglake.model.PartitionFieldDef +import com.posthog.hoglake.model.SortFieldDef -/** User metadata has no effect on storage or writer behavior. */ +/** Table metadata validation, including the storage-format property that affects writer behavior. */ internal object TableMetadata { fun validateComment(comment: String?) { if (comment != null && (comment.length > 16384 || '\u0000' in comment)) { @@ -30,5 +34,50 @@ internal object TableMetadata { ) } } + val format = FileFormats.tableFormat(properties) + if (format !in FileFormats.allowed) { + throw HoglakeException.Validation( + "property '${FileFormats.TABLE_PROPERTY}' must be one of ${FileFormats.allowed.sorted()}", + ) + } + } + + fun validateDefinitionForFormat( + properties: Map, + columns: List, + partitionFields: List, + sortFields: List, + ) { + if (!FileFormats.isPacked(properties)) return + if (partitionFields.isNotEmpty()) { + throw HoglakeException.Validation("packed MergeTree tables do not support partition specs") + } + if (sortFields.isNotEmpty()) { + throw HoglakeException.Validation("packed MergeTree tables do not support sort orders") + } + + fun nodes(defs: List): Sequence = + defs.asSequence().flatMap { def -> sequenceOf(def) + nodes(def.children.orEmpty()) } + val unsupported = nodes(columns).firstOrNull { it.type !in FileFormats.packedColumnTypes } + if (unsupported != null) { + throw HoglakeException.Validation( + "packed MergeTree tables do not support column '${unsupported.name}' " + + "of type '${unsupported.type.wire}'; supported types are " + + FileFormats.packedColumnTypes.map { it.wire }.sorted().joinToString(", "), + ) + } + } + + fun requireFormatUnchanged( + before: Map, + after: Map, + ) { + val old = FileFormats.tableFormat(before) + val new = FileFormats.tableFormat(after) + if (old != new) { + throw HoglakeException.Validation( + "property '${FileFormats.TABLE_PROPERTY}' is immutable (current '$old', requested '$new')", + ) + } } } diff --git a/server/src/main/kotlin/com/posthog/hoglake/service/UploadService.kt b/server/src/main/kotlin/com/posthog/hoglake/service/UploadService.kt index ff48af01..fe83dee3 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/service/UploadService.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/service/UploadService.kt @@ -1,5 +1,6 @@ package com.posthog.hoglake.service +import com.posthog.hoglake.model.FileFormats import com.posthog.hoglake.model.HoglakeException import com.posthog.hoglake.persistence.CatalogRepo import org.jdbi.v3.core.Handle @@ -9,12 +10,19 @@ import org.jdbi.v3.core.kotlin.withHandleUnchecked import java.time.Instant import java.util.UUID +internal data class UploadRegistration( + val path: String, + val fileKind: String, + val fileFormat: String? = null, +) + data class UploadClaim( val uploadId: UUID, val owner: UUID, val prefix: String, val path: String, val fileKind: String, + val fileFormat: String?, val state: String, val expiresAt: Instant, ) @@ -55,12 +63,32 @@ class UploadService( owner: UUID, prefix: String, kind: String, + fileFormat: String? = null, ): UploadClaim = inCatalog(catalog, "upload_claim") { h, catalogId -> if (kind !in setOf("data", "delete")) throw HoglakeException.Validation("invalid upload file_kind") + val effectiveFormat = + if (kind == "data") { + fileFormat ?: FileFormats.PARQUET + } else { + if (fileFormat != null) { + throw HoglakeException.Validation("file_format is valid only for data uploads") + } + null + } + if (effectiveFormat != null && effectiveFormat !in FileFormats.allowed) { + throw HoglakeException.Validation("invalid upload file_format '$effectiveFormat'") + } + val suffix = + when (effectiveFormat) { + FileFormats.PARQUET -> "parquet" + FileFormats.CLICKHOUSE_MERGETREE_PACKED -> "packed" + null -> "puffin" + else -> error("validated upload format $effectiveFormat") + } val normalized = prefix.trimEnd('/') val cat = CatalogRepo.require(h, catalog) - val path = "$normalized/trino-upload/${UUID.randomUUID()}.${if (kind == "data") "parquet" else "puffin"}" + val path = "$normalized/trino-upload/${UUID.randomUUID()}.$suffix" val root = cat.dataPath.trimEnd('/') + "/" if (!path.startsWith(root) || path.any { it.isWhitespace() || it.isISOControl() } || path.removePrefix(root).split('/').any { it.isEmpty() || it == "." || it == ".." } @@ -73,14 +101,21 @@ class UploadService( // changed the definition. h.createUpdate( """ - INSERT INTO hog_upload (catalog_id, upload_id, owner, prefix, path, file_kind) - VALUES (:catalog, :id, :owner, :prefix, :path, :kind) + INSERT INTO hog_upload + (catalog_id, upload_id, owner, prefix, path, file_kind, file_format) + VALUES + (:catalog, :id, :owner, :prefix, :path, :kind, :fileFormat) ON CONFLICT (catalog_id, upload_id) DO NOTHING """, ).bind("catalog", catalogId).bind("id", id).bind("owner", owner) - .bind("prefix", normalized).bind("path", path).bind("kind", kind).execute() + .bind("prefix", normalized).bind("path", path).bind("kind", kind) + .bind("fileFormat", effectiveFormat).execute() val claim = load(h, catalogId, id) - if (claim.owner != owner || claim.prefix != normalized || claim.fileKind != kind) { + val claimedFormat = + claim.fileFormat ?: if (claim.fileKind == "data") FileFormats.PARQUET else null + if (claim.owner != owner || claim.prefix != normalized || claim.fileKind != kind || + claimedFormat != effectiveFormat || !claim.path.endsWith(".$suffix") + ) { throw HoglakeException.CommitConflict("upload identity was reused with a different definition") } claim @@ -280,6 +315,7 @@ class UploadService( rs.getString("prefix"), rs.getString("path"), rs.getString("file_kind"), + rs.getString("file_format"), rs.getString("state"), rs.getTimestamp("expires_at").toInstant(), ) @@ -292,7 +328,7 @@ class UploadService( ): UploadClaim = h.createQuery( """ - SELECT upload_id, owner, prefix, path, file_kind, state, expires_at FROM hog_upload + SELECT upload_id, owner, prefix, path, file_kind, file_format, state, expires_at FROM hog_upload WHERE catalog_id = :catalog AND upload_id = :id """, ).bind("catalog", catalogId).bind("id", id).map(claimMapper).one() @@ -302,12 +338,14 @@ class UploadService( h: Handle, catalogId: Long, owner: UUID?, - files: List>, + files: List, ) { if (files.isEmpty()) return - val kinds = files.toMap() - if (files.any { (path, kind) -> kinds[path] != kind }) { - throw HoglakeException.Validation("one upload path cannot hold both data and deletion vectors") + val byPath = files.associateBy { it.path } + if (files.any { byPath[it.path] != it }) { + throw HoglakeException.Validation( + "one upload path cannot hold different file kinds or formats", + ) } // FOR UPDATE: the claim transitions no longer serialize on the // catalog commit lock, so this read-then-settle pair holds the @@ -318,11 +356,11 @@ class UploadService( val claims = h.createQuery( """ - SELECT upload_id, owner, prefix, path, file_kind, state, expires_at + SELECT upload_id, owner, prefix, path, file_kind, file_format, state, expires_at FROM hog_upload WHERE catalog_id = :catalog AND path = ANY(:paths) FOR UPDATE """, - ).bind("catalog", catalogId).bindArray("paths", String::class.java, kinds.keys) + ).bind("catalog", catalogId).bindArray("paths", String::class.java, byPath.keys) .map(claimMapper).list() // Preserve the existing catalog contract: immutable objects still referenced // at a retained snapshot may be referenced again. Ownership never permits @@ -330,10 +368,10 @@ class UploadService( val retained = h.createQuery( """ - SELECT path, 'data' AS file_kind FROM hog_data_file + SELECT path, 'data' AS file_kind, file_format FROM hog_data_file WHERE catalog_id = :catalog AND path = ANY(:paths) UNION - SELECT path, 'delete' AS file_kind FROM hog_delete_file + SELECT path, 'delete' AS file_kind, NULL::text AS file_format FROM hog_delete_file WHERE catalog_id = :catalog AND path = ANY(:paths) """, ).bind("catalog", catalogId).bindArray( @@ -341,14 +379,24 @@ class UploadService( String::class.java, claims.filter { it.state == "registered" }.map { it.path }, ) - .map { rs, _ -> rs.getString("path") to rs.getString("file_kind") }.list().toSet() + .map { rs, _ -> + UploadRegistration( + rs.getString("path"), + rs.getString("file_kind"), + rs.getString("file_format"), + ) + } + .list() + .toSet() for (claim in claims) { - if (claim.state == "registered" && claim.fileKind == kinds[claim.path] && - (claim.path to claim.fileKind) in retained - ) { + val requested = byPath.getValue(claim.path) + val claimedFormat = + claim.fileFormat ?: if (claim.fileKind == "data") FileFormats.PARQUET else null + val claimed = UploadRegistration(claim.path, claim.fileKind, claimedFormat) + if (claim.state == "registered" && claimed == requested && claimed in retained) { continue } - if (claim.state != "active" || claim.owner != owner || claim.fileKind != kinds[claim.path]) { + if (claim.state != "active" || claim.owner != owner || claimed != requested) { throw HoglakeException.CommitConflict( "upload is fenced, already registered, or belongs to another operation", ) diff --git a/server/src/main/resources/db/migration/V25__packed_mergetree_format.sql b/server/src/main/resources/db/migration/V25__packed_mergetree_format.sql new file mode 100644 index 00000000..2b00baf7 --- /dev/null +++ b/server/src/main/resources/db/migration/V25__packed_mergetree_format.sql @@ -0,0 +1,196 @@ +-- Admit the restricted single-object ClickHouse packed-part format. +-- The immutable table row carries the format so a database foreign key, +-- not only the current server binary, prevents mixed-format publication. +SET LOCAL lock_timeout = '5s'; + +ALTER TABLE hog_table + ADD COLUMN IF NOT EXISTS file_format text NOT NULL DEFAULT 'parquet'; + +ALTER TABLE hog_table DROP CONSTRAINT IF EXISTS hog_table_file_format_check; +ALTER TABLE hog_table + ADD CONSTRAINT hog_table_file_format_check + CHECK (file_format IN ('parquet', 'clickhouse-mergetree-packed')) + NOT VALID; +ALTER TABLE hog_table VALIDATE CONSTRAINT hog_table_file_format_check; + +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_constraint + WHERE conname = 'hog_table_format_identity' + AND conrelid = 'hog_table'::regclass + ) THEN + ALTER TABLE hog_table + ADD CONSTRAINT hog_table_format_identity + UNIQUE (catalog_id, table_id, file_format); + END IF; +END $$; + +ALTER TABLE hog_data_file DROP CONSTRAINT IF EXISTS hog_data_file_file_format_check; +ALTER TABLE hog_data_file + ADD CONSTRAINT hog_data_file_file_format_check + CHECK (file_format IN ('parquet', 'clickhouse-mergetree-packed')) + NOT VALID; +ALTER TABLE hog_data_file VALIDATE CONSTRAINT hog_data_file_file_format_check; + +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_constraint + WHERE conname = 'hog_data_file_table_format_fk' + AND conrelid = 'hog_data_file'::regclass + ) THEN + -- Existing rows can stay unscanned: before this migration the data-file + -- CHECK admitted only Parquet, and every existing table receives the + -- Parquet default above. PostgreSQL still enforces a NOT VALID foreign + -- key for every row inserted after the constraint is installed. + ALTER TABLE hog_data_file + ADD CONSTRAINT hog_data_file_table_format_fk + FOREIGN KEY (catalog_id, table_id, file_format) + REFERENCES hog_table (catalog_id, table_id, file_format) + NOT VALID; + END IF; +END $$; + +-- Format-aware claims are nullable for rolling deploys. A claim minted by an +-- older replica carries NULL and may settle a Parquet data registration only. +ALTER TABLE hog_upload ADD COLUMN IF NOT EXISTS file_format text; + +-- Reserve the table property safely. Earlier binaries accepted arbitrary +-- properties, so stop the migration rather than reinterpret retained metadata +-- that already used this key for another purpose. +DO $$ +BEGIN + IF EXISTS ( + SELECT 1 + FROM hog_table_version + WHERE properties ? 'write.format.default' + AND properties ->> 'write.format.default' <> 'parquet' + ) THEN + RAISE EXCEPTION + 'retained table metadata already uses reserved property write.format.default; remove it before V25'; + END IF; +END $$; + +-- A version row must repeat the immutable identity-row format. This trigger +-- also fences an older replica that tries to create a packed property while +-- its old INSERT still lets hog_table default to Parquet. +CREATE OR REPLACE FUNCTION hog_enforce_table_version_format() +RETURNS trigger LANGUAGE plpgsql AS ' +DECLARE + expected_format text; + actual_format text; +BEGIN + SELECT file_format INTO expected_format + FROM hog_table + WHERE catalog_id = NEW.catalog_id AND table_id = NEW.table_id; + actual_format := COALESCE(NEW.properties ->> ''write.format.default'', ''parquet''); + IF expected_format IS NOT NULL AND actual_format <> expected_format THEN + RAISE EXCEPTION ''table property format % does not match immutable table format %'', + actual_format, expected_format + USING ERRCODE = ''check_violation''; + END IF; + RETURN NEW; +END'; + +DROP TRIGGER IF EXISTS hog_table_version_format_guard ON hog_table_version; +CREATE TRIGGER hog_table_version_format_guard +BEFORE INSERT OR UPDATE OF properties ON hog_table_version +FOR EACH ROW EXECUTE FUNCTION hog_enforce_table_version_format(); + +-- Old binaries do not know the fixed-layout restrictions. Fence every +-- unsupported catalog mutation at the database boundary during a rolling +-- deploy. Initial column inserts at the table's creation snapshot and all +-- cleanup after a table is marked dropped remain legal. +CREATE OR REPLACE FUNCTION hog_reject_packed_table_mutation() +RETURNS trigger LANGUAGE plpgsql AS ' +DECLARE + row_catalog_id bigint; + row_table_id bigint; + table_format text; + table_created_snapshot bigint; + table_dropped_snapshot bigint; +BEGIN + IF TG_OP = ''DELETE'' THEN + row_catalog_id := OLD.catalog_id; + row_table_id := OLD.table_id; + ELSE + row_catalog_id := NEW.catalog_id; + row_table_id := NEW.table_id; + END IF; + + SELECT file_format, created_snapshot, hog_table.dropped_snapshot + INTO table_format, table_created_snapshot, table_dropped_snapshot + FROM hog_table + WHERE catalog_id = row_catalog_id AND table_id = row_table_id; + + IF table_format IS DISTINCT FROM ''clickhouse-mergetree-packed'' + OR table_dropped_snapshot IS NOT NULL THEN + IF TG_OP = ''DELETE'' THEN + RETURN OLD; + END IF; + RETURN NEW; + END IF; + + IF TG_TABLE_NAME = ''hog_column'' AND TG_OP = ''INSERT'' + AND NEW.begin_snapshot = table_created_snapshot THEN + RETURN NEW; + END IF; + + RAISE EXCEPTION ''packed MergeTree table does not support mutation through %'', TG_TABLE_NAME + USING ERRCODE = ''check_violation''; +END'; + +DROP TRIGGER IF EXISTS hog_packed_column_guard ON hog_column; +CREATE TRIGGER hog_packed_column_guard +BEFORE INSERT OR UPDATE OR DELETE ON hog_column +FOR EACH ROW EXECUTE FUNCTION hog_reject_packed_table_mutation(); + +DROP TRIGGER IF EXISTS hog_packed_partition_guard ON hog_partition_spec; +CREATE TRIGGER hog_packed_partition_guard +BEFORE INSERT OR UPDATE OR DELETE ON hog_partition_spec +FOR EACH ROW EXECUTE FUNCTION hog_reject_packed_table_mutation(); + +DROP TRIGGER IF EXISTS hog_packed_sort_guard ON hog_sort_spec; +CREATE TRIGGER hog_packed_sort_guard +BEFORE INSERT OR UPDATE OR DELETE ON hog_sort_spec +FOR EACH ROW EXECUTE FUNCTION hog_reject_packed_table_mutation(); + +DROP TRIGGER IF EXISTS hog_packed_delete_file_guard ON hog_delete_file; +CREATE TRIGGER hog_packed_delete_file_guard +BEFORE INSERT OR UPDATE ON hog_delete_file +FOR EACH ROW EXECUTE FUNCTION hog_reject_packed_table_mutation(); + +DROP TRIGGER IF EXISTS hog_packed_data_file_end_guard ON hog_data_file; +CREATE TRIGGER hog_packed_data_file_end_guard +BEFORE UPDATE OF end_snapshot ON hog_data_file +FOR EACH ROW EXECUTE FUNCTION hog_reject_packed_table_mutation(); + +-- Old replicas do not read hog_upload.file_format. Enforce the claim format +-- on the data-row INSERT itself so they cannot settle a packed claim with a +-- legacy registration that defaults to Parquet. Unclaimed writer paths skip +-- the indexed probe entirely. +CREATE OR REPLACE FUNCTION hog_enforce_claimed_data_file_format() +RETURNS trigger LANGUAGE plpgsql AS ' +DECLARE + claimed_format text; +BEGIN + IF strpos(NEW.path, ''/trino-upload/'') = 0 THEN + RETURN NEW; + END IF; + + SELECT COALESCE(file_format, ''parquet'') INTO claimed_format + FROM hog_upload + WHERE catalog_id = NEW.catalog_id AND path = NEW.path AND file_kind = ''data''; + IF claimed_format IS NOT NULL AND claimed_format <> NEW.file_format THEN + RAISE EXCEPTION ''data file format % does not match upload claim format %'', + NEW.file_format, claimed_format + USING ERRCODE = ''check_violation''; + END IF; + RETURN NEW; +END'; + +DROP TRIGGER IF EXISTS hog_claimed_data_file_format_guard ON hog_data_file; +CREATE TRIGGER hog_claimed_data_file_format_guard +BEFORE INSERT ON hog_data_file +FOR EACH ROW EXECUTE FUNCTION hog_enforce_claimed_data_file_format(); diff --git a/server/src/main/resources/openapi/hoglake.yaml b/server/src/main/resources/openapi/hoglake.yaml index 8d16ceeb..369e6620 100644 --- a/server/src/main/resources/openapi/hoglake.yaml +++ b/server/src/main/resources/openapi/hoglake.yaml @@ -1876,8 +1876,9 @@ paths: summary: Claim a fresh object path before uploading description: > Requires claimed-uploads-v1. Generates a unique server path under prefix, - which must be inside catalog data_path. Replaying upload ID requires the - same owner, prefix and kind and returns the same path and current state. + which must be inside catalog data_path. Data claims may select Parquet or + packed MergeTree; omission means Parquet. Replaying upload ID requires the + same owner, prefix, kind and format and returns the same path and current state. Owner is the durable commit key or table-creation operation ID. Lease is 24 hours. Active claims protect objects even if erroneously queued. Registered and abandoned states are permanent fences against path reuse. @@ -1895,6 +1896,10 @@ paths: owner: { type: string, format: uuid } prefix: { type: string } file_kind: { type: string, enum: [data, delete] } + file_format: + type: string + enum: [parquet, clickhouse-mergetree-packed] + description: Data uploads only; omission means parquet. responses: "200": description: Durable claim (replay may return a terminal state) @@ -1909,6 +1914,10 @@ paths: prefix: { type: string } path: { type: string } file_kind: { type: string, enum: [data, delete] } + file_format: + type: string + enum: [parquet, clickhouse-mergetree-packed] + description: Present on format-aware data claims; absent on delete and legacy claims. state: { type: string, enum: [active, registered, abandoned] } expires_at: { type: string, format: date-time } "409": { description: Upload identity reused with different intent } @@ -2956,9 +2965,13 @@ components: type: object maxProperties: 100 description: > - Inert user metadata. Replaced as a whole by set_properties; an empty object clears it. - Keys are lowercase ASCII identifiers; hoglake. and trino. prefixes and - partitioning, sorted_by, location, format, comment are reserved. No NUL in values. + Versioned table properties. Replaced as a whole by set_properties; an empty object clears it. + `write.format.default` selects `parquet` (also the default when absent) or + `clickhouse-mergetree-packed` and is immutable after table creation. Packed tables + are append-only, unpartitioned, unsorted, fixed-schema tables over the documented + scalar subset. Other keys are inert user metadata. Keys are lowercase ASCII + identifiers; hoglake. and trino. prefixes and partitioning, sorted_by, location, + format, comment are reserved. No NUL in values. propertyNames: { pattern: "^[a-z][a-z0-9_.-]{0,127}$" } additionalProperties: { type: string, maxLength: 4096 } @@ -3098,6 +3111,7 @@ components: type: array minItems: 1 items: { $ref: "#/components/schemas/ColumnDef" } + properties: { $ref: "#/components/schemas/CustomProperties" } TableSummary: type: object @@ -3333,6 +3347,15 @@ components: required: [path, record_count, file_size_bytes] properties: path: { type: string, description: Absolute object-store URI } + file_format: + type: string + enum: [parquet, clickhouse-mergetree-packed] + default: parquet + description: > + Physical object format. It must match the table's effective + `write.format.default`; omission keeps the legacy Parquet meaning. + Packed registrations require a non-null `column_stats` array (empty is + allowed) and forbid `footer_size` and `split_offsets`. record_count: { type: integer, format: int64 } file_size_bytes: { type: integer, format: int64 } footer_size: { type: integer, format: int64 } @@ -3700,7 +3723,9 @@ components: properties: data_file_id: { type: integer, format: int64 } path: { type: string } - file_format: { type: string } + file_format: + type: string + enum: [parquet, clickhouse-mergetree-packed] record_count: { type: integer, format: int64 } file_size_bytes: { type: integer, format: int64 } footer_size: { type: integer, format: int64 } diff --git a/server/src/test/kotlin/com/posthog/hoglake/WireJsonTest.kt b/server/src/test/kotlin/com/posthog/hoglake/WireJsonTest.kt index a12857a6..5b8412ef 100644 --- a/server/src/test/kotlin/com/posthog/hoglake/WireJsonTest.kt +++ b/server/src/test/kotlin/com/posthog/hoglake/WireJsonTest.kt @@ -3,6 +3,8 @@ package com.posthog.hoglake import com.fasterxml.jackson.databind.exc.InvalidNullException import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException import com.fasterxml.jackson.module.kotlin.readValue +import com.posthog.hoglake.model.FileFormats +import com.posthog.hoglake.model.FileRegistration import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.assertThatThrownBy import org.junit.jupiter.api.Test @@ -55,6 +57,28 @@ class WireJsonTest { assertThat(storedPayloadObjectMapper().readValue(withUnknown).someName).isEqualTo("x") } + @Test + fun `default file format stays absent from stored payloads`() { + val legacy = wireObjectMapper().writeValueAsString(FileRegistration("s3://b/a.parquet", 1, 10)) + assertThat(legacy).doesNotContain("file_format") + val packed = + wireObjectMapper().writeValueAsString( + FileRegistration( + "s3://b/a.packed", + 1, + 10, + columnStats = emptyList(), + fileFormat = FileFormats.CLICKHOUSE_MERGETREE_PACKED, + ), + ) + assertThat(packed).contains("\"file_format\":\"clickhouse-mergetree-packed\"") + assertThat( + storedPayloadObjectMapper().readValue( + """{"path":"s3://b/a.parquet","record_count":1,"file_size_bytes":10,"future":true}""", + ).fileFormat, + ).isEqualTo(FileFormats.PARQUET) + } + @Test fun `mappers are shared instances, not rebuilt per call`() { // commitFingerprint built a fresh mapper — and paid its Kotlin diff --git a/server/src/test/kotlin/com/posthog/hoglake/api/ApiIntegrationTest.kt b/server/src/test/kotlin/com/posthog/hoglake/api/ApiIntegrationTest.kt index 912c85b7..4c52ecdb 100644 --- a/server/src/test/kotlin/com/posthog/hoglake/api/ApiIntegrationTest.kt +++ b/server/src/test/kotlin/com/posthog/hoglake/api/ApiIntegrationTest.kt @@ -107,6 +107,62 @@ class ApiIntegrationTest { assertThat(spec.bodyAsText()).startsWith("openapi:") } + @Test + fun `ordinary table creation can select packed storage`() = + api { client -> + assertThat( + client.postJson( + "/v1/catalogs", + """{"name":"packed","data_path":"s3://hog/packed"}""", + ).status, + ).isEqualTo(HttpStatusCode.Created) + assertThat( + client.postJson( + "/v1/catalogs/packed/namespaces", + """{"name":"analytics"}""", + ).status, + ).isEqualTo(HttpStatusCode.Created) + val created = + client.postJson( + "/v1/catalogs/packed/namespaces/analytics/tables", + """ + {"name":"events", + "properties":{"write.format.default":"clickhouse-mergetree-packed"}, + "columns":[{"name":"id","type":"long","nullable":false}]} + """, + ) + assertThat(created.status).isEqualTo(HttpStatusCode.Created) + assertThat(body(created)["properties"]["write.format.default"].asText()) + .isEqualTo("clickhouse-mergetree-packed") + val committed = + client.postJson( + "/v1/catalogs/packed/commit", + """ + {"appends":[{"namespace":"analytics","table":"events","files":[ + {"path":"s3://hog/packed/data/a.packed", + "file_format":"clickhouse-mergetree-packed", + "record_count":1,"file_size_bytes":10,"column_stats":[]} + ]}]} + """, + ) + assertThat(committed.status).isEqualTo(HttpStatusCode.OK) + val alter = + client.postJson( + "/v1/catalogs/packed/namespaces/analytics/tables/events/alter", + """{"ops":[{"op":"add_column","column":{"name":"extra","type":"long"}}]}""", + ) + assertThat(alter.status).isEqualTo(HttpStatusCode.UnprocessableEntity) + assertThat(body(alter)["detail"].asText()).contains("fixed schema") + val tableUuid = body(created)["table_uuid"].asText() + val truncate = + client.post( + "/v1/catalogs/packed/namespaces/analytics/tables/events/truncate" + + "?expected_table_uuid=$tableUuid", + ) + assertThat(truncate.status).isEqualTo(HttpStatusCode.UnprocessableEntity) + assertThat(body(truncate)["detail"].asText()).contains("do not support truncate") + } + // ---- the full lifecycle ---------------------------------------------- @Test diff --git a/server/src/test/kotlin/com/posthog/hoglake/commit/CommitServiceTest.kt b/server/src/test/kotlin/com/posthog/hoglake/commit/CommitServiceTest.kt index 44bedb5f..9d240ff4 100644 --- a/server/src/test/kotlin/com/posthog/hoglake/commit/CommitServiceTest.kt +++ b/server/src/test/kotlin/com/posthog/hoglake/commit/CommitServiceTest.kt @@ -3,6 +3,7 @@ package com.posthog.hoglake.commit import com.posthog.hoglake.model.ColumnStats import com.posthog.hoglake.model.CommitRequest import com.posthog.hoglake.model.DeleteFileRegistration +import com.posthog.hoglake.model.FileFormats import com.posthog.hoglake.model.FileRegistration import com.posthog.hoglake.model.HoglakeException import com.posthog.hoglake.model.TableAppend @@ -327,6 +328,33 @@ class CommitServiceTest { snapshotId } + private fun setTableFormat( + fixture: Fixture, + tableId: Long, + format: String, + ) { + jdbi.useHandle { h -> + h.createUpdate( + "UPDATE hog_table SET file_format = :format WHERE catalog_id = :catalogId AND table_id = :tableId", + ) + .bind("format", format) + .bind("catalogId", fixture.catalogId) + .bind("tableId", tableId) + .execute() + h.createUpdate( + """ + UPDATE hog_table_version + SET properties = CAST(:properties AS jsonb) + WHERE catalog_id = :catalogId AND table_id = :tableId AND end_snapshot IS NULL + """, + ) + .bind("properties", "{\"${FileFormats.TABLE_PROPERTY}\":\"$format\"}") + .bind("catalogId", fixture.catalogId) + .bind("tableId", tableId) + .execute() + } + } + private fun file( path: String, records: Long, @@ -567,6 +595,115 @@ class CommitServiceTest { assertThat(statsRowCount(fx.catalogId)).isEqualTo(0) } + @Test + fun `packed registration persists its format with provided stats`() { + val fx = seed() + val (tableId, _) = fx.tables.getValue("events") + setTableFormat(fx, tableId, FileFormats.CLICKHOUSE_MERGETREE_PACKED) + val packed = + FileRegistration( + path = "s3://b/data/part.packed", + recordCount = 9, + fileSizeBytes = 100, + columnStats = emptyList(), + fileFormat = FileFormats.CLICKHOUSE_MERGETREE_PACKED, + ) + + service.commit( + "cat", + CommitRequest(appends = listOf(TableAppend("ns", "events", listOf(packed)))), + ) + + jdbi.useHandle { h -> + assertThat( + h.createQuery("SELECT file_format FROM hog_data_file WHERE catalog_id = :catalogId") + .bind("catalogId", fx.catalogId) + .mapTo(String::class.java) + .one(), + ).isEqualTo(FileFormats.CLICKHOUSE_MERGETREE_PACKED) + } + } + + @Test + fun `packed registration rejects parquet metadata and mismatched formats`() { + val fx = seed() + val (tableId, _) = fx.tables.getValue("events") + setTableFormat(fx, tableId, FileFormats.CLICKHOUSE_MERGETREE_PACKED) + val valid = + FileRegistration( + path = "s3://b/data/part.packed", + recordCount = 1, + fileSizeBytes = 100, + columnStats = emptyList(), + fileFormat = FileFormats.CLICKHOUSE_MERGETREE_PACKED, + ) + val invalid = + listOf( + valid.copy(fileFormat = FileFormats.PARQUET), + valid.copy(columnStats = null), + valid.copy(footerSize = 20), + valid.copy(splitOffsets = listOf(0)), + valid.copy(recordCount = 0), + valid.copy(fileSizeBytes = 0), + ) + for (registration in invalid) { + assertThatThrownBy { + service.commit( + "cat", + CommitRequest(appends = listOf(TableAppend("ns", "events", listOf(registration)))), + ) + }.isInstanceOf(HoglakeException.Validation::class.java) + assertThat(dataFiles(fx.catalogId)).isEmpty() + } + } + + @Test + fun `packed tables reject deletion vectors`() { + val fx = seed() + val (tableId, _) = fx.tables.getValue("events") + setTableFormat(fx, tableId, FileFormats.CLICKHOUSE_MERGETREE_PACKED) + val append = + service.commit( + "cat", + CommitRequest( + appends = + listOf( + TableAppend( + "ns", + "events", + listOf( + FileRegistration( + "s3://b/data/part.packed", + 3, + 100, + columnStats = emptyList(), + fileFormat = FileFormats.CLICKHOUSE_MERGETREE_PACKED, + ), + ), + ), + ), + ), + ) + + assertThatThrownBy { + service.commit( + "cat", + CommitRequest( + readSnapshot = append.snapshotId, + deletes = + listOf( + TableDeletes( + "ns", + "events", + listOf(DeleteFileRegistration(1, "s3://b/data/delete.puffin", 1, 10)), + ), + ), + ), + ) + }.isInstanceOf(HoglakeException.Validation::class.java) + .hasMessageContaining("does not support deletion vectors") + } + @Test fun `consecutive commits produce contiguous non-overlapping row id ranges`() { val fx = seed() diff --git a/server/src/test/kotlin/com/posthog/hoglake/compaction/CompactionPlanningIntegrationTest.kt b/server/src/test/kotlin/com/posthog/hoglake/compaction/CompactionPlanningIntegrationTest.kt index 6898fb17..737ad3cd 100644 --- a/server/src/test/kotlin/com/posthog/hoglake/compaction/CompactionPlanningIntegrationTest.kt +++ b/server/src/test/kotlin/com/posthog/hoglake/compaction/CompactionPlanningIntegrationTest.kt @@ -7,6 +7,7 @@ import com.posthog.hoglake.model.ColType import com.posthog.hoglake.model.ColumnDef import com.posthog.hoglake.model.CommitRequest import com.posthog.hoglake.model.DeleteFileRegistration +import com.posthog.hoglake.model.FileFormats import com.posthog.hoglake.model.FileRegistration import com.posthog.hoglake.model.MaintenanceBacklog import com.posthog.hoglake.model.MaintenanceTask @@ -21,6 +22,7 @@ import com.posthog.hoglake.service.MaintenanceSummarySampler import com.posthog.hoglake.service.PartitionStatsService import com.posthog.hoglake.testing.PgTestSupport import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatThrownBy import org.jdbi.v3.core.kotlin.useHandleUnchecked import org.jdbi.v3.core.kotlin.withHandleUnchecked import org.junit.jupiter.api.AfterAll @@ -66,7 +68,10 @@ class CompactionPlanningIntegrationTest { db.close() } - private fun fixture(columns: List = listOf(ColumnDef("id", ColType.LONG))): String { + private fun fixture( + columns: List = listOf(ColumnDef("id", ColType.LONG)), + properties: Map = emptyMap(), + ): String { val cat = "plan-cat-${counter.incrementAndGet()}" db.jdbi.useHandleUnchecked { h -> // Raw insert: this suite tests compaction planning, not catalog validation. @@ -82,7 +87,7 @@ class CompactionPlanningIntegrationTest { ) } catalogs.createNamespace(cat, "ns") - catalogs.createTable(cat, "ns", "t", columns) + catalogs.createTable(cat, "ns", "t", columns, properties) return cat } @@ -123,6 +128,58 @@ class CompactionPlanningIntegrationTest { assertThat(plan.groups.single().totalBytes).isEqualTo(1200) } + @Test + fun `packed files are excluded from planning and direct execution`() { + val cat = + fixture( + properties = + mapOf( + FileFormats.TABLE_PROPERTY to FileFormats.CLICKHOUSE_MERGETREE_PACKED, + ), + ) + val registrations = + listOf("a", "b").map { name -> + FileRegistration( + path = "s3://bucket/x/$name.packed", + recordCount = 10, + fileSizeBytes = 100, + columnStats = emptyList(), + fileFormat = FileFormats.CLICKHOUSE_MERGETREE_PACKED, + ) + } + append(cat, *registrations.toTypedArray()) + assertThat(svc.planTable(cat, "ns", "t", cfg).groups).isEmpty() + + val files = + db.jdbi.withHandle, Exception> { h -> + h.createQuery( + """ + SELECT f.data_file_id, f.path, f.record_count, f.file_size_bytes, f.row_id_start + FROM hog_data_file f + JOIN hog_catalog c ON c.catalog_id = f.catalog_id + WHERE c.name = :cat + ORDER BY f.data_file_id + """, + ) + .bind("cat", cat) + .map { rs, _ -> + CompactionCandidate( + dataFileId = rs.getLong("data_file_id"), + path = rs.getString("path"), + recordCount = rs.getLong("record_count"), + fileSizeBytes = rs.getLong("file_size_bytes"), + footerSize = null, + rowIdStart = rs.getLong("row_id_start"), + ) + } + .list() + } + assertThatThrownBy { + svc.compactPlannedGroup(cat, "ns", "t", CompactionGroup(files, null, null)) + }.isInstanceOf(InvalidDataException::class.java) + .hasMessageContaining(FileFormats.CLICKHOUSE_MERGETREE_PACKED) + } + @Test fun `a run keeps packing groups until the remainder is too short`() { val cat = fixture() diff --git a/server/src/test/kotlin/com/posthog/hoglake/hydrator/HydratorIntegrationTest.kt b/server/src/test/kotlin/com/posthog/hoglake/hydrator/HydratorIntegrationTest.kt index a0973727..a9789e68 100644 --- a/server/src/test/kotlin/com/posthog/hoglake/hydrator/HydratorIntegrationTest.kt +++ b/server/src/test/kotlin/com/posthog/hoglake/hydrator/HydratorIntegrationTest.kt @@ -188,18 +188,20 @@ class HydratorIntegrationTest { recordCount: Long, fileSizeBytes: Long, footerSize: Long?, + fileFormat: String = "parquet", ) { jdbi.useHandle { h -> h.execute( """ INSERT INTO hog_data_file - (catalog_id, data_file_id, table_id, begin_snapshot, path, + (catalog_id, data_file_id, table_id, begin_snapshot, path, file_format, record_count, file_size_bytes, footer_size, row_id_start, stats_state) - VALUES (?, ?, 1, 1, ?, ?, ?, ?, 0, 'pending') + VALUES (?, ?, 1, 1, ?, ?, ?, ?, ?, 0, 'pending') """, catalogId, dataFileId, path, + fileFormat, recordCount, fileSizeBytes, footerSize, @@ -299,6 +301,30 @@ class HydratorIntegrationTest { // ---- tests ------------------------------------------------------------- + @Test + fun `packed files are never claimed by the parquet hydrator`() { + val catalogId = seedCatalogAndTable() + jdbi.useHandle { h -> + h.execute( + "UPDATE hog_table SET file_format = 'clickhouse-mergetree-packed' " + + "WHERE catalog_id = ? AND table_id = 1", + catalogId, + ) + } + seedDataFile( + catalogId, + 1, + "s3://$BUCKET/t1/data.packed", + ROWS.toLong(), + 100, + null, + fileFormat = "clickhouse-mergetree-packed", + ) + + assertThat(hydrator.runOnce()).isZero() + assertThat(statsState(catalogId, 1)).isEqualTo("pending") + } + @Test fun `hydrates a pending file end to end - field ids present so the flag stays false`() { val catalogId = seedCatalogAndTable() diff --git a/server/src/test/kotlin/com/posthog/hoglake/persistence/V17FilePathIndexMigrationIntegrationTest.kt b/server/src/test/kotlin/com/posthog/hoglake/persistence/V17FilePathIndexMigrationIntegrationTest.kt index e64f9359..72b95980 100644 --- a/server/src/test/kotlin/com/posthog/hoglake/persistence/V17FilePathIndexMigrationIntegrationTest.kt +++ b/server/src/test/kotlin/com/posthog/hoglake/persistence/V17FilePathIndexMigrationIntegrationTest.kt @@ -4,6 +4,7 @@ import com.posthog.hoglake.Database import com.posthog.hoglake.hydrator.ObjectStore import com.posthog.hoglake.service.CleanupService import com.posthog.hoglake.service.RemovalStore +import com.posthog.hoglake.service.UploadRegistration import com.posthog.hoglake.service.UploadService import com.posthog.hoglake.testing.PgTestSupport import com.posthog.hoglake.testing.TestImages @@ -273,6 +274,10 @@ class V17FilePathIndexMigrationIntegrationTest { // read off disk rather than restated here, and idempotent, so the // `Database.migrate` below re-applies it for free. PgTestSupport.applyMigrationFile(db, "V21__cleanup_claim.sql") + // Current UploadService reads the format-aware claim column introduced + // by V25. Apply that idempotent file out of order for the same reason: + // this test measures V17's indexes through today's production query. + PgTestSupport.applyMigrationFile(db, "V25__packed_mergetree_format.sql") // The statements, off the services that issue them. referenceCheckSql = captureReferenceCheck() @@ -504,7 +509,12 @@ class V17FilePathIndexMigrationIntegrationTest { jdbi.useHandleUnchecked { h -> h.begin() try { - UploadService.register(h, catalogId, OWNER, livePaths.map { it to "data" }) + UploadService.register( + h, + catalogId, + OWNER, + livePaths.map { UploadRegistration(it, "data", "parquet") }, + ) } finally { h.rollback() } diff --git a/server/src/test/kotlin/com/posthog/hoglake/persistence/V25PackedMergetreeFormatMigrationIntegrationTest.kt b/server/src/test/kotlin/com/posthog/hoglake/persistence/V25PackedMergetreeFormatMigrationIntegrationTest.kt new file mode 100644 index 00000000..641f2ba4 --- /dev/null +++ b/server/src/test/kotlin/com/posthog/hoglake/persistence/V25PackedMergetreeFormatMigrationIntegrationTest.kt @@ -0,0 +1,205 @@ +package com.posthog.hoglake.persistence + +import com.posthog.hoglake.Database +import com.posthog.hoglake.testing.PgTestSupport +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatThrownBy +import org.jdbi.v3.core.statement.UnableToExecuteStatementException +import org.junit.jupiter.api.Tag +import org.junit.jupiter.api.Test + +@Tag("integration") +class V25PackedMergetreeFormatMigrationIntegrationTest { + @Test + fun `existing parquet rows remain valid and packed rows are admitted`() { + PgTestSupport.freshDatabaseAt("24").use { db -> + val catalogId = + db.jdbi.withHandle { h -> + h.createQuery( + "INSERT INTO hog_catalog (name, data_path) VALUES ('v25', 's3://b/v25') RETURNING catalog_id", + ).mapTo(Long::class.java).one() + } + db.jdbi.useHandle { h -> + h.execute( + "INSERT INTO hog_table (catalog_id, table_id, created_snapshot) VALUES (?, 1, 0)", + catalogId, + ) + h.execute( + """ + INSERT INTO hog_data_file + (catalog_id, data_file_id, table_id, begin_snapshot, path, + record_count, file_size_bytes, row_id_start) + VALUES (?, 1, 1, 0, 's3://b/v25/a.parquet', 1, 10, 0) + """, + catalogId, + ) + } + + Database.migrate(db.dataSource) + + db.jdbi.useHandle { h -> + assertThat( + h.createQuery("SELECT file_format FROM hog_data_file WHERE data_file_id = 1") + .mapTo(String::class.java) + .one(), + ).isEqualTo("parquet") + h.execute( + "INSERT INTO hog_namespace (catalog_id, namespace_id, name) VALUES (?, 1, 'ns')", + catalogId, + ) + h.execute( + """ + INSERT INTO hog_table + (catalog_id, table_id, created_snapshot, file_format) + VALUES (?, 2, 0, 'clickhouse-mergetree-packed') + """, + catalogId, + ) + h.execute( + """ + INSERT INTO hog_table_version + (catalog_id, table_id, begin_snapshot, namespace_id, name, properties) + VALUES (?, 2, 0, 1, 'packed', + '{"write.format.default":"clickhouse-mergetree-packed"}'::jsonb) + """, + catalogId, + ) + h.execute( + """ + INSERT INTO hog_column + (catalog_id, table_id, field_id, begin_snapshot, name, col_type, ordinal) + VALUES (?, 2, 1, 0, 'id', 'long', 0) + """, + catalogId, + ) + h.execute( + """ + INSERT INTO hog_data_file + (catalog_id, data_file_id, table_id, begin_snapshot, path, file_format, + record_count, file_size_bytes, row_id_start) + VALUES (?, 2, 2, 0, 's3://b/v25/b.packed', + 'clickhouse-mergetree-packed', 1, 10, 1) + """, + catalogId, + ) + + h.execute( + """ + INSERT INTO hog_upload + (catalog_id, upload_id, owner, prefix, path, file_kind, file_format) + VALUES (?, '00000000-0000-4000-8000-000000000001', + '00000000-0000-4000-8000-000000000002', 's3://b/v25', + 's3://b/v25/trino-upload/claimed.packed', 'data', + 'clickhouse-mergetree-packed') + """, + catalogId, + ) + assertThatThrownBy { + h.execute( + """ + INSERT INTO hog_data_file + (catalog_id, data_file_id, table_id, begin_snapshot, path, + record_count, file_size_bytes, row_id_start) + VALUES (?, 20, 1, 0, + 's3://b/v25/trino-upload/claimed.packed', 1, 10, 20) + """, + catalogId, + ) + }.isInstanceOf(UnableToExecuteStatementException::class.java) + + data class InvalidFile(val id: Long, val tableId: Long, val format: String, val path: String) + for ((id, tableId, format, path) in listOf( + InvalidFile(3, 1, "clickhouse-mergetree-packed", "s3://b/v25/mixed.packed"), + InvalidFile(4, 2, "parquet", "s3://b/v25/mixed.parquet"), + InvalidFile(5, 1, "orc", "s3://b/v25/c.orc"), + )) { + assertThatThrownBy { + h.createUpdate( + """ + INSERT INTO hog_data_file + (catalog_id, data_file_id, table_id, begin_snapshot, path, file_format, + record_count, file_size_bytes, row_id_start) + VALUES (:catalog, :id, :table, 0, :path, :format, 1, 10, :id) + """, + ) + .bind("catalog", catalogId) + .bind("id", id) + .bind("table", tableId) + .bind("path", path) + .bind("format", format) + .execute() + }.isInstanceOf(UnableToExecuteStatementException::class.java) + } + val forbiddenMutations = + listOf( + "UPDATE hog_column SET end_snapshot = 1 WHERE catalog_id = $catalogId AND table_id = 2", + "INSERT INTO hog_partition_spec (catalog_id, table_id, spec_id, begin_snapshot) " + + "VALUES ($catalogId, 2, 1, 1)", + "UPDATE hog_data_file SET end_snapshot = 1 " + + "WHERE catalog_id = $catalogId AND data_file_id = 2", + "INSERT INTO hog_delete_file " + + "(catalog_id, delete_file_id, table_id, data_file_id, begin_snapshot, path, " + + "delete_count, file_size_bytes) VALUES " + + "($catalogId, 10, 2, 2, 1, 's3://b/v25/dv.puffin', 1, 10)", + "UPDATE hog_table_version SET properties = '{}'::jsonb " + + "WHERE catalog_id = $catalogId AND table_id = 2 AND end_snapshot IS NULL", + ) + forbiddenMutations.forEach { sql -> + assertThatThrownBy { h.execute(sql) } + .isInstanceOf(UnableToExecuteStatementException::class.java) + } + h.execute( + "UPDATE hog_table SET dropped_snapshot = 2 WHERE catalog_id = ? AND table_id = 2", + catalogId, + ) + assertThat( + h.execute( + "UPDATE hog_column SET end_snapshot = 2 WHERE catalog_id = ? AND table_id = 2", + catalogId, + ), + ).isEqualTo(1) + assertThat( + h.createQuery( + """ + SELECT is_nullable FROM information_schema.columns + WHERE table_name = 'hog_upload' AND column_name = 'file_format' + """, + ).mapTo(String::class.java).one(), + ).isEqualTo("YES") + } + } + } + + @Test + fun `a preexisting use of the reserved property blocks migration`() { + PgTestSupport.freshDatabaseAt("24").use { db -> + db.jdbi.useHandle { h -> + val catalogId = + h.createQuery( + "INSERT INTO hog_catalog (name, data_path) VALUES ('v25-reserved', 's3://b/v25r') " + + "RETURNING catalog_id", + ).mapTo(Long::class.java).one() + h.execute( + "INSERT INTO hog_namespace (catalog_id, namespace_id, name) VALUES (?, 1, 'ns')", + catalogId, + ) + h.execute( + "INSERT INTO hog_table (catalog_id, table_id, created_snapshot) VALUES (?, 1, 0)", + catalogId, + ) + h.execute( + """ + INSERT INTO hog_table_version + (catalog_id, table_id, begin_snapshot, namespace_id, name, properties) + VALUES (?, 1, 0, 1, 't', + '{"write.format.default":"clickhouse-mergetree-packed"}'::jsonb) + """, + catalogId, + ) + } + + assertThatThrownBy { Database.migrate(db.dataSource) } + .hasMessageContaining("write.format.default") + } + } +} diff --git a/server/src/test/kotlin/com/posthog/hoglake/service/TableMetadataTest.kt b/server/src/test/kotlin/com/posthog/hoglake/service/TableMetadataTest.kt index 4b9718a8..2ccc2a59 100644 --- a/server/src/test/kotlin/com/posthog/hoglake/service/TableMetadataTest.kt +++ b/server/src/test/kotlin/com/posthog/hoglake/service/TableMetadataTest.kt @@ -1,6 +1,14 @@ package com.posthog.hoglake.service +import com.posthog.hoglake.model.ColType +import com.posthog.hoglake.model.ColumnDef +import com.posthog.hoglake.model.FileFormats import com.posthog.hoglake.model.HoglakeException +import com.posthog.hoglake.model.NullOrder +import com.posthog.hoglake.model.PartitionFieldDef +import com.posthog.hoglake.model.SortDirection +import com.posthog.hoglake.model.SortFieldDef +import com.posthog.hoglake.model.Transform import org.assertj.core.api.Assertions.assertThatThrownBy import org.junit.jupiter.api.Test @@ -26,4 +34,66 @@ class TableMetadataTest { assertThatThrownBy { TableMetadata.validateComment("\u0000") } .isInstanceOf(HoglakeException.Validation::class.java) } + + @Test + fun `packed format property and fixed layout are validated`() { + val packed = mapOf(FileFormats.TABLE_PROPERTY to FileFormats.CLICKHOUSE_MERGETREE_PACKED) + TableMetadata.validateProperties(emptyMap()) + TableMetadata.validateProperties(mapOf(FileFormats.TABLE_PROPERTY to FileFormats.PARQUET)) + TableMetadata.validateProperties(packed) + assertThatThrownBy { + TableMetadata.validateProperties(mapOf(FileFormats.TABLE_PROPERTY to "orc")) + }.isInstanceOf(HoglakeException.Validation::class.java) + + TableMetadata.validateDefinitionForFormat( + packed, + FileFormats.packedColumnTypes.mapIndexed { index, type -> ColumnDef("c$index", type) }, + emptyList(), + emptyList(), + ) + assertThatThrownBy { + TableMetadata.validateDefinitionForFormat( + packed, + listOf(ColumnDef("amount", ColType.DECIMAL)), + emptyList(), + emptyList(), + ) + }.isInstanceOf(HoglakeException.Validation::class.java) + .hasMessageContaining("decimal") + assertThatThrownBy { + TableMetadata.validateDefinitionForFormat( + packed, + listOf(ColumnDef("id", ColType.LONG)), + listOf(PartitionFieldDef(1, Transform.IDENTITY)), + emptyList(), + ) + }.isInstanceOf(HoglakeException.Validation::class.java) + .hasMessageContaining("partition") + assertThatThrownBy { + TableMetadata.validateDefinitionForFormat( + packed, + listOf(ColumnDef("id", ColType.LONG)), + emptyList(), + listOf(SortFieldDef(1, SortDirection.ASC, NullOrder.NULLS_FIRST)), + ) + }.isInstanceOf(HoglakeException.Validation::class.java) + .hasMessageContaining("sort") + } + + @Test + fun `table format is immutable while unrelated properties may change`() { + val packed = mapOf(FileFormats.TABLE_PROPERTY to FileFormats.CLICKHOUSE_MERGETREE_PACKED) + TableMetadata.requireFormatUnchanged( + packed, + packed + ("owner.team" to "analytics"), + ) + assertThatThrownBy { + TableMetadata.requireFormatUnchanged(emptyMap(), packed) + }.isInstanceOf(HoglakeException.Validation::class.java) + .hasMessageContaining("immutable") + assertThatThrownBy { + TableMetadata.requireFormatUnchanged(packed, emptyMap()) + }.isInstanceOf(HoglakeException.Validation::class.java) + .hasMessageContaining("immutable") + } } diff --git a/server/src/test/kotlin/com/posthog/hoglake/service/UploadServiceIntegrationTest.kt b/server/src/test/kotlin/com/posthog/hoglake/service/UploadServiceIntegrationTest.kt index 26750bb5..7205564e 100644 --- a/server/src/test/kotlin/com/posthog/hoglake/service/UploadServiceIntegrationTest.kt +++ b/server/src/test/kotlin/com/posthog/hoglake/service/UploadServiceIntegrationTest.kt @@ -4,6 +4,7 @@ import com.posthog.hoglake.commit.CommitService import com.posthog.hoglake.model.ColType import com.posthog.hoglake.model.ColumnDef import com.posthog.hoglake.model.CommitRequest +import com.posthog.hoglake.model.FileFormats import com.posthog.hoglake.model.FileRegistration import com.posthog.hoglake.model.HoglakeException import com.posthog.hoglake.model.TableAppend @@ -148,6 +149,51 @@ class UploadServiceIntegrationTest { assertThat(uploads.renew(catalog, owner)).isZero() } + @Test + fun `data claims select a format-specific immutable path`() { + val catalog = catalog() + val owner = UUID.randomUUID() + val prefix = catalogs.getCatalog(catalog).dataPath + val uploadId = UUID.randomUUID() + val packed = + uploads.claim( + catalog, + uploadId, + owner, + prefix, + "data", + FileFormats.CLICKHOUSE_MERGETREE_PACKED, + ) + assertThat(packed.path).endsWith(".packed") + assertThat( + uploads.claim( + catalog, + uploadId, + owner, + prefix, + "data", + FileFormats.CLICKHOUSE_MERGETREE_PACKED, + ), + ).isEqualTo(packed) + assertThatThrownBy { + uploads.claim(catalog, uploadId, owner, prefix, "data", FileFormats.PARQUET) + }.isInstanceOf(HoglakeException.CommitConflict::class.java) + assertThatThrownBy { + commits.commit(catalog, request(catalog, owner, packed)) + }.isInstanceOf(HoglakeException.CommitConflict::class.java) + .hasMessageContaining("upload is fenced") + assertThatThrownBy { + uploads.claim( + catalog, + UUID.randomUUID(), + owner, + prefix, + "delete", + FileFormats.CLICKHOUSE_MERGETREE_PACKED, + ) + }.isInstanceOf(HoglakeException.Validation::class.java) + } + @Test fun `publication and unknown-response replay survive cleanup and table drop`() { val catalog = catalog() diff --git a/server/src/test/resources/com/posthog/hoglake/fuzz/WireDtoParseFuzzTestInputs/wireParseFailsOnlyWithMappedExceptions/commit_full b/server/src/test/resources/com/posthog/hoglake/fuzz/WireDtoParseFuzzTestInputs/wireParseFailsOnlyWithMappedExceptions/commit_full index b69fb2c3..a05b7738 100644 --- a/server/src/test/resources/com/posthog/hoglake/fuzz/WireDtoParseFuzzTestInputs/wireParseFailsOnlyWithMappedExceptions/commit_full +++ b/server/src/test/resources/com/posthog/hoglake/fuzz/WireDtoParseFuzzTestInputs/wireParseFailsOnlyWithMappedExceptions/commit_full @@ -1,5 +1,5 @@ {"read_snapshot":7,"appends":[{"namespace":"ns","table":"t","files":[ - {"path":"s3://b/f.parquet","record_count":10,"file_size_bytes":1024, + {"path":"s3://b/f.parquet","file_format":"parquet","record_count":10,"file_size_bytes":1024, "footer_size":256,"column_stats":[{"field_id":1,"value_count":10, "null_count":0,"lower_bound":"AAAAAA==","upper_bound":"/////w=="}], "partition_values":["2026-01-01",null]}], From 8bebc3d74e2d1b3cb5401cf282b68123e7458fe1 Mon Sep 17 00:00:00 2001 From: fuziontech Date: Sat, 3 Oct 2026 04:23:34 +0000 Subject: [PATCH 3/6] fix: gate packed MergeTree rollout and CI coverage Co-authored-by: Shelley --- .github/workflows/pyhoglake-checks.yml | 48 +++++++++++++++++++ pyhoglake/README.md | 3 +- server/README.md | 17 ++++++- .../main/kotlin/com/posthog/hoglake/App.kt | 2 +- .../main/kotlin/com/posthog/hoglake/Config.kt | 9 ++++ .../posthog/hoglake/service/AlterService.kt | 3 +- .../posthog/hoglake/service/CatalogService.kt | 23 ++++++++- .../hoglake/service/TableCreationService.kt | 2 +- .../src/main/resources/openapi/hoglake.yaml | 4 +- .../com/posthog/hoglake/ConfigBootProbe.kt | 4 +- .../hoglake/PackedMergeTreeConfigTest.kt | 38 +++++++++++++++ .../posthog/hoglake/api/ApiIntegrationTest.kt | 17 ++++++- .../hoglake/api/CatalogApiIntegrationTest.kt | 40 ++++++++++++++++ .../CompactionPlanningIntegrationTest.kt | 2 +- .../service/TableCreationIntegrationTest.kt | 23 +++++++++ 15 files changed, 222 insertions(+), 13 deletions(-) create mode 100644 server/src/test/kotlin/com/posthog/hoglake/PackedMergeTreeConfigTest.kt diff --git a/.github/workflows/pyhoglake-checks.yml b/.github/workflows/pyhoglake-checks.yml index fa5c5d82..1f44fe98 100644 --- a/.github/workflows/pyhoglake-checks.yml +++ b/.github/workflows/pyhoglake-checks.yml @@ -61,6 +61,54 @@ jobs: - name: Tests run: uv run pytest -q + packed-clickhouse: + name: Packed ClickHouse round trip + runs-on: ubuntu-latest + timeout-minutes: 20 + defaults: + run: + working-directory: pyhoglake + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - uses: astral-sh/setup-uv@bec219d24cd3e171d82865faccec33120bb574f4 # v10.1.0 + + - run: uv sync --locked + + - name: Prepare pinned clickhouse-local wrapper + env: + CLICKHOUSE_IMAGE: >- + clickhouse/clickhouse-server:26.9.8.3@sha256:230b973a00b5bac5b8925bd2898fae95a039aa6f25063fa8c507ffe9e2c7d770 + run: | + docker pull "$CLICKHOUSE_IMAGE" + cat > "$RUNNER_TEMP/clickhouse-local" <> "$GITHUB_ENV" + + # This is the real ClickHouse writer/reader path: it verifies the + # one-file data.packed layout, Arrow type round-trip, readonly attach, + # and an exact two-part snapshot. The output assertion makes a skip a + # failure instead of a green job. + - name: Mandatory packed adapter round trip and multi-part read + run: | + output=$(uv run pytest -q -rA \ + tests/test_packed.py::test_real_clickhouse_packed_append_and_snapshot_read) + printf '%s\n' "$output" + case "$output" in + *"1 passed"*) ;; + *) echo "::error::mandatory packed ClickHouse test did not pass"; exit 1 ;; + esac + build: name: Build sdist and wheel runs-on: ubuntu-latest diff --git a/pyhoglake/README.md b/pyhoglake/README.md index 754479d9..ca3627c0 100644 --- a/pyhoglake/README.md +++ b/pyhoglake/README.md @@ -110,7 +110,8 @@ for s in catalog.snapshots(before=head + 1): ## Packed MergeTree adapter `ClickHousePackedAdapter` is the explicit writer and reader for tables created with -`write.format.default=clickhouse-mergetree-packed`: +`write.format.default=clickhouse-mergetree-packed`. The server must first complete the rollout +sequence in `server/README.md` and enable `HOGLAKE_PACKED_MERGETREE_ENABLED=true`: ```python import pyarrow as pa diff --git a/server/README.md b/server/README.md index 0a2df65c..b34456c0 100644 --- a/server/README.md +++ b/server/README.md @@ -244,11 +244,24 @@ The packed contract is deliberately narrow: one registered object contains the b ClickHouse `data.packed` part, `column_stats` must be present (an empty array means no bounds), and `footer_size` plus Parquet `split_offsets` are forbidden. +Creation is behind `HOGLAKE_PACKED_MERGETREE_ENABLED`, default `false`. The rollout order is: + +1. Deploy the migration and packed-aware server binary to every replica with the gate off. +2. Verify no older API replica remains. Database fences protect existing packed tables, but the gate + prevents creation while the fleet is mixed. +3. Set `HOGLAKE_PACKED_MERGETREE_ENABLED=true` on every replica that can receive table-creation + requests and complete that rollout. +4. Only then create packed tables and start packed writers. + +Turning the gate off again prevents new packed tables; it does not make existing packed tables +unreadable or change their immutable format. + Packed tables are append-only, unpartitioned, unsorted, and fixed-schema. They admit only the scalar types covered by the Python `ClickHousePackedAdapter`: boolean, signed and unsigned integers, float, double, string, binary, date, and timestamp variants, within the configured ClickHouse -version's `Date32` and `DateTime64` value ranges. Column add, drop, rename, promotion, -partition or sort changes, truncate, and deletion-vector commits are refused. Drop remains valid. +version's `Date32` and `DateTime64` value ranges. Column add, drop, rename, promotion, column-comment +changes, partition or sort changes, truncate, and deletion-vector commits are refused. Table comments, +unrelated table properties, table rename, and drop remain valid. The Python adapter exports only a part directory containing exactly `data.packed`; projections and ClickHouse metadata that escape that file are outside this format. diff --git a/server/src/main/kotlin/com/posthog/hoglake/App.kt b/server/src/main/kotlin/com/posthog/hoglake/App.kt index 4bd5a14a..7c3091e0 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/App.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/App.kt @@ -82,7 +82,7 @@ class App private constructor( */ val requestDispatcher = RequestDispatcher(cfg.requestThreads) - private val catalogService = CatalogService(jdbi) + private val catalogService = CatalogService(jdbi, cfg.packedMergeTreeEnabled) private val commitService = CommitService( jdbi, diff --git a/server/src/main/kotlin/com/posthog/hoglake/Config.kt b/server/src/main/kotlin/com/posthog/hoglake/Config.kt index 55fcd941..3f803dee 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/Config.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/Config.kt @@ -13,6 +13,15 @@ data class Config( val dbUser: String = env("HOGLAKE_DB_USER", "hoglake"), val dbPassword: String = env("HOGLAKE_DB_PASSWORD", "hoglake"), val dbPoolSize: Int = env("HOGLAKE_DB_POOL_SIZE", "10").toInt(), + /** + * Rollout gate for creating `clickhouse-mergetree-packed` tables. + * + * Default false. Enable only after every server replica runs the packed + * format contract; database fences keep old replicas from mutating an + * existing packed table, but creation stays opt-in so rollout order is an + * explicit operator decision. + */ + val packedMergeTreeEnabled: Boolean = boolEnv("HOGLAKE_PACKED_MERGETREE_ENABLED", false), /** * How many threads serve blocking route handlers (#218). * diff --git a/server/src/main/kotlin/com/posthog/hoglake/service/AlterService.kt b/server/src/main/kotlin/com/posthog/hoglake/service/AlterService.kt index 14b96cb7..d9b901a3 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/service/AlterService.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/service/AlterService.kt @@ -98,7 +98,8 @@ class AlterService(private val jdbi: Jdbi) { ops.firstOrNull { it is AlterOp.AddColumn || it is AlterOp.DropColumn || it is AlterOp.RenameColumn || it is AlterOp.PromoteColumn || - it is AlterOp.SetPartitionSpec || it is AlterOp.SetSortOrder + it is AlterOp.SetPartitionSpec || it is AlterOp.SetSortOrder || + it is AlterOp.SetColumnComment } if (unsupported != null) { throw HoglakeException.Validation( diff --git a/server/src/main/kotlin/com/posthog/hoglake/service/CatalogService.kt b/server/src/main/kotlin/com/posthog/hoglake/service/CatalogService.kt index f38b21bd..d403766e 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/service/CatalogService.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/service/CatalogService.kt @@ -54,7 +54,10 @@ import java.util.UUID * Reads honor the versioned-row rule: visible at S iff * begin_snapshot <= S AND (end_snapshot IS NULL OR S < end_snapshot). */ -class CatalogService(private val jdbi: Jdbi) { +class CatalogService( + private val jdbi: Jdbi, + private val packedMergeTreeEnabled: Boolean = false, +) { companion object { /** * Ceiling on a catalog's `data_path`. @@ -307,7 +310,7 @@ class CatalogService(private val jdbi: Jdbi) { ): TableInfo { TableMetadata.validateComment(comment) TableMetadata.validateProperties(properties) - TableMetadata.validateDefinitionForFormat(properties, columns, partitionFields, sortFields) + validateTableFormatDefinition(properties, columns, partitionFields, sortFields) validateTableDefinition(name, columns) val cols = initialColumns(columns) // Publication catches definition validation and records a rejected receipt. @@ -405,6 +408,22 @@ class CatalogService(private val jdbi: Jdbi) { ) } + internal fun validateTableFormatDefinition( + properties: Map, + columns: List, + partitionFields: List, + sortFields: List, + ) { + if (FileFormats.isPacked(properties) && !packedMergeTreeEnabled) { + throw HoglakeException.Validation( + "packed MergeTree table creation is disabled; enable " + + "HOGLAKE_PACKED_MERGETREE_ENABLED only after every server replica " + + "supports the packed format contract", + ) + } + TableMetadata.validateDefinitionForFormat(properties, columns, partitionFields, sortFields) + } + internal fun validateTableDefinition( name: String, columns: List, diff --git a/server/src/main/kotlin/com/posthog/hoglake/service/TableCreationService.kt b/server/src/main/kotlin/com/posthog/hoglake/service/TableCreationService.kt index 5eb67004..3744ac68 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/service/TableCreationService.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/service/TableCreationService.kt @@ -93,7 +93,7 @@ class TableCreationService( // the forest prepare refused. TableMetadata.validateComment(definition.comment) TableMetadata.validateProperties(definition.properties) - TableMetadata.validateDefinitionForFormat( + catalogs.validateTableFormatDefinition( definition.properties, definition.columns, definition.partitionFields, diff --git a/server/src/main/resources/openapi/hoglake.yaml b/server/src/main/resources/openapi/hoglake.yaml index 369e6620..465f6950 100644 --- a/server/src/main/resources/openapi/hoglake.yaml +++ b/server/src/main/resources/openapi/hoglake.yaml @@ -2967,7 +2967,9 @@ components: description: > Versioned table properties. Replaced as a whole by set_properties; an empty object clears it. `write.format.default` selects `parquet` (also the default when absent) or - `clickhouse-mergetree-packed` and is immutable after table creation. Packed tables + `clickhouse-mergetree-packed` and is immutable after table creation. Creating the packed + value requires the server rollout gate `HOGLAKE_PACKED_MERGETREE_ENABLED=true`; the + default-disabled gate returns 422 until the fleet is explicitly enabled. Packed tables are append-only, unpartitioned, unsorted, fixed-schema tables over the documented scalar subset. Other keys are inert user metadata. Keys are lowercase ASCII identifiers; hoglake. and trino. prefixes and partitioning, sorted_by, location, diff --git a/server/src/test/kotlin/com/posthog/hoglake/ConfigBootProbe.kt b/server/src/test/kotlin/com/posthog/hoglake/ConfigBootProbe.kt index 6014a84b..e3d873ad 100644 --- a/server/src/test/kotlin/com/posthog/hoglake/ConfigBootProbe.kt +++ b/server/src/test/kotlin/com/posthog/hoglake/ConfigBootProbe.kt @@ -29,8 +29,8 @@ object ConfigBootProbe { @JvmStatic fun main(args: Array) { try { - Config() - println("CONSTRUCTED") + val config = Config() + println("CONSTRUCTED packedMergeTreeEnabled=${config.packedMergeTreeEnabled}") } catch (e: IllegalArgumentException) { println("REFUSED: ${e.message}") // Not `exitProcess(2)`: the point is an exit code the parent diff --git a/server/src/test/kotlin/com/posthog/hoglake/PackedMergeTreeConfigTest.kt b/server/src/test/kotlin/com/posthog/hoglake/PackedMergeTreeConfigTest.kt new file mode 100644 index 00000000..adcd19e2 --- /dev/null +++ b/server/src/test/kotlin/com/posthog/hoglake/PackedMergeTreeConfigTest.kt @@ -0,0 +1,38 @@ +package com.posthog.hoglake + +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import java.util.concurrent.TimeUnit + +class PackedMergeTreeConfigTest { + private fun boot(value: String?): Pair { + val java = System.getProperty("java.home") + "/bin/java" + val builder = + ProcessBuilder(java, "-cp", System.getProperty("java.class.path"), ConfigBootProbe::class.java.name) + .redirectErrorStream(true) + if (value == null) { + builder.environment().remove("HOGLAKE_PACKED_MERGETREE_ENABLED") + } else { + builder.environment()["HOGLAKE_PACKED_MERGETREE_ENABLED"] = value + } + val process = builder.start() + val output = process.inputStream.bufferedReader().readText() + assertThat(process.waitFor(120, TimeUnit.SECONDS)).isTrue() + return process.exitValue() to output + } + + @Test + fun `packed table creation rollout gate defaults off and can be enabled`() { + assertThat(Config(packedMergeTreeEnabled = true).packedMergeTreeEnabled).isTrue() + assertThat(boot(null)).isEqualTo(0 to "CONSTRUCTED packedMergeTreeEnabled=false\n") + assertThat(boot("true")).isEqualTo(0 to "CONSTRUCTED packedMergeTreeEnabled=true\n") + } + + @Test + fun `packed table creation rollout gate refuses ambiguous boolean spellings`() { + val (code, output) = boot("yes") + assertThat(code).isEqualTo(2) + assertThat(output) + .contains("HOGLAKE_PACKED_MERGETREE_ENABLED must be 'true' or 'false', got 'yes'") + } +} diff --git a/server/src/test/kotlin/com/posthog/hoglake/api/ApiIntegrationTest.kt b/server/src/test/kotlin/com/posthog/hoglake/api/ApiIntegrationTest.kt index 4c52ecdb..76ff0998 100644 --- a/server/src/test/kotlin/com/posthog/hoglake/api/ApiIntegrationTest.kt +++ b/server/src/test/kotlin/com/posthog/hoglake/api/ApiIntegrationTest.kt @@ -40,7 +40,15 @@ class ApiIntegrationTest { // the probe no longer borrows from the request pool, so the fixture // has to say which database it should reach. Without it the probe // would dial Config's localhost:5432 default and report 503. - private val app = App.build(Config(hydratorIntervalMs = 0, jdbcUrl = db.jdbcUrl), db.jdbi) + private val app = + App.build( + Config( + hydratorIntervalMs = 0, + jdbcUrl = db.jdbcUrl, + packedMergeTreeEnabled = true, + ), + db.jdbi, + ) private val json = ObjectMapper() @AfterAll @@ -153,6 +161,13 @@ class ApiIntegrationTest { ) assertThat(alter.status).isEqualTo(HttpStatusCode.UnprocessableEntity) assertThat(body(alter)["detail"].asText()).contains("fixed schema") + val commentAlter = + client.postJson( + "/v1/catalogs/packed/namespaces/analytics/tables/events/alter", + """{"ops":[{"op":"set_column_comment","name":"id","comment":"identifier"}]}""", + ) + assertThat(commentAlter.status).isEqualTo(HttpStatusCode.UnprocessableEntity) + assertThat(body(commentAlter)["detail"].asText()).contains("SetColumnComment") val tableUuid = body(created)["table_uuid"].asText() val truncate = client.post( diff --git a/server/src/test/kotlin/com/posthog/hoglake/api/CatalogApiIntegrationTest.kt b/server/src/test/kotlin/com/posthog/hoglake/api/CatalogApiIntegrationTest.kt index e663b2a5..9f88fde0 100644 --- a/server/src/test/kotlin/com/posthog/hoglake/api/CatalogApiIntegrationTest.kt +++ b/server/src/test/kotlin/com/posthog/hoglake/api/CatalogApiIntegrationTest.kt @@ -44,4 +44,44 @@ class CatalogApiIntegrationTest { } } } + + @Test + fun `packed table creation is disabled by default with typed validation`() { + PgTestSupport.freshDatabase().use { db -> + App.build(Config(hydratorIntervalMs = 0, metricsIntervalMs = 0), db.jdbi).use { app -> + testApplication { + application { app.module(this) } + assertThat( + client.post("/v1/catalogs") { + contentType(ContentType.Application.Json) + setBody("""{"name":"rollout","data_path":"s3://bucket/rollout/"}""") + }.status, + ).isEqualTo(HttpStatusCode.Created) + assertThat( + client.post("/v1/catalogs/rollout/namespaces") { + contentType(ContentType.Application.Json) + setBody("""{"name":"main"}""") + }.status, + ).isEqualTo(HttpStatusCode.Created) + val refused = + client.post("/v1/catalogs/rollout/namespaces/main/tables") { + contentType(ContentType.Application.Json) + setBody( + """ + {"name":"packed", + "properties":{"write.format.default":"clickhouse-mergetree-packed"}, + "columns":[{"name":"id","type":"long"}]} + """, + ) + } + assertThat(refused.status).isEqualTo(HttpStatusCode.UnprocessableEntity) + val error = ObjectMapper().readTree(refused.bodyAsText()) + assertThat(error["error"].asText()).isEqualTo("validation") + assertThat(error["detail"].asText()).contains("HOGLAKE_PACKED_MERGETREE_ENABLED") + assertThat(client.get("/v1/catalogs/rollout/namespaces/main/tables").bodyAsText()) + .isEqualTo("[]") + } + } + } + } } diff --git a/server/src/test/kotlin/com/posthog/hoglake/compaction/CompactionPlanningIntegrationTest.kt b/server/src/test/kotlin/com/posthog/hoglake/compaction/CompactionPlanningIntegrationTest.kt index 737ad3cd..4f81cc81 100644 --- a/server/src/test/kotlin/com/posthog/hoglake/compaction/CompactionPlanningIntegrationTest.kt +++ b/server/src/test/kotlin/com/posthog/hoglake/compaction/CompactionPlanningIntegrationTest.kt @@ -45,7 +45,7 @@ import java.util.concurrent.atomic.AtomicInteger @TestInstance(TestInstance.Lifecycle.PER_CLASS) class CompactionPlanningIntegrationTest { private val db = PgTestSupport.freshDatabase() - private val catalogs = CatalogService(db.jdbi) + private val catalogs = CatalogService(db.jdbi, packedMergeTreeEnabled = true) private val commits = CommitService(db.jdbi) private val alter = AlterService(db.jdbi) private val counter = AtomicInteger(0) diff --git a/server/src/test/kotlin/com/posthog/hoglake/service/TableCreationIntegrationTest.kt b/server/src/test/kotlin/com/posthog/hoglake/service/TableCreationIntegrationTest.kt index fdb99890..00af7a8a 100644 --- a/server/src/test/kotlin/com/posthog/hoglake/service/TableCreationIntegrationTest.kt +++ b/server/src/test/kotlin/com/posthog/hoglake/service/TableCreationIntegrationTest.kt @@ -3,6 +3,7 @@ package com.posthog.hoglake.service import com.posthog.hoglake.commit.CommitService import com.posthog.hoglake.model.ColType import com.posthog.hoglake.model.ColumnDef +import com.posthog.hoglake.model.FileFormats import com.posthog.hoglake.model.FileRegistration import com.posthog.hoglake.model.HoglakeException import com.posthog.hoglake.model.NullOrder @@ -46,6 +47,28 @@ class TableCreationIntegrationTest { private fun file(operation: TableCreation) = FileRegistration(operation.writePath + "part.parquet", 7, 100, 20) + @Test + fun `atomic packed creation obeys the rollout gate`() { + val catalog = catalog() + val packed = + definition.copy( + properties = + mapOf( + FileFormats.TABLE_PROPERTY to FileFormats.CLICKHOUSE_MERGETREE_PACKED, + ), + ) + assertThatThrownBy { creations.prepare(catalog, UUID.randomUUID(), packed) } + .isInstanceOf(HoglakeException.Validation::class.java) + .hasMessageContaining("HOGLAKE_PACKED_MERGETREE_ENABLED") + + val enabledCatalogs = CatalogService(db.jdbi, packedMergeTreeEnabled = true) + val enabledCreations = TableCreationService(db.jdbi, enabledCatalogs, CommitService(db.jdbi)) + val prepared = enabledCreations.prepare(catalog, UUID.randomUUID(), packed) + assertThat(prepared.definition.properties) + .containsEntry(FileFormats.TABLE_PROPERTY, FileFormats.CLICKHOUSE_MERGETREE_PACKED) + enabledCreations.abort(catalog, prepared.operationId) + } + @Test fun `metadata survives creation replay versioned edits rename and replacement`() { val catalog = catalog() From 803d0352a7a1e03106f81dfb24b4308b9e3984ab Mon Sep 17 00:00:00 2001 From: fuziontech Date: Mon, 5 Oct 2026 16:16:18 +0000 Subject: [PATCH 4/6] Rework packed format fencing to service checks and address review feedback - Rework DB triggers and manifest FK into Kotlin service-layer checks - Leave hog_data_file format check NOT VALID without ACCESS EXCLUSIVE full scan - Make hog_table.file_format the single source of truth and derive write.format.default - Filter packed tables from MaintenanceSummarySampler to prevent small-file debt leaks - Remove per-group format re-query from CompactionService - Relax packed stats requirement to counts-only (no per-column bounds required) - Fix pyhoglake numeric block ordering for 10+ parts with test coverage - Lower pyhoglake resource defaults and stream Arrow output - Document format invariants and no-rollback constraint in AGENT.md and README.md Co-authored-by: Shelley --- AGENT.md | 18 ++ pyhoglake/src/pyhoglake/packed.py | 68 +++++-- pyhoglake/tests/test_packed.py | 71 ++++++- server/README.md | 18 +- server/schema.sql | 113 +----------- .../posthog/hoglake/commit/CommitService.kt | 6 - .../hoglake/compaction/CompactionService.kt | 35 +--- .../com/posthog/hoglake/model/FileFormats.kt | 2 +- .../posthog/hoglake/persistence/TableRepo.kt | 50 ++++- .../posthog/hoglake/service/AlterService.kt | 2 +- .../posthog/hoglake/service/CatalogService.kt | 13 +- .../service/MaintenanceSummarySampler.kt | 16 +- .../V25__packed_mergetree_format.sql | 173 ++---------------- .../hoglake/commit/CommitServiceTest.kt | 7 +- ...MergetreeFormatMigrationIntegrationTest.kt | 84 ++------- 15 files changed, 265 insertions(+), 411 deletions(-) diff --git a/AGENT.md b/AGENT.md index 1160dbc2..091720e6 100644 --- a/AGENT.md +++ b/AGENT.md @@ -421,6 +421,24 @@ there would break that gate on every build. therefore put the subclass arm FIRST — `ErrorMapping`, `Metrics.commitFailureResult` and `Audit.failureOutcome` all do, and each says so. +13. **Table file format is immutable and homogeneous per table**, stored + authoritatively on `hog_table.file_format` (the single source of truth; + `write.format.default` is derived for clients at read time). + - Parquet is the default format across all tables. + - `clickhouse-mergetree-packed` is admitted for fixed-schema, + unpartitioned, unsorted append-only tables when the rollout gate + `HOGLAKE_PACKED_MERGETREE_ENABLED=true` is enabled. + - Packed tables do not support schema evolution (columns, partition specs, + or sort orders cannot be added, dropped, renamed, promoted, or commented), + deletion vectors, explicit row IDs, truncation, or Parquet compaction. + - Compaction and maintenance planners select Parquet files only. The + maintenance debt sampler excludes non-Parquet tables so packed tables + never leak permanent small-file debt into debt scores or metrics. + - The server never opens stored data files (Parquet or packed parts); + validation is enforced at registration/commit without I/O. + - **No rollback past V25 once a packed table exists**: pre-V25 binaries + lack format filters in their compaction candidate queries and will fail + if run against a database containing packed tables. ## Scale doctrine (read before touching a query, a loop or a lock) diff --git a/pyhoglake/src/pyhoglake/packed.py b/pyhoglake/src/pyhoglake/packed.py index f2e59425..537dac33 100644 --- a/pyhoglake/src/pyhoglake/packed.py +++ b/pyhoglake/src/pyhoglake/packed.py @@ -78,11 +78,11 @@ "binary": "String", } -_DEFAULT_MAX_PART_BYTES = 8 * 1024**3 -_DEFAULT_MAX_SNAPSHOT_BYTES = 64 * 1024**3 -_DEFAULT_MAX_SNAPSHOT_PARTS = 10_000 -_DEFAULT_MAX_MEMORY_BYTES = 2 * 1024**3 -_DEFAULT_MAX_RESULT_BYTES = 8 * 1024**3 +_DEFAULT_MAX_PART_BYTES = 2 * 1024**3 +_DEFAULT_MAX_SNAPSHOT_BYTES = 8 * 1024**3 +_DEFAULT_MAX_SNAPSHOT_PARTS = 500 +_DEFAULT_MAX_MEMORY_BYTES = 1 * 1024**3 +_DEFAULT_MAX_RESULT_BYTES = 2 * 1024**3 _ARROW_TYPES: dict[str, pa.DataType] = { "boolean": pa.bool_(), @@ -308,7 +308,6 @@ def prepare_append( f"{self._max_part_bytes}", status_code=None, ) - stats = _column_stats(aligned, info.columns) body = { "owner": str(operation), "prefix": ( @@ -334,7 +333,7 @@ def prepare_append( "file_format": CLICKHOUSE_MERGETREE_PACKED_FORMAT, "record_count": aligned.num_rows, "file_size_bytes": packed_size, - "column_stats": stats, + "column_stats": [], } return { "idempotency_key": str(operation), @@ -497,13 +496,57 @@ def read( + " MODIFY SETTING table_readonly=1", ) columns = ", ".join(_quote_identifier(field.name) for field in schema) - raw = self._run( - root, + query = ( f"SELECT {columns} FROM {_quote_identifier('packed_read')} " - "ORDER BY _part, _part_offset FORMAT ArrowStream", + "ORDER BY toUInt64(splitByChar('_', _part)[2]), _part_offset FORMAT ArrowStream" ) - with pa.ipc.open_stream(raw) as reader: - result = reader.read_all() + command = [ + *self._command, + "local", + "--path", + str(root), + "--background_schedule_pool_size", + str(self._max_threads), + "--max_threads", + str(self._max_threads), + "--max_memory_usage", + str(self._max_memory_bytes), + "--max_result_bytes", + str(self._max_result_bytes), + "--result_overflow_mode", + "throw", + "--output_format_arrow_string_as_string", + "0", + "--query", + query, + ] + try: + process = subprocess.Popen( + command, + cwd=root, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + except OSError as error: + raise HoglakeError( + f"could not execute clickhouse local: {error}" + ) from error + try: + assert process.stdout is not None + with pa.ipc.open_stream(process.stdout) as reader: + result = reader.read_all() + _, stderr = process.communicate(timeout=self._timeout) + except subprocess.TimeoutExpired as error: + process.kill() + process.communicate() + raise HoglakeError( + f"clickhouse local exceeded the {self._timeout:g}s timeout" + ) from error + if process.returncode != 0: + detail = stderr.decode("utf-8", errors="replace").strip() + raise HoglakeError( + "clickhouse local failed" + (f": {detail[:2000]}" if detail else "") + ) try: result = result.cast(schema) except (pa.ArrowInvalid, pa.ArrowNotImplementedError) as error: @@ -594,6 +637,7 @@ def _run(self, root: Path, sql: str, *, input_bytes: bytes | None = None) -> byt input=input_bytes, capture_output=True, check=False, + cwd=root, timeout=self._timeout, ) except subprocess.TimeoutExpired as error: diff --git a/pyhoglake/tests/test_packed.py b/pyhoglake/tests/test_packed.py index ebae4b5d..21607c84 100644 --- a/pyhoglake/tests/test_packed.py +++ b/pyhoglake/tests/test_packed.py @@ -430,7 +430,7 @@ def claim(request: httpx.Request) -> httpx.Response: assert registration["file_format"] == CLICKHOUSE_MERGETREE_PACKED_FORMAT assert "footer_size" not in registration assert "split_offsets" not in registration - assert len(registration["column_stats"]) == len(data.column_names) + assert registration["column_stats"] == [] key = claimed.removeprefix("s3://") assert fake_s3.files[key] @@ -505,6 +505,75 @@ def claim(request: httpx.Request) -> httpx.Response: table._namespace._catalog._client.close() +def test_real_clickhouse_packed_multi_part_numeric_ordering(httpx_mock) -> None: + executable = os.environ.get("PYHOGLAKE_CLICKHOUSE") + if not executable: + pytest.skip("set PYHOGLAKE_CLICKHOUSE to a clickhouse executable or wrapper") + fake_s3 = FakeS3() + table = _table(fake_s3) + adapter = ClickHousePackedAdapter(Path(executable), timeout=180) + table_url = f"{BASE}/v1/catalogs/cat/namespaces/ns1/tables/events" + + schema = pa.schema([pa.field("id", pa.int64(), nullable=False)]) + table_wire = _table_wire() + table_wire["columns"] = [ + {"field_id": 1, "name": "id", "type": "long", "nullable": False, "ordinal": 0} + ] + table_wire["read_snapshot_id"] = 12 + + # Produce 12 independent single-row parts + scan_entries = [] + expected_tables = [] + for i in range(1, 13): + row_table = pa.table({"id": pa.array([i], pa.int64())}, schema=schema) + expected_tables.append(row_table) + claimed_uri = f"s3://bkt/lake/data/ns1/events/test/part_{i}.packed" + + httpx_mock.add_response( + method="GET", url=f"{table_url}?totals=false", json=table_wire + ) + httpx_mock.add_response( + method="PUT", + url=re.compile(f"{re.escape(BASE)}/v1/catalogs/cat/uploads/.*"), + json={"path": claimed_uri}, + ) + _ = adapter.prepare_append(table, row_table) + part_key = claimed_uri.removeprefix("s3://") + scan_entries.append( + { + "data_file": { + "data_file_id": i, + "path": claimed_uri, + "file_format": CLICKHOUSE_MERGETREE_PACKED_FORMAT, + "record_count": 1, + "file_size_bytes": len(fake_s3.files[part_key]), + "row_id_start": i - 1, + "stats_state": "provided", + "begin_snapshot": i, + } + } + ) + + read_wire = dict(table_wire) + read_wire["read_snapshot_id"] = 12 + httpx_mock.add_response( + method="GET", + url=f"{table_url}?snapshot=12&totals=false", + json=read_wire, + ) + httpx_mock.add_response( + method="GET", + url=f"{table_url}/scan?snapshot=12", + json=scan_entries, + ) + + result = adapter.read(table, snapshot=12) + expected = pa.concat_tables(expected_tables) + assert result.column("id").to_pylist() == list(range(1, 13)) + assert result.equals(expected) + table._namespace._catalog._client.close() + + @pytest.mark.integration def test_live_server_claim_commit_and_multi_part_snapshot_read(live_server_url) -> None: executable = os.environ.get("PYHOGLAKE_CLICKHOUSE") diff --git a/server/README.md b/server/README.md index b34456c0..3bdaedee 100644 --- a/server/README.md +++ b/server/README.md @@ -241,18 +241,20 @@ is the one that grows fastest (hoglake#240). A table may set `write.format.default=clickhouse-mergetree-packed` at creation. Absence means `parquet`, and the effective value is immutable. Every registration must match its table format. The packed contract is deliberately narrow: one registered object contains the bytes of one -ClickHouse `data.packed` part, `column_stats` must be present (an empty array means no bounds), and +ClickHouse `data.packed` part, counts-only stats are sufficient (per-column bounds are not required), and `footer_size` plus Parquet `split_offsets` are forbidden. Creation is behind `HOGLAKE_PACKED_MERGETREE_ENABLED`, default `false`. The rollout order is: 1. Deploy the migration and packed-aware server binary to every replica with the gate off. -2. Verify no older API replica remains. Database fences protect existing packed tables, but the gate - prevents creation while the fleet is mixed. +2. Verify no older API replica remains. The gate prevents packed table creation while the fleet is mixed. 3. Set `HOGLAKE_PACKED_MERGETREE_ENABLED=true` on every replica that can receive table-creation requests and complete that rollout. 4. Only then create packed tables and start packed writers. +**No rollback past V25 once a packed table exists**: older server binaries do not filter on format in +their compaction candidate queries and will fail if run against a catalog containing packed tables. + Turning the gate off again prevents new packed tables; it does not make existing packed tables unreadable or change their immutable format. @@ -267,12 +269,12 @@ ClickHouse metadata that escape that file are outside this format. Upload claims accept `file_format=clickhouse-mergetree-packed`, persist that selection, and mint a fresh `.packed` path. Publication verifies the claimed format and settles the claim in the same -transaction as the file row. The table identity row also stores the immutable format, and a database -foreign key prevents any server version from publishing a differently formatted data row. The -Parquet hydrator filters +transaction as the file row. The table identity row stores the immutable format on `hog_table.file_format` +as the single source of truth, enforced strictly at the service layer. The Parquet hydrator filters to `file_format='parquet'`, and both compaction candidate selection and direct planned-group execution -refuse packed inputs. Expiry, retirement, removal-queue fencing, snapshots, row-range allocation, and -exact-path cleanup keep their existing one-row/one-object behavior. +refuse packed inputs. The maintenance debt sampler excludes non-Parquet tables so packed tables never +leak permanent small-file debt into debt scores or metrics. Expiry, retirement, removal-queue fencing, +snapshots, row-range allocation, and exact-path cleanup keep their existing one-row/one-object behavior. The DuckDB extension and Hedgerow reject packed tables and files. The Python adapter is the only reader and writer in this repository. It materializes an exact snapshot's registered part list into diff --git a/server/schema.sql b/server/schema.sql index 4853fb1b..59179813 100644 --- a/server/schema.sql +++ b/server/schema.sql @@ -153,8 +153,6 @@ CREATE TABLE hog_table ( UNIQUE (catalog_id, table_uuid), CONSTRAINT hog_table_file_format_check CHECK (file_format IN ('parquet', 'clickhouse-mergetree-packed')), - CONSTRAINT hog_table_format_identity - UNIQUE (catalog_id, table_id, file_format), -- The edge has no FK, so this is its only structural defence: a -- self-edge would be a 1-cycle for the recursive walk that follows it. CONSTRAINT hog_table_no_self_replacement CHECK (replaced_table_id <> table_id) @@ -289,8 +287,7 @@ CREATE TABLE hog_data_file ( begin_snapshot bigint NOT NULL, end_snapshot bigint, path text NOT NULL, -- absolute object-store URI; no relative chains - file_format text NOT NULL DEFAULT 'parquet' - CHECK (file_format IN ('parquet', 'clickhouse-mergetree-packed')), + file_format text NOT NULL DEFAULT 'parquet', record_count bigint NOT NULL CHECK (record_count >= 0), file_size_bytes bigint NOT NULL CHECK (file_size_bytes >= 0), footer_size bigint, @@ -331,12 +328,9 @@ CREATE TABLE hog_data_file ( FOREIGN KEY (catalog_id, table_id) REFERENCES hog_table ON DELETE CASCADE, CHECK (end_snapshot IS NULL OR end_snapshot > begin_snapshot) ); --- NOT VALID avoids a manifest-wide deployment scan. Existing rows predate --- the alternate format and are therefore Parquet; new rows are checked. ALTER TABLE hog_data_file - ADD CONSTRAINT hog_data_file_table_format_fk - FOREIGN KEY (catalog_id, table_id, file_format) - REFERENCES hog_table (catalog_id, table_id, file_format) + ADD CONSTRAINT hog_data_file_file_format_check + CHECK (file_format IN ('parquet', 'clickhouse-mergetree-packed')) NOT VALID; CREATE INDEX hog_data_file_live ON hog_data_file (catalog_id, table_id, begin_snapshot) @@ -792,107 +786,6 @@ CREATE TABLE hog_upload ( CREATE INDEX hog_upload_owner ON hog_upload (catalog_id, owner) WHERE state = 'active'; CREATE INDEX hog_upload_cleanup ON hog_upload (catalog_id, last_scheduled_at, upload_id) WHERE state <> 'registered'; -CREATE FUNCTION hog_enforce_table_version_format() -RETURNS trigger LANGUAGE plpgsql AS ' -DECLARE - expected_format text; - actual_format text; -BEGIN - SELECT file_format INTO expected_format - FROM hog_table - WHERE catalog_id = NEW.catalog_id AND table_id = NEW.table_id; - actual_format := COALESCE(NEW.properties ->> ''write.format.default'', ''parquet''); - IF expected_format IS NOT NULL AND actual_format <> expected_format THEN - RAISE EXCEPTION ''table property format % does not match immutable table format %'', - actual_format, expected_format - USING ERRCODE = ''check_violation''; - END IF; - RETURN NEW; -END'; - -CREATE TRIGGER hog_table_version_format_guard -BEFORE INSERT OR UPDATE OF properties ON hog_table_version -FOR EACH ROW EXECUTE FUNCTION hog_enforce_table_version_format(); - -CREATE FUNCTION hog_reject_packed_table_mutation() -RETURNS trigger LANGUAGE plpgsql AS ' -DECLARE - row_catalog_id bigint; - row_table_id bigint; - table_format text; - table_created_snapshot bigint; - table_dropped_snapshot bigint; -BEGIN - IF TG_OP = ''DELETE'' THEN - row_catalog_id := OLD.catalog_id; - row_table_id := OLD.table_id; - ELSE - row_catalog_id := NEW.catalog_id; - row_table_id := NEW.table_id; - END IF; - - SELECT file_format, created_snapshot, hog_table.dropped_snapshot - INTO table_format, table_created_snapshot, table_dropped_snapshot - FROM hog_table - WHERE catalog_id = row_catalog_id AND table_id = row_table_id; - - IF table_format IS DISTINCT FROM ''clickhouse-mergetree-packed'' - OR table_dropped_snapshot IS NOT NULL THEN - IF TG_OP = ''DELETE'' THEN - RETURN OLD; - END IF; - RETURN NEW; - END IF; - - IF TG_TABLE_NAME = ''hog_column'' AND TG_OP = ''INSERT'' - AND NEW.begin_snapshot = table_created_snapshot THEN - RETURN NEW; - END IF; - - RAISE EXCEPTION ''packed MergeTree table does not support mutation through %'', TG_TABLE_NAME - USING ERRCODE = ''check_violation''; -END'; - -CREATE TRIGGER hog_packed_column_guard -BEFORE INSERT OR UPDATE OR DELETE ON hog_column -FOR EACH ROW EXECUTE FUNCTION hog_reject_packed_table_mutation(); -CREATE TRIGGER hog_packed_partition_guard -BEFORE INSERT OR UPDATE OR DELETE ON hog_partition_spec -FOR EACH ROW EXECUTE FUNCTION hog_reject_packed_table_mutation(); -CREATE TRIGGER hog_packed_sort_guard -BEFORE INSERT OR UPDATE OR DELETE ON hog_sort_spec -FOR EACH ROW EXECUTE FUNCTION hog_reject_packed_table_mutation(); -CREATE TRIGGER hog_packed_delete_file_guard -BEFORE INSERT OR UPDATE ON hog_delete_file -FOR EACH ROW EXECUTE FUNCTION hog_reject_packed_table_mutation(); -CREATE TRIGGER hog_packed_data_file_end_guard -BEFORE UPDATE OF end_snapshot ON hog_data_file -FOR EACH ROW EXECUTE FUNCTION hog_reject_packed_table_mutation(); - -CREATE FUNCTION hog_enforce_claimed_data_file_format() -RETURNS trigger LANGUAGE plpgsql AS ' -DECLARE - claimed_format text; -BEGIN - IF strpos(NEW.path, ''/trino-upload/'') = 0 THEN - RETURN NEW; - END IF; - - SELECT COALESCE(file_format, ''parquet'') INTO claimed_format - FROM hog_upload - WHERE catalog_id = NEW.catalog_id AND path = NEW.path AND file_kind = ''data''; - IF claimed_format IS NOT NULL AND claimed_format <> NEW.file_format THEN - RAISE EXCEPTION ''data file format % does not match upload claim format %'', - NEW.file_format, claimed_format - USING ERRCODE = ''check_violation''; - END IF; - RETURN NEW; -END'; - -CREATE TRIGGER hog_claimed_data_file_format_guard -BEFORE INSERT ON hog_data_file -FOR EACH ROW EXECUTE FUNCTION hog_enforce_claimed_data_file_format(); - -- Replacement lineage (V14): the access path for the walk -- OffsetRepo.releaseSupersededOffsets runs on every offset commit and -- inside the expiry sweep, following `replaced_table_id` forward from a diff --git a/server/src/main/kotlin/com/posthog/hoglake/commit/CommitService.kt b/server/src/main/kotlin/com/posthog/hoglake/commit/CommitService.kt index 1e91bac7..ee78c6f0 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/commit/CommitService.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/commit/CommitService.kt @@ -1600,12 +1600,6 @@ class CommitService( "packed MergeTree registration ${file.path} must contain at least one row and one byte", ) } - if (file.columnStats == null) { - throw HoglakeException.Validation( - "packed MergeTree registration ${file.path} must provide column_stats; " + - "use an empty array when no bounds are available", - ) - } if (file.footerSize != null) { throw HoglakeException.Validation( "packed MergeTree registration ${file.path} must not provide footer_size", diff --git a/server/src/main/kotlin/com/posthog/hoglake/compaction/CompactionService.kt b/server/src/main/kotlin/com/posthog/hoglake/compaction/CompactionService.kt index f7443a37..5a600564 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/compaction/CompactionService.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/compaction/CompactionService.kt @@ -1341,7 +1341,7 @@ class CompactionService( namespace = ns.name, table = t.name, tableId = t.tableId, - fileFormat = FileFormats.tableFormat(t.properties), + fileFormat = t.fileFormat, columns = TableRepo.columnsAt(h, cat.catalogId, t.tableId, cat.headSnapshotId), sortFields = SortRepo.sortSpecAt(h, cat.catalogId, t.tableId, cat.headSnapshotId) @@ -3242,33 +3242,7 @@ class CompactionService( val ctx = jdbi.inTransactionUnchecked { h -> h.execute("SET TRANSACTION ISOLATION LEVEL REPEATABLE READ READ ONLY") - val resolved = tableContext(h, catalog, namespace, table, defaults) - if (group.files.isNotEmpty()) { - val actualFormats = - h.createQuery( - """ - SELECT data_file_id, file_format - FROM hog_data_file - WHERE catalog_id = :catalogId AND table_id = :tableId - AND data_file_id IN () - """, - ) - .bind("catalogId", resolved.catalogId) - .bind("tableId", resolved.tableId) - .bindList("ids", group.files.map { it.dataFileId }) - .map { rs, _ -> rs.getLong("data_file_id") to rs.getString("file_format") } - .list() - .toMap() - group.files.firstOrNull { - actualFormats[it.dataFileId] != FileFormats.PARQUET - }?.let { file -> - throw InvalidDataException( - "compaction supports parquet inputs only; data_file_id ${file.dataFileId} " + - "uses '${actualFormats[file.dataFileId] ?: "unknown"}'", - ) - } - } - resolved + tableContext(h, catalog, namespace, table, defaults) } return compactGroup(ctx, group) } @@ -3280,6 +3254,11 @@ class CompactionService( ctx: TableContext, group: CompactionGroup, ): GroupOutcome { + if (ctx.fileFormat != FileFormats.PARQUET) { + throw InvalidDataException( + "compaction supports parquet tables only; table '${ctx.table}' uses '${ctx.fileFormat}'", + ) + } group.files.firstOrNull { it.fileFormat != FileFormats.PARQUET }?.let { file -> throw InvalidDataException( "compaction supports parquet inputs only; data_file_id ${file.dataFileId} " + diff --git a/server/src/main/kotlin/com/posthog/hoglake/model/FileFormats.kt b/server/src/main/kotlin/com/posthog/hoglake/model/FileFormats.kt index bb3a407d..683faee6 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/model/FileFormats.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/model/FileFormats.kt @@ -35,7 +35,7 @@ object FileFormats { ColType.BINARY, ) - fun tableFormat(properties: Map): String = properties[TABLE_PROPERTY] ?: PARQUET + fun tableFormat(properties: Map): String = properties[TABLE_PROPERTY]?.lowercase() ?: PARQUET fun isPacked(properties: Map): Boolean = tableFormat(properties) == CLICKHOUSE_MERGETREE_PACKED } diff --git a/server/src/main/kotlin/com/posthog/hoglake/persistence/TableRepo.kt b/server/src/main/kotlin/com/posthog/hoglake/persistence/TableRepo.kt index de791e88..5c028620 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/persistence/TableRepo.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/persistence/TableRepo.kt @@ -24,6 +24,7 @@ data class TableRow( val name: String, val comment: String? = null, val properties: Map = emptyMap(), + val fileFormat: String = FileFormats.PARQUET, ) /** Rollup row from hog_table_stats. */ @@ -43,13 +44,27 @@ data class TableStatsRow( object TableRepo { private val tableRowMapper = RowMapper { rs, _ -> + val fileFormat = + try { + rs.getString("file_format") ?: FileFormats.PARQUET + } catch (_: Exception) { + FileFormats.PARQUET + } + val rawProperties = Pg.fromJson(rs.getString("properties"))!!.mapValues { (_, value) -> value as String } + val properties = + if (fileFormat != FileFormats.PARQUET) { + rawProperties + (FileFormats.TABLE_PROPERTY to fileFormat) + } else { + rawProperties - FileFormats.TABLE_PROPERTY + } TableRow( tableId = rs.getLong("table_id"), tableUuid = rs.getObject("table_uuid") as UUID, namespaceId = rs.getLong("namespace_id"), name = rs.getString("name"), comment = rs.getString("comment"), - properties = Pg.fromJson(rs.getString("properties"))!!.mapValues { (_, value) -> value as String }, + properties = properties, + fileFormat = fileFormat, ) } @@ -204,6 +219,7 @@ object TableRepo { comment: String? = null, properties: Map = emptyMap(), ) { + val storedProperties = properties - FileFormats.TABLE_PROPERTY try { handle.createUpdate( """ @@ -217,7 +233,7 @@ object TableRepo { .bind("namespaceId", namespaceId) .bind("name", name) .bind("comment", comment) - .bind("properties", Pg.toJson(properties)) + .bind("properties", Pg.toJson(storedProperties)) .execute() } catch (e: UnableToExecuteStatementException) { if (Pg.isUniqueViolation(e)) { @@ -316,6 +332,27 @@ object TableRepo { * retirement gate (`dropped_snapshot <= earliest_snapshot_id`) * preserves until the floor has passed the drop. */ + @Volatile + private var hasTableFormatColumn: Boolean? = null + + private fun formatColumnExpr(handle: Handle): String { + if (hasTableFormatColumn == true) return "t.file_format" + val exists = + handle.createQuery( + """ + SELECT EXISTS ( + SELECT 1 FROM information_schema.columns + WHERE table_name = 'hog_table' AND column_name = 'file_format' + ) + """, + ).mapTo(Boolean::class.java).one() + if (exists) { + hasTableFormatColumn = true + return "t.file_format" + } + return "'parquet' AS file_format" + } + fun findAt( handle: Handle, catalogId: Long, @@ -325,7 +362,8 @@ object TableRepo { ): TableRow? = handle.createQuery( """ - SELECT t.table_id, t.table_uuid, tv.namespace_id, tv.name, tv.comment, tv.properties + SELECT t.table_id, t.table_uuid, tv.namespace_id, tv.name, tv.comment, tv.properties, + ${formatColumnExpr(handle)} FROM hog_table_version tv JOIN hog_table t ON t.catalog_id = tv.catalog_id AND t.table_id = tv.table_id @@ -367,7 +405,8 @@ object TableRepo { ): TableRow? = handle.createQuery( """ - SELECT t.table_id, t.table_uuid, tv.namespace_id, tv.name, tv.comment, tv.properties + SELECT t.table_id, t.table_uuid, tv.namespace_id, tv.name, tv.comment, tv.properties, + ${formatColumnExpr(handle)} FROM hog_table_version tv JOIN hog_table t ON t.catalog_id = tv.catalog_id AND t.table_id = tv.table_id @@ -410,7 +449,8 @@ object TableRepo { ): List = handle.createQuery( """ - SELECT t.table_id, t.table_uuid, tv.namespace_id, tv.name, tv.comment, tv.properties + SELECT t.table_id, t.table_uuid, tv.namespace_id, tv.name, tv.comment, tv.properties, + ${formatColumnExpr(handle)} FROM hog_table_version tv JOIN hog_table t ON t.catalog_id = tv.catalog_id AND t.table_id = tv.table_id diff --git a/server/src/main/kotlin/com/posthog/hoglake/service/AlterService.kt b/server/src/main/kotlin/com/posthog/hoglake/service/AlterService.kt index d9b901a3..852ea798 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/service/AlterService.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/service/AlterService.kt @@ -93,7 +93,7 @@ class AlterService(private val jdbi: Jdbi) { TableMetadata.validateProperties(it.properties) TableMetadata.requireFormatUnchanged(t.properties, it.properties) } - if (com.posthog.hoglake.model.FileFormats.isPacked(t.properties)) { + if (t.fileFormat == com.posthog.hoglake.model.FileFormats.CLICKHOUSE_MERGETREE_PACKED) { val unsupported = ops.firstOrNull { it is AlterOp.AddColumn || it is AlterOp.DropColumn || diff --git a/server/src/main/kotlin/com/posthog/hoglake/service/CatalogService.kt b/server/src/main/kotlin/com/posthog/hoglake/service/CatalogService.kt index d403766e..0a7efde7 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/service/CatalogService.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/service/CatalogService.kt @@ -355,6 +355,7 @@ class CatalogService( // Record the replacement edge with the row itself: the lineage a // consumer's offset release depends on must be a fact, not a // convention re-derived later (OffsetRepo.releaseSupersededOffsets). + val format = FileFormats.tableFormat(properties) val createdUuid = TableRepo.insertTable( h, @@ -363,7 +364,7 @@ class CatalogService( alloc.snapshotId, tableUuid, replacementTableId, - FileFormats.tableFormat(properties), + format, ) // nodeCount, not columns.size: a nested column needs one id per // NODE, not one per top-level column. Allocating by size would @@ -379,11 +380,17 @@ class CatalogService( AlterService( jdbi, ).installPartitionSpec(h, cat.catalogId, tableId, alloc.snapshotId, cols, partitionFields) + val effectiveProperties = + if (format != FileFormats.PARQUET) { + properties + (FileFormats.TABLE_PROPERTY to format) + } else { + properties - FileFormats.TABLE_PROPERTY + } return TableInfo( tableId = tableId, tableUuid = createdUuid, comment = comment, - properties = properties, + properties = effectiveProperties, namespace = ns.name, name = name, columns = cols, @@ -530,7 +537,7 @@ class CatalogService( currentTableUuid = t.tableUuid, ) } - if (com.posthog.hoglake.model.FileFormats.isPacked(t.properties)) { + if (t.fileFormat == FileFormats.CLICKHOUSE_MERGETREE_PACKED) { throw HoglakeException.Validation("packed MergeTree tables do not support truncate") } val alloc = CatalogRepo.allocateSnapshot(h, cat.catalogId) diff --git a/server/src/main/kotlin/com/posthog/hoglake/service/MaintenanceSummarySampler.kt b/server/src/main/kotlin/com/posthog/hoglake/service/MaintenanceSummarySampler.kt index 6a5738d1..0344c807 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/service/MaintenanceSummarySampler.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/service/MaintenanceSummarySampler.kt @@ -256,11 +256,15 @@ class MaintenanceSummarySampler( // mutable set in it would throw away every in-flight // scan on the deploy that added it — and the set would // be stale by construction anyway. - val dropped = + // Exclude dropped tables and non-Parquet (packed) tables from + // compaction debt and tier summarization. Packed tables are + // never compacted and must not leak permanent small-file debt. + val excluded = h.createQuery( """ SELECT table_id FROM hog_table - WHERE catalog_id = :id AND dropped_snapshot IS NOT NULL + WHERE catalog_id = :id + AND (dropped_snapshot IS NOT NULL OR file_format IS DISTINCT FROM 'parquet') """, ).bind("id", job.catalogId).mapTo(Long::class.javaObjectType).list().toSet() val rows = @@ -323,15 +327,15 @@ class MaintenanceSummarySampler( // dropped table per generation is the price of an O(1) // skip; paging the table instead costs one of them per // 10,000 ROWS. - val leadsInDroppedTable = rows.firstOrNull()?.takeIf { it.table in dropped } - if (leadsInDroppedTable != null) { - scan.table = leadsInDroppedTable.table + val leadsInExcludedTable = rows.firstOrNull()?.takeIf { it.table in excluded } + if (leadsInExcludedTable != null) { + scan.table = leadsInExcludedTable.table scan.size = Long.MAX_VALUE scan.file = Long.MAX_VALUE checkpoint(h, job.catalogId, generation, scan) return@inTransactionUnchecked true } - accumulate(h, job.catalogId, generation, scan, rows.filter { it.table !in dropped }) + accumulate(h, job.catalogId, generation, scan, rows.filter { it.table !in excluded }) rows.lastOrNull()?.let { scan.table = it.table scan.size = it.size diff --git a/server/src/main/resources/db/migration/V25__packed_mergetree_format.sql b/server/src/main/resources/db/migration/V25__packed_mergetree_format.sql index 2b00baf7..6d212f69 100644 --- a/server/src/main/resources/db/migration/V25__packed_mergetree_format.sql +++ b/server/src/main/resources/db/migration/V25__packed_mergetree_format.sql @@ -1,6 +1,7 @@ -- Admit the restricted single-object ClickHouse packed-part format. --- The immutable table row carries the format so a database foreign key, --- not only the current server binary, prevents mixed-format publication. +-- Format is stored on the immutable hog_table row as the single source of +-- truth. Enforcement is performed at the service layer on creation, alter, +-- and commit, gated by HOGLAKE_PACKED_MERGETREE_ENABLED. SET LOCAL lock_timeout = '5s'; ALTER TABLE hog_table @@ -13,184 +14,32 @@ ALTER TABLE hog_table NOT VALID; ALTER TABLE hog_table VALIDATE CONSTRAINT hog_table_file_format_check; -DO $$ -BEGIN - IF NOT EXISTS ( - SELECT 1 FROM pg_constraint - WHERE conname = 'hog_table_format_identity' - AND conrelid = 'hog_table'::regclass - ) THEN - ALTER TABLE hog_table - ADD CONSTRAINT hog_table_format_identity - UNIQUE (catalog_id, table_id, file_format); - END IF; -END $$; - +-- Existing data-file rows remain valid without a full table scan under +-- ACCESS EXCLUSIVE lock: V1 only admitted Parquet, and new rows are validated +-- on insert. The constraint stays NOT VALID to avoid a multi-minute lock on +-- large production catalogs. ALTER TABLE hog_data_file DROP CONSTRAINT IF EXISTS hog_data_file_file_format_check; ALTER TABLE hog_data_file ADD CONSTRAINT hog_data_file_file_format_check CHECK (file_format IN ('parquet', 'clickhouse-mergetree-packed')) NOT VALID; -ALTER TABLE hog_data_file VALIDATE CONSTRAINT hog_data_file_file_format_check; - -DO $$ -BEGIN - IF NOT EXISTS ( - SELECT 1 FROM pg_constraint - WHERE conname = 'hog_data_file_table_format_fk' - AND conrelid = 'hog_data_file'::regclass - ) THEN - -- Existing rows can stay unscanned: before this migration the data-file - -- CHECK admitted only Parquet, and every existing table receives the - -- Parquet default above. PostgreSQL still enforces a NOT VALID foreign - -- key for every row inserted after the constraint is installed. - ALTER TABLE hog_data_file - ADD CONSTRAINT hog_data_file_table_format_fk - FOREIGN KEY (catalog_id, table_id, file_format) - REFERENCES hog_table (catalog_id, table_id, file_format) - NOT VALID; - END IF; -END $$; -- Format-aware claims are nullable for rolling deploys. A claim minted by an -- older replica carries NULL and may settle a Parquet data registration only. ALTER TABLE hog_upload ADD COLUMN IF NOT EXISTS file_format text; --- Reserve the table property safely. Earlier binaries accepted arbitrary --- properties, so stop the migration rather than reinterpret retained metadata --- that already used this key for another purpose. +-- Reserve the table property safely (case-insensitively). Earlier binaries +-- accepted arbitrary properties, so stop the migration rather than reinterpret +-- retained metadata that already used this key for another purpose. DO $$ BEGIN IF EXISTS ( SELECT 1 FROM hog_table_version WHERE properties ? 'write.format.default' - AND properties ->> 'write.format.default' <> 'parquet' + AND lower(properties ->> 'write.format.default') <> 'parquet' ) THEN RAISE EXCEPTION 'retained table metadata already uses reserved property write.format.default; remove it before V25'; END IF; END $$; - --- A version row must repeat the immutable identity-row format. This trigger --- also fences an older replica that tries to create a packed property while --- its old INSERT still lets hog_table default to Parquet. -CREATE OR REPLACE FUNCTION hog_enforce_table_version_format() -RETURNS trigger LANGUAGE plpgsql AS ' -DECLARE - expected_format text; - actual_format text; -BEGIN - SELECT file_format INTO expected_format - FROM hog_table - WHERE catalog_id = NEW.catalog_id AND table_id = NEW.table_id; - actual_format := COALESCE(NEW.properties ->> ''write.format.default'', ''parquet''); - IF expected_format IS NOT NULL AND actual_format <> expected_format THEN - RAISE EXCEPTION ''table property format % does not match immutable table format %'', - actual_format, expected_format - USING ERRCODE = ''check_violation''; - END IF; - RETURN NEW; -END'; - -DROP TRIGGER IF EXISTS hog_table_version_format_guard ON hog_table_version; -CREATE TRIGGER hog_table_version_format_guard -BEFORE INSERT OR UPDATE OF properties ON hog_table_version -FOR EACH ROW EXECUTE FUNCTION hog_enforce_table_version_format(); - --- Old binaries do not know the fixed-layout restrictions. Fence every --- unsupported catalog mutation at the database boundary during a rolling --- deploy. Initial column inserts at the table's creation snapshot and all --- cleanup after a table is marked dropped remain legal. -CREATE OR REPLACE FUNCTION hog_reject_packed_table_mutation() -RETURNS trigger LANGUAGE plpgsql AS ' -DECLARE - row_catalog_id bigint; - row_table_id bigint; - table_format text; - table_created_snapshot bigint; - table_dropped_snapshot bigint; -BEGIN - IF TG_OP = ''DELETE'' THEN - row_catalog_id := OLD.catalog_id; - row_table_id := OLD.table_id; - ELSE - row_catalog_id := NEW.catalog_id; - row_table_id := NEW.table_id; - END IF; - - SELECT file_format, created_snapshot, hog_table.dropped_snapshot - INTO table_format, table_created_snapshot, table_dropped_snapshot - FROM hog_table - WHERE catalog_id = row_catalog_id AND table_id = row_table_id; - - IF table_format IS DISTINCT FROM ''clickhouse-mergetree-packed'' - OR table_dropped_snapshot IS NOT NULL THEN - IF TG_OP = ''DELETE'' THEN - RETURN OLD; - END IF; - RETURN NEW; - END IF; - - IF TG_TABLE_NAME = ''hog_column'' AND TG_OP = ''INSERT'' - AND NEW.begin_snapshot = table_created_snapshot THEN - RETURN NEW; - END IF; - - RAISE EXCEPTION ''packed MergeTree table does not support mutation through %'', TG_TABLE_NAME - USING ERRCODE = ''check_violation''; -END'; - -DROP TRIGGER IF EXISTS hog_packed_column_guard ON hog_column; -CREATE TRIGGER hog_packed_column_guard -BEFORE INSERT OR UPDATE OR DELETE ON hog_column -FOR EACH ROW EXECUTE FUNCTION hog_reject_packed_table_mutation(); - -DROP TRIGGER IF EXISTS hog_packed_partition_guard ON hog_partition_spec; -CREATE TRIGGER hog_packed_partition_guard -BEFORE INSERT OR UPDATE OR DELETE ON hog_partition_spec -FOR EACH ROW EXECUTE FUNCTION hog_reject_packed_table_mutation(); - -DROP TRIGGER IF EXISTS hog_packed_sort_guard ON hog_sort_spec; -CREATE TRIGGER hog_packed_sort_guard -BEFORE INSERT OR UPDATE OR DELETE ON hog_sort_spec -FOR EACH ROW EXECUTE FUNCTION hog_reject_packed_table_mutation(); - -DROP TRIGGER IF EXISTS hog_packed_delete_file_guard ON hog_delete_file; -CREATE TRIGGER hog_packed_delete_file_guard -BEFORE INSERT OR UPDATE ON hog_delete_file -FOR EACH ROW EXECUTE FUNCTION hog_reject_packed_table_mutation(); - -DROP TRIGGER IF EXISTS hog_packed_data_file_end_guard ON hog_data_file; -CREATE TRIGGER hog_packed_data_file_end_guard -BEFORE UPDATE OF end_snapshot ON hog_data_file -FOR EACH ROW EXECUTE FUNCTION hog_reject_packed_table_mutation(); - --- Old replicas do not read hog_upload.file_format. Enforce the claim format --- on the data-row INSERT itself so they cannot settle a packed claim with a --- legacy registration that defaults to Parquet. Unclaimed writer paths skip --- the indexed probe entirely. -CREATE OR REPLACE FUNCTION hog_enforce_claimed_data_file_format() -RETURNS trigger LANGUAGE plpgsql AS ' -DECLARE - claimed_format text; -BEGIN - IF strpos(NEW.path, ''/trino-upload/'') = 0 THEN - RETURN NEW; - END IF; - - SELECT COALESCE(file_format, ''parquet'') INTO claimed_format - FROM hog_upload - WHERE catalog_id = NEW.catalog_id AND path = NEW.path AND file_kind = ''data''; - IF claimed_format IS NOT NULL AND claimed_format <> NEW.file_format THEN - RAISE EXCEPTION ''data file format % does not match upload claim format %'', - NEW.file_format, claimed_format - USING ERRCODE = ''check_violation''; - END IF; - RETURN NEW; -END'; - -DROP TRIGGER IF EXISTS hog_claimed_data_file_format_guard ON hog_data_file; -CREATE TRIGGER hog_claimed_data_file_format_guard -BEFORE INSERT ON hog_data_file -FOR EACH ROW EXECUTE FUNCTION hog_enforce_claimed_data_file_format(); diff --git a/server/src/test/kotlin/com/posthog/hoglake/commit/CommitServiceTest.kt b/server/src/test/kotlin/com/posthog/hoglake/commit/CommitServiceTest.kt index 9d240ff4..0ee62086 100644 --- a/server/src/test/kotlin/com/posthog/hoglake/commit/CommitServiceTest.kt +++ b/server/src/test/kotlin/com/posthog/hoglake/commit/CommitServiceTest.kt @@ -640,7 +640,6 @@ class CommitServiceTest { val invalid = listOf( valid.copy(fileFormat = FileFormats.PARQUET), - valid.copy(columnStats = null), valid.copy(footerSize = 20), valid.copy(splitOffsets = listOf(0)), valid.copy(recordCount = 0), @@ -655,6 +654,12 @@ class CommitServiceTest { }.isInstanceOf(HoglakeException.Validation::class.java) assertThat(dataFiles(fx.catalogId)).isEmpty() } + val receipt = + service.commit( + "cat", + CommitRequest(appends = listOf(TableAppend("ns", "events", listOf(valid.copy(columnStats = null))))), + ) + assertThat(receipt.snapshotId).isPositive() } @Test diff --git a/server/src/test/kotlin/com/posthog/hoglake/persistence/V25PackedMergetreeFormatMigrationIntegrationTest.kt b/server/src/test/kotlin/com/posthog/hoglake/persistence/V25PackedMergetreeFormatMigrationIntegrationTest.kt index 641f2ba4..6647f3c5 100644 --- a/server/src/test/kotlin/com/posthog/hoglake/persistence/V25PackedMergetreeFormatMigrationIntegrationTest.kt +++ b/server/src/test/kotlin/com/posthog/hoglake/persistence/V25PackedMergetreeFormatMigrationIntegrationTest.kt @@ -83,81 +83,31 @@ class V25PackedMergetreeFormatMigrationIntegrationTest { catalogId, ) - h.execute( - """ - INSERT INTO hog_upload - (catalog_id, upload_id, owner, prefix, path, file_kind, file_format) - VALUES (?, '00000000-0000-4000-8000-000000000001', - '00000000-0000-4000-8000-000000000002', 's3://b/v25', - 's3://b/v25/trino-upload/claimed.packed', 'data', - 'clickhouse-mergetree-packed') - """, - catalogId, - ) assertThatThrownBy { - h.execute( + h.createUpdate( + """ + INSERT INTO hog_table + (catalog_id, table_id, created_snapshot, file_format) + VALUES (:catalog, 3, 0, 'orc') + """, + ) + .bind("catalog", catalogId) + .execute() + }.isInstanceOf(UnableToExecuteStatementException::class.java) + + assertThatThrownBy { + h.createUpdate( """ INSERT INTO hog_data_file - (catalog_id, data_file_id, table_id, begin_snapshot, path, + (catalog_id, data_file_id, table_id, begin_snapshot, path, file_format, record_count, file_size_bytes, row_id_start) - VALUES (?, 20, 1, 0, - 's3://b/v25/trino-upload/claimed.packed', 1, 10, 20) + VALUES (:catalog, 3, 1, 0, 's3://b/v25/invalid.orc', 'orc', 1, 10, 3) """, - catalogId, ) + .bind("catalog", catalogId) + .execute() }.isInstanceOf(UnableToExecuteStatementException::class.java) - data class InvalidFile(val id: Long, val tableId: Long, val format: String, val path: String) - for ((id, tableId, format, path) in listOf( - InvalidFile(3, 1, "clickhouse-mergetree-packed", "s3://b/v25/mixed.packed"), - InvalidFile(4, 2, "parquet", "s3://b/v25/mixed.parquet"), - InvalidFile(5, 1, "orc", "s3://b/v25/c.orc"), - )) { - assertThatThrownBy { - h.createUpdate( - """ - INSERT INTO hog_data_file - (catalog_id, data_file_id, table_id, begin_snapshot, path, file_format, - record_count, file_size_bytes, row_id_start) - VALUES (:catalog, :id, :table, 0, :path, :format, 1, 10, :id) - """, - ) - .bind("catalog", catalogId) - .bind("id", id) - .bind("table", tableId) - .bind("path", path) - .bind("format", format) - .execute() - }.isInstanceOf(UnableToExecuteStatementException::class.java) - } - val forbiddenMutations = - listOf( - "UPDATE hog_column SET end_snapshot = 1 WHERE catalog_id = $catalogId AND table_id = 2", - "INSERT INTO hog_partition_spec (catalog_id, table_id, spec_id, begin_snapshot) " + - "VALUES ($catalogId, 2, 1, 1)", - "UPDATE hog_data_file SET end_snapshot = 1 " + - "WHERE catalog_id = $catalogId AND data_file_id = 2", - "INSERT INTO hog_delete_file " + - "(catalog_id, delete_file_id, table_id, data_file_id, begin_snapshot, path, " + - "delete_count, file_size_bytes) VALUES " + - "($catalogId, 10, 2, 2, 1, 's3://b/v25/dv.puffin', 1, 10)", - "UPDATE hog_table_version SET properties = '{}'::jsonb " + - "WHERE catalog_id = $catalogId AND table_id = 2 AND end_snapshot IS NULL", - ) - forbiddenMutations.forEach { sql -> - assertThatThrownBy { h.execute(sql) } - .isInstanceOf(UnableToExecuteStatementException::class.java) - } - h.execute( - "UPDATE hog_table SET dropped_snapshot = 2 WHERE catalog_id = ? AND table_id = 2", - catalogId, - ) - assertThat( - h.execute( - "UPDATE hog_column SET end_snapshot = 2 WHERE catalog_id = ? AND table_id = 2", - catalogId, - ), - ).isEqualTo(1) assertThat( h.createQuery( """ From eae212d7a40390d1796cfaed1361a4e9282bdca2 Mon Sep 17 00:00:00 2001 From: James Greenhill Date: Tue, 6 Oct 2026 16:18:10 +0000 Subject: [PATCH 5/6] packed: bounded per-part reads, single-part appends, live CI coverage pyhoglake packed adapter: - Read each part into its own Arrow file (INTO OUTFILE, sorted by _part_offset within the part) through the bounded _run path. The old streamed read never applied its timeout while reading, could deadlock on an undrained stderr pipe, leaked the child on a mid-stream failure, and sorted the whole snapshot under one memory limit. - Verify attached parts against the scan plan (block order + per-part row counts) instead of assuming ATTACH numbering. - Squash inserts into one block so appends over ~256 MiB stay one part. - Coherent defaults: 4 GiB memory, 1 GiB part, 2 GiB snapshot and result. - Reject column names starting with '_' (they shadow ClickHouse virtual columns and silently reorder reads); the server refuses them for packed tables too. - Drop dead min/max stats code; README states counts-only registration, the idempotency-key contract and the limits. CI: - ci/clickhouse-local.sh is the one digest-pinned ClickHouse wrapper. python-live pulls it, enables HOGLAKE_PACKED_MERGETREE_ENABLED on the live server and runs the server-backed packed test plus both real-ClickHouse tests (now marked integration), where a skip fails the run. This replaces the separate packed-clickhouse job, whose test only ran one of the two. - Dev stack (compose + just server run) opens the packed gate so the duckdb-client live suite runs against a default stack. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/pyhoglake-checks.yml | 48 ---- ci/clickhouse-local.sh | 16 ++ ci/live-python.sh | 10 + pyhoglake/README.md | 17 +- pyhoglake/pyproject.toml | 2 +- pyhoglake/src/pyhoglake/packed.py | 208 ++++++++---------- pyhoglake/tests/test_packed.py | 30 +-- server/docker-compose.yml | 3 + server/justfile | 5 +- .../posthog/hoglake/service/TableMetadata.kt | 9 + .../hoglake/service/TableMetadataTest.kt | 18 ++ 11 files changed, 169 insertions(+), 197 deletions(-) create mode 100755 ci/clickhouse-local.sh diff --git a/.github/workflows/pyhoglake-checks.yml b/.github/workflows/pyhoglake-checks.yml index 578491c9..8a3ffe40 100644 --- a/.github/workflows/pyhoglake-checks.yml +++ b/.github/workflows/pyhoglake-checks.yml @@ -64,54 +64,6 @@ jobs: - name: Tests run: uv run pytest -m "not integration" -q - packed-clickhouse: - name: Packed ClickHouse round trip - runs-on: ubuntu-latest - timeout-minutes: 20 - defaults: - run: - working-directory: pyhoglake - steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - persist-credentials: false - - - uses: astral-sh/setup-uv@bec219d24cd3e171d82865faccec33120bb574f4 # v10.1.0 - - - run: uv sync --locked - - - name: Prepare pinned clickhouse-local wrapper - env: - CLICKHOUSE_IMAGE: >- - clickhouse/clickhouse-server:26.9.8.3@sha256:230b973a00b5bac5b8925bd2898fae95a039aa6f25063fa8c507ffe9e2c7d770 - run: | - docker pull "$CLICKHOUSE_IMAGE" - cat > "$RUNNER_TEMP/clickhouse-local" <> "$GITHUB_ENV" - - # This is the real ClickHouse writer/reader path: it verifies the - # one-file data.packed layout, Arrow type round-trip, readonly attach, - # and an exact two-part snapshot. The output assertion makes a skip a - # failure instead of a green job. - - name: Mandatory packed adapter round trip and multi-part read - run: | - output=$(uv run pytest -q -rA \ - tests/test_packed.py::test_real_clickhouse_packed_append_and_snapshot_read) - printf '%s\n' "$output" - case "$output" in - *"1 passed"*) ;; - *) echo "::error::mandatory packed ClickHouse test did not pass"; exit 1 ;; - esac - build: name: Build sdist and wheel runs-on: ubuntu-latest diff --git a/ci/clickhouse-local.sh b/ci/clickhouse-local.sh new file mode 100755 index 00000000..3fe30aee --- /dev/null +++ b/ci/clickhouse-local.sh @@ -0,0 +1,16 @@ +#!/usr/bin/env bash +# The pinned ClickHouse that pyhoglake's packed adapter is tested against, +# exposed as a `clickhouse` executable (PYHOGLAKE_CLICKHOUSE). Packed parts +# are version-coupled bytes, so writer and reader are pinned by digest. +# The adapter only uses temporary directories, which the container sees at +# the same path through the TMPDIR bind mount. +set -euo pipefail + +image=clickhouse/clickhouse-server:26.9.8.3@sha256:230b973a00b5bac5b8925bd2898fae95a039aa6f25063fa8c507ffe9e2c7d770 +tmp=${TMPDIR:-/tmp} +exec docker run --rm -i --network none \ + --user "$(id -u):$(id -g)" \ + --volume "$tmp:$tmp" \ + --env TMPDIR="$tmp" \ + --entrypoint clickhouse \ + "$image" "$@" diff --git a/ci/live-python.sh b/ci/live-python.sh index 5d491ae7..84629475 100644 --- a/ci/live-python.sh +++ b/ci/live-python.sh @@ -47,6 +47,15 @@ ci_python=(uv run --no-project --with defusedxml==0.7.1 python) ./gradlew --no-daemon :installDist -x test -x ktlintCheck --console=plain ) 2>&1 | tee "$report_dir/build.log" "${compose[@]}" up -d --wait --wait-timeout 90 postgres minio +# The packed MergeTree tests run real ClickHouse. Pull the pinned image up +# front so its pull time does not count against the adapter's timeouts. +clickhouse_image=$(sed -n 's/^image=//p' "$repo_dir/ci/clickhouse-local.sh") +for ((attempt = 1; attempt <= 3; attempt++)); do + docker pull --quiet "$clickhouse_image" && break + [[ $attempt -lt 3 ]] || exit 1 + sleep $((attempt * 5)) +done +export PYHOGLAKE_CLICKHOUSE="$repo_dir/ci/clickhouse-local.sh" # The deferred-stats integration test requires the hydrator loop. env HOGLAKE_JDBC_URL="jdbc:postgresql://localhost:$HOGLAKE_PG_PORT/hoglake" \ @@ -56,6 +65,7 @@ env HOGLAKE_JDBC_URL="jdbc:postgresql://localhost:$HOGLAKE_PG_PORT/hoglake" \ HOGLAKE_CLEANUP_INTERVAL_MS=0 HOGLAKE_COMPACTION_INTERVAL_MS=0 \ HOGLAKE_RETIREMENT_INTERVAL_MS=0 HOGLAKE_REINDEX_INTERVAL_MS=0 \ HOGLAKE_METRICS_INTERVAL_MS=0 HOGLAKE_MAINTENANCE_SUMMARY_INTERVAL_MS=0 \ + HOGLAKE_PACKED_MERGETREE_ENABLED=true \ "$repo_dir/server/build/install/hoglake-server/bin/hoglake-server" \ > "$report_dir/server.log" 2>&1 & server_pid=$! diff --git a/pyhoglake/README.md b/pyhoglake/README.md index c19e2800..15ee0ff1 100644 --- a/pyhoglake/README.md +++ b/pyhoglake/README.md @@ -146,7 +146,11 @@ For durable publication, call `prepare_append`, persist the returned JSON payloa idempotency key; replay `commit_prepared` with the persisted payload. Preparation creates one MergeTree part in an isolated local directory, refuses any layout except one `data.packed` file, claims a fresh `.packed` object -path from Hoglake, uploads it, and includes Arrow-derived Iceberg bounds in the registration. +path from Hoglake, uploads it, and registers it with counts only (`column_stats: []`; packed +files carry no per-column bounds). An `idempotency_key` must be a UUID and is sent in canonical +form (`str(uuid.UUID(key))`). `append(idempotency_key=...)` is not a retry handle: calling it again +uploads a second object under a fresh claim, which the server refuses as a different request under +the same key. Retry through the persisted `prepare_append` payload instead. `abandon_prepared` fences a payload that is known not to have committed. Do not abandon after an unknown commit outcome; replay the exact payload instead. @@ -155,13 +159,18 @@ isolated local table, enable ClickHouse `table_readonly`, and return an Arrow ta or scan an object-storage prefix. The adapter currently supports boolean, signed and unsigned integer, float, double, string, binary, date, and second/millisecond/microsecond/nanosecond timestamp columns. Date and timestamp values must also fit the configured ClickHouse version's `Date32`/`DateTime64` -ranges; the adapter does not widen those engine domains. Schemas are fixed. Partition specs, sort +ranges; the adapter does not widen those engine domains. Column names starting with `_` are +refused, because a real column shadows a ClickHouse virtual column (`_part`, `_part_offset`, ...) +that reads depend on for ordering. Schemas are fixed. Partition specs, sort orders, deletion vectors, explicit row IDs, packed-part compaction, and mixed-format tables are refused. The writer and reader should use the same ClickHouse version; no cross-version compatibility or remote-read performance claim is made. -The adapter also bounds each part, part count and total registered bytes in a read, ClickHouse -memory, result bytes, worker threads, and process time. Constructor arguments can lower or raise those limits for a +Rows come back in scan-plan order and, within a part, in insertion order. Each part is read and +sorted on its own, so memory is bounded by the largest part rather than the snapshot. The adapter +also bounds each part (1 GiB), part count (500) and total registered bytes (2 GiB) in a read, +ClickHouse memory (4 GiB), result bytes (2 GiB), worker threads, and process time. One append is +always exactly one part. Constructor arguments can lower or raise those limits for a known workload. The ordinary `Table.append` and `prepare_append_*` methods remain Parquet-only and reject packed tables before writing an object. diff --git a/pyhoglake/pyproject.toml b/pyhoglake/pyproject.toml index 97788013..8d7142dc 100644 --- a/pyhoglake/pyproject.toml +++ b/pyhoglake/pyproject.toml @@ -98,7 +98,7 @@ ignore_missing_imports = true [tool.pytest.ini_options] markers = [ - "integration: tests that require a live hoglake server (HOGLAKE_URL)", + "integration: tests that require a live hoglake server (HOGLAKE_URL) or a real ClickHouse (PYHOGLAKE_CLICKHOUSE)", ] testpaths = ["tests"] # qe_*.py: QE/property-based fuzzing suites live alongside test_*.py diff --git a/pyhoglake/src/pyhoglake/packed.py b/pyhoglake/src/pyhoglake/packed.py index 537dac33..ec780ae7 100644 --- a/pyhoglake/src/pyhoglake/packed.py +++ b/pyhoglake/src/pyhoglake/packed.py @@ -14,17 +14,13 @@ from typing import Any import pyarrow as pa -import pyarrow.compute as pc -from .bounds import encode_bound, normalize_bound from .client import Table, _align_table from .errors import HoglakeError, ValidationError from .formats import CLICKHOUSE_MERGETREE_PACKED_FORMAT, table_format from .models import ( AppendedFile, AppendResult, - Column, - ColumnStats, CommitResult, TableInfo, ) @@ -78,10 +74,14 @@ "binary": "String", } -_DEFAULT_MAX_PART_BYTES = 2 * 1024**3 -_DEFAULT_MAX_SNAPSHOT_BYTES = 8 * 1024**3 +# Coherent by construction: ClickHouse needs ~2.5x a block's size to parse an +# Arrow insert, and a read sorts one part at a time, so the memory limit +# covers the largest part; a snapshot within its byte limit is readable +# within the result limit (packed parts compress, results do not). +_DEFAULT_MAX_PART_BYTES = 1 * 1024**3 +_DEFAULT_MAX_SNAPSHOT_BYTES = 2 * 1024**3 _DEFAULT_MAX_SNAPSHOT_PARTS = 500 -_DEFAULT_MAX_MEMORY_BYTES = 1 * 1024**3 +_DEFAULT_MAX_MEMORY_BYTES = 4 * 1024**3 _DEFAULT_MAX_RESULT_BYTES = 2 * 1024**3 _ARROW_TYPES: dict[str, pa.DataType] = { @@ -123,6 +123,14 @@ def _schema(info: TableInfo) -> pa.Schema: f"packed MergeTree requires a safe catalog column identifier, got {column.name!r}", status_code=None, ) + if column.name.startswith("_"): + # ClickHouse resolves a real column before a virtual one, so a column + # named _part or _part_offset would silently reorder every read. + raise ValidationError( + "packed MergeTree reserves column names starting with '_' for " + f"ClickHouse virtual columns, got {column.name!r}", + status_code=None, + ) if column.children or column.type not in _SUPPORTED_TYPES: raise ValidationError( f"packed MergeTree does not support column {column.name!r} " @@ -163,60 +171,6 @@ def _arrow_stream(table: pa.Table) -> bytes: return sink.getvalue().to_pybytes() -def _min_max(array: pa.Array, column: Column) -> tuple[Any, Any] | None: - values = pc.drop_null(array) - if len(values) == 0: - return None - if column.type in {"float", "double"}: - values = pc.filter(values, pc.invert(pc.is_nan(values))) - if len(values) == 0: - return None - if column.type == "boolean": - any_true = bool(pc.any(values).as_py()) - all_true = bool(pc.all(values).as_py()) - return all_true, any_true - result = pc.min_max(values) - lower = result["min"] - upper = result["max"] - if column.type == "timestamp_ns": - return lower.value, upper.value - return lower.as_py(), upper.as_py() - - -def _column_stats(table: pa.Table, columns: tuple[Column, ...]) -> list[dict[str, Any]]: - stats: list[dict[str, Any]] = [] - for column in columns: - array = table.column(column.name).combine_chunks() - nan_count: int | None = None - if column.type in {"float", "double"}: - non_null = pc.drop_null(array) - nan_count = int(pc.sum(pc.is_nan(non_null)).as_py() or 0) - bounds = _min_max(array, column) - lower = upper = None - if bounds is not None: - lower = normalize_bound( - column.type, - encode_bound(column.type, bounds[0], column.type_params), - lower=True, - ) - upper = normalize_bound( - column.type, - encode_bound(column.type, bounds[1], column.type_params), - lower=False, - ) - stats.append( - ColumnStats( - field_id=column.field_id, - value_count=len(array), - null_count=array.null_count, - nan_count=nan_count, - lower_bound=lower, - upper_bound=upper, - ).to_wire() - ) - return stats - - class ClickHousePackedAdapter: """Produce and read packed parts with a trusted ``clickhouse local`` executable. @@ -294,10 +248,19 @@ def prepare_append( with tempfile.TemporaryDirectory(prefix="pyhoglake-packed-write-") as directory: root = Path(directory) self._run(root, self._create_sql(info, "packed_write")) + # Squash the whole input into one block: by default ClickHouse cuts + # inserts at min_insert_block_size_bytes (~256 MiB uncompressed), + # which would produce several parts for one append. + unbounded = str(2**62) self._run( root, f"INSERT INTO {_quote_identifier('packed_write')} FORMAT ArrowStream", input_bytes=_arrow_stream(aligned), + settings={ + "min_insert_block_size_rows": unbounded, + "min_insert_block_size_bytes": unbounded, + "max_insert_block_size": unbounded, + }, ) part = self._single_part(root, "packed_write", aligned.num_rows) packed_file = part / "data.packed" @@ -488,71 +451,65 @@ def read( f"'all_{index}_{index}_0'" for index in range(1, len(plan) + 1) ) - self._run( + raw = self._run( root, attach + ";ALTER TABLE " + _quote_identifier("packed_read") - + " MODIFY SETTING table_readonly=1", + + " MODIFY SETTING table_readonly=1" + + ";SELECT name, rows FROM system.parts WHERE active AND " + + "table='packed_read' ORDER BY min_block_number FORMAT JSONEachRow", ) + # ATTACH numbers blocks in statement order, so block order is plan + # order. Check it rather than assume it: every part must hold + # exactly the rows its own registration records. + parts = [json.loads(line) for line in raw.splitlines() if line] + attached = [int(part["rows"]) for part in parts] + registered = [item.data_file.record_count for item in plan] + if attached != registered: + raise HoglakeError( + f"attached packed parts hold {attached} rows, the scan plan " + f"registers {registered}" + ) + # One output file per part, sorted within the part only. A single + # ORDER BY across parts would materialize and sort the whole + # snapshot under max_memory_usage; per part, the sort is bounded by + # one part. Files rather than stdout, so the run goes through + # _run's bounded path (both pipes drained, timeout enforced, exit + # status checked before any output is trusted). columns = ", ".join(_quote_identifier(field.name) for field in schema) - query = ( - f"SELECT {columns} FROM {_quote_identifier('packed_read')} " - "ORDER BY toUInt64(splitByChar('_', _part)[2]), _part_offset FORMAT ArrowStream" + outputs = [root / f"result-{index}.arrow" for index in range(len(parts))] + self._run( + root, + ";".join( + f"SELECT {columns} FROM {_quote_identifier('packed_read')} " + f"WHERE _part = {_quote_string(str(part['name']))} " + f"ORDER BY _part_offset INTO OUTFILE {_quote_string(str(output))} " + "FORMAT ArrowStream" + for part, output in zip(parts, outputs, strict=True) + ), ) - command = [ - *self._command, - "local", - "--path", - str(root), - "--background_schedule_pool_size", - str(self._max_threads), - "--max_threads", - str(self._max_threads), - "--max_memory_usage", - str(self._max_memory_bytes), - "--max_result_bytes", - str(self._max_result_bytes), - "--result_overflow_mode", - "throw", - "--output_format_arrow_string_as_string", - "0", - "--query", - query, - ] - try: - process = subprocess.Popen( - command, - cwd=root, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - ) - except OSError as error: - raise HoglakeError( - f"could not execute clickhouse local: {error}" - ) from error - try: - assert process.stdout is not None - with pa.ipc.open_stream(process.stdout) as reader: - result = reader.read_all() - _, stderr = process.communicate(timeout=self._timeout) - except subprocess.TimeoutExpired as error: - process.kill() - process.communicate() - raise HoglakeError( - f"clickhouse local exceeded the {self._timeout:g}s timeout" - ) from error - if process.returncode != 0: - detail = stderr.decode("utf-8", errors="replace").strip() - raise HoglakeError( - "clickhouse local failed" + (f": {detail[:2000]}" if detail else "") + result_bytes = sum(output.stat().st_size for output in outputs) + if result_bytes > self._max_result_bytes: + raise ValidationError( + f"packed snapshot result is {result_bytes} bytes, over " + f"max_result_bytes {self._max_result_bytes}", + status_code=None, ) - try: - result = result.cast(schema) - except (pa.ArrowInvalid, pa.ArrowNotImplementedError) as error: - raise HoglakeError( - f"ClickHouse returned an Arrow schema incompatible with the packed table: {error}" - ) from error + tables: list[pa.Table] = [] + for output in outputs: + try: + with ( + pa.OSFile(str(output)) as source, + pa.ipc.open_stream(source) as reader, + ): + tables.append(reader.read_all().cast(schema)) + except (pa.ArrowInvalid, pa.ArrowNotImplementedError, OSError) as error: + raise HoglakeError( + "ClickHouse returned an Arrow result incompatible with the " + f"packed table: {error}" + ) from error + result = pa.concat_tables(tables) expected_rows = sum(item.data_file.record_count for item in plan) if result.num_rows != expected_rows: raise HoglakeError( @@ -610,7 +567,19 @@ def _single_part(self, root: Path, table: str, expected_rows: int) -> Path: ) return part - def _run(self, root: Path, sql: str, *, input_bytes: bytes | None = None) -> bytes: + def _run( + self, + root: Path, + sql: str, + *, + input_bytes: bytes | None = None, + settings: Mapping[str, str] | None = None, + ) -> bytes: + extra = [ + arg + for key, value in (settings or {}).items() + for arg in (f"--{key}", value) + ] command = [ *self._command, "local", @@ -628,6 +597,7 @@ def _run(self, root: Path, sql: str, *, input_bytes: bytes | None = None) -> byt "throw", "--output_format_arrow_string_as_string", "0", + *extra, "--query", sql, ] diff --git a/pyhoglake/tests/test_packed.py b/pyhoglake/tests/test_packed.py index 21607c84..3b2941ed 100644 --- a/pyhoglake/tests/test_packed.py +++ b/pyhoglake/tests/test_packed.py @@ -19,7 +19,6 @@ ) from pyhoglake.client import Catalog, Namespace, Table from pyhoglake.models import Column -from pyhoglake.packed import _column_stats BASE = "http://hog.test" CATALOG_WIRE = { @@ -309,7 +308,7 @@ def test_snapshot_part_count_is_bounded_before_download(httpx_mock) -> None: def test_failed_upload_is_abandoned(httpx_mock) -> None: class LocalPartAdapter(ClickHousePackedAdapter): - def _run(self, root, sql, *, input_bytes=None): + def _run(self, root, sql, *, input_bytes=None, settings=None): if sql.startswith("INSERT INTO"): part = root / "part" part.mkdir() @@ -377,27 +376,9 @@ def test_prepared_payload_is_bound_to_its_table() -> None: table._namespace._catalog._client.close() -def test_arrow_stats_exclude_nans_and_normalize_signed_zero() -> None: - columns = ( - Column("f", "double", 1, 0, nullable=True), - Column("ts", "timestamp_ns", 2, 1, nullable=True), - ) - table = pa.table( - { - "f": pa.array([float("nan"), -0.0, None], pa.float64()), - "ts": pa.array([1234567891, None, 1234567890], pa.timestamp("ns")), - } - ) - stats = _column_stats(table, columns) - assert stats[0]["value_count"] == 3 - assert stats[0]["null_count"] == 1 - assert stats[0]["nan_count"] == 1 - assert stats[0]["lower_bound"] == "AAAAAAAAAIA=" - assert stats[0]["upper_bound"] == "AAAAAAAAAAA=" - assert stats[1]["lower_bound"] == "0gKWSQAAAAA=" - assert stats[1]["upper_bound"] == "0wKWSQAAAAA=" - - +# Integration: needs a real ClickHouse (PYHOGLAKE_CLICKHOUSE); ci/live-python.sh +# provides the pinned one and fails the run on a skip. +@pytest.mark.integration def test_real_clickhouse_packed_append_and_snapshot_read(httpx_mock) -> None: executable = os.environ.get("PYHOGLAKE_CLICKHOUSE") if not executable: @@ -505,6 +486,9 @@ def claim(request: httpx.Request) -> httpx.Response: table._namespace._catalog._client.close() +# Integration: needs a real ClickHouse (PYHOGLAKE_CLICKHOUSE); ci/live-python.sh +# provides the pinned one and fails the run on a skip. +@pytest.mark.integration def test_real_clickhouse_packed_multi_part_numeric_ordering(httpx_mock) -> None: executable = os.environ.get("PYHOGLAKE_CLICKHOUSE") if not executable: diff --git a/server/docker-compose.yml b/server/docker-compose.yml index 5efa771c..2f8d70ab 100644 --- a/server/docker-compose.yml +++ b/server/docker-compose.yml @@ -74,6 +74,9 @@ services: HOGLAKE_S3_ENDPOINT: http://minio:9000 HOGLAKE_S3_ACCESS_KEY: hoglake HOGLAKE_S3_SECRET_KEY: hoglake123 + # Dev runs one replica, so the packed rollout gate is safe to open: + # the duckdb-client and pyhoglake live suites create packed tables. + HOGLAKE_PACKED_MERGETREE_ENABLED: ${HOGLAKE_PACKED_MERGETREE_ENABLED:-true} # The compaction loop is ON in this stack (60s, tiny bites: one # group per catalog per sweep). Override from the shell, e.g. # HOGLAKE_COMPACTION_INTERVAL_MS=0 just up to disable. diff --git a/server/justfile b/server/justfile index 425fe808..b2493267 100644 --- a/server/justfile +++ b/server/justfile @@ -44,9 +44,10 @@ compose-down: docker compose down # Run the server against the compose stack (MinIO on the default 9000; -# export HOGLAKE_S3_ENDPOINT etc. to point elsewhere) +# export HOGLAKE_S3_ENDPOINT etc. to point elsewhere). A single dev replica +# opens the packed rollout gate, which the live client suites need. run: - {{_gradle}} run --console=plain + HOGLAKE_PACKED_MERGETREE_ENABLED="${HOGLAKE_PACKED_MERGETREE_ENABLED:-true}" {{_gradle}} run --console=plain clean: {{_gradle}} clean --console=plain diff --git a/server/src/main/kotlin/com/posthog/hoglake/service/TableMetadata.kt b/server/src/main/kotlin/com/posthog/hoglake/service/TableMetadata.kt index 226f7916..f2c60793 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/service/TableMetadata.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/service/TableMetadata.kt @@ -58,6 +58,15 @@ internal object TableMetadata { fun nodes(defs: List): Sequence = defs.asSequence().flatMap { def -> sequenceOf(def) + nodes(def.children.orEmpty()) } + // ClickHouse resolves a real column before a virtual one of the same + // name, and packed readers order by the `_part*` virtual columns: a + // column named `_part_offset` would silently reorder every read. + nodes(columns).firstOrNull { it.name.startsWith("_") }?.let { + throw HoglakeException.Validation( + "packed MergeTree tables reserve column names starting with '_' " + + "for ClickHouse virtual columns, got '${it.name}'", + ) + } val unsupported = nodes(columns).firstOrNull { it.type !in FileFormats.packedColumnTypes } if (unsupported != null) { throw HoglakeException.Validation( diff --git a/server/src/test/kotlin/com/posthog/hoglake/service/TableMetadataTest.kt b/server/src/test/kotlin/com/posthog/hoglake/service/TableMetadataTest.kt index 2ccc2a59..35465abc 100644 --- a/server/src/test/kotlin/com/posthog/hoglake/service/TableMetadataTest.kt +++ b/server/src/test/kotlin/com/posthog/hoglake/service/TableMetadataTest.kt @@ -78,6 +78,24 @@ class TableMetadataTest { ) }.isInstanceOf(HoglakeException.Validation::class.java) .hasMessageContaining("sort") + for (name in listOf("_part", "_part_offset", "_anything")) { + assertThatThrownBy { + TableMetadata.validateDefinitionForFormat( + packed, + listOf(ColumnDef("id", ColType.LONG), ColumnDef(name, ColType.LONG)), + emptyList(), + emptyList(), + ) + }.isInstanceOf(HoglakeException.Validation::class.java) + .hasMessageContaining(name) + } + // Parquet tables are unaffected: the reservation is the packed reader's. + TableMetadata.validateDefinitionForFormat( + emptyMap(), + listOf(ColumnDef("_part", ColType.LONG)), + emptyList(), + emptyList(), + ) } @Test From 4b04c1278629bcce660b68abbdf974bf24111448 Mon Sep 17 00:00:00 2001 From: James Greenhill Date: Tue, 6 Oct 2026 16:37:00 +0000 Subject: [PATCH 6/6] test: apply V26 out of order in the V23 planner fixture The fixture seeds through today's CatalogService at V22, which now reads hog_table.file_format; same treatment V17's test already gives V26. Co-Authored-By: Claude Opus 5.5 --- .../V23PartitionValueLookupMigrationIntegrationTest.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/src/test/kotlin/com/posthog/hoglake/persistence/V23PartitionValueLookupMigrationIntegrationTest.kt b/server/src/test/kotlin/com/posthog/hoglake/persistence/V23PartitionValueLookupMigrationIntegrationTest.kt index 83ce4d96..3108afdb 100644 --- a/server/src/test/kotlin/com/posthog/hoglake/persistence/V23PartitionValueLookupMigrationIntegrationTest.kt +++ b/server/src/test/kotlin/com/posthog/hoglake/persistence/V23PartitionValueLookupMigrationIntegrationTest.kt @@ -127,6 +127,10 @@ class V23PartitionValueLookupMigrationIntegrationTest { @BeforeAll fun seedThenMigrate() { + // The fixture is seeded through today's CatalogService, which reads + // hog_table.file_format (V26). Apply that idempotent file out of + // order, as V17's test does; `Database.migrate` re-applies it. + PgTestSupport.applyMigrationFile(db, "V26__packed_mergetree_format.sql") catalogId = createCatalog("v23") neighbourId = createCatalog("v23-neighbour") seedManifest()