Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds a DialectAwareSql helper (jOOQ CustomCondition/CustomField delegating to ctx.family() at render time) so the shared SQL-building code consumed by both the meta (always Postgres) and analytics (Postgres or ClickHouse) datasources can pick dialect-specific SQL without a vendor-gated bean. - PostgresJsonPathAccessor -> DialectAwareJsonPathAccessor: jsonbAt/AsText/ AsNumeric render ClickHouse JSONExtract*/JSONExtractRaw on the CLICKHOUSE family, byte-identical Postgres jsonb operators otherwise. - FilterTranslator: nullSatisfies/negate and the co/nc array-containment seams (string-element, scalar-element, case-insensitive) dialect-switch; negation is folded into the same family closure to avoid jOOQ's NOT combinator adding a defensive extra parenthesis around a CustomCondition. - RocAucFunction: CLICKHOUSE branch delegates to arrayAUC(scores, labels) (argument order swapped vs roc_auc_score(labels, scores)). - percentile_cont/disc and width_bucket render valid ClickHouse SQL via jOOQ's existing DSL.percentileCont/Disc/widthBucket unchanged; a pinning render test documents and locks in that outcome. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jOOQ's default ClickHouse rendering of DSL.percentileCont is CH's approximate, sampling-based quantile() aggregate, not Postgres' exact linear-interpolation percentile_cont; this would make P10/P90/P99 metric-score statistics nondeterministic and semantically diverge from Postgres. BuiltInQueryFunctions.percentile(...) now dialect-switches via DialectAwareSql to the exact ClickHouse equivalents instead: quantileExactInclusive(fraction)(column) for percentile_cont and quantileExactLow(fraction)(column) for percentile_disc, with the already-validated [0,1] fraction inlined. Non-CLICKHOUSE families keep today's DSL.percentileCont/Disc rendering byte-identical. width_bucket was re-checked and left untouched: jOOQ's CLICKHOUSE rendering is a documented ClickHouse alias, with no approximate/exact divergence risk. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds ClickHouse* repositories (TestCaseRunResult, EvalSummary, RunMetricSnapshot, MetricScoreResult) and ClickHouse* query entity resolvers (EvalSummary, MetricScoreResult), each extending its Postgres twin and gated by datasource.analytics.vendor=CLICKHOUSE. Reads are inherited unchanged (rendered dialect-correctly by the injected CLICKHOUSE-dialect analyticsDsl); saveAll drops ON CONFLICT (ClickHouse dedups via ReplacingMergeTree); EvalSummary's metric accessor switches to JSONExtract and countMatches' FILTER (WHERE ...) aggregates switch to CASE WHEN, both proven necessary by render probes against SQLDialect.CLICKHOUSE. Minimal private->protected visibility relaxations in the Postgres classes enable the overrides; no behavior change to POSTGRES vendor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Boots the application against a live ClickHouse 25.8 container with datasource.analytics.vendor=CLICKHOUSE for the first time, and fixes the two P1 wiring bugs that only a real server could reveal. fix(P1): replace the ClickHouse Flyway bean with ClickHouseSchemaInitializer. flyway-database-clickhouse probes schema existence with "SELECT COUNT() FROM system.databases WHERE name = ?"; the ClickHouse V2 JDBC driver parses every PreparedStatement with an ANTLR grammar that cannot parse a bare `name` column reference, so it reports zero bind parameters and setString() throws ArrayIndexOutOfBoundsException before any SQL is sent. The SQL is hard-coded in the plugin, so no Flyway configuration avoids it; the dependency is dropped entirely (which also removes the 10.x-vs-12.x skew risk). fix(P1): "SET final = 1" as connectionInitSql does not stick. The V2 driver sends every statement as an independent stateless HTTP request, so the setting is forgotten immediately - verified against a live server (system.settings reported final=0 and a duplicated ReplacingMergeTree key still returned two rows). Replaced with the clickhouse_setting_final=1 connection property, which the driver attaches to every request (verified: setting=1, dedup exact). test: remove org.testcontainers:clickhouse (1.x line, drags 1.x jdbc / database-commons next to the BOM-managed 2.0.5 prefixed modules) in favour of a plain GenericContainer; extract the shared DIAL client mocks into DialClientMockingFunctionalTests and the shared meta pg_dump/pg_restore into PostgresMetaSnapshotSupport so both vendor entry points reuse them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…xes (P4)
Attaches the 14 core analytics data suites to ClickHouseFunctionalTests
(152 tests) and fixes everything a live server surfaced. All four bugs were
invisible to render-pinning tests: each renders valid ClickHouse SQL and only
misbehaves on execution.
fix(P1): add join_use_nulls=1 to the required server settings. ClickHouse fills
the right-hand columns of an unmatched LEFT JOIN row with the column type's
DEFAULT value ('' for String), not NULL, so every anti-match predicate in the
run-comparison queries (probeKey IS NULL / IS NOT NULL, count(probeKey)) read as
"everything matched" - 14 failing assertions across RunComparison* alone.
fix(P2): dialect-aware numeric cast (DialectAwareSql.numericCast). A bare
`decimal` on ClickHouse means Decimal(10, 0) - scale zero - so jOOQ's
cast(x as decimal), emitted by every DSL add/subtract/multiply/divide/coalesce,
silently truncated 0.5 to 0. ClickHouse now casts to Float64 (the type the
analytics schema stores these values in); Postgres rendering is untouched.
fix(P2): width_bucket needs an unsigned bucket count on ClickHouse. jOOQ's
cast(count as integer) (Int32) is rejected with ILLEGAL_TYPE_OF_ARGUMENT; the
ClickHouse branch now wraps it in toUInt32. P2 had cleared width_bucket on
rendering alone.
fix(P3): ClickHouseMetricScoreResultRepository writes `value` as
toFloat64('<plain decimal>'). jOOQ's multi-query batch inlines a Double in
scientific notation (8.500000000000001E-1) and ClickHouse's textual Float64
parser is one ULP off for the exponent form, so a persisted metric score no
longer equalled the same statistic recomputed on the fly.
test: extract the pg_indexes assertion out of EvalSummaryFunctionalTests into
EvalSummaryIndexFunctionalTests (Postgres-only; ClickHouse serves that access
path from the table's ORDER BY key, not a secondary index).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…F8 fixes (P4)
Brings ClickHouseFunctionalTests to 342 tests across 31 nested suites - every
analytics-touching abstract suite except RocAucScoreFunctionalTests, which
exercises the Postgres roc_auc_score stored function directly (ClickHouse uses
the built-in arrayAUC via RocAucFunction, covered by the structured-query and
metric-score suites).
fix(P3): ClickHouseEvalSummaryRepository overrides existsByRunIdAndComputationId.
The inherited fetchExists sends "select exists (select 1 ... where ... = ?)";
the ClickHouse V2 driver's ANTLR parser cannot parse a SELECT nested in a scalar
expression, reports zero bind parameters, and the bind throws before any SQL is
sent (6 export tests returned HTTP 500). Replaced with a "select 1 ... limit 1"
probe, which the parser handles.
fix(P3): run-comparison case-folding uses lowerUTF8 on ClickHouse. ClickHouse's
lower() is ASCII-only, so two test case names differing only in the case of a
non-ASCII letter would never match each other and would sort inconsistently,
where Postgres' lower() matches them. Dialect-switched via DialectAwareSql, so
Postgres rendering is unchanged, and covered by a new test that runs on BOTH
vendors ("ETAPE"/"etape" with acute accents).
test: new ClickHouseAnalyticsSemanticsFunctionalTests pins the two vendor
invariants that have no Postgres counterpart - a duplicated eval-summary natural
key reads back as one row (proves final=1 actually reaches the server), and a
17-significant-digit metric score round-trips bit-for-bit.
docs: configuration.md / database-schema.md corrected - schema management is
ClickHouseSchemaInitializer (idempotent scripts, no history table), not Flyway,
and the two pinned server settings are documented as load-bearing.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ray containment, shared type constants
Fix wave for the ClickHouse analytics vendor review (P5):
- Correct every CH repository/migration comment that claimed dedup relies on a
session-wide SET final=1: the real mechanism is the clickhouse_setting_final=1
connection property (SET does not persist on the V2 HTTP driver); point at
AnalyticsClickHouseConfiguration as the single source of truth.
- Correct the ORDER BY claim for test_case_run_results: its key is a superset of
the Postgres onConflict key (leading test_suite_id has no PG counterpart), not
the same key; document the functional-dependency invariant a future
suite-reassignment feature must revisit.
- FilterTranslator#arrayContainsScalarElement now dispatches ClickHouse rendering
on the literal's ValueType instead of always comparing as Nullable(Float64):
numeric/timestamp literals keep the Float64 form, boolean compares as
Nullable(Bool), and any other type (date/uuid) is rejected with a
ValidationException (surfaces as HTTP 400) since ClickHouse can't recover its
original form from JSON text - the Postgres branch is unchanged for every type.
- Add render-pinning tests for the previously untested negated ('nc') array
containment forms (string/numeric/ignore-case) and a 'not' node wrapping a
'co' array containment, plus Number/Boolean/rejection coverage for the new
typed dispatch.
- Extract the duplicated ClickHouse JSONExtract type-name literals into a single
ClickHouseTypeNames constants class, referenced from
DialectAwareJsonPathAccessor, ClickHouseEvalSummaryRepository, and
FilterTranslator.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Documents the ClickHouse analytics vendor's unique patterns (render-time dialect switching + the DSL.not()/plain-SQL-template extra-paren pitfall, vendor twins, ReplacingMergeTree-as-ON-CONFLICT + connection-property FINAL, ClickHouseSchemaInitializer instead of Flyway, known engine semantics), linked from AGENTS.md's Unique Patterns table and docs/patterns/README.md. Also notes the new ClickHouse-related classes in docs/key-packages.md at its existing per-package granularity. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
clickhouse-jdbc 0.10.0 fixes the ANTLR parser bug that made flyway-database-clickhouse unusable on 0.9.0, so Flyway now owns the ClickHouse analytics schema (flyway_schema_history, one-time migrations) instead of the hand-rolled ClickHouseSchemaInitializer. The plugin only recognizes jdbc:clickhouse:// (not jdbc:ch://), so that prefix becomes the documented default everywhere. Also drops the now-unneeded fetchExists override on ClickHouseEvalSummaryRepository, verified fixed on the same driver bump. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Split jOOQ codegen into two tasks with different sources of truth:
* generateJooq -> META only, from the Postgres meta migrations (Zonky)
* generateClickHouseJooq -> ANALYTICS, from the CLICKHOUSE analytics migrations
(Testcontainers ClickHouse 25.8 + Flyway + jOOQ
ClickHouseDatabase)
The analytics data model is now vendor-independent: schema evolution happens in
db/migration/analytics/CLICKHOUSE first and the Postgres analytics migrations are
the derived twin. Each task pre-cleans only its own subtree of src/main/java-generated.
Codegen configuration reproduces the model's Java/jOOQ surface on top of ClickHouse
metadata: outputSchema "analytics", forcedTypes for VARCHAR(36) ids (length is
load-bearing for the uuid inference in JooqTableSchemaResolver), JSONB payload
columns, and the bounded VARCHAR widths. ClickHouse reports each table's ORDER BY
key as PRIMARY, so records stay UpdatableRecordImpl without synthetic keys. Index
constants are generated for meta only -- ClickHouse data-skipping index names are
table-scoped, so the analytics schema declares idx_id twice.
Generated column types, nullability and defaults are unchanged. Non-cosmetic deltas:
column declaration order now follows the consolidated ClickHouse DDL, the Keys
constants are ClickHouse-derived, and the unused Indexes/Routines classes are gone.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add ClickHouseSchemaDriftTest: a plain (Docker, no Spring context) JUnit test that migrates a live ClickHouse 25.8 instance with the CLICKHOUSE analytics migrations and diffs system.columns against the four committed generated table classes -- columns in both directions, nullability, the ClickHouse-type -> jOOQ-type mapping, and the load-bearing VARCHAR(36) width on id columns. It catches "edited a CLICKHOUSE migration but forgot to rerun generateClickHouseJooq". JooqSchemaDriftTest becomes the Postgres-side guard: its analytics table list gains the previously missing METRIC_SCORE_RESULT, and its failure hints now say what each side means (regenerate for meta; realign the derived Postgres twin for analytics). Docs: rewrite the codegen/drift sections of the jOOQ pattern doc, add the model provenance section to the ClickHouse pattern doc, and mention both tasks in AGENTS.md and docs/code-templates.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
P7 flipped the single analytics package's source from the Postgres analytics
migrations to the ClickHouse ones. Restore the Postgres-generated model and give
ClickHouse its own package instead, so each vendor's model is generated from that
vendor's own migrations:
* generateJooq -> ...jooq.meta + ...jooq.analytics, from the POSTGRES
meta and analytics migrations (Zonky, no Docker)
* generateClickHouseJooq -> ...jooq.clickhouse, from the CLICKHOUSE analytics
migrations (Testcontainers ClickHouse 25.8 + Flyway)
...jooq.analytics is byte-identical to its pre-P7 content again: the PG-specific
Indexes/Routines/RocAucScore classes, the Postgres Keys and the Postgres column
order are all back. ...jooq.clickhouse is P7's output verbatim, renamed.
Each task still pre-cleans only its own subtree (P7's improvement), so running one
never deletes another's output.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The four ClickHouse repositories' self-contained overrides (batch inserts, the JSONExtract metric accessors, the CASE WHEN aggregates) and the two ClickHouse entity resolvers' table + bindings now come from ...jooq.clickhouse instead of the Postgres-generated ...jooq.analytics, so each vendor's own code is described by its own model. Everything the two vendors share — mappers, schema providers, filter whitelists, the Postgres repositories and the inherited read paths — stays on the Postgres model, which is the canonical API surface; jOOQ fields render by name and every DSLContext uses withRenderSchema(false), so the mix is invisible in SQL. Add AnalyticsModelParityTest: a plain unit test (no Docker, no Spring) that holds the two generated models column-for-column identical — same tables, same columns in both directions, same Java type / jOOQ type name / length / precision / scale / nullability, the same '['-prefixed JSON-array default decision that drives the resolver's array-vs-object inference, and the same published QueryFieldType map. Its failure message spells out the dual-authored evolution workflow. Retarget ClickHouseSchemaDriftTest and the ClickHouse render test to the new package and correct JooqSchemaDriftTest's analytics hint: the Postgres analytics model is regenerated with './gradlew generateJooq' again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rewrite the model story in the jOOQ and ClickHouse pattern docs: three generated packages, each from its own vendor's migrations; generateJooq owns meta + analytics-PG (Zonky, no Docker), generateClickHouseJooq owns the ClickHouse twin (Docker); AnalyticsModelParityTest binds the two analytics models; shared query code, mappers and schema providers stay on the Postgres model while ClickHouse-owned paths use the ClickHouse one. Update the AGENTS.md pattern rows, the java-generated DON'T bullet, the codegen debugging tip, docs/code-templates.md and docs/key-packages.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… functional test MetricDeclarationFunctionalTests$GetLatestVersion needs its own context and so re-registers the datasource properties by hand, but it never re-registered datasource.analytics.vendor. Since application.yml's default flipped to CLICKHOUSE the class booted the ClickHouse Flyway bean against a server that isn't running and failed the whole context (5 tests). It imports PostgresFunctionalTestConfiguration, so POSTGRES is the right value. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…pe corruption dsl.batch(List<Query>) is jOOQ's multi-query batch, which renders STATIC statements with parameters inlined. jOOQ inlines string literals identically for POSTGRES and CLICKHOUSE (quotes doubled, backslashes untouched), but ClickHouse — unlike Postgres with standard_conforming_strings — interprets backslash escapes inside single-quoted literals. Any JSON payload whose string values contain characters Jackson escapes (\n, \t, \", \\) was therefore silently corrupted at rest (\n became a raw linefeed, \" a bare quote), the stored column stopped being valid JSON, and metric evaluation failed on read with 'Illegal unquoted character (CTRL-CHAR, code 10)'. All four ClickHouse repositories now write through a single prepared statement executed as a JDBC batch with bind values (dsl.batch(insertTemplate).bind(row)...), which the driver transmits verbatim (verified against a live server for the bound, batch-bound and static-inlined paths). metric_score_result keeps its exact-Float64 mechanism as toFloat64(?) with a bound plain-decimal string. Regression-guarded by ClickHouseAnalyticsSemanticsFunctionalTests #escapeWorthyCharactersSurviveBatchWrites (newline, tab, escaped quote and backslash round-tripped verbatim through all four repositories; reproduced the corruption before the fix). Existing rows written by the old path remain corrupted at rest — re-run suites or reset the local ClickHouse volume. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
…Java migration ClickHouseAnalyticsBackfillMigration copies the four analytics tables via ClickHouse's postgresql() table function (INSERT INTO ... SELECT ... FROM postgresql(...)), so the copy runs server-side and the source credentials come from clickhouse.analytics.backfill.* application properties instead of a checked-in SQL script. Repeatable + checksum-keyed on the config (password excluded): a versioned migration would be recorded as an applied no-op on any environment that boots the vendor before backfill is configured and could never run again; flipping enabled=true is what re-applies it. Re-runs are idempotent (ReplacingMergeTree + final=1). Per-table count verification fails fast on a shortfall. Functional coverage runs the migration twice against a scratch Postgres carrying the real analytics POSTGRES schema (applied via the production Flyway migrations), reached container-to-container over the Docker bridge, asserting escape-heavy JSON payloads and a 17-significant-digit double arrive intact. Full ClickHouse suite: 332 tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ckHouse Adds a ClickHouse-only acceleration column to test_case_eval_summaries (folded into V1.1 - WIP schema, no new migration): metric_values_map Map(String, Map(String, Nullable(Float64))) MATERIALIZED JSONExtract(metric_values, ...). The JSON text is parsed once at insert into typed columnar storage; metric_values stays the serving source of truth (explicit nulls, verbatim bytes). All two-level numeric metric-path reads switch to map access with bound-parameter keys: the aggregate endpoint (ClickHouseEvalSummaryRepository#buildNumericMetricAccessor), Stack A's JSONB_NUMERIC filters and the query DSL's metric::<name>::<field> family (twin registry in DialectAwareJsonPathAccessor's CLICKHOUSE branch, keyed by table.column with a JSONExtract fallback). The text accessor (presence counting) and the Postgres branch are byte-unchanged, render-pinned. Why Map, not the native JSON type (verified live on 25.8): JSON drops explicit nulls, flattens dotted metric names into nested paths, fails the whole INSERT on a path collision, and its typed subcolumn reads return NULL for Int64-stored integer scores under .:Float64. The twin stays out of both generated jOOQ models (codegen excludes + includeExcludeColumns; regenerated model is byte-identical) so AnalyticsModelParityTest is unaffected; ClickHouseSchemaDriftTest carves it out of the live side. Guarded by a semantics functional test (integer-valued scores, 17-digit doubles, dotted names, explicit nulls) and render-pinning unit tests. Full ClickHouse suite green. Existing local ClickHouse volumes must be reset: V1.1 was rewritten in place (checksum change) and IF NOT EXISTS will not add the column to old tables. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tics payload columns Skip indexes over JSONExtract expressions work on String columns (added via ALTER + MATERIALIZE INDEX, granule pruning verified live), so test_case_data needs no type change to become indexable; native JSON would also expose the dotted-field-name collision hazard (field names allow dots). extracted_columns has a verified faithful Map twin recipe for when a use-case lands; metric_infos stays raw permanently. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…l-run defaults Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… accessor Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dependency ReviewThe following issues were found:
Vulnerabilitiessettings.gradle
OpenSSF ScorecardScorecard details
Scanned Files
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Applicable issues
Description of changes
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.