feat(execution): add validate mode and a row guardrail - #116
Merged
Conversation
Nothing bounded what a chart query returned. Altair carries a 5,000-row cap, but altair/utils/save.py disables it while saving because vl-convert needs the data inlined -- which is the path glyf renders through, so the cap never applied here. Measured on the real path, one chart at 1k rows is a 0.26 MB SVG, at 10k 2.55 MB, at 100k 25.7 MB; `embed_charts` inlines that markup, so the dashboard page *is* that size. `glyf build --validate` runs each query wrapped in `limit 0`, checks that every column bound by VISUALISE is present, writes the compiled SQL and stops -- no images, no data files, no dashboards, no export. That is what CI actually wants to know: whether the SQL still runs and still binds the columns the chart draws, not what the numbers are this morning. On the example project it writes five compiled SQL files and nothing else. Nothing is drawn on purpose. A chart rendered from a sample looks exactly like a real one, and would be reviewed, or published, as if it were. `execution.max_rows` bounds a normal build and fails when a query exceeds it, naming the chart and saying to aggregate. It never truncates: a chart drawn from an arbitrary slice of a result is indistinguishable from a correct one. Unset by default, so no existing project changes behaviour. Both bounds are applied in SQL so the warehouse sends less over the wire, and both the docs and the commit say plainly what that does not do -- `LIMIT` bounds transfer and render time, not what the warehouse scans. ARCHITECTURE.md records the ceiling this implies: glyf is a compile-time renderer, not a data engine, and a chart is a picture of an aggregate.
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.
§1 of the scale and exposure plan. Two bounds on what a chart query returns, plus the architectural ceiling they imply.
Nothing bounded the input, and not by accident
Altair carries a 5,000-row cap, so this looked like it was already handled. It is not:
altair/utils/save.py:283runsdata_transformers.disable_max_rows()while saving, because vl-convert needs the data inlined — and saving is exactly the path glyf renders through. Confirmed directly: at 20k rowschart.to_dict()raisesMaxRowsErrorwhilechart.save()happily writes a 5 MB SVG.Measured on the real render path, one chart:
SVG spends a DOM node per mark; PNG rasterises.
embed_charts: trueis the default and inlines that SVG into the dashboard page, so the page is the 25 MB.glyf build --validateEach query runs wrapped in
limit 0— columns, no rows. glyf checks every column bound byVISUALISEis present, writes the compiled SQL, and stops: no images, no data files, no dashboards, no export.execution.mode: validatedoes the same fromglyf.yml.On
examples/simple_dbtwith a clean output directory it writes five compiled SQL files and nothing else, in 0.47s against 0.87s for a full render — and that gap is on four rows of demo data, where fetching and drawing cost almost nothing.Nothing is drawn deliberately. A chart rendered from a sample looks exactly like a real one and would be reviewed, or published, as if it were. The CLI says what it skipped rather than printing a green tick that reads like a build.
execution.max_rowsUnset by default — no existing project changes behaviour. When set, a query returning more fails:
It never truncates. A chart drawn from an arbitrary slice of a result is indistinguishable from a correct one, which is the worst way for this to fail.
Both bounds are applied in SQL via
wrap_row_limit(the original becomes a subquery, so a trailingorder bycannot swallow the limit), so the warehouse sends less over the wire. The docs state plainly what that does not do:LIMITbounds transfer and render time, not what the warehouse scans — an aggregate computes in full whatever follows it.Ceiling recorded
ARCHITECTURE.mdgains one Design Decisions entry: glyf is a compile-time renderer, not a data engine; a chart is a picture of an aggregate, and theSELECTis where aggregation belongs. A line chart is ~2,000 pixels wide, so a million points is 500 marks per pixel column — invisible to the reader, paid for by everyone. Rendering large results faster is noted as a separate question, and is next up as a spike.Checks
247 passed, up from 225 — 15 for validate mode and the guardrail (including CLI coverage), 7 for the config keys.
Beyond the suite: validated the example project on a clean target (five
compiled/*.sql, nocharts/, nodata/), confirmedmax_rows: 3fails andmax_rows: 100renders normally, and confirmed a full build still produces dashboards and the site unchanged.