diff --git a/AGENT.md b/AGENT.md index e0c7c823..eb633ead 100644 --- a/AGENT.md +++ b/AGENT.md @@ -482,6 +482,37 @@ 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 a packed part. Registration validates packed + files from metadata only (non-zero rows and bytes, no `footer_size`, + no `split_offsets`, file format equal to the table format). The two + server surfaces that DO open Parquet — the stats hydrator and + compaction (invariant 2) — select `file_format = 'parquet'` only, so a + packed object can never reach a Parquet reader server-side. + - Packed stats are counts only: `column_stats` is optional and a packed + row is always `stats_state = 'provided'` (the hydrator never claims + it, so `pending` would be permanent). Packed files carry positional + row-id ranges like any other file and never `explicit_row_ids`. + - Every client and engine must refuse a format it cannot read, typed, + before handing the object to a reader: DuckDB (`duckdb-client/`), + Hedgerow and Trino (`plugin/trino-hoglake`) refuse packed tables and + per-file non-Parquet formats in scan plans; `pyhoglake.packed` is the + only reader. + - **No rollback past V26 once a packed table exists**: pre-V26 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/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/duckdb-client/README.md b/duckdb-client/README.md index 6f286ada..8cf96565 100644 --- a/duckdb-client/README.md +++ b/duckdb-client/README.md @@ -112,3 +112,10 @@ They cover DDL between file preparation and commit, table recreation, eager DDL, multi-table atomicity, and retries that preserve the request. Run against a server with `HOGLAKE_REFUSE_BLIND_PARTITIONED_APPENDS=true` to verify that partitioned INSERTs satisfy the strict server setting. + +## 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 64733236..05d26db6 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 57279147..5f2960af 100644 --- a/duckdb-client/src/storage/hoglake_insert.cpp +++ b/duckdb-client/src/storage/hoglake_insert.cpp @@ -227,6 +227,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 7e11d908..74cb6ef3 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/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..34fb3a74 100644 --- a/hedgerow/src/hedgerow/daemon.py +++ b/hedgerow/src/hedgerow/daemon.py @@ -72,6 +72,7 @@ from .config import HedgerowConfig from .filtering import RowFilter, build_filter +from .formats import require_parquet from .halts import ( DataIntegrityError, DeletesPresentError, @@ -193,11 +194,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 +258,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 +278,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 +365,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..b11365d8 100644 --- a/hedgerow/src/hedgerow/ingestion.py +++ b/hedgerow/src/hedgerow/ingestion.py @@ -22,6 +22,7 @@ write_duckdb_event_partition, ) from .events import EventTransform +from .formats import require_parquet from .halts import ( DataIntegrityError, DeletesPresentError, @@ -146,6 +147,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 +188,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..91e5be48 100644 --- a/hedgerow/tests/fakes.py +++ b/hedgerow/tests/fakes.py @@ -94,6 +94,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 +146,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..37d0ae87 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 @@ -32,6 +32,7 @@ ReplicationConfig, SchemaMismatchError, SourceConfig, + UnsupportedFormatError, ) SRC_COLS = ( @@ -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..061fe7be 100644 --- a/hedgerow/tests/test_events_discovery.py +++ b/hedgerow/tests/test_events_discovery.py @@ -16,7 +16,11 @@ from hedgerow.discovery import discover_window from hedgerow.events import EventTransform -from hedgerow.halts import DataIntegrityError, SchemaMismatchError +from hedgerow.halts import ( + DataIntegrityError, + SchemaMismatchError, + UnsupportedFormatError, +) from hedgerow.pending import PendingStore @@ -239,3 +243,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() diff --git a/pyhoglake/README.md b/pyhoglake/README.md index 58d66fbc..15ee0ff1 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 @@ -115,6 +116,64 @@ 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`. The server must first complete the rollout +sequence in `server/README.md` and enable `HOGLAKE_PACKED_MERGETREE_ENABLED=true`: + +```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 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. + +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. 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. + +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. + ## Configuration | What | How | 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/__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..ec780ae7 --- /dev/null +++ b/pyhoglake/src/pyhoglake/packed.py @@ -0,0 +1,704 @@ +"""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 + +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, + 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", +} + +# 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 = 4 * 1024**3 +_DEFAULT_MAX_RESULT_BYTES = 2 * 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.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} " + 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() + + +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")) + # 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" + 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, + ) + 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": [], + } + 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) + ) + raw = self._run( + root, + attach + + ";ALTER TABLE " + + _quote_identifier("packed_read") + + " 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) + 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) + ), + ) + 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, + ) + 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( + 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, + 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", + "--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", + *extra, + "--query", + sql, + ] + try: + completed = subprocess.run( + command, + input=input_bytes, + capture_output=True, + check=False, + cwd=root, + 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..3b2941ed --- /dev/null +++ b/pyhoglake/tests/test_packed.py @@ -0,0 +1,627 @@ +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 + +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, settings=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() + + +# 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: + 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 registration["column_stats"] == [] + 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() + + +# 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: + 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") + 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 4ef58dbd..88d45aa0 100644 --- a/server/README.md +++ b/server/README.md @@ -236,6 +236,52 @@ 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, 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. 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 V26 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. + +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, 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. + +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 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. 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 +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/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/schema.sql b/server/schema.sql index 4c5d3cef..9679cdbe 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,8 @@ 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')), -- 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) @@ -284,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')), + 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, @@ -326,6 +328,10 @@ 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) ); +ALTER TABLE hog_data_file + 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) WHERE end_snapshot IS NULL; @@ -770,6 +776,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, diff --git a/server/src/main/kotlin/com/posthog/hoglake/App.kt b/server/src/main/kotlin/com/posthog/hoglake/App.kt index 146e32c6..6a89a735 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/App.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/App.kt @@ -83,7 +83,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 089421eb..fe6d0aae 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/Config.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/Config.kt @@ -15,6 +15,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/api/Dto.kt b/server/src/main/kotlin/com/posthog/hoglake/api/Dto.kt index 0dec4bf9..98a025aa 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 84df3e51..a6be8f9d 100644 --- a/server/src/main/kotlin/com/posthog/hoglake/api/Routes.kt +++ b/server/src/main/kotlin/com/posthog/hoglake/api/Routes.kt @@ -131,6 +131,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..ce13563b 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,11 +1151,22 @@ 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) .bind("rowIdStart", rowId) - .bind("statsState", if (file.columnStats != null) "provided" else "pending") + // A packed part is never hydrated (the hydrator reads Parquet footers + // only), so it has no 'pending' state to leave: counts-only stats are + // complete at registration, with or without column_stats. + .bind( + "statsState", + if (file.columnStats != null || file.fileFormat != FileFormats.PARQUET) { + "provided" + } else { + "pending" + }, + ) .bind("specId", append.spec?.specId) .bindBigintArrayOrNull("splitOffsets", file.splitOffsets) .add() @@ -1351,8 +1395,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 +1467,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 +1484,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 +1568,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 +1604,23 @@ 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.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 +1737,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 b126a426..20090908 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 = t.fileFormat, 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 { @@ -3240,6 +3254,17 @@ 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} " + + "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..cd855825 --- /dev/null +++ b/server/src/main/kotlin/com/posthog/hoglake/model/FileFormats.kt @@ -0,0 +1,61 @@ +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]?.lowercase() ?: PARQUET + + fun isPacked(properties: Map): Boolean = tableFormat(properties) == CLICKHOUSE_MERGETREE_PACKED + + /** + * [properties] as every read reports them: the format key is never stored + * (`hog_table.file_format` is authoritative), so it is absent for Parquet + * and the canonical lowercase value otherwise. Write paths that answer with + * the properties they were given go through this so the response equals + * the next read. + */ + fun canonicalProperties(properties: Map): Map { + val format = tableFormat(properties) + val rest = properties - TABLE_PROPERTY + return if (format == PARQUET) rest else rest + (TABLE_PROPERTY to format) + } +} + +/** 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 2c6f2e7b..525a56a0 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..6d42f3e3 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 @@ -23,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. */ @@ -42,13 +44,22 @@ data class TableStatsRow( object TableRepo { private val tableRowMapper = RowMapper { rs, _ -> + val fileFormat = rs.getString("file_format") + 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, ) } @@ -133,21 +144,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 @@ -188,6 +214,7 @@ object TableRepo { comment: String? = null, properties: Map = emptyMap(), ) { + val storedProperties = properties - FileFormats.TABLE_PROPERTY try { handle.createUpdate( """ @@ -201,7 +228,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)) { @@ -309,7 +336,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, + t.file_format FROM hog_table_version tv JOIN hog_table t ON t.catalog_id = tv.catalog_id AND t.table_id = tv.table_id @@ -351,7 +379,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, + t.file_format FROM hog_table_version tv JOIN hog_table t ON t.catalog_id = tv.catalog_id AND t.table_id = tv.table_id @@ -394,7 +423,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, + t.file_format 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 9e199819..046f1505 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,26 @@ class AlterService(private val jdbi: Jdbi) { currentTableUuid = t.tableUuid, ) } + ops.filterIsInstance().forEach { + TableMetadata.validateProperties(it.properties) + TableMetadata.requireFormatUnchanged(t.properties, it.properties) + } + if (t.fileFormat != com.posthog.hoglake.model.FileFormats.PARQUET) { + // An allow-list, so an AlterOp added later fails closed on a + // fixed-schema table instead of silently rewriting its columns + // or specs. + val unsupported = + ops.firstOrNull { + it !is AlterOp.RenameTable && it !is AlterOp.SetTableComment && + it !is AlterOp.SetProperties + } + 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) { @@ -249,7 +269,7 @@ class AlterService(private val jdbi: Jdbi) { } is AlterOp.SetProperties -> { TableMetadata.validateProperties(op.properties) - state.properties = op.properties.toMap() + state.properties = com.posthog.hoglake.model.FileFormats.canonicalProperties(op.properties) rewriteMetadata(h, catalogId, tableId, namespaceId, snapshot, state) } } 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..7cee1cc6 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 @@ -53,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`. @@ -276,6 +280,7 @@ class CatalogService(private val jdbi: Jdbi) { namespace: String, name: String, columns: List, + properties: Map = emptyMap(), ): TableInfo = Audit.audited( "table_create", @@ -283,7 +288,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 +310,7 @@ class CatalogService(private val jdbi: Jdbi) { ): TableInfo { TableMetadata.validateComment(comment) TableMetadata.validateProperties(properties) + validateTableFormatDefinition(properties, columns, partitionFields, sortFields) validateTableDefinition(name, columns) val cols = initialColumns(columns) // Publication catches definition validation and records a rejected receipt. @@ -346,8 +355,17 @@ class CatalogService(private val jdbi: Jdbi) { // 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, cat.catalogId, tableId, alloc.snapshotId, tableUuid, replacementTableId) + TableRepo.insertTable( + h, + cat.catalogId, + tableId, + alloc.snapshotId, + tableUuid, + replacementTableId, + format, + ) // 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 @@ -362,11 +380,12 @@ class CatalogService(private val jdbi: Jdbi) { AlterService( jdbi, ).installPartitionSpec(h, cat.catalogId, tableId, alloc.snapshotId, cols, partitionFields) + val effectiveProperties = FileFormats.canonicalProperties(properties) return TableInfo( tableId = tableId, tableUuid = createdUuid, comment = comment, - properties = properties, + properties = effectiveProperties, namespace = ns.name, name = name, columns = cols, @@ -391,6 +410,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, @@ -497,6 +532,9 @@ class CatalogService(private val jdbi: Jdbi) { currentTableUuid = t.tableUuid, ) } + if (t.fileFormat == FileFormats.CLICKHOUSE_MERGETREE_PACKED) { + 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/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/kotlin/com/posthog/hoglake/service/TableCreationService.kt b/server/src/main/kotlin/com/posthog/hoglake/service/TableCreationService.kt index dd99e34f..3744ac68 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) + catalogs.validateTableFormatDefinition( + 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..f2c60793 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,59 @@ 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()) } + // 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( + "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 573705a4..dc4a368b 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, ) @@ -67,12 +75,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 == ".." } @@ -85,14 +113,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 @@ -344,7 +379,7 @@ class UploadService( */ internal const val REGISTER_CLAIMS_SQL: String = """ - 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) ORDER BY upload_id FOR UPDATE @@ -398,6 +433,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(), ) @@ -410,7 +446,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() @@ -420,12 +456,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 @@ -438,7 +476,7 @@ class UploadService( // the same rows at the same time. val claims = h.createQuery(REGISTER_CLAIMS_SQL) - .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 @@ -446,10 +484,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( @@ -457,14 +495,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/V26__packed_mergetree_format.sql b/server/src/main/resources/db/migration/V26__packed_mergetree_format.sql new file mode 100644 index 00000000..2ece018a --- /dev/null +++ b/server/src/main/resources/db/migration/V26__packed_mergetree_format.sql @@ -0,0 +1,45 @@ +-- Admit the restricted single-object ClickHouse packed-part format. +-- 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 + 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; + +-- 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; + +-- 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 (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 lower(properties ->> 'write.format.default') <> 'parquet' + ) THEN + RAISE EXCEPTION + 'retained table metadata already uses reserved property write.format.default; remove it before V26'; + END IF; +END $$; diff --git a/server/src/main/resources/openapi/hoglake.yaml b/server/src/main/resources/openapi/hoglake.yaml index 1e437ae8..cdf0d5b9 100644 --- a/server/src/main/resources/openapi/hoglake.yaml +++ b/server/src/main/resources/openapi/hoglake.yaml @@ -1882,8 +1882,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. @@ -1901,6 +1902,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) @@ -1915,6 +1920,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 } @@ -2971,9 +2980,15 @@ 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. 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, + format, comment are reserved. No NUL in values. propertyNames: { pattern: "^[a-z][a-z0-9_.-]{0,127}$" } additionalProperties: { type: string, maxLength: 4096 } @@ -3113,6 +3128,7 @@ components: type: array minItems: 1 items: { $ref: "#/components/schemas/ColumnDef" } + properties: { $ref: "#/components/schemas/CustomProperties" } TableSummary: type: object @@ -3348,6 +3364,17 @@ 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 carry counts only: `column_stats` is optional (an + empty array or omission are equivalent) and the file is recorded with + `stats_state: provided`, never hydrated. They 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 } @@ -3715,7 +3742,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/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/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..8465c5bc 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 @@ -107,6 +115,116 @@ 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 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") + // The layout ops are refused even though no hog_column row moves: a + // packed table's layout is fixed. Each one separately, so dropping + // any single arm of the allow-list reds. + for ( + (op, name) in + listOf( + """{"op":"set_sort_order","sort_fields":[ + {"source_field_id":1,"direction":"asc","null_order":"nulls_last"}]}""" to + "SetSortOrder", + """{"op":"set_partition_spec","fields":[ + {"source_field_id":1,"transform":"identity"}]}""" to "SetPartitionSpec", + """{"op":"rename_column","from":"id","to":"ident"}""" to "RenameColumn", + """{"op":"drop_column","name":"id"}""" to "DropColumn", + ) + ) { + val refused = + client.postJson( + "/v1/catalogs/packed/namespaces/analytics/tables/events/alter", + """{"ops":[$op]}""", + ) + assertThat(refused.status).describedAs(op).isEqualTo(HttpStatusCode.UnprocessableEntity) + assertThat(body(refused)["detail"].asText()).contains(name) + } + // The table format is immutable through set_properties. Without the + // service check this would succeed as a silent no-op (the format is + // derived from hog_table.file_format, not the stored properties). + val reformat = + client.postJson( + "/v1/catalogs/packed/namespaces/analytics/tables/events/alter", + """{"ops":[{"op":"set_properties","properties":{"write.format.default":"parquet"}}]}""", + ) + assertThat(reformat.status).isEqualTo(HttpStatusCode.UnprocessableEntity) + assertThat(body(reformat)["detail"].asText()).contains("immutable") + // Metadata-only ops stay allowed, and a read-modify-write of the + // properties keeps the (derived) format. + val metadata = + client.postJson( + "/v1/catalogs/packed/namespaces/analytics/tables/events/alter", + """{"ops":[ + {"op":"set_table_comment","comment":"packed events"}, + {"op":"set_properties","properties":{ + "write.format.default":"CLICKHOUSE-MERGETREE-PACKED","owner":"team"}}]}""", + ) + assertThat(metadata.status).describedAs(metadata.bodyAsText()).isEqualTo(HttpStatusCode.OK) + assertThat(body(metadata)["properties"]["write.format.default"].asText()) + .isEqualTo("clickhouse-mergetree-packed") + assertThat(body(metadata)["properties"]["owner"].asText()).isEqualTo("team") + 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/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/commit/CommitServiceTest.kt b/server/src/test/kotlin/com/posthog/hoglake/commit/CommitServiceTest.kt index 44bedb5f..e43e5a40 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,129 @@ 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, + ) + // column_stats is optional for packed (counts only). The hydrator never + // claims a packed file, so a 'pending' row would stay pending forever. + val withoutStats = packed.copy(path = "s3://b/data/part2.packed", columnStats = null) + + service.commit( + "cat", + CommitRequest(appends = listOf(TableAppend("ns", "events", listOf(packed, withoutStats)))), + ) + + jdbi.useHandle { h -> + assertThat( + h.createQuery( + "SELECT file_format || '/' || stats_state FROM hog_data_file " + + "WHERE catalog_id = :catalogId ORDER BY path", + ) + .bind("catalogId", fx.catalogId) + .mapTo(String::class.java) + .list(), + ).containsExactly( + "${FileFormats.CLICKHOUSE_MERGETREE_PACKED}/provided", + "${FileFormats.CLICKHOUSE_MERGETREE_PACKED}/provided", + ) + } + } + + @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(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() + } + val receipt = + service.commit( + "cat", + CommitRequest(appends = listOf(TableAppend("ns", "events", listOf(valid.copy(columnStats = null))))), + ) + assertThat(receipt.snapshotId).isPositive() + } + + @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 70f162ea..a28c8e6c 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 @@ -43,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) @@ -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,150 @@ 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) + } + + private fun setFileFormat( + cat: String, + path: String, + format: String, + ) = db.jdbi.useHandleUnchecked { h -> + // Raw update: neither state is producible through the service, which is + // the point — each test removes one of the two exclusions' cover. + h.createUpdate( + """ + UPDATE hog_data_file f SET file_format = :format + FROM hog_catalog c + WHERE c.catalog_id = f.catalog_id AND c.name = :cat AND f.path = :path + """, + ).bind("format", format).bind("cat", cat).bind("path", path).execute() + } + + @Test + fun `the candidate query alone excludes a packed row from a parquet table`() { + // Pins the SQL `file_format = 'parquet'` predicate: the table-level + // early return cannot see this row, the table is Parquet. + val cat = fixture() + append(cat, file("a", 300), file("b", 300), file("c", 300)) + setFileFormat(cat, "s3://bucket/x/b.parquet", FileFormats.CLICKHOUSE_MERGETREE_PACKED) + val plan = svc.planTable(cat, "ns", "t", cfg) + assertThat(plan.groups).hasSize(1) + assertThat(plan.groups.single().files.map { it.path }) + .containsExactly("s3://bucket/x/a.parquet", "s3://bucket/x/c.parquet") + } + + @Test + fun `a packed table is refused from metadata before the candidate query`() { + // Pins the table-format early return: these rows claim Parquet, so the + // SQL predicate alone would plan them. + val cat = + fixture( + properties = mapOf(FileFormats.TABLE_PROPERTY to FileFormats.CLICKHOUSE_MERGETREE_PACKED), + ) + append( + cat, + *listOf("a", "b", "c").map { + FileRegistration( + path = "s3://bucket/x/$it.packed", + recordCount = 10, + fileSizeBytes = 300, + fileFormat = FileFormats.CLICKHOUSE_MERGETREE_PACKED, + ) + }.toTypedArray(), + ) + listOf("a", "b", "c").forEach { setFileFormat(cat, "s3://bucket/x/$it.packed", FileFormats.PARQUET) } + assertThat(svc.planTable(cat, "ns", "t", cfg).groups).isEmpty() + } + + @Test + fun `packed files are not counted as compaction debt by the sampler`() { + // Packed tables are never compacted, so their small files are not debt: + // counting them would publish a permanent backlog on /maintenance/status, + // the debt page and the small-file gauges (all read the sampler's tiers). + val cat = + fixture( + properties = mapOf(FileFormats.TABLE_PROPERTY to FileFormats.CLICKHOUSE_MERGETREE_PACKED), + ) + append( + cat, + *(0..4).map { + FileRegistration( + path = "s3://bucket/x/p$it.packed", + recordCount = 10, + fileSizeBytes = 16, + fileFormat = FileFormats.CLICKHOUSE_MERGETREE_PACKED, + ) + }.toTypedArray(), + ) + // Page size 1 drives the excluded-table skip, 10,000 the in-page filter. + for (batch in listOf(1, 10_000)) { + db.jdbi.useHandleUnchecked { h -> + h.execute( + "UPDATE hog_maintenance_summary SET generation = generation + 1, " + + "scan_state = NULL, sample = NULL, sampled_at = NULL, next_batch_at = now()", + ) + } + val sampler = + MaintenanceSummarySampler(db.jdbi, 1024, cfg.minInputFiles, cfg.maxInputFiles, 3600) + var steps = 0 + while (sampler.runOnce(batch)) check(++steps < 100_000) + val sample = + db.jdbi.withHandleUnchecked { h -> + MaintenanceSummarySampler.read(h, listOf(catalogId(cat))).values.single() + } + assertThat(sample.sample.smallFiles).describedAs("page size %d", batch).isZero() + } + } + @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..e06cf456 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 V26. 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, "V26__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/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() diff --git a/server/src/test/kotlin/com/posthog/hoglake/persistence/V26PackedMergetreeFormatMigrationIntegrationTest.kt b/server/src/test/kotlin/com/posthog/hoglake/persistence/V26PackedMergetreeFormatMigrationIntegrationTest.kt new file mode 100644 index 00000000..83b66020 --- /dev/null +++ b/server/src/test/kotlin/com/posthog/hoglake/persistence/V26PackedMergetreeFormatMigrationIntegrationTest.kt @@ -0,0 +1,157 @@ +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 V26PackedMergetreeFormatMigrationIntegrationTest { + @Test + fun `existing parquet rows remain valid and packed rows are admitted`() { + PgTestSupport.freshDatabaseAt("25").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, + ) + + assertThatThrownBy { + 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) + .hasMessageContaining("hog_table_file_format_check") + + 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, 3, 1, 0, 's3://b/v25/invalid.orc', 'orc', 1, 10, 3) + """, + ) + .bind("catalog", catalogId) + .execute() + }.isInstanceOf(UnableToExecuteStatementException::class.java) + .hasMessageContaining("hog_data_file_file_format_check") + + 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("25").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/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() 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..35465abc 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,84 @@ 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") + 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 + 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 ee93bff5..865cb817 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 @@ -153,6 +154,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]}],