From 27991434a07fcf20028f4af08c329cfbbc69fc3f Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sun, 11 Oct 2026 01:10:31 -0400 Subject: [PATCH] spike(historian): T0 review fixes Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- .../2026-10-10-questdb-historian-design.md | 31 ++++++++++---- docs/plans/2026-10-10-questdb-spike.md | 42 ++++++++++++++++++- 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/docs/plans/2026-10-10-questdb-historian-design.md b/docs/plans/2026-10-10-questdb-historian-design.md index 63ec36ed..c2f1ec99 100644 --- a/docs/plans/2026-10-10-questdb-historian-design.md +++ b/docs/plans/2026-10-10-questdb-historian-design.md @@ -262,9 +262,17 @@ Every query filters `NOT deleted`. Every value goes through `StoredSample`, so t Server's. - **Raw.** A range on `ts`, `ORDER BY ts [DESC]`, `LIMIT @top` (`maxValuesPerNode == 0` ⇒ unbounded), at most `top` samples, `ContinuationPoint: null`; the node manager pages by re-reading from the last timestamp (plan R8). The dedup key forbids same-instant ties. Bounds follow the existing data-source contract. + - **T0-confirmed form (review fix):** `SELECT ts, kind, num_value, int_value, str_value, status FROM {p}sample WHERE + tag = $1 AND ts >= $2 AND ts < $3 AND NOT deleted ORDER BY ts [DESC] LIMIT $4`. + - `$4` is a **bound** parameter, `NpgsqlDbType.Integer` (Int32) or `Bigint` (Int64); both work on 10.0.1, ASC and + DESC. Use `Integer`, since `top` is an `int`. + - `int.MaxValue` (the R8 "unbounded" value) and `long.MaxValue` both return every row. `LIMIT 0` returns no rows, + but R8 never sends 0. + - The inlined-literal fallback is not needed. - **AtTime (T0-confirmed Q9).** Find the previous-or-equal neighbour and the next neighbour of each requested time - with `LIMIT 1` seeks, `UNION ALL`-ed, **at most 50 request times (100 branches) per statement**. QuestDB rejects - 128 or more branches with `SQL model is too complex to evaluate`. Per time `k`, with `$1` = tag and `$j` = the + with `LIMIT 1` seeks, `UNION ALL`-ed, **at most 50 request times (100 branches) per statement**. The exact QuestDB + 10.0.1 limit is **126 `UNION ALL` branches per statement (63 request times)**: 126 work, and 127 fail with `SQL + model is too complex to evaluate`. So 50 times leaves a 26-branch margin. Per time `k`, with `$1` = tag and `$j` = the rounded time: ```sql SELECT * FROM (SELECT k AS req, 0 AS dir, ts, kind, num_value, int_value, str_value, status FROM {p}sample @@ -289,8 +297,9 @@ Server's. - It does **not** use `SAMPLE BY`, whose alignment rules differ from ours. Computing the index ourselves keeps the bucket edges identical to SQL Server: start inclusive, end exclusive, a partial final bucket, interval 0, and the 100k cap. - - The time of the min or max value needs an arg-min style lookup. It is computed with a window function, or with a - second statement if QuestDB lacks one; T0 decides. `AggregateEvaluator` then produces the values. + - The time of the min or max value (ActualTime family) comes from a `row_number()` window function in the same single + statement: `ORDER BY num_value ASC|DESC, ts ASC`, with ties going to the earliest sample (T0-confirmed; the form is + below). No second statement is used. `AggregateEvaluator` then produces the values. - **Confirmed forms (T0 Q8).** They mirror `SqlAggregateQueries`'s three families and column order. They were checked against hand-computed seeds and a 60k-row randomized cross-check across 4 partitions: 617 buckets, 0 mismatches. - Classified subquery `s`: @@ -362,11 +371,15 @@ The Gateway keys (`Endpoint`, `ApiKey`, `UseTls`) are not required for QuestDb, ## 6. Environment -- docker-dev gains a `questdb` service, using an exact image tag that T0 pins (at least two weeks old). It sits behind - a compose profile (`questdb`) so the default rig is unchanged. -- It exposes PG wire `8812` and the web console `9000` on host ports that T0 chooses so they don't collide with - AdminUI `:9000`/`:9200`. -- It uses a named volume. +- QuestDB is an **optional overlay file**, `docker-dev/docker-compose.questdb.yml`, so the default rig is unchanged. + It defines the `questdb` service: + - image `questdb/questdb:10.0.1`, pinned by T0 and pushed 2026-08-24; + - container `otopcua-dev-questdb-1`; + - started with `docker compose -f docker-dev/docker-compose.yml -f docker-dev/docker-compose.questdb.yml up -d + --no-deps questdb`. +- It exposes PG wire on host port **18812** (→ 8812) and the web console/REST on host port **19000** (→ 9000), so it + does not collide with AdminUI `:9000`/`:9200`. +- It uses the named volume `otopcua-questdb-data`. - The central nodes gain `ServerHistorian__QuestDb__ConnectionString` (`OTOPCUA_HISTORIAN_QUESTDB`), defaulting to the `questdb` service. - No new NuGet packages. diff --git a/docs/plans/2026-10-10-questdb-spike.md b/docs/plans/2026-10-10-questdb-spike.md index 510dc1cc..1a028cab 100644 --- a/docs/plans/2026-10-10-questdb-spike.md +++ b/docs/plans/2026-10-10-questdb-spike.md @@ -287,7 +287,9 @@ one exact hit and one after the last sample. ``` **Decision: form A, chunked `LIMIT 1` seeks, at most 50 request times (100 `UNION ALL` branches) per statement.** -QuestDB rejects 128 or more branches. The literal `req` index and `dir` (0 = previous-or-equal, 1 = next) columns are +The exact limit is **126 `UNION ALL` branches (63 request times) per statement**. A re-measure with single-seek +branches (review fix, below) showed 126 work and 127 fail. The form A runs above agree: 63 times (126 branches) work, +and 64 times (128 branches) fail. The literal `req` index and `dir` (0 = previous-or-equal, 1 = next) columns are generated integers, not user input; `$1` is the tag and `$2..` the rounded times. It returns both neighbours in one shape and costs ~0.2 ms per time. ASOF JOIN is faster, but it only gives the previous neighbour, so a second seek form would still be needed for the next one. That does not justify two code paths at this scale. @@ -393,6 +395,44 @@ DeleteRaw loop, as confirmed: `SELECT ts FROM {p}sample WHERE tag = $1 AND ts >= ts LIMIT 1000` → one 1,000-row tombstone INSERT → the Q4 wait → continue from `last ts + 10 ticks`. The wait matters: without it the next page can re-read rows it just tombstoned. +## Review fixes (2026-10-11) + +### Raw `LIMIT $n` with a bound parameter + +`dotnet run -- limit` against 10.0.1, with 10 rows seeded in `[t0, t0+10 s)`. The SQL was `SELECT ts, num_value FROM +spike_lim WHERE tag = $1 AND ts >= $2 AND ts < $3 AND NOT deleted ORDER BY ts [DESC] LIMIT $4`: + +``` + rows=3 first=0 last=2 [OK] ASC LIMIT $4 (Integer 3) + rows=3 first=9 last=7 [OK] DESC LIMIT $4 (Integer 3) + rows=3 first=0 last=2 [OK] ASC LIMIT $4 (Bigint 3) + rows=3 first=9 last=7 [OK] DESC LIMIT $4 (Bigint 3) + rows=10 first=0 last=9 [OK] ASC LIMIT $4 (Integer int.MaxValue) + rows=10 first=9 last=0 [OK] DESC LIMIT $4 (Integer int.MaxValue) + rows=10 first=0 last=9 [OK] ASC LIMIT $4 (Bigint long.MaxValue) + rows=10 first=9 last=0 [OK] DESC LIMIT $4 (Bigint long.MaxValue) + rows=0 first=NULL last=NULL [OK] ASC LIMIT $4 (Integer 0) + rows=0 first=NULL last=NULL [OK] DESC LIMIT $4 (Integer 0) +``` + +**Decision:** a bound `LIMIT $n` works, with `NpgsqlDbType.Integer` (Int32) or `Bigint` (Int64). Use `Integer`, since +`top` is an `int`. R8's unbounded value `int.MaxValue` returns every row. `LIMIT 0` returns nothing, but R8 never sends +it. The inlined-literal fallback is not needed. + +### Exact `UNION ALL` branch limit + +`dotnet run -- union`. Each branch is `SELECT * FROM (SELECT k AS req, ts FROM spike_un WHERE tag = $1 AND ts <= $2 AND +NOT deleted ORDER BY ts DESC LIMIT 1)`: + +``` + [OK] 100 branches [OK] 120 branches [OK] 125 branches [OK] 126 branches + [FAIL] 127 branches: PostgresException: 00000: SQL model is too complex to evaluate + [FAIL] 128 branches / 129 / 130: same +``` + +**Decision:** the limit is **126 branches, which is 63 AtTime request times** (two branches per time). The chosen chunk +of **50 times (100 branches)** is unchanged. + ## Rulings candidates raised by the spike 1. **Pre-1970 timestamps cannot be stored** (Q13). QuestDB rejects a designated timestamp before 1970-01-01, and the