feat(0072): forward new current_prices columns through the read view - #158
Merged
Conversation
PR #150 filled the four DEFAULT-stub columns in mv_current_prices and switched the HTTP handlers to pass-through, but left the in-cluster read surface on its v1 shape: current_price_usd exposed only price_usd and updated_at. BE consumes prices.* views IN-CLUSTER (their 0199 contract — named views, no HTTP), so sources / price_xlm / change_*_pct / vwap_24h were unreachable to that consumer no matter what the MV wrote. Closes the task's remaining acceptance criterion. The view moves to CREATE OR REPLACE. CREATE VIEW IF NOT EXISTS does not redefine a view that already exists, so on any target that already has the v1 definition the apply would silently no-op and the new columns would never land. A plain view replaces atomically, so unlike the refreshable MV in current.sql this needs no DROP window. The other five views in the file still use IF NOT EXISTS and carry the same latent footgun; left alone here rather than widened beyond this task. New columns are APPENDED, not inserted, so the six that shipped keep their positions and a SELECT * consumer is not re-ordered underneath itself.
All seven acceptance criteria are now met in code. The task stays active: production still runs the v1 six-column MV and the stubbed API build, and nothing is done until that is verified. The rollout was deferred by PR #150 until task 0114's coarse repair had finished, so its merge pressure could not be mistaken for MV refresh cost. 0114 is archived, so the hold is released. The runbook leads with a read-only cost probe because the new SELECT does materially more work per 60s refresh than the v1 one — a two-level aggregation, an array pipeline, a JSON build and an extra 7-day price_ohlcv_1h FINAL scan — and it carries explicit abort thresholds rather than assuming it fits. Also records the rollback artifact capture (a refreshable MV's old definition survives nowhere else once DROPped) and the fact that the API deploy is the same deploy-production-compute that 0132 sidestepped, so it heals that CFN drift and ships every Lambda in the stack from the tree. Spawns 0134 (the remaining views.sql IF NOT EXISTS footgun) and 0135 (price_usd is not outlier-protected, flagged but unfixed in PR #150).
Every existing assertion in views_it lands on a database setup_scratch just created, so views.sql is only ever applied to a target with no pre-existing current_price_usd — and all of them would pass identically under the old CREATE VIEW IF NOT EXISTS form. The regression this PR exists to prevent was guarded only by a string match on the statement text in src/lib.rs. Seeds the v1 six-column view first and re-applies, which is the actual production upgrade path: ch-prod-01 already holds that shape. The IF NOT EXISTS control run is the load-bearing half — it asserts the old form leaves the v1 view standing at 6 columns, so an edit back to it fails here rather than as a silent no-op against prod. Also pins that the six original columns keep positions 1-6. Raised by the PR #158 review (finding 5); overlaps 0134 AC 4.
…able Three corrections from the PR #158 review, no behaviour change. The "SELECT * consumers are unaffected" note was half true: appending protects column ORDER but not ARITY, and the view now returns 13 columns where it returned 6. Anything decoding positionally — a fixed-arity tuple, a row struct, INSERT INTO t SELECT * — breaks on the extra columns. Since BE reading this view in-cluster is the premise of the PR, the note now says to pin an explicit column list, and the runbook tells them the arity changed rather than only that the columns are live. The JOIN interop contract block did not cover the seven new columns, and their sentinels are load-bearing: current_prices is non-nullable, so "unavailable" and a real value share a type. sources in particular has three states and '' is not valid JSON — our own API special-cases it in dto.rs::parse_sources, BE had no documented equivalent. 0 on the numeric columns is indistinguishable from a genuine 0% change. Rollback: SHOW CREATE emits bare CREATE statements, so neither captured artifact replays — both fail Code: 57 TABLE_ALREADY_EXISTS. Step 0 now rewrites them into runnable form at capture time (OR REPLACE for the view, a leading DROP for the MV) and greps the MV artifact for arrayReduce('median') to prove it captured the v1 definition, rather than leaving the operator to work that out mid-incident. Step 5's "price_xlm must be exactly 1" was a false-abort trigger: the xlm_usd scalar filters close_usd > 0 and price_usd does not, so an un-enriched XLM tip yields 0, diagnosing enrichment lag rather than a broken view. Softened, with the query that tells them apart.
The PR #158 review found that CREATE OR REPLACE VIEW requires a DROP VIEW privilege on CH 26.3.10.60 — unconditionally, even when the view does not exist. apply_sql propagates the first error and prices-clickhouse-init applies VIEWS_SQL unconditionally, so the init binary aborts for any scoped applier. Task 0072 made that true for one view; 0134 would make it true for all six. It is not a SQL-GRANT away: the prod prices_* users are XML-managed in BE's services.xml, the same constraint that moved 0133's alarm to BE. Not currently biting, because the 0072 rollout applies views.sql via docker exec as the container's default user. Records three resolution options with the trade-off of each, including the one to reject (tolerating Code: 497 re-introduces the silent no-op this task exists to remove), so it is not re-derived later. Deciding between them is now the substance of the task rather than the sed. Also carries the correction of the "every statement is CREATE … IF NOT EXISTS" claims on prices-clickhouse-init.rs:3 and lib.rs:120, which 0072 falsified and did not fix, and points at 0072's upgrade-path test as the pattern for the remaining five views.
The PR #158 review implied 0072 broke prices-clickhouse-init for scoped appliers. Measuring ch-prod-01 shows there was no scoped applier to break. prices_writer holds only SELECT, INSERT, ALTER DELETE, OPTIMIZE on prices.* plus SELECT on system.parts; prices_reader only SELECT. Neither has DROP VIEW — nor CREATE VIEW, CREATE TABLE or CREATE DATABASE, so init.sql would have failed on its first statement at any point, including before 0072. The DROP VIEW requirement is real; the regression framing was not. Schema DDL on ch-prod-01 has always been an operator action as the container's default user over the loopback native port, which bypasses Caddy and the mTLS CN map entirely. That is how 0076 applied the 0039/0053 schema and how the 0072 runbook applies current.sql and views.sql. The runtime users' DDL-free grants are the intended design. Decision: option 2 — keep view DDL off the scoped path and document it. Option 1 is now known to be insufficient as well as over-broad (they would need CREATE VIEW and table DDL too, a large grant to the ingestion writer for no operational gain). Option 3 stays rejected for re-introducing the silent no-op. Decision only. The conversion of the remaining five views is unstarted and the task stays in backlog.
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
1 participant
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
mv_current_pricesand switched the HTTP handlers to pass-through, but leftcurrent_price_usdon its v1 shape —price_usd+updated_atonly. BE consumesprices.*views in-cluster (their 0199 contract: named views, no HTTP), sosources/price_xlm/change_*_pct/vwap_24hwere unreachable to that consumer no matter what the MV wrote.CREATE OR REPLACE.CREATE VIEW IF NOT EXISTSdoes not redefine a view that already exists, so against a target already holding the v1 definition — i.e. ch-prod-01 — the apply silently no-ops and the columns never land. Proven on local CH 26.3.10.60 against a hand-built v1 view: theIF NOT EXISTSform left it at 6 columns,OR REPLACEtook it to 13. A plain view replaces atomically, so no DROP window (unlike the refreshable MV incurrent.sql).SELECT *consumer is not re-ordered underneath itself.docs/runbooks/0072-current-prices-mv-rollout.md— the deferred prod rollout, which PR feat(lore-0072): materialize the current_prices deferred columns #150 held until 0114's coarse repair finished (now archived). Leads with a read-onlyFORMAT Nullcost probe with explicit abort thresholds, because the new SELECT does materially more work per 60s refresh: two-level aggregation, array pipeline, JSON build, plus an extra 7-dayprice_ohlcv_1h FINALscan. Covers rollback-artifact capture and per-layer rollback.views.sqlviews carry the sameIF NOT EXISTSfootgun) and 0135 (price_usdis still not outlier-protected — flagged but deliberately unfixed in PR feat(lore-0072): materialize the current_prices deferred columns #150, and it propagates intomarket_cap_usd).Verification
prices-clickhouseandprices-api,-- --ignored)views_itassertions fail withUNKNOWN_IDENTIFIERagainst the pre-0072 view shape, so they cannot pass vacuouslyvwap_24hand thesourcesJSON, whilevolume_24h_usdstill counts its volume as a traded total