feat: continuous analytics (lastYears=0) support for Doris#24440
Draft
jason-p-pickering wants to merge 23 commits into
Draft
feat: continuous analytics (lastYears=0) support for Doris#24440jason-p-pickering wants to merge 23 commits into
jason-p-pickering wants to merge 23 commits into
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…no-op negative case
…able catalog qualification - DorisSqlBuilder.createTable() now reorders the rendered column list so primary key columns lead, since Doris requires key columns to be an ordered leading prefix of the schema. Aggregate and completeness analytics tables (where id is not already column 0) would otherwise fail CREATE TABLE. - AbstractJdbcTableManager.swapTable()'s Doris insert-into-main branch now references the staging table via quote() instead of qualifyTable(), since the staging table is a native Doris table, not a federated one (same fix already applied to removeUpdatedData() in earlier commits).
…artition requirement Doris's UNIQUE KEY (Merge-on-Write) model requires the partition column to be part of the key. Analytics tables are partitioned by year, so the key must be composite: (id, year) for aggregate/completeness, (event, year) for event. Confirmed working end-to-end against a real Doris 3.0.7 instance. Also fixes DorisSqlBuilder.getOrderedColumns(), which built the leading key-column block from natural column order rather than the primary key's declared order. For a composite key, Doris requires the leading schema columns to match the key clause's declared order exactly -- for the aggregate/completeness tables, year appears before id in natural column order (FIXED_COLS), which would have produced a mismatched schema. removeUpdatedData() DELETE...USING SQL is unchanged in all three managers -- it matches on id/event alone and was confirmed to still correctly find and delete rows regardless of partition.
…st isPartialUpdate() skipMasterTable is computed from params.isPartialUpdate() (lastYears != null || isLatestUpdate()), not isLatestUpdate() alone, so the new Doris insert-into-main branch was also firing for lastYears=1..N (a normal bounded-years rebuild), not just lastYears=0 (the continuous/latest-partition case this plan is scoped to). removeUpdatedData() never runs for lastYears=N>0, so the branch was merging staging data into main without ever purging rows deleted at source -- a real regression versus the pre-PR no-op-drop-only behavior for that path. Gate the insert branch on params.isLatestUpdate() as well, restoring the exact pre-PR (safe, no-op) behavior for lastYears=N>0 on Doris while keeping the insert working for lastYears=0.
jason-p-pickering
marked this pull request as draft
July 14, 2026 05:58
…ey column) Real Doris CI (Sierra Leone e2e run) failed with errCode = 2, "String Type should not be used in key column[id]" while creating the DATA_VALUE staging table -- aborting the whole analytics table generation job, including everything after DATA_VALUE in iteration order. The synthetic id column (concat(...) of several dimension UIDs) was defined as DataType.TEXT, which DorisSqlBuilder maps to the unbounded "string" type. Doris rejects unbounded string/text as a key column on any key model. event/dx don't have this problem -- they're already CHARACTER_11 (bounded char(11)). Give id a bounded VARCHAR_255 (Doris: varchar(1020)) type specifically when sqlBuilder.requiresUniqueKeyAnalyticsTables() is true (Doris only), leaving Postgres/ClickHouse's TEXT unchanged, in both JdbcAnalyticsTableManager and JdbcCompletenessTableManager.
… into feat/doris-continuous-analytics
…inuous-analytics history preservation
tableExists() filtered on t.table_schema = 'public', but native Doris
analytics tables live in whatever database the connection is on (e.g.
'dhis2'), not 'public' ('public' is only correct for the federated
Postgres catalog). This meant tableExists() always returned false for
real Doris tables, so swapTable()'s skipMasterTable was always false,
so every continuous (lastYears=0) run silently replaced the entire
main analytics table with just the latest-window data instead of
inserting into the existing table - destroying history.
Found via a live manual test against a real Doris instance: triggered
lastYears=0, and confirmed the main table's row count and partition
history matched exactly what a full replace (not a merge) would
produce. Fixed by using database() instead of the hardcoded schema
name, which correctly resolves to the connection's actual database.
Re-verified live after the fix: history preserved, create/update/delete
all correctly reflected, "skip master table: true" confirmed in logs.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
JUnit 5 test lifecycle methods don't need public visibility; the rest of the class already uses package-private.
2 tasks
Contributor
|
The Doris analytics CI failures (12 download tests with HTTP 406) are a master regression from #24463, not from this PR. Fix opened: #24479 Stripped-path content negotiation was matching the JSON |
|
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
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.



Summary
Adds continuous/incremental analytics table updates (
lastYears=0) for Apache Doris, matching functionality that already exists for PostgreSQL. Previously, Doris only supported full analytics table rebuilds.SqlBuildercapability flags:supportsContinuousAnalytics()(Postgres/Doris=true, ClickHouse=false — ClickHouse continuous analytics was concluded not feasible in earlier investigation) andrequiresUniqueKeyAnalyticsTables()(Doris-only, kept separate so Postgres/ClickHouse table DDL is unchanged).DATA_VALUE,COMPLETENESS, andEVENTDoris analytics tables now use a compositeUNIQUE KEY(id, year)/UNIQUE KEY(event, year)model instead ofDUPLICATE KEY— required because Doris rejectsDELETE ... WHERE x IN (SELECT ...)outright, only supportsDELETE ... USING <join>onUNIQUE KEYtables, and requires the partition column (year) to be part of the key for its Merge-on-Write model.removeUpdatedData()rewritten in the three affected table managers to emit Doris'sDELETE...USINGform.swapTable()/DefaultAnalyticsTableServicewired so the mechanism only activates for the true continuous (lastYears=0) case; ClickHouse and Postgres behavior is unchanged, and bounded partial rebuilds (lastYears=N>0) are unaffected.Bugs found and fixed during review/validation (beyond the initial implementation)
This PR went through unusually thorough post-implementation validation — a full whole-branch code review, then live testing against a real Doris instance, then a full manual create/update/delete/
lastYears=0lifecycle test with before/after evidence. That process found and fixed six real bugs:CREATE TABLEcolumn ordering — Doris requires unique-key columns to be a leading schema prefix; fixed by reordering the rendered column list.swapTable()staging-table catalog qualification — was referencing the native Doris staging table through the federated Postgres catalog prefix; fixed to use nativequote(...).year) to be part of the key; validated live against real Doris before implementing.swapTable()insert-gate scope leak — was reachable forlastYears=N>0(not justlastYears=0), which would have started merging fresh data into the main table for bounded rebuilds without ever purging deleted-at-source rows. Narrowed toisLatestUpdate()only.idkey column type —DataType.TEXTmaps to Doris's unboundedstringtype, which Doris rejects as a key-column type. Caught by real CI (run-api-analytics-tests-doris.yml) aborting the entire analytics build. Fixed with a boundedVARCHAR_255on Doris only.DorisSqlBuilder.tableExists()checked the wrong schema ('public', the Postgres-catalog convention, instead of the actual connected database) — so it always returnedfalsefor real Doris tables, soswapTable()'sskipMasterTablewould always evaluatefalse. Concretely: had this shipped as-is, every continuous (lastYears=0) run on Doris would have replaced the entire main analytics table with just the latest-window data, destroying all historical data outside that window.mastertoday, not introduced by this PR.removeUpdatedData()unconditionally issuedDELETE ... WHERE x IN (SELECT ...), a form Doris rejects outright. AnylastYears=0attempt against Doris pre-PR would have failed with a SQL error at that earlier step, beforeswapTable()— and therefore beforetableExists()— was ever reached. This is consistent with continuous analytics on Doris never having been supported before this PR.removeUpdatedData()DELETE...USINGrewrite andsupportsContinuousAnalytics()gating added here are what first madeswapTable()'sskipMasterTablebranch reachable for Doris at all. Left unfixed, this PR would have introduced the silent-data-loss bug as part of shipping the feature — not resurrected a dormant production issue. Caught via manual create→update→delete→lastYears=0testing against a live Doris instance before merge, invisible to unit tests (mocked JDBC), the whole-branch review, and CI's E2E test (which never exercises a secondlastYears=0run against pre-existing data — see the follow-up ticket below). Fixed by usingdatabase()instead of the hardcoded schema literal.Manual validation performed
After the
tableExists()fix (finding 6 above), ran a full live validation against a real Doris 3.0.7 instance + the Sierra Leone DB (same stack CI uses), for bothDATA_VALUE(aggregate) andEVENT:lastYears=0→ confirmed new data appears.lastYears=0→ confirmed new value reflected, no duplicate row.lastYears=0→ confirmed row removed.swapTable()takes the insert-into-main path (skip master table: true), not the full-replace path.Known follow-ups (filed separately, not blocking this PR)
lastYears=0) — CI only validates full-build table creation and read-only queries. This is exactly the gap that let thetableExists()schema bug (finding 6 above) through. Scoped as a separate follow-up PR (aggregate/DATA_VALUEfirst).lastYears=N>0(bounded partial rebuilds, as opposed tolastYears=0) appears to be a pre-existing silent no-op on Doris, unrelated to this PR's scope.removeUpdatedData()performance gap (missinglastupdatedindex ontrackerevent/completedatasetregistration/singleevent) was found on the Postgres side while investigating — pre-existing, not Doris-specific, not introduced by this PR.idcolumn in favor of the already-presentdx/co/ao/pe/oucolumns) was identified but deliberately deferred as a follow-up, not needed for correctness.Test plan
dhis-support-sql,dhis-service-analyticsmodules)mvn spotless:checkcleanrun-api-analytics-tests-doris.yml(real Doris + Sierra Leone DB) — passinglastYears=0lifecycle for aggregate and event types, with before/after database evidence