diff --git a/ggsql-cli/CLAUDE.md b/ggsql-cli/CLAUDE.md index cf78d13b4..f9defa7c6 100644 --- a/ggsql-cli/CLAUDE.md +++ b/ggsql-cli/CLAUDE.md @@ -87,7 +87,7 @@ The macOS codesign step uses [`/entitlements.plist`](../entitlements.plist) at t ## Features ```toml -default = ["duckdb", "sqlite", "vegalite", "parquet", "builtin-data", "odbc", "svg", "pdf", "hep"] +default = ["duckdb", "sqlite", "vegalite", "html", "parquet", "builtin-data", "odbc", "svg", "pdf", "hep"] ``` Each feature passes through to `ggsql/`. A writer feature gates only its own row's render function in `writers.rs`; the row itself is always present. @@ -124,7 +124,7 @@ open target/visual-test/index.html Four properties are worth preserving when changing it: -- **One reader per source file, cells in document order.** Doc pages build a table in one cell and plot it in the next, so per-cell isolation would break the corpus. A cell with no `VISUALISE` (`validate(..).has_visual()` is false) runs as setup through `execute_sql`. +- **One reader per source file, cells in document order.** Doc pages build a table in one cell and plot it in the next, so per-cell isolation would break the corpus. A cell with no `VISUALISE`/`TABULATE` (`validate(..).has_spec()` is false) runs as setup through `execute_sql`. - **Cells run in their own page's directory**, as Quarto runs them, so a query reading `FROM 'minard_troops.csv'` finds the CSV sitting beside the `.qmd`. The report and its `assets/` are resolved to an absolute path up front, since they outlive that switch. - **Nothing aborts the run.** Execution errors, render errors and *panics* inside a writer are captured per cell (`capture`), so one report surfaces every problem in the corpus at once. This is the point of the tool — a run that stops at the first failure tells you almost nothing. - **Renders are files, specs are inline.** PNGs are written to `assets/`; Vega-Lite specs are embedded in `"), - "<script>alert('xss')</script>" - ); - } - /// A render backend with no GPU, so tests are fast and identical /// everywhere. The SVG path it leaves is the one that always works. fn backend() -> PlotBackend { diff --git a/ggsql-jupyter/src/executor.rs b/ggsql-jupyter/src/executor.rs index ac7adbddd..6e79b2ba6 100644 --- a/ggsql-jupyter/src/executor.rs +++ b/ggsql-jupyter/src/executor.rs @@ -12,16 +12,17 @@ use anyhow::Result; use ggsql::{ reader::{ connection::{extract_odbc_value, reader_from_uri}, - Reader, Spec, + Reader, ResolvedPlot, ResolvedSpec, }, validate::validate, + writer::{HtmlWriter, Writer}, DataFrame, }; /// A resolved plot has to reach a render thread, so the design rests on this. const _: () = { fn assert_send() {} - let _ = assert_send::; + let _ = assert_send::; }; /// Result of executing a ggsql query @@ -33,14 +34,17 @@ pub enum ExecutionResult { /// /// Not pre-rendered: the format depends on where the output is going, and /// once a plot comm is open it is asked again on every resize. Boxed because - /// a `Spec` carries the post-stat DataFrames and dwarfs the other variants. - Visualization(Box), + /// a `ResolvedPlot` carries the post-stat DataFrames and dwarfs the other + /// variants. + Visualization(Box), + /// TABULATE query, already rendered as an HTML table via `HtmlWriter`. + Table { html: String }, /// Connection changed via meta-command ConnectionChanged { display_name: String }, } -// `Spec` is neither `Debug` nor `Clone`, so this summarises rather than -// deriving. What a log wants from a result is its shape and size anyway. +// `ResolvedPlot` is neither `Debug` nor `Clone`, so this summarises rather +// than deriving. What a log wants from a result is its shape and size anyway. impl std::fmt::Debug for ExecutionResult { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -56,6 +60,10 @@ impl std::fmt::Debug for ExecutionResult { .field("layers", &metadata.layer_count) .finish() } + Self::Table { html } => f + .debug_struct("Table") + .field("html_len", &html.len()) + .finish(), Self::ConnectionChanged { display_name } => f .debug_struct("ConnectionChanged") .field("display_name", display_name) @@ -265,7 +273,7 @@ impl QueryExecutor { let validated = validate(code)?; // 2. Check if there's a visualization - if !validated.has_visual() { + if !validated.has_spec() { // Pure SQL query - execute directly and return DataFrame. let df = self.reader.execute_sql(code)?; tracing::info!( @@ -279,15 +287,38 @@ impl QueryExecutor { // 3. Execute ggsql query using reader let spec = self.reader.execute(code)?; - tracing::info!( - "Query executed: {} rows, {} layers", - spec.metadata().rows, - spec.metadata().layer_count - ); + // 4. A table is rendered here, since a static HTML table needs no + // choice about where the output is going. A plot is not: choosing a + // format is the display layer's job, because only it knows that. + match spec { + ResolvedSpec::Table(table) => { + tracing::info!( + "Query executed: {} rows, {} cols", + table.body().height(), + table.body().width() + ); + for warning in table.warnings() { + tracing::warn!("{}", warning.message); + } + + let html = HtmlWriter::new().write_table(table.table(), table.body())?; + tracing::debug!("Generated HTML table: {} chars", html.len()); + + Ok(ExecutionResult::Table { html }) + } + ResolvedSpec::Plot(plot) => { + tracing::info!( + "Query executed: {} rows, {} layers", + plot.metadata().rows, + plot.metadata().layer_count + ); + for warning in plot.warnings() { + tracing::warn!("{}", warning.message); + } - // 4. Hand back the resolved plot. Choosing a format is the display - // layer's job, because only it knows where the output is going. - Ok(ExecutionResult::Visualization(Box::new(spec))) + Ok(ExecutionResult::Visualization(plot)) + } + } } } @@ -304,6 +335,21 @@ mod tests { assert!(matches!(result, ExecutionResult::Visualization(_))); } + #[test] + fn test_tabulate() { + let mut executor = QueryExecutor::new().unwrap(); + let code = "SELECT 1 AS x, 2 AS y TABULATE"; + let result = executor.execute(code).unwrap(); + + match result { + ExecutionResult::Table { html } => { + assert!(html.contains("")); + assert!(html.contains("")); + } + other => panic!("expected Table, got {other:?}"), + } + } + #[test] fn test_pure_sql() { let mut executor = QueryExecutor::new().unwrap(); @@ -384,7 +430,7 @@ mod tests { ) .unwrap(); assert_eq!(executor.reader_uri(), "duckdb://memory"); - assert!(matches!(result, ExecutionResult::Visualization { .. })); + assert!(matches!(result, ExecutionResult::Visualization(_))); } #[test] @@ -406,7 +452,7 @@ mod tests { let result = executor .execute("-- @uncache\nSELECT 1 AS x, 2 AS y VISUALISE x, y DRAW point") .unwrap(); - assert!(matches!(result, ExecutionResult::Visualization { .. })); + assert!(matches!(result, ExecutionResult::Visualization(_))); } #[test] diff --git a/ggsql-jupyter/src/kernel.rs b/ggsql-jupyter/src/kernel.rs index 369061d74..4f957cc73 100644 --- a/ggsql-jupyter/src/kernel.rs +++ b/ggsql-jupyter/src/kernel.rs @@ -11,7 +11,7 @@ use crate::message::{ConnectionInfo, JupyterMessage, MessageHeader}; use crate::plot::comm::{PlotMetadata, RenderParams, RpcError}; use crate::plot::{PlotBackend, RenderOutcome, RenderTicket}; use anyhow::Result; -use ggsql::reader::Spec; +use ggsql::reader::ResolvedPlot; use hmac::{Hmac, Mac}; use serde_json::{json, Value}; use sha2::Sha256; @@ -38,7 +38,7 @@ pub struct KernelServer { /// Taken out of `self` before the event loop, because `select!` cannot /// borrow `self` mutably for this arm while the other arms do the same. render_outcomes: Option>, - /// Open plot comms, metadata only — each plot's `Spec` lives on the render + /// Open plot comms, metadata only — each plot's `ResolvedPlot` lives on the render /// thread, keeping the retained `DataFrame`s off the async task. plot_comms: HashMap, /// Comm ids in the order they were opened, for oldest-first eviction. @@ -937,7 +937,7 @@ impl KernelServer { /// from the `_recentExecutions` map that message populates. async fn open_plot_comm( &mut self, - spec: Box, + spec: Box, code: &str, parent: &JupyterMessage, ) -> Result<()> { @@ -1136,7 +1136,7 @@ impl KernelServer { // An error rather than `result: null`, so a future Positron method // fails visibly instead of being satisfied with garbage. `show` and // `update` belong here: they mean "re-fetch this figure", but a - // `Spec` is immutable per execution, so re-running a cell opens a + // `ResolvedPlot` is immutable per execution, so re-running a cell opens a // new comm as the R and matplotlib backends do. Do not add them. other => { let e = RpcError::MethodNotFound(format!("the plot comm has no '{other}' method")); diff --git a/ggsql-jupyter/src/plot/backend.rs b/ggsql-jupyter/src/plot/backend.rs index e72482f6d..20f64936d 100644 --- a/ggsql-jupyter/src/plot/backend.rs +++ b/ggsql-jupyter/src/plot/backend.rs @@ -28,7 +28,7 @@ use std::collections::HashMap; use std::sync::mpsc::{self, Receiver, Sender}; use anyhow::{anyhow, Result}; -use ggsql::reader::Spec; +use ggsql::reader::ResolvedPlot; use super::{Format, RenderRequest, RenderTicket}; @@ -56,7 +56,10 @@ pub struct RenderOutcome { enum Job { /// Keep `spec` so the plot can be re-drawn at a new size without re-running /// the query. Its `DataFrame`s stay here rather than on the async task. - Store { comm_id: String, spec: Box }, + Store { + comm_id: String, + spec: Box, + }, /// Forget a stored plot, because its comm closed or it was evicted. Forget { comm_id: String }, /// Re-render a stored plot and answer `reply` directly. @@ -79,7 +82,7 @@ enum Job { /// Render a plot we were handed and will not keep, answering `reply` /// directly. The one-shot path, for a static output bundle. RenderOnce { - spec: Box, + spec: Box, request: RenderRequest, reply: Sender>>, }, @@ -181,7 +184,7 @@ impl PlotBackend { /// /// Returns an error if the render thread has stopped, or if the render /// itself failed. - pub fn render_once(&self, spec: Box, request: RenderRequest) -> Result> { + pub fn render_once(&self, spec: Box, request: RenderRequest) -> Result> { let (reply, answer) = mpsc::channel(); self.jobs .send(Job::RenderOnce { @@ -196,7 +199,7 @@ impl PlotBackend { } /// Keep a plot so its comm can re-render it at any size. - pub fn store(&self, comm_id: String, spec: Box) { + pub fn store(&self, comm_id: String, spec: Box) { let _ = self.jobs.send(Job::Store { comm_id, spec }); } @@ -291,7 +294,7 @@ fn render_loop( // The retained plots. They live here rather than beside the comm state so // the post-stat `DataFrame`s stay off the async task entirely. - let mut stored: HashMap> = HashMap::new(); + let mut stored: HashMap> = HashMap::new(); while let Ok(job) = inbox.recv() { match job { @@ -359,7 +362,13 @@ fn warm_up(renderer: Option<&mut Renderer>) { let spec = match ggsql::reader::connection::reader_from_uri("duckdb://memory") .and_then(|reader| reader.execute(QUERY)) { - Ok(spec) => spec, + Ok(spec) => match spec.into_plot() { + Some(plot) => plot, + None => { + tracing::debug!("renderer warm-up skipped: not a plot"); + return; + } + }, Err(e) => { tracing::debug!("renderer warm-up skipped: {e}"); return; @@ -438,7 +447,7 @@ fn comm_namespace(comm_id: &str) -> String { /// `id_namespace` prefixes the ids the SVG writer generates; the other formats /// have no such thing and ignore it. fn render_one( - spec: &Spec, + spec: &ResolvedPlot, request: &RenderRequest, renderer: Option<&mut Renderer>, id_namespace: &str, @@ -514,7 +523,7 @@ mod tests { /// A plot with a colour scale, so the SVG carries a gradient — the ids that /// collide are the ones a legend gradient defines and references. - fn a_spec() -> Box { + fn a_spec() -> Box { use ggsql::reader::{DuckDBReader, Reader}; let query = "SELECT * FROM (VALUES (1,2,10),(2,3,50),(3,1,90)) t(x,y,c) \ VISUALISE x AS x, y AS y, c AS color DRAW point"; @@ -522,6 +531,8 @@ mod tests { DuckDBReader::from_connection_string("duckdb://memory") .unwrap() .execute(query) + .unwrap() + .into_plot() .unwrap(), ) } diff --git a/ggsql-wasm/src/lib.rs b/ggsql-wasm/src/lib.rs index 7f4daa203..93ab72aea 100644 --- a/ggsql-wasm/src/lib.rs +++ b/ggsql-wasm/src/lib.rs @@ -6,7 +6,7 @@ use ggsql::array_util::value_to_string; use ggsql::naming::DATA_PREFIX; use ggsql::reader::sqlite::SqliteReader; use ggsql::reader::Reader; -use ggsql::reader::Spec; +use ggsql::reader::ResolvedPlot; use ggsql::validate::validate; use ggsql::writer::SvgWriter; use ggsql::DataFrame; @@ -253,13 +253,16 @@ impl GgsqlContext { let spec = reader .execute(query) .map_err(|e| JsValue::from_str(&format!("Execute error: {:?}", e)))?; + let spec = spec.into_plot().ok_or_else(|| { + JsValue::from_str("TABULATE queries are not supported in the browser playground") + })?; Ok(GgsqlPlot { spec }) } /// Check whether a query contains a VISUALISE clause pub fn has_visual(&self, query: &str) -> bool { match validate(query) { - Ok(v) => v.has_visual(), + Ok(v) => v.has_spec(), Err(_) => false, } } @@ -398,7 +401,7 @@ impl GgsqlContext { /// query — see [`GgsqlContext::execute`]. #[wasm_bindgen] pub struct GgsqlPlot { - spec: Spec, + spec: ResolvedPlot, } #[wasm_bindgen] diff --git a/src/CLAUDE.md b/src/CLAUDE.md index 35d2ace40..3ee6a2546 100644 --- a/src/CLAUDE.md +++ b/src/CLAUDE.md @@ -17,14 +17,16 @@ src/ ├── dataframe.rs DataFrame wrapper around arrow RecordBatch ├── format.rs Label/number/date formatting ├── naming.rs Internal column-name conventions (__ggsql_*) +├── spec.rs Spec: parse-time result of one VISUALISE/TABULATE statement (Plot or Table) ├── util.rs String helpers (and_list, or_list, …) ├── validate.rs validate(): syntax + semantic checks without SQL execution ├── fonts.rs Font registration, for hosts with no font database │ -├── parser/ Tree-sitter integration → typed AST (Plot) +├── parser/ Tree-sitter integration → typed AST (Spec: Plot or Table) ├── plot/ AST: Plot, Layer, Geom, Scale, Facet, Projection, Mappings (see plot/CLAUDE.md) +├── table/ AST stub for TABULATE, parallel to plot/ (no fields yet) ├── reader/ Reader trait + drivers (DuckDB, SQLite, ODBC, Snowflake, …) -├── execute/ Pipeline that turns Plot + Reader → executed Spec +├── execute/ Pipeline that turns Plot + Reader → ResolvedPlot ├── writer/ Writer trait + Vega-Lite implementation (see writer/vegalite/CLAUDE.md) ├── data/ Bundled sample datasets (penguins, airquality) └── doc/ API.md — public Rust API reference @@ -32,9 +34,9 @@ src/ ### `parser/` -- `mod.rs` exposes `parse_query()` which builds a `Vec` from a query string. -- `source_tree.rs` is the parse-once wrapper: holds the tree-sitter `Tree`, source text, and language; offers a declarative query API (`find_node`, `find_text`, …) plus lazy `extract_sql()` / `extract_visualise()` extractors. It also handles the `VISUALISE FROM ` shorthand by injecting `SELECT * FROM `. -- `builder.rs` walks the CST and produces typed `Plot` values. This is where new grammar nodes become `Plot` fields. +- `mod.rs` exposes `parse_query()` which builds a `Vec` from a query string — one `Spec` per `VISUALISE`/`TABULATE` statement, in source order. Today `build_ast` only ever produces `Spec::Plot`; `TABULATE` isn't wired into the grammar yet. +- `source_tree.rs` is the parse-once wrapper: holds the tree-sitter `Tree`, source text, and language; offers a declarative query API (`find_node`, `find_text`, …) plus lazy `extract_sql()` / `extract_spec()` extractors (the latter covers both `VISUALISE` and `TABULATE`). It also handles the `VISUALISE FROM ` shorthand by injecting `SELECT * FROM `. +- `builder.rs` walks the CST and produces typed `Spec` values (`Plot`, boxed for size, or `Table`). This is where new grammar nodes become `Plot`/`Table` fields. - `sql.rs` extracts structure from SQL fragments over the parse tree. Grammar lives in [`/tree-sitter-ggsql/`](../tree-sitter-ggsql/) — when adding syntax, edit `grammar.js`, regenerate, then teach `builder.rs` about the new nodes. @@ -50,7 +52,7 @@ Grammar lives in [`/tree-sitter-ggsql/`](../tree-sitter-ggsql/) — when adding | `odbc.rs` | ODBC | `odbc` (default) | | `cache.rs` | `CachingReader` — wraps any primary `Reader` with an in-memory cache | `duckdb` or `sqlite` | | `connection.rs` | Connection-string parsing for all of the above | — | -| `spec.rs` | `Spec` type returned by `execute()`, plus DataFrame conversion | — | +| `spec.rs` | `ResolvedPlot` type returned by `execute()`, plus DataFrame conversion | — | | `data.rs` | Bundled sample datasets — the `ggsql:` builtins | `builtin-data` | `SqlDialect` trait in `mod.rs` lets each driver supply its own type names, information-schema queries, and spatial helper methods (`sql_st_transform`, `sql_geometry_to_wkb`, `sql_geometry_bbox`, `sql_ensure_geometry`, `sql_select_replace`, `sql_spatial_setup`). @@ -59,7 +61,7 @@ Grammar lives in [`/tree-sitter-ggsql/`](../tree-sitter-ggsql/) — when adding ### `execute/` -The pipeline that takes a parsed `Plot` plus a `Reader` and produces a fully-resolved `Spec` (typed data per layer, scales resolved, casts applied). Submodules: +The pipeline that takes a parsed `Plot` plus a `Reader` and produces a `ResolvedPlot` (typed data per layer, scales resolved, casts applied). Submodules: - `mod.rs` — top-level `prepare_data_with_reader()` and validation glue. - `cte.rs` — CTE extraction / materialization for shared subqueries. @@ -74,6 +76,7 @@ The pipeline that takes a parsed `Plot` plus a `Reader` and produces a fully-res `Writer` trait in `mod.rs` (associated `Output` type so writers can return text or bytes, and `from_options` for configuration a frontend collects as key–value pairs — `options.rs`'s `WriterOptions`, parsed from the CLI's `--writer-option`). Two families: - **Vega-Lite** (`vegalite` feature, default) — emits Vega-Lite JSON. Deep-dive: [`writer/vegalite/CLAUDE.md`](writer/vegalite/CLAUDE.md). +- **HTML** (`html` feature, default) — `HtmlWriter` renders a resolved `Table` (a `TABULATE` query) as a bare `
x
`; the only writer that supports tables at all. Plot-only otherwise (`vegalite` and the hephaestus writers below). - **The renderer-backed writers** (seven of them; `svg`, `pdf` and `hep` default, the four raster ones not) — all live in `writer/hephaestus/`, named after the renderer they wrap; that name is internal, and the module is private so only the writers, `Canvas` and `RasterRenderer` are public. They share their whole pipeline — `Canvas` for configuration, `compose` for the plot composition, then either `raster` for pixels or `vector` for drawing commands — and differ only in what they do with the result. Deep-dive (architecture + known gaps): [`writer/hephaestus/CLAUDE.md`](writer/hephaestus/CLAUDE.md). | Feature | Default | Writer | Output | GPU | @@ -106,13 +109,13 @@ Sufficiently large to have its own [`plot/CLAUDE.md`](plot/CLAUDE.md). It holds ### `doc/` -Just `API.md` — the public Rust API reference for `Reader::execute`, `Writer::render`, `validate`, `Spec`, `Validated`, `Metadata`. End-user docs live in `/doc/`, not here. +Just `API.md` — the public Rust API reference for `Reader::execute`, `Writer::render`, `validate`, `ResolvedPlot`, `Validated`, `Metadata`. End-user docs live in `/doc/`, not here. ## Public API quick reference Two-stage pipeline: -1. **`reader.execute(query)`** → `Spec` (parses, runs SQL, resolves mappings, applies stats). +1. **`reader.execute(query)`** → `ResolvedPlot` (parses, runs SQL, resolves mappings, applies stats). 2. **`writer.render(&spec)`** → output (Vega-Lite JSON for `VegaLiteWriter`). `validate(query)` performs syntax + semantic checks without touching a reader. @@ -131,6 +134,7 @@ Defined in `Cargo.toml`: | `parquet` | ✓ | Parquet support in readers/data | | `spatial` | ✓ | Spatial/geometry support (geozero for WKT↔GeoJSON) | | `vegalite` | ✓ | Vega-Lite writer | +| `html` | ✓ | `HtmlWriter` — renders a `TABULATE` query as a bare `
` | | `graphics` | — | *Internal.* The shared plot-composition layer; no GPU | | `raster` | — | *Internal.* `graphics` + the GPU rasteriser (wgpu/vello-hybrid) | | `png` | — | PNG writer (`raster`; genuinely 1.88+, excluded from the MSRV check) | diff --git a/src/Cargo.toml b/src/Cargo.toml index e77ef2d51..d2dd86495 100644 --- a/src/Cargo.toml +++ b/src/Cargo.toml @@ -82,7 +82,7 @@ adbc_datafusion = "0.23" adbc_driver_manager = "0.23" [features] -default = ["adbc", "duckdb", "sqlite", "vegalite", "parquet", "builtin-data", "odbc", "spatial", "svg", "pdf", "hep"] +default = ["adbc", "duckdb", "sqlite", "vegalite", "html", "parquet", "builtin-data", "odbc", "spatial", "svg", "pdf", "hep"] duckdb = ["dep:duckdb"] parquet = ["dep:parquet"] sqlite = ["dep:rusqlite"] @@ -90,6 +90,9 @@ adbc = ["dep:adbc_core"] odbc = ["dep:toml_edit", "dep:libloading"] spatial = ["dep:geozero", "rusqlite?/load_extension"] vegalite = [] +# Pure string formatting, no extra dependencies — like `vegalite`, gated only +# so a minimal build can drop it, not because it needs anything to compile. +html = [] # Internal, enabled by the writer features below rather than named directly. # `graphics` is the shared plot-composition layer; `raster` adds the GPU # rasteriser on top of it. Splitting them is what lets a vector-only build skip @@ -143,4 +146,4 @@ webfonts = ["graphics", "dep:wuff"] builtin-data = [] all-readers = ["duckdb", "sqlite", "odbc"] -all-writers = ["vegalite", "png", "jpeg", "tiff", "webp", "svg", "pdf", "hep"] +all-writers = ["vegalite", "html", "png", "jpeg", "tiff", "webp", "svg", "pdf", "hep"] diff --git a/src/doc/API.md b/src/doc/API.md index 1c8e8cbb1..c9449059d 100644 --- a/src/doc/API.md +++ b/src/doc/API.md @@ -4,7 +4,7 @@ This document provides a comprehensive reference for the ggsql public API. ## Overview -- **Stage 1: `reader.execute()`** - Parse query, execute SQL, resolve mappings, create Spec +- **Stage 1: `reader.execute()`** - Parse query, execute SQL, resolve mappings, create a ResolvedSpec (a resolved Plot or Table) - **Stage 2: `writer.render()`** - Generate output (Vega-Lite JSON, SVG, PDF, PNG, …) ### API Functions @@ -12,7 +12,7 @@ This document provides a comprehensive reference for the ggsql public API. | Function | Use Case | | ------------------ | ---------------------------------------------------- | | `reader.execute()` | Main entry point - full visualization pipeline | -| `writer.render()` | Generate output from Spec | +| `writer.render()` | Generate output from a ResolvedSpec (Plot or Table) | | `validate()` | Validate syntax + semantics, inspect query structure | --- @@ -22,10 +22,10 @@ This document provides a comprehensive reference for the ggsql public API. ### `Reader::execute` ```rust -fn execute(&self, query: &str) -> Result +fn execute(&self, query: &str) -> Result ``` -Execute a ggsql query for visualization. This is the main entry point - a default method on the Reader trait. +Execute a ggsql query for visualization or tabulation. This is the main entry point - a required method on the Reader trait. `ResolvedSpec` is `Plot(Box)` or `Table(ResolvedTable)`, depending on whether the query used `VISUALISE` or `TABULATE`; `.as_plot()` / `.as_table()` (or the consuming `.into_plot()` / `.into_table()`) narrow it. **What happens during execution:** @@ -43,7 +43,7 @@ Execute a ggsql query for visualization. This is the main entry point - a defaul **Returns:** -- `Ok(Spec)` - Ready for rendering +- `Ok(ResolvedSpec)` - Ready for rendering - `Err(GgsqlError)` - Parse, validation, or execution error **Example:** @@ -57,11 +57,14 @@ let spec = reader.execute( "SELECT x, y FROM data VISUALISE x, y DRAW point" )?; -// Access metadata -println!("Rows: {}", spec.metadata().rows); -println!("Columns: {:?}", spec.metadata().columns); +// Access metadata (Plot-specific) +if let Some(plot) = spec.as_plot() { + println!("Rows: {}", plot.metadata().rows); + println!("Columns: {:?}", plot.metadata().columns); +} -// Render to Vega-Lite +// Render to Vega-Lite — dispatches to write_plot/write_table based on +// which variant `spec` is let writer = VegaLiteWriter::new(); let result = writer.render(&spec)?; ``` @@ -118,7 +121,7 @@ if !validated.valid() { } // Inspect query structure -if validated.has_visual() { +if validated.has_spec() { println!("SQL: {}", validated.sql()); println!("Visual: {}", validated.visual()); } @@ -150,7 +153,7 @@ pub struct Validated { | Method | Signature | Description | | ------------ | -------------------------------------------- | ---------------------------------- | -| `has_visual` | `fn has_visual(&self) -> bool` | Whether query contains VISUALISE | +| `has_spec` | `fn has_spec(&self) -> bool` | Whether query contains a Spec (VISUALISE or TABULATE) | | `sql` | `fn sql(&self) -> &str` | The SQL portion (before VISUALISE) | | `visual` | `fn visual(&self) -> &str` | The VISUALISE portion (raw text) | | `tree` | `fn tree(&self) -> Option<&Tree>` | CST for advanced inspection | @@ -171,7 +174,7 @@ if !validated.valid() { } // Inspect query structure -assert!(validated.has_visual()); +assert!(validated.has_spec()); assert_eq!(validated.sql(), "SELECT 1 as x"); assert!(validated.visual().starts_with("VISUALISE")); @@ -183,13 +186,16 @@ if let Some(tree) = validated.tree() { --- -### `Spec` +### `ResolvedPlot` -Result of executing a ggsql query, ready for rendering. +Result of executing a `VISUALISE` query, ready for rendering. `reader.execute()` +returns this wrapped in `ResolvedSpec::Plot(Box)`; get here via +`spec.as_plot()` (or the consuming `spec.into_plot()`). #### Rendering -Use `writer.render(&spec)` to generate output. +Pass the `ResolvedSpec` itself to `writer.render()` — it dispatches to +`write_plot`/`write_table` depending on which variant it is. **Example:** @@ -297,9 +303,9 @@ for i in 0..spec.layer_count() { ```rust let spec = reader.execute(query)?; -// Check for warnings -if !spec.warnings().is_empty() { - for warning in spec.warnings() { +// Check for warnings (Plot-specific) +if let Some(plot) = spec.as_plot() { + for warning in plot.warnings() { eprintln!("Warning: {}", warning.message); } } @@ -397,13 +403,18 @@ pub trait Writer { fn from_options(options: &WriterOptions) -> Result where Self: Sized; /// Render a plot specification and its data to the output format - fn write(&self, spec: &Plot, data: &HashMap) -> Result; + fn write_plot(&self, spec: &Plot, data: &HashMap) -> Result; + + /// Check whether a plot can be rendered by this writer, without rendering it + fn validate_plot(&self, spec: &Plot) -> Result<()>; - /// Check whether a spec can be rendered by this writer, without rendering it - fn validate(&self, spec: &Plot) -> Result<()>; + /// Render a resolved table and its body data. Defaults to an "unsupported" + /// error; only `HtmlWriter` overrides it as of this writing. + fn write_table(&self, table: &Table, body: &DataFrame) -> Result { .. } - /// Render a prepared `Spec` from `reader.execute()` — the usual entry point - fn render(&self, spec: &Spec) -> Result; + /// Render a `ResolvedSpec` from `reader.execute()` — the usual entry point. + /// Dispatches to `write_plot`/`write_table` depending on the variant. + fn render(&self, spec: &ResolvedSpec) -> Result; } ``` diff --git a/src/execute/cte.rs b/src/execute/cte.rs index 65b343681..85ae80317 100644 --- a/src/execute/cte.rs +++ b/src/execute/cte.rs @@ -463,7 +463,7 @@ pub fn transform_global_sql( let viz_from_query = source_tree .find_text( &root, - r#"(visualise_statement (visualise_from source: (_) @source))"#, + r#"(visualise_statement (single_source_from source: (_) @source))"#, ) .map(|table| { let q = format!("SELECT * FROM {}", table); @@ -520,7 +520,7 @@ pub fn has_executable_sql(source_tree: &SourceTree) -> bool { // Check for VISUALISE FROM (which injects SELECT * FROM ) let visualise_from = r#" (visualise_statement - (visualise_from) @from) + (single_source_from) @from) "#; if source_tree.find_node(&root, visualise_from).is_some() { return true; @@ -738,7 +738,7 @@ mod tests { fn register(&self, _name: &str, _df: crate::DataFrame, _replace: bool) -> Result<()> { Ok(()) } - fn execute(&self, _query: &str) -> Result { + fn execute(&self, _query: &str) -> Result { unreachable!() } fn caches_sources(&self) -> bool { diff --git a/src/execute/mod.rs b/src/execute/mod.rs index 9f633d10f..9ada5a9a1 100644 --- a/src/execute/mod.rs +++ b/src/execute/mod.rs @@ -9,6 +9,7 @@ //! - `casting`: Type requirements determination and casting logic //! - `layer`: Layer query building, data transforms, and stat application //! - `scale`: Scale creation, resolution, type coercion, and OOB handling +//! - `table`: Table (TABULATE) resolution mod casting; mod cte; @@ -16,11 +17,13 @@ mod layer; mod position; mod scale; mod schema; +mod table; // Re-export public API pub use casting::TypeRequirement; pub use cte::CteDefinition; pub use schema::TypeInfo; +pub use table::resolve_table_with_reader; use crate::naming; use crate::parser; @@ -29,7 +32,7 @@ use crate::plot::facet::{resolve_properties as resolve_facet_properties, FacetDa use crate::plot::layer::is_transposed; use crate::plot::projection::resolve_projection_properties; use crate::plot::{AestheticValue, Layer, Scale, ScaleTypeKind, Schema}; -use crate::{DataFrame, DataSource, GgsqlError, Plot, Result}; +use crate::{DataFrame, DataSource, GgsqlError, Plot, Result, Spec}; use std::collections::{HashMap, HashSet}; use crate::reader::Reader; @@ -1090,6 +1093,19 @@ pub struct PreparedData { pub visual: String, } +/// Execute setup statements (INSTALL, LOAD, SET, etc.) ahead of the main +/// query. Shared by the Plot and Table pipelines (`prepare_data_with_reader` +/// and `table::resolve_table_with_reader`). Structured DML (CREATE, INSERT, +/// UPDATE, DELETE) is out of scope here — see `cte::extract_side_effects`, +/// which only the Plot pipeline currently runs. +fn execute_setup_statements(source_tree: &parser::SourceTree, reader: &dyn Reader) -> Result<()> { + let root = source_tree.root(); + for stmt in source_tree.find_texts(&root, "(sql_statement (other_sql_statement) @stmt)") { + reader.execute_sql(&stmt)?; + } + Ok(()) +} + /// Build data map from a query using a Reader /// /// This is the main entry point for preparing visualization data from a ggsql query. @@ -1108,7 +1124,10 @@ pub fn prepare_data_with_reader(query: &str, reader: &dyn Reader) -> Result Result = parser::build_ast(&source_tree)? + .into_iter() + .filter_map(Spec::into_plot) + .collect(); if specs.is_empty() { return Err(GgsqlError::ValidationError( @@ -1128,12 +1152,7 @@ pub fn prepare_data_with_reader(query: &str, reader: &dyn Reader) -> Result Result assert!(e + .to_string() + .contains("No visualization specifications found")), + Ok(_) => panic!("expected an error for a TABULATE-only query"), + } + } + #[cfg(feature = "duckdb")] #[test] fn test_prepare_data_layer_source() { diff --git a/src/execute/table.rs b/src/execute/table.rs new file mode 100644 index 000000000..8c6ebb3fa --- /dev/null +++ b/src/execute/table.rs @@ -0,0 +1,115 @@ +//! Table resolution: turns a TABULATE query + Reader into a ResolvedTable. +//! +//! A Table has no layers, so there's no per-layer CTE materialization, scale +//! resolution, or facet handling to do here — just the one query that +//! produces `body`. + +use crate::parser::{self, SourceTree}; +use crate::reader::{Reader, ResolvedTable}; +use crate::validate::{validate, ValidationWarning}; +use crate::{GgsqlError, Result, Spec}; + +/// Resolve a TABULATE query into a `ResolvedTable`. +/// +/// This is the Table-side substitute for *two* Plot-side functions combined: +/// `execute::prepare_data_with_reader` (parses, resolves layers/scales/facets, +/// returns the intermediate `PreparedData`) and `reader::resolve_plot_with_reader` +/// (takes the first `Plot` from that, wraps it into `ResolvedPlot`). Table +/// collapses both into one function because there's no per-layer/scale/facet +/// resolution step for a `PreparedTable`-equivalent to do — `ResolvedTable` +/// already holds everything this function produces. +/// +/// Takes the *first* `Table` spec found in the query (mirroring how Plot +/// execution takes the first `Plot` spec) — a query with several TABULATE +/// statements, or a mix of VISUALISE and TABULATE, isn't disambiguated any +/// further than that yet. +/// +/// Setup statements (INSTALL, LOAD, SET, etc.) ahead of a TABULATE are +/// executed here too, via the same `execute_setup_statements` helper +/// `prepare_data_with_reader` uses — structured DML (CREATE, INSERT, UPDATE, +/// DELETE) ahead of a TABULATE isn't handled, since there's no CTE/side-effect +/// extraction step in this pipeline to mirror `prepare_data_with_reader`'s use +/// of `cte::extract_side_effects`. +pub fn resolve_table_with_reader(query: &str, reader: &dyn Reader) -> Result { + let validated = validate(query)?; + let warnings: Vec = validated.warnings().to_vec(); + + let source_tree = SourceTree::new(query)?; + source_tree.validate()?; + + let table = parser::build_ast(&source_tree)? + .into_iter() + .find_map(Spec::into_table) + .ok_or_else(|| GgsqlError::ValidationError("No table specification found".to_string()))?; + + super::execute_setup_statements(&source_tree, reader)?; + + let sql = source_tree.extract_sql().ok_or_else(|| { + GgsqlError::ValidationError( + "TABULATE has no data source: add a FROM, or a SQL query before it".to_string(), + ) + })?; + + let body = reader.execute_sql(&sql)?; + + Ok(ResolvedTable::new(table, body, sql, warnings)) +} + +#[cfg(test)] +#[cfg(feature = "duckdb")] +mod tests { + use super::*; + use crate::reader::DuckDBReader; + + fn reader_with_sales() -> DuckDBReader { + let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); + reader + .execute_sql("CREATE TABLE sales AS SELECT * FROM (VALUES (1, 'a'), (2, 'b'), (3, 'c')) AS t(id, name)") + .unwrap(); + reader + } + + #[test] + fn test_tabulate_from() { + let reader = reader_with_sales(); + let resolved = resolve_table_with_reader("TABULATE FROM sales", &reader).unwrap(); + + assert_eq!(resolved.sql(), "SELECT * FROM sales"); + assert_eq!(resolved.body().height(), 3); + assert_eq!(resolved.body().width(), 2); + } + + #[test] + fn test_bare_tabulate_uses_preceding_select() { + let reader = reader_with_sales(); + + let from_only = resolve_table_with_reader("TABULATE FROM sales", &reader).unwrap(); + let select_then_tabulate = + resolve_table_with_reader("SELECT * FROM sales TABULATE", &reader).unwrap(); + + assert_eq!(from_only.sql(), select_then_tabulate.sql()); + assert_eq!( + from_only.body().height(), + select_then_tabulate.body().height() + ); + } + + #[test] + fn test_tabulate_with_no_source_errors() { + let reader = reader_with_sales(); + let result = resolve_table_with_reader("TABULATE", &reader); + assert!(result.is_err()); + } + + #[test] + fn test_tabulate_does_not_borrow_a_later_visualise_from() { + // A source-less TABULATE followed by an unrelated VISUALISE FROM must + // still error "no data source", not silently resolve against the + // VISUALISE's FROM — regression for a bug where extract_sql matched + // any statement's FROM in the whole query, not just the one being + // resolved. + let reader = reader_with_sales(); + let result = resolve_table_with_reader("TABULATE VISUALISE FROM sales DRAW point", &reader); + assert!(result.is_err()); + } +} diff --git a/src/lib.rs b/src/lib.rs index 44554ef00..99f11a165 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -42,6 +42,8 @@ pub mod format; pub mod naming; pub mod parser; pub mod plot; +pub mod spec; +pub mod table; pub mod util; pub mod reader; @@ -68,6 +70,10 @@ pub use plot::{ SqlExpression, }; +// Re-export the parse-time Plot/Table result and the Table stub +pub use spec::Spec; +pub use table::Table; + // Re-export aesthetic classification utilities pub use plot::aesthetic::{ is_position_aesthetic, AestheticContext, MATERIAL_AESTHETICS, POSITION_SUFFIXES, @@ -169,7 +175,7 @@ mod integration_tests { // Generate Vega-Lite JSON let writer = VegaLiteWriter::new(); - let json_str = writer.write(&spec, &wrap_data(df)).unwrap(); + let json_str = writer.write_plot(&spec, &wrap_data(df)).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); // CRITICAL ASSERTION: x-axis should be automatically inferred as "temporal" @@ -229,7 +235,7 @@ mod integration_tests { // Generate Vega-Lite JSON let writer = VegaLiteWriter::new(); - let json_str = writer.write(&spec, &wrap_data(df)).unwrap(); + let json_str = writer.write_plot(&spec, &wrap_data(df)).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); // x-axis should be automatically inferred as "temporal" @@ -287,7 +293,7 @@ mod integration_tests { // Generate Vega-Lite JSON let writer = VegaLiteWriter::new(); - let json_str = writer.write(&spec, &wrap_data(df)).unwrap(); + let json_str = writer.write_plot(&spec, &wrap_data(df)).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); // Types should be inferred as quantitative @@ -342,7 +348,7 @@ mod integration_tests { spec.transform_aesthetics_to_internal(); let writer = VegaLiteWriter::new(); - let json_str = writer.write(&spec, &wrap_data(df)).unwrap(); + let json_str = writer.write_plot(&spec, &wrap_data(df)).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); // Check null handling in JSON @@ -379,7 +385,7 @@ mod integration_tests { spec.transform_aesthetics_to_internal(); let writer = VegaLiteWriter::new(); - let json_str = writer.write(&spec, &wrap_data(df)).unwrap(); + let json_str = writer.write_plot(&spec, &wrap_data(df)).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); // String columns should be inferred as nominal @@ -437,7 +443,7 @@ mod integration_tests { spec.transform_aesthetics_to_internal(); let writer = VegaLiteWriter::new(); - let json_str = writer.write(&spec, &wrap_data(df)).unwrap(); + let json_str = writer.write_plot(&spec, &wrap_data(df)).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); // x-axis should be temporal @@ -485,7 +491,7 @@ mod integration_tests { spec.transform_aesthetics_to_internal(); let writer = VegaLiteWriter::new(); - let json_str = writer.write(&spec, &wrap_data(df)).unwrap(); + let json_str = writer.write_plot(&spec, &wrap_data(df)).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); // Check values are preserved @@ -543,7 +549,7 @@ mod integration_tests { spec.transform_aesthetics_to_internal(); let writer = VegaLiteWriter::new(); - let json_str = writer.write(&spec, &wrap_data(df)).unwrap(); + let json_str = writer.write_plot(&spec, &wrap_data(df)).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); // All integer types should be quantitative @@ -612,7 +618,9 @@ mod integration_tests { // Generate Vega-Lite let writer = VegaLiteWriter::new(); - let json_str = writer.write(&prepared.specs[0], &prepared.data).unwrap(); + let json_str = writer + .write_plot(&prepared.specs[0], &prepared.data) + .unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); // Verify we have two layers @@ -770,7 +778,7 @@ mod integration_tests { // Verify the spec has the facet configuration assert!( prepared.specs[0].facet.is_some(), - "Spec should have facet configuration" + "ResolvedPlot should have facet configuration" ); } @@ -793,7 +801,9 @@ mod integration_tests { // Render to Vega-Lite let writer = VegaLiteWriter::new(); - let json_str = writer.write(&prepared.specs[0], &prepared.data).unwrap(); + let json_str = writer + .write_plot(&prepared.specs[0], &prepared.data) + .unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); // Find the point annotation layer (should be second layer) @@ -912,7 +922,9 @@ mod integration_tests { // Generate Vega-Lite and verify it works let writer = VegaLiteWriter::new(); - let json_str = writer.write(&prepared.specs[0], &prepared.data).unwrap(); + let json_str = writer + .write_plot(&prepared.specs[0], &prepared.data) + .unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); // Both layers should have stroke field-mapped to prefixed aesthetic-named column @@ -1035,7 +1047,9 @@ mod integration_tests { let prepared = execute::prepare_data_with_reader(query, &reader).unwrap(); let writer = VegaLiteWriter::new(); - let json_str = writer.write(&prepared.specs[0], &prepared.data).unwrap(); + let json_str = writer + .write_plot(&prepared.specs[0], &prepared.data) + .unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); let layers = vl_spec["layer"].as_array().unwrap(); @@ -1071,7 +1085,9 @@ mod integration_tests { let prepared = execute::prepare_data_with_reader(query, &reader).unwrap(); let writer = VegaLiteWriter::new(); - let json_str = writer.write(&prepared.specs[0], &prepared.data).unwrap(); + let json_str = writer + .write_plot(&prepared.specs[0], &prepared.data) + .unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); assert_eq!(vl_spec["layer"][0]["mark"]["type"], "geoshape"); @@ -1118,7 +1134,9 @@ mod integration_tests { let prepared = execute::prepare_data_with_reader(&query, &reader).unwrap(); let writer = VegaLiteWriter::new(); - let json_str = writer.write(&prepared.specs[0], &prepared.data).unwrap(); + let json_str = writer + .write_plot(&prepared.specs[0], &prepared.data) + .unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); let data = vl_spec["data"]["values"].as_array().unwrap(); @@ -1246,7 +1264,9 @@ mod integration_tests { let prepared = execute::prepare_data_with_reader(query, &reader).unwrap(); let writer = VegaLiteWriter::new(); - let json_str = writer.write(&prepared.specs[0], &prepared.data).unwrap(); + let json_str = writer + .write_plot(&prepared.specs[0], &prepared.data) + .unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); let data = vl_spec["data"]["values"].as_array().unwrap(); @@ -1278,7 +1298,9 @@ mod integration_tests { let prepared = execute::prepare_data_with_reader(query, &reader).unwrap(); let writer = VegaLiteWriter::new(); - let json_str = writer.write(&prepared.specs[0], &prepared.data).unwrap(); + let json_str = writer + .write_plot(&prepared.specs[0], &prepared.data) + .unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); let data = vl_spec["data"]["values"].as_array().unwrap(); diff --git a/src/parser/builder.rs b/src/parser/builder.rs index d2f44e735..ea1aa53dd 100644 --- a/src/parser/builder.rs +++ b/src/parser/builder.rs @@ -8,7 +8,7 @@ use crate::plot::layer::geom::Geom; use crate::plot::projection::resolve_coord; use crate::plot::scale::{color_to_hex, is_color_aesthetic, is_user_facet_aesthetic, Transform}; use crate::plot::*; -use crate::{GgsqlError, Result}; +use crate::{GgsqlError, Result, Spec, Table}; use std::collections::HashMap; use tree_sitter::Node; @@ -194,7 +194,7 @@ fn parse_literal_value(node: &Node, source: &SourceTree) -> Result Result> { +pub fn build_ast(source: &SourceTree) -> Result> { let root = source.root(); // Check if root is a query node @@ -216,22 +216,51 @@ pub fn build_ast(source: &SourceTree) -> Result> { false }; - // Find all visualise_statement nodes - let query = "(visualise_statement) @viz"; - let viz_nodes = source.find_nodes(&root, query); + // A single alternation query visits the tree once and yields + // visualise_statement/tabulate_statement nodes already in document order, + // so they arrive correctly interleaved (e.g. `VISUALISE ... TABULATE ...`). + let stmt_nodes = source.find_nodes( + &root, + r#" + [ + (visualise_statement) @stmt + (tabulate_statement) @stmt + ] + "#, + ); let mut specs = Vec::new(); - for viz_node in viz_nodes { - let spec = build_visualise_statement(&viz_node, source)?; - - // Validate VISUALISE FROM usage - if spec.source.is_some() && last_is_select { - return Err(GgsqlError::ParseError( - "Cannot use VISUALISE FROM when the last SQL statement is SELECT. \ - Use either 'SELECT ... VISUALISE' or remove the SELECT and use \ - 'VISUALISE FROM ...'." - .to_string(), - )); + for stmt_node in stmt_nodes { + // Build the spec, then check the shared "FROM after a trailing + // SELECT" restriction once for whichever kind it is — VISUALISE FROM + // and TABULATE FROM both forbid it, differing only in keyword. + let (has_from, keyword, spec) = match stmt_node.kind() { + "visualise_statement" => { + let plot = build_visualise_statement(&stmt_node, source)?; + ( + plot.source.is_some(), + "VISUALISE", + Spec::Plot(Box::new(plot)), + ) + } + "tabulate_statement" => { + let table = build_tabulate_statement(&stmt_node, source); + (table.source.is_some(), "TABULATE", Spec::Table(table)) + } + other => { + return Err(GgsqlError::InternalError(format!( + "Unexpected top-level statement kind: '{}'", + other + ))); + } + }; + + if has_from && last_is_select { + return Err(GgsqlError::ParseError(format!( + "Cannot use {keyword} FROM when the last SQL statement is SELECT. \ + Use either 'SELECT ... {keyword}' or remove the SELECT and use \ + '{keyword} FROM ...'." + ))); } specs.push(spec); @@ -239,7 +268,7 @@ pub fn build_ast(source: &SourceTree) -> Result> { if specs.is_empty() { return Err(GgsqlError::ParseError( - "No VISUALISE statements found in query".to_string(), + "No VISUALISE or TABULATE statements found in query".to_string(), )); } @@ -266,7 +295,7 @@ fn build_visualise_statement(node: &Node, source: &SourceTree) -> Result { // Handle standalone wildcard (*) mapping spec.global_mappings.wildcard = true; } - "visualise_from" => { + "single_source_from" => { if let Some(source_node) = child.child_by_field_name("source") { spec.source = Some(parse_data_source(&source_node, source)); } @@ -313,6 +342,22 @@ fn build_visualise_statement(node: &Node, source: &SourceTree) -> Result { Ok(spec) } +/// Build a single Table from a tabulate_statement node +fn build_tabulate_statement(node: &Node, source: &SourceTree) -> Table { + let mut table = Table::new(); + + let mut cursor = node.walk(); + for child in node.children(&mut cursor) { + if child.kind() == "single_source_from" { + if let Some(source_node) = child.child_by_field_name("source") { + table.source = Some(parse_data_source(&source_node, source)); + } + } + } + + table +} + /// Process a visualization clause node fn process_viz_clause(node: &Node, source: &SourceTree, spec: &mut Plot) -> Result<()> { let mut cursor = node.walk(); @@ -1216,9 +1261,67 @@ mod tests { let source = SourceTree::new(query)?; source.validate()?; + Ok(build_ast(&source)? + .into_iter() + .filter_map(Spec::into_plot) + .collect()) + } + + /// Like `parse_test_query`, but keeps `Table` specs instead of filtering + /// them out — for tests that need to see the raw `Spec` variants. + fn parse_test_specs(query: &str) -> Result> { + let source = SourceTree::new(query)?; + source.validate()?; build_ast(&source) } + // ======================================== + // TABULATE Tests + // ======================================== + + #[test] + fn test_tabulate_bare() { + let specs = parse_test_specs("SELECT 1 TABULATE").unwrap(); + assert_eq!(specs.len(), 1); + let table = specs[0].as_table().expect("expected a Table spec"); + assert!(table.source.is_none()); + } + + #[test] + fn test_tabulate_from() { + let specs = parse_test_specs("TABULATE FROM sales").unwrap(); + assert_eq!(specs.len(), 1); + let table = specs[0].as_table().expect("expected a Table spec"); + assert!(matches!(table.source, Some(DataSource::Identifier(ref name)) if name == "sales")); + } + + #[test] + fn test_tabulate_from_file_path() { + let specs = parse_test_specs("TABULATE FROM 'data.csv'").unwrap(); + assert_eq!(specs.len(), 1); + let table = specs[0].as_table().expect("expected a Table spec"); + assert!(matches!(table.source, Some(DataSource::FilePath(ref path)) if path == "data.csv")); + } + + #[test] + fn test_tabulate_from_after_select_errors() { + // Mirrors VISUALISE FROM's own "last statement is SELECT" restriction. + let result = parse_test_specs("SELECT 1 TABULATE FROM sales"); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("Cannot use TABULATE FROM")); + } + + #[test] + fn test_visualise_then_tabulate_interleaved() { + let specs = parse_test_specs("SELECT 1 AS x VISUALISE x DRAW point TABULATE").unwrap(); + assert_eq!(specs.len(), 2); + assert!(matches!(specs[0], Spec::Plot(_))); + assert!(matches!(specs[1], Spec::Table(_))); + } + // ======================================== // PROJECT Property Validation Tests // ======================================== @@ -3508,7 +3611,7 @@ mod tests { let source = make_source("VISUALISE FROM sales DRAW bar"); let root = source.root(); - let query = "(visualise_from source: (_) @source)"; + let query = "(single_source_from source: (_) @source)"; let from_node = source.find_node(&root, query).unwrap(); let parsed = parse_data_source(&from_node, &source); diff --git a/src/parser/mod.rs b/src/parser/mod.rs index a9171c64f..15bab19db 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -19,7 +19,7 @@ the visualization specification into a typed AST. ```rust # use ggsql::parser::parse_query; -# use ggsql::Geom; +# use ggsql::{Geom, Spec}; # fn main() -> Result<(), Box> { let query = r#" SELECT date, revenue, region FROM sales WHERE year = 2024 @@ -31,14 +31,17 @@ let query = r#" let specs = parse_query(query)?; assert_eq!(specs.len(), 1); -assert_eq!(specs[0].layers.len(), 1); -assert_eq!(specs[0].layers[0].geom, Geom::line()); +let Spec::Plot(plot) = &specs[0] else { + panic!("expected a Plot"); +}; +assert_eq!(plot.layers.len(), 1); +assert_eq!(plot.layers[0].geom, Geom::line()); # Ok(()) # } ``` */ -use crate::{Plot, Result}; +use crate::{Result, Spec}; pub mod builder; pub mod source_tree; @@ -55,7 +58,7 @@ pub use sql::{ /// /// Takes a complete ggsql query (SQL + VISUALISE) and returns a vector of /// parsed specifications (one per VISUALISE statement). -pub fn parse_query(query: &str) -> Result> { +pub fn parse_query(query: &str) -> Result> { // Parse the full query and create SourceTree let source_tree = SourceTree::new(query)?; @@ -72,7 +75,17 @@ pub fn parse_query(query: &str) -> Result> { mod tests { use super::*; use crate::plot::ParameterValue; - use crate::{AestheticValue, DataSource, Geom}; + use crate::{AestheticValue, DataSource, Geom, Plot}; + + /// Test helper: `parse_query`, then unwrap every result down to its `Plot`. + /// These tests predate the `Table` variant and only ever exercise VISUALISE + /// queries, so every `Spec` here is a `Spec::Plot`. + fn parse_query_plots(query: &str) -> Result> { + Ok(parse_query(query)? + .into_iter() + .filter_map(Spec::into_plot) + .collect()) + } #[test] fn test_simple_query_parsing() { @@ -82,7 +95,7 @@ mod tests { DRAW point "#; - let result = parse_query(query); + let result = parse_query_plots(query); assert!(result.is_ok(), "Failed to parse simple query: {:?}", result); let specs = result.unwrap(); @@ -115,7 +128,7 @@ mod tests { DRAW point MAPPING z AS y, 'value' AS color "#; - let specs = parse_query(query).unwrap(); + let specs = parse_query_plots(query).unwrap(); assert_eq!(specs.len(), 1); assert_eq!(specs[0].layers.len(), 2); // First layer is line, second layer is point @@ -140,7 +153,7 @@ mod tests { DRAW bar MAPPING x AS x, y AS y "#; - let specs = parse_query(query).unwrap(); + let specs = parse_query_plots(query).unwrap(); assert_eq!(specs.len(), 2); assert_eq!(specs[0].layers.len(), 1); assert_eq!(specs[1].layers.len(), 1); @@ -154,7 +167,7 @@ mod tests { DRAW point "#; - let specs = parse_query(query).unwrap(); + let specs = parse_query_plots(query).unwrap(); assert_eq!(specs.len(), 1); assert_eq!(specs[0].layers[0].geom, Geom::point()); } @@ -171,7 +184,7 @@ mod tests { DRAW point "#; - let specs = parse_query(query).unwrap(); + let specs = parse_query_plots(query).unwrap(); assert_eq!(specs.len(), 3); assert_eq!(specs[0].layers.len(), 1); assert_eq!(specs[1].layers.len(), 1); @@ -186,7 +199,7 @@ mod tests { DRAW point MAPPING x AS x, y AS y "#; - let specs = parse_query(query).unwrap(); + let specs = parse_query_plots(query).unwrap(); assert_eq!(specs.len(), 1); assert!(specs[0].global_mappings.is_empty()); } @@ -202,7 +215,7 @@ mod tests { DRAW bar MAPPING x AS x, y AS y "#; - let specs = parse_query(query).unwrap(); + let specs = parse_query_plots(query).unwrap(); assert_eq!(specs.len(), 2); // First viz should have layers and labels @@ -226,7 +239,7 @@ mod tests { DRAW bar MAPPING x AS x, y AS y "#; - let specs = parse_query(query).unwrap(); + let specs = parse_query_plots(query).unwrap(); assert_eq!(specs.len(), 3); assert_eq!(specs[0].layers[0].geom, Geom::line()); assert_eq!(specs[1].layers[0].geom, Geom::point()); @@ -249,7 +262,7 @@ mod tests { DRAW point MAPPING date AS x, revenue AS y "#; - let specs = parse_query(query).unwrap(); + let specs = parse_query_plots(query).unwrap(); assert_eq!(specs.len(), 3); // Plot with 2 layers, scale, labels @@ -288,7 +301,7 @@ mod tests { source_tree ); - let specs = parse_query(query).unwrap(); + let specs = parse_query_plots(query).unwrap(); assert_eq!(specs.len(), 1); } @@ -300,7 +313,7 @@ mod tests { DRAW point "#; - let specs = parse_query(query).unwrap(); + let specs = parse_query_plots(query).unwrap(); assert_eq!(specs.len(), 1); assert!(specs[0].global_mappings.wildcard); assert!(specs[0].global_mappings.aesthetics.is_empty()); @@ -313,7 +326,7 @@ mod tests { DRAW line "#; - let specs = parse_query(query).unwrap(); + let specs = parse_query_plots(query).unwrap(); assert_eq!(specs.len(), 1); let mapping = &specs[0].global_mappings; assert!(!mapping.wildcard); @@ -339,7 +352,7 @@ mod tests { DRAW point "#; - let specs = parse_query(query).unwrap(); + let specs = parse_query_plots(query).unwrap(); assert_eq!(specs.len(), 1); let mapping = &specs[0].global_mappings; assert!(!mapping.wildcard); @@ -363,7 +376,7 @@ mod tests { DRAW point "#; - let specs = parse_query(query).unwrap(); + let specs = parse_query_plots(query).unwrap(); assert_eq!(specs.len(), 1); let mapping = &specs[0].global_mappings; assert!(!mapping.wildcard); @@ -390,7 +403,7 @@ mod tests { DRAW bar "#; - let specs = parse_query(query).unwrap(); + let specs = parse_query_plots(query).unwrap(); assert_eq!(specs.len(), 1); assert_eq!( specs[0].source, @@ -406,7 +419,7 @@ mod tests { DRAW point "#; - let specs = parse_query(query).unwrap(); + let specs = parse_query_plots(query).unwrap(); assert_eq!(specs.len(), 1); assert_eq!( specs[0].source, @@ -423,7 +436,7 @@ mod tests { PLACE text SETTING x => 5, y => 10, label => 'Hello' "#; - let result = parse_query(query); + let result = parse_query_plots(query); assert!(result.is_ok(), "Failed to parse PLACE clause: {:?}", result); let specs = result.unwrap(); diff --git a/src/parser/source_tree.rs b/src/parser/source_tree.rs index 32109f9f5..99f7edd6b 100644 --- a/src/parser/source_tree.rs +++ b/src/parser/source_tree.rs @@ -49,12 +49,13 @@ impl<'a> SourceTree<'a> { let has_sql_from = self .find_node(&root, "(sql_statement (from_statement) @stmt)") .is_some(); - let has_viz_from = self - .find_node(&root, "(visualise_statement (visualise_from) @t)") - .is_some(); - if has_sql_from && has_viz_from { + // single_source_from is shared by visualise_statement and + // tabulate_statement, so this covers both `FROM a VISUALISE FROM b` + // and `FROM a TABULATE FROM b` in one check. + let has_stmt_from = self.find_node(&root, "(single_source_from) @t").is_some(); + if has_sql_from && has_stmt_from { return Err(GgsqlError::ParseError( - "VISUALISE has two FROM clauses (one before VISUALISE and one after). \ + "Query has two FROM clauses (one before VISUALISE/TABULATE and one after). \ Use only one." .to_string(), )); @@ -133,7 +134,28 @@ impl<'a> SourceTree<'a> { .collect() } - /// Extract the SQL portion of the query (before VISUALISE). + /// The first VISUALISE or TABULATE statement node, whichever comes first. + /// `None` if the query has neither. + /// + /// This is the statement `Reader::execute()`'s dispatch actually resolves + /// — it always acts on the first top-level `Spec`, of whichever kind — so + /// it is also the only statement `extract_sql`'s FROM-injection should + /// ever look inside. + fn first_stmt<'b>(&self, root: &Node<'b>) -> Option> { + let viz = self.find_node(root, "(visualise_statement) @viz"); + let tab = self.find_node(root, "(tabulate_statement) @tab"); + match (viz, tab) { + (Some(a), Some(b)) => Some(if a.start_byte() <= b.start_byte() { + a + } else { + b + }), + (Some(a), None) | (None, Some(a)) => Some(a), + (None, None) => None, + } + } + + /// Extract the SQL portion of the query (before VISUALISE/TABULATE). /// /// Two rewrites happen here so the returned SQL is always something a /// plain SQL reader can execute: @@ -142,25 +164,27 @@ impl<'a> SourceTree<'a> { /// `from_statement`. Each such statement is rewritten by prepending /// `SELECT * ` — so `FROM sales VISUALISE …` becomes /// `SELECT * FROM sales`. - /// - `VISUALISE FROM `: the FROM appears on the VISUALISE clause. - /// We append `SELECT * FROM ` to the SQL so the reader sees an - /// executable query. + /// - `VISUALISE FROM ` / `TABULATE FROM `: the FROM + /// appears on the statement itself rather than as SQL. We append + /// `SELECT * FROM ` to the SQL so the reader sees an + /// executable query. Plots and tables share this extraction exactly — + /// the only difference between them is that a table has no per-layer + /// granular sources to also account for. /// - /// Returns `None` if there's no SQL portion and no VISUALISE FROM to - /// inject. The ambiguous double-FROM case (`FROM a VISUALISE FROM b …`) - /// is rejected in `SourceTree::new`, so any tree reaching here has at - /// most one of the two FROMs. + /// Returns `None` if there's no SQL portion and no FROM (on either + /// VISUALISE or TABULATE) to inject. The ambiguous double-FROM case + /// (`FROM a VISUALISE FROM b …`) is rejected in `SourceTree::new`, so any + /// tree reaching here has at most one of the two FROMs. pub fn extract_sql(&self) -> Option { let root = self.root(); - // Check if there's any VISUALISE statement - if self - .find_node(&root, "(visualise_statement) @viz") - .is_none() - { - // No VISUALISE at all - return entire source as SQL + // The statement `Reader::execute()` will actually resolve — the only + // one a FROM belonging to some *other*, later statement should ever + // be allowed to feed into. + let Some(first_stmt) = self.first_stmt(&root) else { + // Neither VISUALISE nor TABULATE at all - return entire source as SQL return Some(self.source.to_string()); - } + }; // Find sql_portion node and extract its text let sql_portion_node = self.find_node(&root, "(sql_portion) @sql"); @@ -191,17 +215,31 @@ impl<'a> SourceTree<'a> { } } - // VISUALISE FROM : append "SELECT * FROM ". - let viz_from = self.find_text( - &root, + // VISUALISE FROM / TABULATE FROM : append + // "SELECT * FROM ". Explicitly anchored to both statement + // kinds (rather than a bare `(single_source_from …)`) so this can't + // silently start matching some unrelated future use of + // single_source_from — today it's only ever a child of one of these + // two, but this doesn't rely on that staying true. + // + // Scoped to `first_stmt` specifically, not `root`: a query can have + // several VISUALISE/TABULATE statements (interleaved, even), and only + // the first one is ever actually resolved. Searching the whole tree + // here would let a *later* statement's FROM leak into the one being + // resolved — e.g. `TABULATE VISUALISE FROM sales DRAW point` would + // silently pick up the VISUALISE's `FROM sales` for the source-less + // TABULATE instead of correctly reporting no data source. + let stmt_from = self.find_text( + &first_stmt, r#" - (visualise_statement - (visualise_from - source: (_) @source)) + [ + (visualise_statement (single_source_from source: (_) @source)) + (tabulate_statement (single_source_from source: (_) @source)) + ] "#, ); - if let Some(from_identifier) = viz_from { + if let Some(from_identifier) = stmt_from { let result = if sql_text.trim().is_empty() { format!("SELECT * FROM {}", from_identifier) } else { @@ -218,20 +256,16 @@ impl<'a> SourceTree<'a> { } } - /// Extract the VISUALISE portion of the query (from first VISUALISE onwards) - /// - /// Returns the raw text of all VISUALISE statements - pub fn extract_visualise(&self) -> Option { + /// Extract the `Spec` portion of the query — the VISUALISE/TABULATE + /// statement(s), from whichever comes first onwards. + pub fn extract_spec(&self) -> Option { let root = self.root(); - // Find byte offset of first VISUALISE - let viz_start = self - .find_node(&root, "(visualise_statement) @viz") - .map(|node| node.start_byte())?; + let spec_start = self.first_stmt(&root)?.start_byte(); - // Extract viz text from first VISUALISE onwards - let viz_text = &self.source[viz_start..]; - Some(viz_text.trim().to_string()) + // Extract spec text from first VISUALISE/TABULATE onwards + let spec_text = &self.source[spec_start..]; + Some(spec_text.trim().to_string()) } } @@ -247,7 +281,7 @@ mod tests { let sql = tree.extract_sql().unwrap(); assert_eq!(sql, "SELECT * FROM data"); - let viz = tree.extract_visualise().unwrap(); + let viz = tree.extract_spec().unwrap(); assert!(viz.starts_with("VISUALISE")); assert!(viz.contains("DRAW point")); } @@ -260,7 +294,7 @@ mod tests { let sql = tree.extract_sql().unwrap(); assert_eq!(sql, "SELECT * FROM data"); - let viz = tree.extract_visualise().unwrap(); + let viz = tree.extract_spec().unwrap(); assert!(viz.starts_with("visualise")); } @@ -284,7 +318,7 @@ mod tests { let sql = tree.extract_sql().unwrap(); assert_eq!(sql, query); - let viz = tree.extract_visualise(); + let viz = tree.extract_spec(); assert!(viz.is_none()); } @@ -297,10 +331,44 @@ mod tests { // Should inject SELECT * FROM mtcars assert_eq!(sql, "SELECT * FROM mtcars"); - let viz = tree.extract_visualise().unwrap(); + let viz = tree.extract_spec().unwrap(); assert!(viz.starts_with("VISUALISE FROM mtcars")); } + #[test] + fn test_extract_sql_tabulate_from_matches_bare_select() { + // TABULATE FROM and SELECT * FROM TABULATE should + // extract to the identical SQL, the same equivalence VISUALISE FROM + // already has with a bare SELECT. + let from_only = SourceTree::new("TABULATE FROM ggsql:penguins").unwrap(); + let select_then_tabulate = + SourceTree::new("SELECT * FROM ggsql:penguins TABULATE").unwrap(); + + assert_eq!( + from_only.extract_sql().unwrap(), + select_then_tabulate.extract_sql().unwrap() + ); + assert_eq!( + from_only.extract_sql().unwrap(), + "SELECT * FROM ggsql:penguins" + ); + } + + #[test] + fn test_extract_sql_does_not_borrow_a_later_statements_from() { + // A source-less TABULATE followed by an unrelated VISUALISE FROM must + // not pick up the VISUALISE's FROM — extract_sql is scoped to the + // first statement (the one Reader::execute() actually resolves), not + // the whole tree. + let tree = SourceTree::new("TABULATE VISUALISE FROM sales DRAW point").unwrap(); + assert_eq!(tree.extract_sql(), None); + + // Same in the other order: a source-less VISUALISE must not borrow a + // later TABULATE's FROM either. + let tree = SourceTree::new("VISUALISE DRAW point TABULATE FROM sales").unwrap(); + assert_eq!(tree.extract_sql(), None); + } + #[test] fn test_extract_sql_visualise_from_jinja_ref() { let query = "VISUALISE FROM {{ ref('fct_orders') }} DRAW point MAPPING x AS x, y AS y"; @@ -309,7 +377,7 @@ mod tests { let sql = tree.extract_sql().unwrap(); assert_eq!(sql, "SELECT * FROM {{ ref('fct_orders') }}"); - let viz = tree.extract_visualise().unwrap(); + let viz = tree.extract_spec().unwrap(); assert!(viz.starts_with("VISUALISE FROM {{ ref('fct_orders') }}")); } @@ -324,7 +392,7 @@ mod tests { assert!(sql.contains("WITH cte AS (SELECT * FROM x)")); assert!(sql.contains("SELECT * FROM cte")); - let viz = tree.extract_visualise().unwrap(); + let viz = tree.extract_spec().unwrap(); assert!(viz.starts_with("VISUALISE FROM cte")); } @@ -337,7 +405,7 @@ mod tests { assert!(sql.contains("CREATE TABLE x AS SELECT 1;")); assert!(sql.contains("SELECT * FROM x")); - let viz = tree.extract_visualise().unwrap(); + let viz = tree.extract_spec().unwrap(); assert!(viz.starts_with("VISUALISE FROM x")); // Without semicolon, the visualise statement should also be recognised @@ -348,7 +416,7 @@ mod tests { assert!(sql2.contains("CREATE TABLE x AS SELECT 1")); assert!(sql2.contains("SELECT * FROM x")); - let viz2 = tree2.extract_visualise().unwrap(); + let viz2 = tree2.extract_spec().unwrap(); assert!(viz2.starts_with("VISUALISE FROM x")); } @@ -360,7 +428,7 @@ mod tests { let sql = tree.extract_sql().unwrap(); assert!(sql.contains("INSERT")); - let viz = tree.extract_visualise().unwrap(); + let viz = tree.extract_spec().unwrap(); assert!(viz.contains("DRAW")); } @@ -387,7 +455,7 @@ mod tests { let sql = tree.extract_sql().unwrap(); assert!(sql.contains("SELECT * FROM mtcars")); - let viz = tree.extract_visualise().unwrap(); + let viz = tree.extract_spec().unwrap(); assert!(viz.starts_with("VISUALISE")); } @@ -537,7 +605,7 @@ mod tests { // Should inject SELECT * FROM 'mtcars.csv' with quotes preserved assert_eq!(sql, "SELECT * FROM 'mtcars.csv'"); - let viz = tree.extract_visualise().unwrap(); + let viz = tree.extract_spec().unwrap(); assert!(viz.starts_with("VISUALISE FROM 'mtcars.csv'")); } @@ -551,7 +619,7 @@ mod tests { // Should inject SELECT * FROM "data/sales.parquet" with quotes preserved assert_eq!(sql, r#"SELECT * FROM "data/sales.parquet""#); - let viz = tree.extract_visualise().unwrap(); + let viz = tree.extract_spec().unwrap(); assert!(viz.starts_with(r#"VISUALISE FROM "data/sales.parquet""#)); } diff --git a/src/plot/layer/geom/density.rs b/src/plot/layer/geom/density.rs index c81139b82..de84bb744 100644 --- a/src/plot/layer/geom/density.rs +++ b/src/plot/layer/geom/density.rs @@ -1082,12 +1082,16 @@ mod tests { // Debug: print what SQL was generated and what data we have println!("Generated stat SQL:"); - if let Some(sql) = spec.stat_sql(0) { + if let Some(sql) = spec.as_plot().unwrap().stat_sql(0) { println!("{}", sql); } // Get the stat-transformed data for layer 0 - let df = spec.stat_data(0).expect("Layer 0 should have stat data"); + let df = spec + .as_plot() + .unwrap() + .stat_data(0) + .expect("Layer 0 should have stat data"); println!("\nActual columns in stat_data: {:?}", df.get_column_names()); println!("Number of rows: {}", df.height()); diff --git a/src/reader/adbc.rs b/src/reader/adbc.rs index 203d31e26..6b0eabc55 100644 --- a/src/reader/adbc.rs +++ b/src/reader/adbc.rs @@ -282,7 +282,7 @@ where Ok(()) } - fn execute(&self, query: &str) -> Result { + fn execute(&self, query: &str) -> Result { crate::reader::execute_with_reader(self, query) } @@ -455,7 +455,7 @@ mod tests { DRAW line "#; let spec = reader.execute(query).expect("ggsql execute ok"); - let meta = spec.metadata(); + let meta = spec.as_plot().unwrap().metadata(); // Full pipeline verification: SQL executed (3 rows after WHERE), // VISUALISE parsed, plot resolved with 1 layer. assert_eq!(meta.rows, 3); diff --git a/src/reader/cache.rs b/src/reader/cache.rs index 411205cff..006bc0e76 100644 --- a/src/reader/cache.rs +++ b/src/reader/cache.rs @@ -12,7 +12,7 @@ //! - [`Reader::dialect`] returns the cache dialect. use crate::array_util::{as_i64, as_str}; -use crate::reader::{execute_with_reader, ColumnInfo, Reader, Spec, SqlDialect, TableInfo}; +use crate::reader::{execute_with_reader, ColumnInfo, Reader, ResolvedSpec, SqlDialect, TableInfo}; use crate::{naming, DataFrame, Result}; use arrow::array::Array; use std::cell::{Cell, RefCell}; @@ -456,7 +456,7 @@ impl Reader for CachingReader { Ok(()) } - fn execute(&self, query: &str) -> Result { + fn execute(&self, query: &str) -> Result { execute_with_reader(self, query) } diff --git a/src/reader/cache_equivalence.rs b/src/reader/cache_equivalence.rs index 789bae65e..382ceee80 100644 --- a/src/reader/cache_equivalence.rs +++ b/src/reader/cache_equivalence.rs @@ -130,6 +130,12 @@ fn assert_equivalent(plain: &dyn Reader, cached: &dyn Reader, query: &str) { b.as_ref().err(), ); let (Ok(sa), Ok(sb)) = (a, b) else { return }; + let sa = sa + .into_plot() + .expect("cache-equivalence corpus is VISUALISE-only"); + let sb = sb + .into_plot() + .expect("cache-equivalence corpus is VISUALISE-only"); assert_eq!( sa.layer_count(), @@ -272,7 +278,7 @@ mod adbc_mode { use super::*; use crate::reader::sqlite::SqliteDialect; use crate::reader::test_support::assert_dataframes_equal; - use crate::reader::{AdbcReader, Spec, SqlDialect}; + use crate::reader::{AdbcReader, ResolvedSpec, SqlDialect}; use crate::{DataFrame, Result}; use adbc_core::options::{AdbcVersion, OptionDatabase, OptionValue}; use adbc_core::LOAD_FLAG_DEFAULT; @@ -329,7 +335,7 @@ mod adbc_mode { fn unregister(&self, name: &str) -> Result<()> { self.inner.unregister(name) } - fn execute(&self, query: &str) -> Result { + fn execute(&self, query: &str) -> Result { crate::reader::execute_with_reader(self, query) } fn dialect(&self) -> &dyn SqlDialect { diff --git a/src/reader/data.rs b/src/reader/data.rs index 47ef711a1..26022a2df 100644 --- a/src/reader/data.rs +++ b/src/reader/data.rs @@ -214,6 +214,7 @@ mod duckdb_tests { ); } + #[cfg(feature = "vegalite")] #[test] fn test_ribbon_transposed_vegalite_encoding() { use crate::reader::Reader; diff --git a/src/reader/duckdb.rs b/src/reader/duckdb.rs index cde53e32e..94ab52b5c 100644 --- a/src/reader/duckdb.rs +++ b/src/reader/duckdb.rs @@ -503,7 +503,7 @@ impl Reader for DuckDBReader { Ok(()) } - fn execute(&self, query: &str) -> Result { + fn execute(&self, query: &str) -> Result { super::execute_with_reader(self, query) } @@ -836,8 +836,9 @@ mod tests { .execute("SELECT * FROM bar_data VISUALISE DRAW bar MAPPING category AS x") .unwrap(); - assert_eq!(spec.plot().layers.len(), 1); - assert!(spec.layer_data(0).is_some()); + let plot = spec.as_plot().unwrap(); + assert_eq!(plot.plot().layers.len(), 1); + assert!(plot.layer_data(0).is_some()); let writer = VegaLiteWriter::new(); let json = writer.render(&spec).unwrap(); @@ -864,8 +865,9 @@ mod tests { .execute("SELECT * FROM hist_data VISUALISE DRAW histogram MAPPING value AS x") .unwrap(); - assert_eq!(spec.plot().layers.len(), 1); - let layer_df = spec.layer_data(0).unwrap(); + let plot = spec.as_plot().unwrap(); + assert_eq!(plot.plot().layers.len(), 1); + let layer_df = plot.layer_data(0).unwrap(); assert!( layer_df.height() < 50, "Histogram should bin data: got {} rows", @@ -897,8 +899,9 @@ mod tests { .execute("SELECT * FROM density_data VISUALISE DRAW density MAPPING value AS x") .unwrap(); - assert_eq!(spec.plot().layers.len(), 1); - assert!(spec.layer_data(0).is_some()); + let plot = spec.as_plot().unwrap(); + assert_eq!(plot.plot().layers.len(), 1); + assert!(plot.layer_data(0).is_some()); let writer = VegaLiteWriter::new(); let json = writer.render(&spec).unwrap(); @@ -928,7 +931,7 @@ mod tests { .execute("SELECT * FROM box_data VISUALISE DRAW boxplot MAPPING grp AS x, value AS y") .unwrap(); - assert!(spec.layer_data(0).is_some()); + assert!(spec.as_plot().unwrap().layer_data(0).is_some()); let writer = VegaLiteWriter::new(); let json = writer.render(&spec).unwrap(); diff --git a/src/reader/mod.rs b/src/reader/mod.rs index aaa961a83..23891e6f7 100644 --- a/src/reader/mod.rs +++ b/src/reader/mod.rs @@ -7,7 +7,7 @@ //! //! All readers implement the `Reader` trait, which provides: //! - SQL query execution → DataFrame conversion -//! - Visualization query execution → Spec +//! - Visualization query execution → ResolvedPlot //! - Optional DataFrame registration for queryable tables //! - Connection management and error handling //! @@ -33,10 +33,11 @@ use std::collections::HashMap; -use crate::execute::prepare_data_with_reader; +use crate::execute::{prepare_data_with_reader, resolve_table_with_reader}; +use crate::parser::{self, SourceTree}; use crate::plot::{CastTargetType, Plot}; use crate::validate::{validate, ValidationWarning}; -use crate::{naming, DataFrame, GgsqlError, Result}; +use crate::{naming, DataFrame, GgsqlError, Result, Spec, Table}; // ============================================================================= // SQL Dialect @@ -502,7 +503,7 @@ pub(crate) fn returns_rows(sql: &str) -> bool { #[cfg(test)] pub(crate) mod test_support { use super::{ - execute_with_reader, returns_rows, ColumnInfo, Reader, Spec, SqlDialect, TableInfo, + execute_with_reader, returns_rows, ColumnInfo, Reader, ResolvedSpec, SqlDialect, TableInfo, }; use crate::{DataFrame, GgsqlError, Result}; use std::sync::{Arc, Mutex}; @@ -541,7 +542,7 @@ pub(crate) mod test_support { fn unregister(&self, name: &str) -> Result<()> { self.inner.unregister(name) } - fn execute(&self, query: &str) -> Result { + fn execute(&self, query: &str) -> Result { execute_with_reader(self, query) } fn dialect(&self) -> &dyn SqlDialect { @@ -591,7 +592,7 @@ pub(crate) mod test_support { fn unregister(&self, _name: &str) -> Result<()> { Err(Self::refuse("unregister")) } - fn execute(&self, query: &str) -> Result { + fn execute(&self, query: &str) -> Result { execute_with_reader(self, query) } fn dialect(&self) -> &dyn SqlDialect { @@ -670,11 +671,11 @@ pub(crate) mod test_support { } // ============================================================================ -// Spec - Result of reader.execute() +// ResolvedPlot - Result of reader.execute() // ============================================================================ /// Result of executing a ggsql query, ready for rendering. -pub struct Spec { +pub struct ResolvedPlot { /// Single resolved plot specification pub(crate) plot: Plot, /// Internal data map (global + layer-specific DataFrames) @@ -701,6 +702,43 @@ pub struct Metadata { pub layer_count: usize, } +// ============================================================================ +// ResolvedTable - Result of reader.execute() for a TABULATE statement +// ============================================================================ + +/// Result of executing a ggsql TABULATE query, ready for rendering. +pub struct ResolvedTable { + /// The resolved table specification + pub(crate) table: Table, + // PROVISIONAL, NOT A FINAL DESIGN DECISION: a plain `DataFrame` is enough + // to design the execution plumbing against, but this was never settled + // as the real representation. It will very likely need to become a + // table-specific intermediate representation once real table writers + // exist (e.g. an HTML/gt-style writer) and we know what they actually + // need `body` to carry. Don't build on this shape assuming it's final. + /// The data resolved from `table.source` (or the main SQL if there was no + /// TABULATE FROM) + pub(crate) body: DataFrame, + /// The SQL query that was executed to produce `body` + pub(crate) sql: String, + /// Validation warnings from preparation + pub(crate) warnings: Vec, +} + +// ============================================================================ +// ResolvedSpec - Result of reader.execute() +// ============================================================================ + +/// Result of executing a ggsql query: either a resolved plot or a resolved +/// table, mirroring the parse-time `Spec` (`Plot` or `Table`). +pub enum ResolvedSpec { + // Boxed for the same reason `Spec::Plot` is: `ResolvedPlot` is far larger + // than `ResolvedTable`, and clippy flags the resulting size gap + // (`large_enum_variant`) otherwise. + Plot(Box), + Table(ResolvedTable), +} + // ============================================================================ // Reader Trait // ============================================================================ @@ -794,24 +832,32 @@ pub trait Reader { ))) } - /// Execute a ggsql query and return the visualization specification. + /// Execute a ggsql query and return the resolved specification. + /// + /// This is the main entry point for creating visualizations or tables. + /// It parses the query, executes the SQL portion, and returns a + /// `ResolvedSpec` ready for rendering — either a `ResolvedPlot` (from a + /// `VISUALISE`) or a `ResolvedTable` (from a `TABULATE`). /// - /// This is the main entry point for creating visualizations. It parses the query, - /// executes the SQL portion, and returns a `Spec` ready for rendering. + /// No default body: implementations delegate to `execute_with_reader` + /// (each with a concrete, `Sized` `self`) so the trait stays object-safe + /// — a default method here would need to unsize `&Self` into + /// `&dyn Reader` to call that same free function, which requires + /// `Self: Sized` and would remove `execute` from `dyn Reader`'s vtable. /// /// # Arguments /// - /// * `query` - The ggsql query (SQL + VISUALISE clause) + /// * `query` - The ggsql query (SQL + VISUALISE/TABULATE clause) /// /// # Returns /// - /// A `Spec` containing the resolved visualization specification and data. + /// A `ResolvedSpec` containing the resolved plot or table. /// /// # Errors /// /// Returns an error if: /// - The query syntax is invalid - /// - The query has no VISUALISE clause + /// - The query has no VISUALISE/TABULATE clause /// - The SQL execution fails /// /// # Example @@ -826,7 +872,7 @@ pub trait Reader { /// let writer = VegaLiteWriter::new(); /// let json = writer.render(&spec)?; /// ``` - fn execute(&self, query: &str) -> Result; + fn execute(&self, query: &str) -> Result; /// Get the SQL dialect for this reader. /// @@ -962,12 +1008,11 @@ pub struct ColumnInfo { pub data_type: String, } -/// Execute a ggsql query using any reader +/// Resolve a VISUALISE query into a `ResolvedPlot`. /// -/// This is the shared implementation behind `Reader::execute()`. Concrete -/// readers delegate to this so the trait stays object-safe (no `Self: Sized` -/// bound on `execute`). -pub fn execute_with_reader(reader: &dyn Reader, query: &str) -> Result { +/// This is the Plot-side counterpart to `execute::resolve_table_with_reader` +/// — see that function's doc comment for how the two relate. +pub fn resolve_plot_with_reader(reader: &dyn Reader, query: &str) -> Result { let validated = validate(query)?; let warnings: Vec = validated.warnings().to_vec(); @@ -981,7 +1026,7 @@ pub fn execute_with_reader(reader: &dyn Reader, query: &str) -> Result { let layer_sql = vec![None; plot.layers.len()]; let stat_sql = vec![None; plot.layers.len()]; - Ok(Spec::new( + Ok(ResolvedPlot::new( plot, prepared_data.data, prepared_data.sql, @@ -992,6 +1037,29 @@ pub fn execute_with_reader(reader: &dyn Reader, query: &str) -> Result { )) } +/// Execute a ggsql query using any reader. +/// +/// This is the shared implementation behind `Reader::execute()`. Concrete +/// readers delegate to this so the trait stays object-safe (no `Self: Sized` +/// bound on `execute`). Dispatches to the Plot or Table pipeline depending on +/// which kind of `Spec` the query's first statement is. +pub fn execute_with_reader(reader: &dyn Reader, query: &str) -> Result { + let source_tree = SourceTree::new(query)?; + source_tree.validate()?; + let specs = parser::build_ast(&source_tree)?; + + match specs.into_iter().next() { + Some(Spec::Table(_)) => { + let resolved = resolve_table_with_reader(query, reader)?; + Ok(ResolvedSpec::Table(resolved)) + } + _ => { + let resolved = resolve_plot_with_reader(reader, query)?; + Ok(ResolvedSpec::Plot(Box::new(resolved))) + } + } +} + #[cfg(test)] #[cfg(all(feature = "duckdb", feature = "vegalite"))] mod tests { @@ -1021,15 +1089,42 @@ mod tests { .execute("SELECT 1 as x, 2 as y VISUALISE x, y DRAW point") .unwrap(); - assert_eq!(spec.plot().layers.len(), 1); - assert_eq!(spec.metadata().layer_count, 1); - assert!(spec.layer_data(0).is_some()); + let plot = spec.as_plot().unwrap(); + assert_eq!(plot.plot().layers.len(), 1); + assert_eq!(plot.metadata().layer_count, 1); + assert!(plot.layer_data(0).is_some()); let writer = VegaLiteWriter::new(); let result = writer.render(&spec).unwrap(); assert!(result.contains("point")); } + /// Confirms the actual dispatch in `execute_with_reader` — not just its + /// two pieces (`resolve_plot_with_reader`, `resolve_table_with_reader`) + /// tested elsewhere — routes each Spec kind correctly through + /// `Reader::execute()`, and that `Writer::render()` rejects a Table + /// cleanly rather than panicking. + #[test] + fn test_execute_dispatches_plot_and_table() { + let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); + reader + .execute_sql("CREATE TABLE sales AS SELECT 1 AS id") + .unwrap(); + + let plot_spec = reader + .execute("SELECT 1 as x, 2 as y VISUALISE x, y DRAW point") + .unwrap(); + assert!(plot_spec.as_plot().is_some()); + assert!(plot_spec.as_table().is_none()); + + let table_spec = reader.execute("TABULATE FROM sales").unwrap(); + assert!(table_spec.as_table().is_some()); + assert!(table_spec.as_plot().is_none()); + + let writer = VegaLiteWriter::new(); + assert!(writer.render(&table_spec).is_err()); + } + #[test] fn test_execute_metadata() { let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); @@ -1039,7 +1134,7 @@ mod tests { ) .unwrap(); - let metadata = spec.metadata(); + let metadata = spec.as_plot().unwrap().metadata(); assert_eq!(metadata.rows, 3); // Columns now includes both user mappings (pos1, pos2) and resolved defaults (size, stroke, fill, opacity, shape, linewidth) // Aesthetics are transformed to internal names (x -> pos1, y -> pos2) @@ -1060,11 +1155,11 @@ mod tests { "#; let spec = reader.execute(query).unwrap(); + let plot = spec.as_plot().unwrap(); - assert_eq!(spec.plot().layers.len(), 1); - assert!(spec.layer_data(0).is_some()); - let df = spec.layer_data(0).unwrap(); - assert_eq!(df.height(), 2); + assert_eq!(plot.plot().layers.len(), 1); + assert!(plot.layer_data(0).is_some()); + assert_eq!(plot.layer_data(0).unwrap().height(), 2); } #[test] @@ -1364,9 +1459,10 @@ mod tests { let query = "SELECT * FROM my_data VISUALISE x, y DRAW point"; let spec = reader.execute(query).unwrap(); - assert_eq!(spec.metadata().rows, 3); + let plot = spec.as_plot().unwrap(); + assert_eq!(plot.metadata().rows, 3); // Aesthetics are transformed to internal names (x -> pos1) - assert!(spec.metadata().columns.contains(&"pos1".to_string())); + assert!(plot.metadata().columns.contains(&"pos1".to_string())); let writer = VegaLiteWriter::new(); let result = writer.render(&spec).unwrap(); @@ -1402,7 +1498,7 @@ mod tests { "#; let spec = reader.execute(query).unwrap(); - assert_eq!(spec.metadata().rows, 3); + assert_eq!(spec.as_plot().unwrap().metadata().rows, 3); } #[test] @@ -1437,10 +1533,11 @@ mod tests { let spec = reader.execute(query).unwrap(); // Verify spec structure - assert_eq!(spec.plot().layers.len(), 1); + let plot = spec.as_plot().unwrap(); + assert_eq!(plot.plot().layers.len(), 1); // Note: scales may include auto-generated x/y scales plus the explicit fill scale assert!( - spec.plot().find_scale("fill").is_some(), + plot.plot().find_scale("fill").is_some(), "Should have a fill scale" ); diff --git a/src/reader/odbc/mod.rs b/src/reader/odbc/mod.rs index 740912e26..1ce87cd8d 100644 --- a/src/reader/odbc/mod.rs +++ b/src/reader/odbc/mod.rs @@ -226,7 +226,7 @@ impl Reader for OdbcReader { Ok(()) } - fn execute(&self, query: &str) -> Result { + fn execute(&self, query: &str) -> Result { super::execute_with_reader(self, query) } @@ -1436,8 +1436,9 @@ mod tests { ) .unwrap(); - assert_eq!(spec.plot.layers.len(), 1); - assert!(spec.layer_data(0).unwrap().height() > 0); + let plot = spec.as_plot().unwrap(); + assert_eq!(plot.plot.layers.len(), 1); + assert!(plot.layer_data(0).unwrap().height() > 0); reader.execute_sql("DROP TABLE __ggsql_countries").unwrap(); } @@ -1481,7 +1482,7 @@ mod tests { .unwrap(); // Only the visible polygon should survive clipping - assert_eq!(spec.layer_data(0).unwrap().height(), 1); + assert_eq!(spec.as_plot().unwrap().layer_data(0).unwrap().height(), 1); reader.execute_sql("DROP TABLE __ggsql_clip_test").unwrap(); } @@ -1524,17 +1525,18 @@ mod tests { ) .unwrap(); - assert_eq!(spec.plot.layers.len(), 1); - let df = spec.layer_data(0).unwrap(); + let plot = spec.as_plot().unwrap(); + assert_eq!(plot.plot.layers.len(), 1); + let df = plot.layer_data(0).unwrap(); assert_eq!(df.height(), 3); let writer = crate::writer::vegalite::VegaLiteWriter::new(); - let json_str = writer.write(&spec.plot, &spec.data).unwrap(); + let json_str = writer.write_plot(&plot.plot, &plot.data).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); // Coordinates should be projected (not raw lon/lat) let data = vl_spec["data"]["values"].as_array().unwrap(); - let layer_key = spec.plot.layers[0].data_key.as_ref().unwrap(); + let layer_key = plot.plot.layers[0].data_key.as_ref().unwrap(); let rows: Vec<_> = data .iter() .filter(|r| r[crate::naming::SOURCE_COLUMN] == layer_key.as_str()) diff --git a/src/reader/spec.rs b/src/reader/spec.rs index aee78f4db..6cd9dec11 100644 --- a/src/reader/spec.rs +++ b/src/reader/spec.rs @@ -1,16 +1,16 @@ -//! Implementation of Spec methods. +//! Implementation of ResolvedPlot, ResolvedTable, and ResolvedSpec methods. use std::collections::HashMap; use crate::naming; use crate::plot::Plot; use crate::validate::ValidationWarning; -use crate::DataFrame; +use crate::{DataFrame, Table}; -use super::{Metadata, Spec}; +use super::{Metadata, ResolvedPlot, ResolvedSpec, ResolvedTable}; -impl Spec { - /// Create a new Spec from PreparedData +impl ResolvedPlot { + /// Create a new ResolvedPlot from PreparedData pub(crate) fn new( plot: Plot, data: HashMap, @@ -110,3 +110,78 @@ impl Spec { &self.warnings } } + +impl ResolvedTable { + /// Create a new ResolvedTable. + pub(crate) fn new( + table: Table, + body: DataFrame, + sql: String, + warnings: Vec, + ) -> Self { + Self { + table, + body, + sql, + warnings, + } + } + + /// Get the resolved table specification. + pub fn table(&self) -> &Table { + &self.table + } + + /// Get the resolved body data. See the PROVISIONAL note on the `body` + /// field in `reader::mod` — this accessor's return type will likely + /// change once real table writers exist. + pub fn body(&self) -> &DataFrame { + &self.body + } + + /// The SQL query that was executed to produce `body`. + pub fn sql(&self) -> &str { + &self.sql + } + + /// Validation warnings from preparation. + pub fn warnings(&self) -> &[ValidationWarning] { + &self.warnings + } +} + +impl ResolvedSpec { + /// Borrow the inner `ResolvedPlot`, or `None` if this is a `ResolvedTable`. + pub fn as_plot(&self) -> Option<&ResolvedPlot> { + match self { + ResolvedSpec::Plot(plot) => Some(plot), + ResolvedSpec::Table(_) => None, + } + } + + /// Borrow the inner `ResolvedTable`, or `None` if this is a `ResolvedPlot`. + pub fn as_table(&self) -> Option<&ResolvedTable> { + match self { + ResolvedSpec::Plot(_) => None, + ResolvedSpec::Table(table) => Some(table), + } + } + + /// Consume this `ResolvedSpec`, returning the inner `ResolvedPlot`, or + /// `None` if it was a `ResolvedTable`. + pub fn into_plot(self) -> Option { + match self { + ResolvedSpec::Plot(plot) => Some(*plot), + ResolvedSpec::Table(_) => None, + } + } + + /// Consume this `ResolvedSpec`, returning the inner `ResolvedTable`, or + /// `None` if it was a `ResolvedPlot`. + pub fn into_table(self) -> Option { + match self { + ResolvedSpec::Plot(_) => None, + ResolvedSpec::Table(table) => Some(table), + } + } +} diff --git a/src/reader/sqlite.rs b/src/reader/sqlite.rs index 6df181633..d1ebe6668 100644 --- a/src/reader/sqlite.rs +++ b/src/reader/sqlite.rs @@ -568,7 +568,7 @@ impl Reader for SqliteReader { Ok(()) } - fn execute(&self, query: &str) -> Result { + fn execute(&self, query: &str) -> Result { super::execute_with_reader(self, query) } @@ -1260,8 +1260,9 @@ mod tests { .execute("SELECT * FROM bar_data VISUALISE DRAW bar MAPPING category AS x") .unwrap(); - assert_eq!(spec.plot().layers.len(), 1); - assert!(spec.layer_data(0).is_some()); + let plot = spec.as_plot().unwrap(); + assert_eq!(plot.plot().layers.len(), 1); + assert!(plot.layer_data(0).is_some()); let writer = VegaLiteWriter::new(); let json = writer.render(&spec).unwrap(); @@ -1293,8 +1294,9 @@ mod tests { .execute("SELECT * FROM hist_data VISUALISE DRAW histogram MAPPING value AS x") .unwrap(); - assert_eq!(spec.plot().layers.len(), 1); - let layer_df = spec.layer_data(0).unwrap(); + let plot = spec.as_plot().unwrap(); + assert_eq!(plot.plot().layers.len(), 1); + let layer_df = plot.layer_data(0).unwrap(); assert!( layer_df.height() < 50, "Histogram should bin data: got {} rows", @@ -1331,8 +1333,9 @@ mod tests { .execute("SELECT * FROM density_data VISUALISE DRAW density MAPPING value AS x") .unwrap(); - assert_eq!(spec.plot().layers.len(), 1); - assert!(spec.layer_data(0).is_some()); + let plot = spec.as_plot().unwrap(); + assert_eq!(plot.plot().layers.len(), 1); + assert!(plot.layer_data(0).is_some()); let writer = VegaLiteWriter::new(); let json = writer.render(&spec).unwrap(); @@ -1370,7 +1373,7 @@ mod tests { .execute("SELECT * FROM box_data VISUALISE DRAW boxplot MAPPING grp AS x, value AS y") .unwrap(); - assert!(spec.layer_data(0).is_some()); + assert!(spec.as_plot().unwrap().layer_data(0).is_some()); let writer = VegaLiteWriter::new(); let json = writer.render(&spec).unwrap(); @@ -1589,8 +1592,9 @@ mod spatialite_tests { ) .unwrap(); - assert_eq!(spec.plot.layers.len(), 1); - assert!(spec.layer_data(0).unwrap().height() > 0); + let plot = spec.as_plot().unwrap(); + assert_eq!(plot.plot.layers.len(), 1); + assert!(plot.layer_data(0).unwrap().height() > 0); } #[cfg(feature = "vegalite")] @@ -1622,7 +1626,7 @@ mod spatialite_tests { .unwrap(); // Only the visible polygon should survive clipping - assert_eq!(spec.layer_data(0).unwrap().height(), 1); + assert_eq!(spec.as_plot().unwrap().layer_data(0).unwrap().height(), 1); } #[cfg(feature = "vegalite")] @@ -1653,16 +1657,17 @@ mod spatialite_tests { ) .unwrap(); - assert_eq!(spec.plot.layers.len(), 1); - let df = spec.layer_data(0).unwrap(); + let plot = spec.as_plot().unwrap(); + assert_eq!(plot.plot.layers.len(), 1); + let df = plot.layer_data(0).unwrap(); assert_eq!(df.height(), 3); let writer = crate::writer::vegalite::VegaLiteWriter::new(); - let json_str = writer.write(&spec.plot, &spec.data).unwrap(); + let json_str = writer.write_plot(&plot.plot, &plot.data).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); let data = vl_spec["data"]["values"].as_array().unwrap(); - let layer_key = spec.plot.layers[0].data_key.as_ref().unwrap(); + let layer_key = plot.plot.layers[0].data_key.as_ref().unwrap(); let rows: Vec<_> = data .iter() .filter(|r| r[crate::naming::SOURCE_COLUMN] == layer_key.as_str()) diff --git a/src/spec.rs b/src/spec.rs new file mode 100644 index 000000000..f37edea76 --- /dev/null +++ b/src/spec.rs @@ -0,0 +1,79 @@ +//! The parse-time result of a single VISUALISE/TABULATE statement. + +use serde::{Deserialize, Serialize}; + +use crate::{Plot, Table}; + +/// One parsed statement from a ggsql query: either a visualization or a table. +/// +/// A query may contain several `VISUALISE`/`TABULATE` statements +/// (`parser::parse_query` returns one `Spec` per statement, in source order). +#[derive(Debug, Clone, Serialize, Deserialize)] +pub enum Spec { + // Boxed: `Plot` is far larger than `Table`, and clippy flags the + // resulting size gap (`large_enum_variant`) otherwise. + Plot(Box), + Table(Table), +} + +impl Spec { + /// Borrow the inner `Plot`, or `None` if this is a `Table`. + pub fn as_plot(&self) -> Option<&Plot> { + match self { + Spec::Plot(plot) => Some(plot), + Spec::Table(_) => None, + } + } + + /// Borrow the inner `Table`, or `None` if this is a `Plot`. + pub fn as_table(&self) -> Option<&Table> { + match self { + Spec::Plot(_) => None, + Spec::Table(table) => Some(table), + } + } + + /// Consume this `Spec`, returning the inner `Plot`, or `None` if it was a `Table`. + pub fn into_plot(self) -> Option { + match self { + Spec::Plot(plot) => Some(*plot), + Spec::Table(_) => None, + } + } + + /// Consume this `Spec`, returning the inner `Table`, or `None` if it was a `Plot`. + pub fn into_table(self) -> Option
{ + match self { + Spec::Plot(_) => None, + Spec::Table(table) => Some(table), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn as_plot_and_as_table_are_mutually_exclusive() { + let plot = Spec::Plot(Box::default()); + assert!(plot.as_plot().is_some()); + assert!(plot.as_table().is_none()); + + let table = Spec::Table(Table::default()); + assert!(table.as_plot().is_none()); + assert!(table.as_table().is_some()); + } + + #[test] + fn into_plot_and_into_table_are_mutually_exclusive() { + let plot = Spec::Plot(Box::default()); + assert!(plot.into_plot().is_some()); + + let table = Spec::Table(Table::default()); + assert!(table.into_table().is_some()); + + assert!(Spec::Plot(Box::default()).into_table().is_none()); + assert!(Spec::Table(Table::default()).into_plot().is_none()); + } +} diff --git a/src/table/mod.rs b/src/table/mod.rs new file mode 100644 index 000000000..75198a784 --- /dev/null +++ b/src/table/mod.rs @@ -0,0 +1,34 @@ +//! Table types for ggsql specification +//! +//! This module will define the typed `Table` structure that represents parsed +//! `TABULATE` statements, parallel to how `plot` defines `Plot` for `VISUALISE` +//! statements. It is currently minimal: only `source` (from `TABULATE FROM`) +//! is populated so far. + +use serde::{Deserialize, Serialize}; + +use crate::DataSource; + +/// Complete ggsql table specification. +/// +/// Parallel to [`crate::Plot`], but for `TABULATE` statements. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct Table { + /// `FROM` source (CTE, table, or file path) from `TABULATE FROM`. Unlike + /// `Plot`, there are no layers to hold a per-layer source override, so + /// this is the only place a `TABULATE`'s data source can come from. + pub source: Option, +} + +impl Table { + /// Create a new empty Table. + pub fn new() -> Self { + Self { source: None } + } +} + +impl Default for Table { + fn default() -> Self { + Self::new() + } +} diff --git a/src/util.rs b/src/util.rs index 9c7b5a073..f257ea228 100644 --- a/src/util.rs +++ b/src/util.rs @@ -87,6 +87,16 @@ pub fn set_union(mut old: Vec, new: &[String]) -> Vec { old } +/// Escape HTML special characters. `&` must be replaced first, or the +/// entities inserted for the others would themselves get escaped. +pub fn escape_html(s: &str) -> String { + s.replace('&', "&") + .replace('<', "<") + .replace('>', ">") + .replace('"', """) + .replace('\'', "'") +} + #[cfg(test)] mod tests { use super::*; @@ -237,4 +247,15 @@ mod tests { assert_eq!(and_list(&[1, 2, 3]), "1, 2, and 3"); assert_eq!(or_list(&[42, 99]), "42 or 99"); } + + #[test] + fn test_escape_html() { + assert_eq!( + escape_html(""), + "<script>alert('xss')</script>" + ); + assert_eq!(escape_html("a & b"), "a & b"); + assert_eq!(escape_html("say \"hi\""), "say "hi""); + assert_eq!(escape_html("plain text"), "plain text"); + } } diff --git a/src/validate.rs b/src/validate.rs index 2199bb956..b15fe7a52 100644 --- a/src/validate.rs +++ b/src/validate.rs @@ -4,7 +4,7 @@ //! any SQL. Use this for IDE integration, syntax checking, and query inspection. use crate::parser; -use crate::Result; +use crate::{Plot, Result, Spec, Table}; // ============================================================================ // Core Types @@ -14,7 +14,7 @@ use crate::Result; pub struct Validated { sql: String, visual: String, - has_visual: bool, + has_spec: bool, tree: Option, valid: bool, errors: Vec, @@ -22,9 +22,9 @@ pub struct Validated { } impl Validated { - /// Whether the query contains a VISUALISE clause. - pub fn has_visual(&self) -> bool { - self.has_visual + /// Whether the query contains a Spec (a VISUALISE or TABULATE clause). + pub fn has_spec(&self) -> bool { + self.has_spec } /// The SQL portion (before VISUALISE). @@ -97,7 +97,7 @@ fn has_error_ancestor(node: &tree_sitter::Node) -> bool { /// Validate query syntax and semantics without executing SQL. pub fn validate(query: &str) -> Result { let mut errors = Vec::new(); - let warnings = Vec::new(); + let mut warnings = Vec::new(); // Parse once and create SourceTree let source_tree = match parser::SourceTree::new(query) { @@ -111,7 +111,7 @@ pub fn validate(query: &str) -> Result { return Ok(Validated { sql: String::new(), visual: String::new(), - has_visual: false, + has_spec: false, tree: None, valid: false, errors, @@ -122,11 +122,12 @@ pub fn validate(query: &str) -> Result { // Extract SQL and viz portions using existing tree let sql_part = source_tree.extract_sql().unwrap_or_default(); - let viz_part = source_tree.extract_visualise().unwrap_or_default(); + let viz_part = source_tree.extract_spec().unwrap_or_default(); let root = source_tree.root(); let visualise_stmt = source_tree.find_node(&root, "(visualise_statement) @viz"); - let has_visual = visualise_stmt.is_some(); + let tabulate_stmt = source_tree.find_node(&root, "(tabulate_statement) @tab"); + let has_spec = visualise_stmt.is_some() || tabulate_stmt.is_some(); if let Err(e) = source_tree.validate() { // The lexer always tokenises VISUALISE / VISUALIZE as @@ -165,7 +166,7 @@ pub fn validate(query: &str) -> Result { return Ok(Validated { sql: sql_part, visual: viz_part, - has_visual, + has_spec, tree: Some(source_tree.tree), valid: false, errors, @@ -173,12 +174,12 @@ pub fn validate(query: &str) -> Result { }); } - // Genuine SQL-only query (no parse errors, no VISUALISE clause). - if !has_visual { + // Genuine SQL-only query (no parse errors, no VISUALISE/TABULATE clause). + if !has_spec { return Ok(Validated { sql: sql_part, visual: viz_part, - has_visual: false, + has_spec: false, tree: None, valid: true, errors, @@ -186,9 +187,9 @@ pub fn validate(query: &str) -> Result { }); } - // Build AST from existing tree for validation - let plots = match parser::build_ast(&source_tree) { - Ok(p) => p, + // Build AST from existing tree for validation. + let specs: Vec = match parser::build_ast(&source_tree) { + Ok(specs) => specs, Err(e) => { errors.push(ValidationError { message: e.to_string(), @@ -197,7 +198,7 @@ pub fn validate(query: &str) -> Result { return Ok(Validated { sql: sql_part, visual: viz_part, - has_visual, + has_spec, tree: Some(source_tree.tree), valid: false, errors, @@ -205,6 +206,22 @@ pub fn validate(query: &str) -> Result { }); } }; + let plots: Vec<&Plot> = specs.iter().filter_map(Spec::as_plot).collect(); + let tables: Vec<&Table> = specs.iter().filter_map(Spec::as_table).collect(); + + // Reader::execute() resolves only the first VISUALISE/TABULATE statement + // and silently drops the rest — a query with more than one gets no + // diagnostic otherwise. This warning is the only signal a caller has that + // part of their query was ignored. + if specs.len() > 1 { + warnings.push(ValidationWarning { + message: format!( + "Query has {} VISUALISE/TABULATE statements; only the first is resolved, the rest are ignored.", + specs.len() + ), + location: None, + }); + } // Validate the single plot (we only support one VISUALISE statement) if let Some(plot) = plots.first() { @@ -265,10 +282,24 @@ pub fn validate(query: &str) -> Result { } } + // Validate the single table (we only support one TABULATE statement). + // `Table` has only `source` today, and `sql_part` already reflects it: + // `extract_sql` synthesizes "SELECT * FROM " for a `TABULATE + // FROM`, so `sql_part` is only empty when there is neither a `FROM` nor + // preceding SQL — the same condition `resolve_table_with_reader` rejects + // at execution time, caught here before any SQL runs. + if !tables.is_empty() && sql_part.trim().is_empty() { + errors.push(ValidationError { + message: "TABULATE has no data source: add a FROM, or a SQL query before it" + .to_string(), + location: None, + }); + } + Ok(Validated { sql: sql_part, visual: viz_part, - has_visual, + has_spec, tree: Some(source_tree.tree), valid: errors.is_empty(), errors, @@ -284,7 +315,7 @@ mod tests { fn test_validate_with_visual() { let validated = validate("SELECT 1 as x, 2 as y VISUALISE DRAW point MAPPING x AS x, y AS y").unwrap(); - assert!(validated.has_visual()); + assert!(validated.has_spec()); assert_eq!(validated.sql(), "SELECT 1 as x, 2 as y"); assert!(validated.visual().starts_with("VISUALISE")); assert!(validated.tree().is_some()); @@ -294,13 +325,51 @@ mod tests { #[test] fn test_validate_without_visual() { let validated = validate("SELECT 1 as x, 2 as y").unwrap(); - assert!(!validated.has_visual()); + assert!(!validated.has_spec()); assert_eq!(validated.sql(), "SELECT 1 as x, 2 as y"); assert!(validated.visual().is_empty()); assert!(validated.tree().is_none()); assert!(validated.valid()); } + #[test] + fn test_validate_warns_on_multiple_spec_statements() { + // Reader::execute() only ever resolves the first VISUALISE/TABULATE + // statement; a query with more than one should warn rather than + // silently drop the rest with no diagnostic at all. + let validated = validate("SELECT 1 AS x VISUALISE x DRAW point TABULATE").unwrap(); + assert!(validated.valid()); + assert!(!validated.warnings().is_empty()); + assert!(validated.warnings()[0] + .message + .contains("only the first is resolved")); + } + + #[test] + fn test_validate_tabulate_from_is_valid() { + // A TABULATE FROM has a data source (extract_sql injects + // "SELECT * FROM " the same way it does for VISUALISE FROM), + // so it's recognized as having a Spec and reported valid. + let validated = validate("TABULATE FROM sales").unwrap(); + assert!(validated.has_spec()); + assert_eq!(validated.sql(), "SELECT * FROM sales"); + assert!(validated.visual().starts_with("TABULATE")); + assert!(validated.valid()); + assert!(validated.errors().is_empty()); + } + + #[test] + fn test_validate_bare_tabulate_has_no_data_source() { + // A bare TABULATE, with neither a FROM nor a preceding SQL query, has + // no data source — caught here before any SQL runs, mirroring + // `resolve_table_with_reader`'s execution-time rejection of the same + // query. + let validated = validate("TABULATE").unwrap(); + assert!(validated.has_spec()); + assert!(!validated.valid()); + assert!(validated.errors()[0].message.contains("no data source")); + } + #[test] fn test_validate_valid_query() { let validated = @@ -335,7 +404,7 @@ mod tests { let query = "SELECT 1 as x, 2 as y VISUALISE DRAW point MAPPING x AS x, y AS y DRAW line MAPPING x AS x, y AS y"; let validated = validate(query).unwrap(); - assert!(validated.has_visual()); + assert!(validated.has_spec()); assert_eq!(validated.sql(), "SELECT 1 as x, 2 as y"); assert!(validated.visual().contains("DRAW point")); assert!(validated.visual().contains("DRAW line")); @@ -408,7 +477,7 @@ mod tests { } // Issue #256: SQL expressions in VISUALISE mappings used to be silently - // consumed as SQL, with validate() reporting valid=true and has_visual=false. + // consumed as SQL, with validate() reporting valid=true and has_spec=false. // The fix detects a stray visualise_keyword node (one that didn't make it // into a visualise_statement) and emits an actionable error. @@ -479,7 +548,7 @@ mod tests { "string literal containing VISUALISE should be valid: {:?}", validated.errors() ); - assert!(!validated.has_visual()); + assert!(!validated.has_spec()); } #[test] @@ -491,6 +560,6 @@ mod tests { "comment containing VISUALISE should be valid: {:?}", validated.errors() ); - assert!(!validated.has_visual()); + assert!(!validated.has_spec()); } } diff --git a/src/writer/hephaestus/CLAUDE.md b/src/writer/hephaestus/CLAUDE.md index 3e7e716e7..da39a2ecf 100644 --- a/src/writer/hephaestus/CLAUDE.md +++ b/src/writer/hephaestus/CLAUDE.md @@ -1,6 +1,6 @@ # `writer/hephaestus/` — renderer-backed writer internals -The writers here render a resolved ggsql `Spec` through +The writers here render a resolved ggsql `ResolvedPlot` through [hephaestus](https://github.com/posit-dev/hephaestus), a 2D scene renderer with a grammar-of-graphics plot API. Seven of them exist, each behind its own cargo feature — the three GPU-free ones on by default, the four raster ones not: @@ -66,7 +66,7 @@ sibling writer's internals, [`../vegalite/CLAUDE.md`](../vegalite/CLAUDE.md). ## The governing principle **ggsql owns every scale domain; the writer never computes its own extents.** -The `Spec` arrives with each `Scale` fully resolved — type, domain (already +The `ResolvedPlot` arrives with each `Scale` fully resolved — type, domain (already expanded, transform-aware, trained globally over all layers and the whole position family), transform, breaks, formatted labels, and a concrete output range for material aesthetics. The writer's job is to *pass those through* to @@ -634,7 +634,7 @@ small omission that only shows up across the whole feature surface — use the visual-test harness instead of one-off queries. It renders every executable ```` ```{ggsql} ```` cell in [`/doc/`](../../../doc/) (≈190 in `doc/syntax/` alone) and writes one HTML report pairing each query with its render, optionally -beside the Vega-Lite render of the same `Spec`: +beside the Vega-Lite render of the same `ResolvedPlot`: ```sh cargo run -p ggsql-cli --features png --example visual_test -- --compare diff --git a/src/writer/hephaestus/hep.rs b/src/writer/hephaestus/hep.rs index 72cdfcfd3..90b7d6da4 100644 --- a/src/writer/hephaestus/hep.rs +++ b/src/writer/hephaestus/hep.rs @@ -130,7 +130,10 @@ impl HepWriter { /// # Errors /// /// As [`Self::write_reporting`]. - pub fn render_reporting(&self, spec: &crate::reader::Spec) -> Result<(Vec, Vec)> { + pub fn render_reporting( + &self, + spec: &crate::reader::ResolvedPlot, + ) -> Result<(Vec, Vec)> { self.write_reporting(spec.plot(), spec.data()) } @@ -173,11 +176,11 @@ impl Writer for HepWriter { }) } - fn validate(&self, spec: &Plot) -> Result<()> { + fn validate_plot(&self, spec: &Plot) -> Result<()> { compose::validate_plot(spec) } - fn write(&self, spec: &Plot, data: &HashMap) -> Result { + fn write_plot(&self, spec: &Plot, data: &HashMap) -> Result { self.write_reporting(spec, data).map(|(bytes, _)| bytes) } } diff --git a/src/writer/hephaestus/jpeg.rs b/src/writer/hephaestus/jpeg.rs index 42d6d5d19..b832d2aa1 100644 --- a/src/writer/hephaestus/jpeg.rs +++ b/src/writer/hephaestus/jpeg.rs @@ -107,7 +107,7 @@ impl JpegWriter { /// As [`Self::write_with`]. pub fn render_with( &self, - spec: &crate::reader::Spec, + spec: &crate::reader::ResolvedPlot, renderer: &mut RasterRenderer, ) -> Result> { self.write_with(spec.plot(), spec.data(), renderer) @@ -147,11 +147,11 @@ impl Writer for JpegWriter { Ok(Self { canvas, quality }) } - fn validate(&self, spec: &Plot) -> Result<()> { + fn validate_plot(&self, spec: &Plot) -> Result<()> { compose::validate_plot(spec) } - fn write(&self, spec: &Plot, data: &HashMap) -> Result { + fn write_plot(&self, spec: &Plot, data: &HashMap) -> Result { let mut renderer = RasterRenderer::new()?; self.write_with(spec, data, &mut renderer) } diff --git a/src/writer/hephaestus/mod.rs b/src/writer/hephaestus/mod.rs index 73894f506..5a564a932 100644 --- a/src/writer/hephaestus/mod.rs +++ b/src/writer/hephaestus/mod.rs @@ -1,8 +1,7 @@ -//! Renderer-backed writers. -//! -//! Every writer here renders a resolved ggsql `Spec` through the [`hephaestus`] -//! 2D scene renderer. Only the writers themselves are public; the renderer -//! behind them is an implementation detail, and this module is private. +//! Every writer here renders a resolved ggsql `ResolvedPlot` through the +//! [`hephaestus`] 2D scene renderer. Only the writers themselves are public; +//! the renderer behind them is an implementation detail, and this module is +//! private. //! //! The work splits three ways, which is what keeps one writer per format small: //! @@ -115,9 +114,9 @@ mod tests { /// look at the picture — only that the whole pipeline ran. const CORPUS_SIZE: (u32, u32, f64) = (640, 480, 96.0); - fn spec_for(query: &str) -> crate::reader::Spec { + fn spec_for(query: &str) -> crate::reader::ResolvedPlot { let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); - reader.execute(query).unwrap() + reader.execute(query).unwrap().into_plot().unwrap() } /// Render `query` through every compiled writer, asserting each output @@ -163,7 +162,7 @@ mod tests { // Last, and the only one that tolerates a headless box. #[cfg(feature = "png")] - assert_png_or_skip(PngWriter::new(w, h, dpi).render(&spec)); + assert_png_or_skip(PngWriter::new(w, h, dpi).write_plot(spec.plot(), spec.data())); } /// The panels' `(top, right)` strip labels, in panel order. Exercises the @@ -171,7 +170,8 @@ mod tests { fn strips(query: &str) -> Vec<(Option, Option)> { let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); let spec = reader.execute(query).unwrap(); - let (_, panels) = facet::build_panels(spec.plot(), spec.data()).unwrap(); + let plot = spec.as_plot().unwrap(); + let (_, panels) = facet::build_panels(plot.plot(), plot.data()).unwrap(); panels .iter() .map(|p| (p.strip_top.clone(), p.strip_right.clone())) @@ -182,7 +182,7 @@ mod tests { fn axis_titles(query: &str) -> Vec<(AxisSide, String)> { let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); let spec = reader.execute(query).unwrap(); - projection::composition_axis_titles(spec.plot()) + projection::composition_axis_titles(spec.as_plot().unwrap().plot()) } /// Just the top strip labels, in panel order. @@ -1311,7 +1311,7 @@ mod svg_text { fn svg(query: &str) -> String { let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); - let spec = reader.execute(query).unwrap(); + let spec = reader.execute(query).unwrap().into_plot().unwrap(); let (svg, warnings) = SvgWriter::new(640, 480, 96.0) .render_reporting(&spec) .unwrap_or_else(|e| panic!("svg render failed: {e}")); @@ -1495,12 +1495,14 @@ mod svg_text { let query = "SELECT x, y FROM (VALUES (1,2),(2,3)) t(x,y) \ VISUALISE x AS x, y AS y DRAW point LABEL title => 'Outlined'"; let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); - let spec = reader.execute(query).unwrap(); + let spec = reader.execute(query).unwrap().into_plot().unwrap(); - let as_text = SvgWriter::new(640, 480, 96.0).render(&spec).unwrap(); + let as_text = SvgWriter::new(640, 480, 96.0) + .write_plot(spec.plot(), spec.data()) + .unwrap(); let as_paths = SvgWriter::new(640, 480, 96.0) .outline_text(true) - .render(&spec) + .write_plot(spec.plot(), spec.data()) .unwrap(); assert!(as_text.contains(""); @@ -1519,13 +1521,13 @@ mod svg_text { let query = "SELECT x, y FROM (VALUES (1,2),(2,3)) t(x,y) \ VISUALISE x AS x, y AS y DRAW point"; let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); - let spec = reader.execute(query).unwrap(); + let spec = reader.execute(query).unwrap().into_plot().unwrap(); let build = |pairs: &[&str]| { let options = WriterOptions::parse(pairs).unwrap(); SvgWriter::from_options(&options) .unwrap() - .render(&spec) + .write_plot(spec.plot(), spec.data()) .unwrap() }; @@ -1554,12 +1556,14 @@ mod svg_text { let query = "SELECT x, y, c FROM (VALUES (1,2,10),(2,3,50),(3,1,90)) t(x,y,c) \ VISUALISE x AS x, y AS y, c AS color DRAW point"; let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); - let spec = reader.execute(query).unwrap(); + let spec = reader.execute(query).unwrap().into_plot().unwrap(); - let bare = SvgWriter::new(640, 480, 96.0).render(&spec).unwrap(); + let bare = SvgWriter::new(640, 480, 96.0) + .write_plot(spec.plot(), spec.data()) + .unwrap(); let prefixed = SvgWriter::new(640, 480, 96.0) .id_prefix("fig1-") - .render(&spec) + .write_plot(spec.plot(), spec.data()) .unwrap(); assert!(bare.contains("id=\"c0\""), "expected an unprefixed id"); @@ -1580,7 +1584,7 @@ mod pdf_structure { use crate::reader::{DuckDBReader, Reader}; use crate::writer::Writer; - fn spec() -> crate::reader::Spec { + fn spec() -> crate::reader::ResolvedPlot { let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); reader .execute( @@ -1588,6 +1592,8 @@ mod pdf_structure { VISUALISE x AS x, y AS y DRAW point LABEL title => 'A page'", ) .unwrap() + .into_plot() + .unwrap() } #[test] @@ -1595,7 +1601,7 @@ mod pdf_structure { // 640 px at 96 dpi is 6⅔ in, which is 480 pt; 480 px is 360 pt. let pdf = PdfWriter::new(640, 480, 96.0) .compress(false) - .render(&spec()) + .write_plot(spec().plot(), spec().data()) .unwrap(); let text = String::from_utf8_lossy(&pdf); assert!( @@ -1608,9 +1614,11 @@ mod pdf_structure { fn an_uncompressed_page_is_readable_and_a_compressed_one_is_smaller() { let readable = PdfWriter::new(640, 480, 96.0) .compress(false) - .render(&spec()) + .write_plot(spec().plot(), spec().data()) + .unwrap(); + let compressed = PdfWriter::new(640, 480, 96.0) + .write_plot(spec().plot(), spec().data()) .unwrap(); - let compressed = PdfWriter::new(640, 480, 96.0).render(&spec()).unwrap(); assert!(readable.starts_with(b"%PDF-")); assert!(compressed.starts_with(b"%PDF-")); @@ -1626,7 +1634,7 @@ mod pdf_structure { // in whatever the reader substitutes. let pdf = PdfWriter::new(640, 480, 96.0) .compress(false) - .render(&spec()) + .write_plot(spec().plot(), spec().data()) .unwrap(); let text = String::from_utf8_lossy(&pdf); assert!(text.contains("/FontFile2"), "no embedded font programme"); @@ -1680,7 +1688,7 @@ mod hep_roundtrip { fn compose_for(query: &str) -> hephaestus::plot::PlotComposition { let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); - let spec = reader.execute(query).unwrap(); + let spec = reader.execute(query).unwrap().into_plot().unwrap(); compose::validate_plot(spec.plot()).unwrap(); compose::build_composition(spec.plot(), spec.data()).unwrap() } @@ -1742,7 +1750,7 @@ mod hep_roundtrip { fn the_writers_hints_travel_with_the_document() { let spec = { let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); - reader.execute(QUERIES[0].1).unwrap() + reader.execute(QUERIES[0].1).unwrap().into_plot().unwrap() }; let (bytes, warnings) = HepWriter::new(1600, 900, 150.0) .background(rgba(0.0, 0.0, 0.0, 1.0)) diff --git a/src/writer/hephaestus/pdf.rs b/src/writer/hephaestus/pdf.rs index aeff21406..02e495c34 100644 --- a/src/writer/hephaestus/pdf.rs +++ b/src/writer/hephaestus/pdf.rs @@ -98,7 +98,10 @@ impl PdfWriter { /// # Errors /// /// As [`Self::write_reporting`]. - pub fn render_reporting(&self, spec: &crate::reader::Spec) -> Result<(Vec, Vec)> { + pub fn render_reporting( + &self, + spec: &crate::reader::ResolvedPlot, + ) -> Result<(Vec, Vec)> { self.write_reporting(spec.plot(), spec.data()) } @@ -132,11 +135,11 @@ impl Writer for PdfWriter { }) } - fn validate(&self, spec: &Plot) -> Result<()> { + fn validate_plot(&self, spec: &Plot) -> Result<()> { compose::validate_plot(spec) } - fn write(&self, spec: &Plot, data: &HashMap) -> Result { + fn write_plot(&self, spec: &Plot, data: &HashMap) -> Result { self.write_reporting(spec, data).map(|(pdf, _)| pdf) } } diff --git a/src/writer/hephaestus/png.rs b/src/writer/hephaestus/png.rs index 4cab1b14f..9570949b8 100644 --- a/src/writer/hephaestus/png.rs +++ b/src/writer/hephaestus/png.rs @@ -100,7 +100,7 @@ impl PngWriter { /// As [`Self::write_with`]. pub fn render_with( &self, - spec: &crate::reader::Spec, + spec: &crate::reader::ResolvedPlot, renderer: &mut RasterRenderer, ) -> Result> { self.write_with(spec.plot(), spec.data(), renderer) @@ -133,11 +133,11 @@ impl Writer for PngWriter { }) } - fn validate(&self, spec: &Plot) -> Result<()> { + fn validate_plot(&self, spec: &Plot) -> Result<()> { compose::validate_plot(spec) } - fn write(&self, spec: &Plot, data: &HashMap) -> Result { + fn write_plot(&self, spec: &Plot, data: &HashMap) -> Result { let mut renderer = RasterRenderer::new()?; self.write_with(spec, data, &mut renderer) } diff --git a/src/writer/hephaestus/svg.rs b/src/writer/hephaestus/svg.rs index 4c22682d0..92d5abe06 100644 --- a/src/writer/hephaestus/svg.rs +++ b/src/writer/hephaestus/svg.rs @@ -119,7 +119,10 @@ impl SvgWriter { /// # Errors /// /// As [`Self::write_reporting`]. - pub fn render_reporting(&self, spec: &crate::reader::Spec) -> Result<(String, Vec)> { + pub fn render_reporting( + &self, + spec: &crate::reader::ResolvedPlot, + ) -> Result<(String, Vec)> { self.write_reporting(spec.plot(), spec.data()) } @@ -162,11 +165,11 @@ impl Writer for SvgWriter { }) } - fn validate(&self, spec: &Plot) -> Result<()> { + fn validate_plot(&self, spec: &Plot) -> Result<()> { compose::validate_plot(spec) } - fn write(&self, spec: &Plot, data: &HashMap) -> Result { + fn write_plot(&self, spec: &Plot, data: &HashMap) -> Result { self.write_reporting(spec, data).map(|(svg, _)| svg) } } diff --git a/src/writer/hephaestus/tiff.rs b/src/writer/hephaestus/tiff.rs index 7fd55dcd1..7a1bcde12 100644 --- a/src/writer/hephaestus/tiff.rs +++ b/src/writer/hephaestus/tiff.rs @@ -97,7 +97,7 @@ impl TiffWriter { /// As [`Self::write_with`]. pub fn render_with( &self, - spec: &crate::reader::Spec, + spec: &crate::reader::ResolvedPlot, renderer: &mut RasterRenderer, ) -> Result> { self.write_with(spec.plot(), spec.data(), renderer) @@ -121,11 +121,11 @@ impl Writer for TiffWriter { }) } - fn validate(&self, spec: &Plot) -> Result<()> { + fn validate_plot(&self, spec: &Plot) -> Result<()> { compose::validate_plot(spec) } - fn write(&self, spec: &Plot, data: &HashMap) -> Result { + fn write_plot(&self, spec: &Plot, data: &HashMap) -> Result { let mut renderer = RasterRenderer::new()?; self.write_with(spec, data, &mut renderer) } diff --git a/src/writer/hephaestus/webp.rs b/src/writer/hephaestus/webp.rs index 427b72b08..4d705ce12 100644 --- a/src/writer/hephaestus/webp.rs +++ b/src/writer/hephaestus/webp.rs @@ -82,7 +82,7 @@ impl WebpWriter { /// As [`Self::write_with`]. pub fn render_with( &self, - spec: &crate::reader::Spec, + spec: &crate::reader::ResolvedPlot, renderer: &mut RasterRenderer, ) -> Result> { self.write_with(spec.plot(), spec.data(), renderer) @@ -98,11 +98,11 @@ impl Writer for WebpWriter { }) } - fn validate(&self, spec: &Plot) -> Result<()> { + fn validate_plot(&self, spec: &Plot) -> Result<()> { compose::validate_plot(spec) } - fn write(&self, spec: &Plot, data: &HashMap) -> Result { + fn write_plot(&self, spec: &Plot, data: &HashMap) -> Result { let mut renderer = RasterRenderer::new()?; self.write_with(spec, data, &mut renderer) } diff --git a/src/writer/hephaestus/window.rs b/src/writer/hephaestus/window.rs index d2a56d62b..6f49766cd 100644 --- a/src/writer/hephaestus/window.rs +++ b/src/writer/hephaestus/window.rs @@ -8,7 +8,7 @@ use hephaestus::plot::PlotComposition; use hephaestus::window::{self, Event, EventCtx, Frame, WindowApp, WindowConfig}; use super::canvas::{parse_background, whole_pixels}; -use crate::reader::Spec; +use crate::reader::ResolvedPlot; use crate::writer::WriterOptions; use crate::{GgsqlError, Result}; @@ -119,7 +119,7 @@ impl PlotViewer { /// /// Returns `GgsqlError::WriterError` if the plot cannot be composed, if no /// GPU adapter can drive a window, or if the event loop fails. - pub fn show(&self, spec: &Spec) -> Result<()> { + pub fn show(&self, spec: &ResolvedPlot) -> Result<()> { let view = super::compose::prepare(spec.plot(), spec.data())?; let config = WindowConfig::new(self.title.clone()) diff --git a/src/writer/html.rs b/src/writer/html.rs new file mode 100644 index 000000000..9d362e60a --- /dev/null +++ b/src/writer/html.rs @@ -0,0 +1,111 @@ +//! A minimal HTML table writer. +//! +//! Renders a `ResolvedTable`'s body as a bare `
` — no styling, no +//! headings/spanners/footnotes, since `Table` has no fields to describe +//! those yet. This is a stub to prove the Table → writer plumbing end to +//! end, not the real grammar-of-tables output; it deliberately does not +//! reuse `ggsql-jupyter`'s existing `dataframe_to_html`, since that's built +//! around `DataFrame` specifically, and `ResolvedTable.body`'s type is +//! itself still provisional (see the note on that field). + +use std::collections::HashMap; + +use crate::array_util::value_to_string; +use crate::util::escape_html; +use crate::writer::{Writer, WriterOptions}; +use crate::{DataFrame, GgsqlError, Plot, Result, Table}; + +/// Renders a resolved table as a bare HTML `
`. Does not support plots. +#[derive(Debug, Default)] +pub struct HtmlWriter; + +impl HtmlWriter { + /// Create a new HtmlWriter. + pub fn new() -> Self { + Self + } +} + +impl Writer for HtmlWriter { + type Output = String; + + /// This writer takes no options and rejects any. + fn from_options(options: &WriterOptions) -> Result { + options.reject_unknown(&[])?; + Ok(Self::new()) + } + + fn write_plot(&self, _spec: &Plot, _data: &HashMap) -> Result { + Err(GgsqlError::WriterError( + "HtmlWriter does not support plots".to_string(), + )) + } + + fn validate_plot(&self, _spec: &Plot) -> Result<()> { + Err(GgsqlError::WriterError( + "HtmlWriter does not support plots".to_string(), + )) + } + + fn write_table(&self, _table: &Table, body: &DataFrame) -> Result { + let mut html = String::from("
\n\n"); + for name in body.get_column_names() { + html.push_str(&format!("", escape_html(&name))); + } + html.push_str("\n\n\n"); + + let columns = body.get_columns(); + for row in 0..body.height() { + html.push_str(""); + for column in columns { + html.push_str(&format!( + "", + escape_html(&value_to_string(column, row)) + )); + } + html.push_str("\n"); + } + html.push_str("\n
{}
{}
"); + + Ok(html) + } +} + +#[cfg(test)] +#[cfg(feature = "duckdb")] +mod tests { + use super::*; + use crate::reader::{DuckDBReader, Reader}; + + #[test] + fn test_write_table_renders_rows_and_escapes_html() { + let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); + reader + .execute_sql( + "CREATE TABLE sales AS SELECT * FROM (VALUES (1, 'a'), (2, 'b')) AS t(id, name)", + ) + .unwrap(); + let spec = reader.execute("TABULATE FROM sales").unwrap(); + + let writer = HtmlWriter::new(); + let html = writer.render(&spec).unwrap(); + + assert!(html.starts_with("")); + assert!(html.contains("")); + assert!(html.contains("")); + assert!(html.contains("")); + assert!(html.contains("<b>a</b>")); + assert!(!html.contains("a")); + } + + #[test] + fn test_write_plot_is_unsupported() { + let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); + let spec = reader + .execute("SELECT 1 AS x, 2 AS y VISUALISE x, y DRAW point") + .unwrap(); + + let writer = HtmlWriter::new(); + assert!(writer.render(&spec).is_err()); + } +} diff --git a/src/writer/mod.rs b/src/writer/mod.rs index 02d235a2b..ac6091a64 100644 --- a/src/writer/mod.rs +++ b/src/writer/mod.rs @@ -6,7 +6,7 @@ //! # Architecture //! //! All writers implement the `Writer` trait, which provides: -//! - Spec + Data → Output conversion +//! - ResolvedPlot + Data → Output conversion //! - Validation for writer compatibility //! - Format-specific rendering logic //! @@ -28,8 +28,8 @@ //! key–value [`WriterOptions`] when a frontend collects settings from a user //! without knowing which writer they picked. -use crate::reader::Spec; -use crate::{DataFrame, Plot, Result}; +use crate::reader::ResolvedSpec; +use crate::{DataFrame, GgsqlError, Plot, Result, Table}; use std::collections::HashMap; pub mod options; @@ -93,6 +93,12 @@ pub use hephaestus::{PngCompression, PngWriter}; #[cfg(feature = "tiff")] pub use hephaestus::{TiffCompression, TiffWriter}; +// Pure string formatting, no extra dependencies — gated for symmetry with +// every other writer, not because it needs anything to compile. +#[cfg(feature = "html")] +pub mod html; +#[cfg(feature = "html")] +pub use html::HtmlWriter; /// Trait for visualization output writers /// /// Writers take a Plot and data sources and produce formatted output @@ -100,9 +106,10 @@ pub use hephaestus::{TiffCompression, TiffWriter}; /// /// # Associated Types /// -/// * `Output` - The type returned by `write()` and `render()`: `String` for a -/// text format, `Vec` for a binary one. Never an `Option` — failure is -/// the `Result`'s business — and a type producing nothing is not a writer. +/// * `Output` - The type returned by `write_plot()`, `write_table()` and +/// `render()`: `String` for a text format, `Vec` for a binary one. +/// Never an `Option` — failure is the `Result`'s business — and a type +/// producing nothing is not a writer. pub trait Writer { /// The output type produced by this writer. type Output; @@ -141,7 +148,7 @@ pub trait Writer { /// - The spec is incompatible with this writer /// - The data doesn't match the spec's requirements /// - Output generation fails - fn write(&self, spec: &Plot, data: &HashMap) -> Result; + fn write_plot(&self, spec: &Plot, data: &HashMap) -> Result; /// Validate that a spec is compatible with this writer /// @@ -155,15 +162,43 @@ pub trait Writer { /// # Returns /// /// Ok(()) if the spec is compatible, otherwise an error - fn validate(&self, spec: &Plot) -> Result<()>; + fn validate_plot(&self, spec: &Plot) -> Result<()>; - /// Render a Spec to output format + /// Generate output from a resolved table specification and its body data + /// + /// The table-side counterpart to `write_plot()`. Defaults to rejecting + /// every table, so a writer that only supports Plot output (every writer, + /// as of this writing) needs no changes; a writer that does support + /// tables overrides this instead. + /// + /// # Arguments + /// + /// * `table` - The parsed TABULATE specification + /// * `body` - The resolved data (see the PROVISIONAL note on + /// `ResolvedTable.body` — this parameter's type may change) + /// + /// # Errors + /// + /// Returns `GgsqlError::WriterError` if this writer doesn't support + /// tables, or output generation fails. + fn write_table(&self, table: &Table, body: &DataFrame) -> Result { + let _ = (table, body); + Err(GgsqlError::WriterError( + "this writer does not support tables".to_string(), + )) + } + + /// Render a ResolvedSpec (a resolved plot or table) to output format /// /// This is the main entry point for generating visualization output. + /// Dispatches to `write_plot()` for a `ResolvedSpec::Plot`, or + /// `write_table()` for a `ResolvedSpec::Table` — whether a writer + /// supports tables is entirely down to whether it overrides + /// `write_table()`. /// /// # Arguments /// - /// * `spec` - The prepared visualization specification from `reader.execute()` + /// * `spec` - The resolved specification from `reader.execute()` /// /// # Returns /// @@ -181,7 +216,10 @@ pub trait Writer { /// let writer = VegaLiteWriter::new(); /// let json = writer.render(&spec)?; /// ``` - fn render(&self, spec: &Spec) -> Result { - self.write(spec.plot(), spec.data()) + fn render(&self, spec: &ResolvedSpec) -> Result { + match spec { + ResolvedSpec::Plot(plot) => self.write_plot(plot.plot(), plot.data()), + ResolvedSpec::Table(table) => self.write_table(table.table(), table.body()), + } } } diff --git a/src/writer/vegalite/layer.rs b/src/writer/vegalite/layer.rs index db59e01f5..953836b73 100644 --- a/src/writer/vegalite/layer.rs +++ b/src/writer/vegalite/layer.rs @@ -3181,7 +3181,7 @@ mod tests { // Generate Vega-Lite JSON let writer = VegaLiteWriter::new(); - let json_str = writer.write(spec, &prepared.data).unwrap(); + let json_str = writer.write_plot(spec, &prepared.data).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); // Text renderer should create nested layers structure @@ -3291,7 +3291,7 @@ mod tests { // Generate Vega-Lite JSON let writer = VegaLiteWriter::new(); - let json_str = writer.write(spec, &prepared.data).unwrap(); + let json_str = writer.write_plot(spec, &prepared.data).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); // Text renderer creates nested layers structure @@ -3362,7 +3362,7 @@ mod tests { // Generate Vega-Lite JSON let writer = VegaLiteWriter::new(); - let json_str = writer.write(spec, &prepared.data).unwrap(); + let json_str = writer.write_plot(spec, &prepared.data).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); // Check that data has formatted labels @@ -3409,7 +3409,7 @@ mod tests { let spec = &prepared.specs[0]; let writer = VegaLiteWriter::new(); - let json_str = writer.write(spec, &prepared.data).unwrap(); + let json_str = writer.write_plot(spec, &prepared.data).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); let data_values = vl_spec["data"]["values"].as_array().unwrap(); @@ -3458,7 +3458,7 @@ mod tests { let spec = &prepared.specs[0]; let writer = VegaLiteWriter::new(); - let json_str = writer.write(spec, &prepared.data).unwrap(); + let json_str = writer.write_plot(spec, &prepared.data).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); let data_values = vl_spec["data"]["values"].as_array().unwrap(); @@ -3533,7 +3533,7 @@ mod tests { // Generate Vega-Lite JSON let writer = VegaLiteWriter::new(); - let json_str = writer.write(spec, &prepared.data).unwrap(); + let json_str = writer.write_plot(spec, &prepared.data).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); // Text renderer creates nested layers structure @@ -3583,7 +3583,7 @@ mod tests { let spec = &prepared.specs[0]; let writer = VegaLiteWriter::new(); - let json_str = writer.write(spec, &prepared.data).unwrap(); + let json_str = writer.write_plot(spec, &prepared.data).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); let top_layers = vl_spec["layer"].as_array().unwrap(); @@ -3621,7 +3621,7 @@ mod tests { let spec = &prepared.specs[0]; let writer = VegaLiteWriter::new(); - let json_str = writer.write(spec, &prepared.data).unwrap(); + let json_str = writer.write_plot(spec, &prepared.data).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); let top_layers = vl_spec["layer"].as_array().unwrap(); @@ -3655,7 +3655,7 @@ mod tests { let spec = &prepared.specs[0]; let writer = VegaLiteWriter::new(); - let json_str = writer.write(spec, &prepared.data).unwrap(); + let json_str = writer.write_plot(spec, &prepared.data).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); let top_layers = vl_spec["layer"].as_array().unwrap(); @@ -3677,7 +3677,7 @@ mod tests { let prepared = execute::prepare_data_with_reader(query, &reader).unwrap(); let spec = &prepared.specs[0]; - let json_str = writer.write(spec, &prepared.data).unwrap(); + let json_str = writer.write_plot(spec, &prepared.data).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); let top_layers = vl_spec["layer"].as_array().unwrap(); @@ -3699,7 +3699,7 @@ mod tests { let prepared = execute::prepare_data_with_reader(query, &reader).unwrap(); let spec = &prepared.specs[0]; - let json_str = writer.write(spec, &prepared.data).unwrap(); + let json_str = writer.write_plot(spec, &prepared.data).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); let top_layers = vl_spec["layer"].as_array().unwrap(); @@ -3721,7 +3721,7 @@ mod tests { let prepared = execute::prepare_data_with_reader(query, &reader).unwrap(); let spec = &prepared.specs[0]; - let json_str = writer.write(spec, &prepared.data).unwrap(); + let json_str = writer.write_plot(spec, &prepared.data).unwrap(); let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); let top_layers = vl_spec["layer"].as_array().unwrap(); diff --git a/src/writer/vegalite/mod.rs b/src/writer/vegalite/mod.rs index 5e462048a..157771450 100644 --- a/src/writer/vegalite/mod.rs +++ b/src/writer/vegalite/mod.rs @@ -16,7 +16,7 @@ //! use ggsql::writer::{Writer, VegaLiteWriter}; //! //! let writer = VegaLiteWriter::new(); -//! let vega_json = writer.write(&spec, &dataframe)?; +//! let vega_json = writer.write_plot(&spec, &dataframe)?; //! // Can be rendered in browser with vega-embed //! ``` @@ -1075,9 +1075,9 @@ impl Writer for VegaLiteWriter { Ok(Self::new()) } - fn write(&self, spec: &Plot, data: &HashMap) -> Result { + fn write_plot(&self, spec: &Plot, data: &HashMap) -> Result { // 1. Validate spec - self.validate(spec)?; + self.validate_plot(spec)?; // 2. Determine layer data keys let layer_data_keys: Vec = spec @@ -1188,7 +1188,7 @@ impl Writer for VegaLiteWriter { }) } - fn validate(&self, spec: &Plot) -> Result<()> { + fn validate_plot(&self, spec: &Plot) -> Result<()> { // Check that we have at least one layer if spec.layers.is_empty() { return Err(GgsqlError::ValidationError( @@ -1478,7 +1478,7 @@ mod tests { fn test_validation_requires_layers() { let writer = VegaLiteWriter::new(); let spec = Plot::new(); - assert!(writer.validate(&spec).is_err()); + assert!(writer.validate_plot(&spec).is_err()); } #[test] @@ -1507,7 +1507,7 @@ mod tests { // Generate Vega-Lite JSON transform_spec(&mut spec); - let json_str = writer.write(&spec, &wrap_data(df)).unwrap(); + let json_str = writer.write_plot(&spec, &wrap_data(df)).unwrap(); assert_valid_vegalite(&json_str); let vl_spec: Value = serde_json::from_str(&json_str).unwrap(); @@ -1553,7 +1553,7 @@ mod tests { .unwrap(); transform_spec(&mut spec); - let json_str = writer.write(&spec, &wrap_data(df)).unwrap(); + let json_str = writer.write_plot(&spec, &wrap_data(df)).unwrap(); assert_valid_vegalite(&json_str); let vl_spec: Value = serde_json::from_str(&json_str).unwrap(); @@ -1591,7 +1591,7 @@ mod tests { let spec = &prepared.specs[0]; let writer = VegaLiteWriter::new(); - let json_str = writer.write(spec, &prepared.data).unwrap(); + let json_str = writer.write_plot(spec, &prepared.data).unwrap(); let vl_spec: Value = serde_json::from_str(&json_str).unwrap(); // Check title (should be object with text and subtitle) @@ -1696,7 +1696,7 @@ mod tests { .unwrap(); // Generate Vega-Lite JSON - let json_str = writer.write(&spec, &wrap_data(df)).unwrap(); + let json_str = writer.write_plot(&spec, &wrap_data(df)).unwrap(); let vl_spec: Value = serde_json::from_str(&json_str).unwrap(); // Verify fontsize maps to size channel @@ -1754,7 +1754,7 @@ mod tests { } .unwrap(); - let json_str = writer.write(&spec, &wrap_data(df)).unwrap(); + let json_str = writer.write_plot(&spec, &wrap_data(df)).unwrap(); let vl_spec: Value = serde_json::from_str(&json_str).unwrap(); let layer = &vl_spec["layer"][0]; @@ -1827,7 +1827,7 @@ mod tests { } .unwrap(); - let json_str = writer.write(&spec, &wrap_data(df)).unwrap(); + let json_str = writer.write_plot(&spec, &wrap_data(df)).unwrap(); let vl_spec: Value = serde_json::from_str(&json_str).unwrap(); let layer = &vl_spec["layer"][0]; @@ -1891,7 +1891,7 @@ mod tests { "y" => vec![1, 2], } .unwrap(); - let json_str = writer.write(&spec, &wrap_data(df)).unwrap(); + let json_str = writer.write_plot(&spec, &wrap_data(df)).unwrap(); let vl_spec: Value = serde_json::from_str(&json_str).unwrap(); vl_spec["layer"][0]["encoding"]["size"]["value"] .as_f64() @@ -1925,7 +1925,7 @@ mod tests { .unwrap(); transform_spec(&mut spec); - let json_str = writer.write(&spec, &wrap_data(df)).unwrap(); + let json_str = writer.write_plot(&spec, &wrap_data(df)).unwrap(); assert_valid_vegalite(&json_str); let vl_spec: Value = serde_json::from_str(&json_str).unwrap(); @@ -1955,7 +1955,7 @@ mod tests { } .unwrap(); - let result = writer.write(&spec, &wrap_data(df)); + let result = writer.write_plot(&spec, &wrap_data(df)); assert!(result.is_err()); let err = result.unwrap_err(); assert!(err.to_string().contains("nonexistent")); @@ -2056,7 +2056,9 @@ mod tests { .unwrap(); transform_spec(&mut spec); - let json_str = writer.write(&spec, &wrap_data_for_layers(df, 2)).unwrap(); + let json_str = writer + .write_plot(&spec, &wrap_data_for_layers(df, 2)) + .unwrap(); assert_valid_vegalite(&json_str); let vl_spec: Value = serde_json::from_str(&json_str).unwrap(); @@ -2167,7 +2169,7 @@ mod tests { .unwrap(); transform_spec(&mut spec); - let json_str = writer.write(&spec, &wrap_data(df)).unwrap(); + let json_str = writer.write_plot(&spec, &wrap_data(df)).unwrap(); assert_valid_vegalite(&json_str); let vl_spec: Value = serde_json::from_str(&json_str).unwrap(); @@ -2266,7 +2268,7 @@ mod tests { // Point geom without explicit size/stroke - should use defaults let spec = build_spec(Geom::point()); - let result = writer.write(&spec, &wrap_data(simple_df())); + let result = writer.write_plot(&spec, &wrap_data(simple_df())); assert!(result.is_ok()); let json_str = result.unwrap(); assert_valid_vegalite(&json_str); @@ -2295,7 +2297,7 @@ mod tests { layer.resolve_aesthetics(); spec.layers.push(layer); - let result = writer.write(&spec, &wrap_data(simple_df())); + let result = writer.write_plot(&spec, &wrap_data(simple_df())); assert!(result.is_ok()); let json_str = result.unwrap(); assert_valid_vegalite(&json_str); @@ -2339,7 +2341,7 @@ mod tests { } .unwrap(); - let result = writer.write(&spec, &wrap_data(df)); + let result = writer.write_plot(&spec, &wrap_data(df)); assert!(result.is_ok()); let json_str = result.unwrap(); assert_valid_vegalite(&json_str); @@ -2379,7 +2381,7 @@ mod tests { } .unwrap(); - let result = writer.write(&spec, &wrap_data(df)); + let result = writer.write_plot(&spec, &wrap_data(df)); assert!(result.is_ok()); let json_str = result.unwrap(); assert_valid_vegalite(&json_str); @@ -2404,7 +2406,7 @@ mod tests { layer.resolve_aesthetics(); spec.layers.push(layer); - let result = writer.write(&spec, &wrap_data(simple_df())); + let result = writer.write_plot(&spec, &wrap_data(simple_df())); assert!(result.is_ok()); let json_str = result.unwrap(); assert_valid_vegalite(&json_str); @@ -2424,7 +2426,7 @@ mod tests { // Line geom has linetype default of "solid" let spec = build_spec(Geom::line()); - let result = writer.write(&spec, &wrap_data(simple_df())); + let result = writer.write_plot(&spec, &wrap_data(simple_df())); assert!(result.is_ok()); let json_str = result.unwrap(); assert_valid_vegalite(&json_str); @@ -2842,7 +2844,7 @@ mod tests { .unwrap(); transform_spec(&mut spec); - let json_str = writer.write(&spec, &wrap_data(df)).unwrap(); + let json_str = writer.write_plot(&spec, &wrap_data(df)).unwrap(); assert_valid_vegalite(&json_str); let vl_spec: Value = serde_json::from_str(&json_str).unwrap(); @@ -2926,7 +2928,7 @@ mod tests { .unwrap(); transform_spec(&mut spec); - let json_str = writer.write(&spec, &wrap_data(df)).unwrap(); + let json_str = writer.write_plot(&spec, &wrap_data(df)).unwrap(); assert_valid_vegalite(&json_str); let vl_spec: Value = serde_json::from_str(&json_str).unwrap(); @@ -3007,7 +3009,7 @@ mod tests { .unwrap(); transform_spec(&mut spec); - let json_str = writer.write(&spec, &wrap_data(df)).unwrap(); + let json_str = writer.write_plot(&spec, &wrap_data(df)).unwrap(); assert_valid_vegalite(&json_str); let vl_spec: Value = serde_json::from_str(&json_str).unwrap(); @@ -3035,7 +3037,7 @@ mod tests { let mut spec = build_spec(Geom::point()); let df = simple_df(); transform_spec(&mut spec); - let json_str = writer.write(&spec, &wrap_data(df)).unwrap(); + let json_str = writer.write_plot(&spec, &wrap_data(df)).unwrap(); assert_valid_vegalite(&json_str); let invalid = r#"{"$schema": "https://vega.github.io/schema/vega-lite/v6.json", "mark": "not_a_mark"}"#; @@ -3101,7 +3103,7 @@ mod tests { .unwrap(); transform_spec(&mut spec); - let json_str = writer.write(&spec, &wrap_data(df)).unwrap(); + let json_str = writer.write_plot(&spec, &wrap_data(df)).unwrap(); let vl_spec: Value = serde_json::from_str(&json_str).unwrap(); for channel in ["x2", "y2"] { @@ -3164,7 +3166,7 @@ mod tests { transform_spec(&mut spec); - let msg = match writer.write(&spec, &wrap_data(df)) { + let msg = match writer.write_plot(&spec, &wrap_data(df)) { Err(GgsqlError::ValidationError(s)) => s, Err(other) => panic!("expected ValidationError, got: {}", other), Ok(_) => panic!("expected error, got success"), @@ -3206,7 +3208,7 @@ mod tests { transform_spec(&mut spec); - let msg = match writer.write(&spec, &wrap_data(df)) { + let msg = match writer.write_plot(&spec, &wrap_data(df)) { Err(GgsqlError::ValidationError(s)) => s, Err(other) => panic!("expected ValidationError, got: {}", other), Ok(_) => panic!("expected error, got success"), @@ -3245,7 +3247,7 @@ mod tests { transform_spec(&mut spec); - let msg = match writer.write(&spec, &wrap_data(df)) { + let msg = match writer.write_plot(&spec, &wrap_data(df)) { Err(GgsqlError::ValidationError(s)) => s, Err(other) => panic!("expected ValidationError, got: {}", other), Ok(_) => panic!("expected error, got success"), diff --git a/tree-sitter-ggsql/CLAUDE.md b/tree-sitter-ggsql/CLAUDE.md index 4c6ea1b75..c31624350 100644 --- a/tree-sitter-ggsql/CLAUDE.md +++ b/tree-sitter-ggsql/CLAUDE.md @@ -32,7 +32,7 @@ The files under `src/` are generated by `tree-sitter generate` from `grammar.js` ## Grammar at a glance -`grammar.js` defines a `query` rule that is `optional(sql_portion) + repeat(visualise_statement)`. The SQL portion is recognised structurally enough to know where it ends (statement boundaries, recursive subqueries, WITH compound statements) without re-implementing a SQL parser — the actual SQL is handed off to the configured `Reader`. The VISUALISE side is parsed in detail (clauses, layer types, mappings, settings, scales, facets, projections, labels) so [`/src/parser/builder.rs`](../src/parser/builder.rs) can build a typed AST from it. +`grammar.js` defines a `query` rule that is `optional(sql_portion) + repeat(choice(visualise_statement, tabulate_statement))`. The SQL portion is recognised structurally enough to know where it ends (statement boundaries, recursive subqueries, WITH compound statements) without re-implementing a SQL parser — the actual SQL is handed off to the configured `Reader`. The VISUALISE side is parsed in detail (clauses, layer types, mappings, settings, scales, facets, projections, labels) so [`/src/parser/builder.rs`](../src/parser/builder.rs) can build a typed AST from it. `tabulate_statement` (the plain-table counterpart to VISUALISE — a query result with no plot) is a **work in progress**: today it only recognises the keyword plus an optional `single_source_from` (the same single-source `FROM` rule VISUALISE uses), with no clauses of its own yet. For ggsql language semantics, see [`/doc/syntax/`](../doc/syntax/) — this package only defines *how text is parsed*, not what the resulting tree means. diff --git a/tree-sitter-ggsql/grammar.js b/tree-sitter-ggsql/grammar.js index 66c301ed4..293d0f564 100644 --- a/tree-sitter-ggsql/grammar.js +++ b/tree-sitter-ggsql/grammar.js @@ -26,10 +26,10 @@ module.exports = grammar({ ], rules: { - // Main entry point - SQL followed by VISUALISE statements + // Main entry point - SQL followed by VISUALISE/TABULATE statements query: $ => seq( optional($.sql_portion), - repeat($.visualise_statement) + repeat(choice($.visualise_statement, $.tabulate_statement)) ), // SQL portion - multiple statements separated by semicolons @@ -650,13 +650,14 @@ module.exports = grammar({ visualise_statement: $ => prec.dynamic(1, seq( $.visualise_keyword, optional($.global_mapping), - optional($.visualise_from), + optional($.single_source_from), repeat($.viz_clause) )), - // The VISUALISE-level source: exactly one table, CTE, or file path. - // Unlike a SQL FROM clause this admits no joins and no comma list. - visualise_from: $ => seq( + // The statement-level source shared by VISUALISE and TABULATE: exactly + // one table, CTE, or file path. Unlike a SQL FROM clause this admits no + // joins and no comma list. + single_source_from: $ => seq( token(prec(1, caseInsensitive('FROM'))), field('source', $.source_ref) ), @@ -667,6 +668,18 @@ module.exports = grammar({ caseInsensitive("VISUALIZE") ))), + // TABULATE — placeholder for tabular output, parallel to VISUALISE: an + // optional FROM right after the keyword, same single_source_from as + // visualise_statement (no joins, no comma list). No other clauses yet: + // the Table AST it builds has no fields to populate. + tabulate_statement: $ => prec.dynamic(1, seq( + $.tabulate_keyword, + optional($.single_source_from), + )), + + // TABULATE keyword as explicit high-precedence token (mirrors visualise_keyword) + tabulate_keyword: $ => token(prec(10, caseInsensitive("TABULATE"))), + // Shared mapping list: comma-separated mapping elements // Used by both global (VISUALISE) and layer (MAPPING) mappings mapping_list: $ => seq( diff --git a/tree-sitter-ggsql/test/corpus/basic.txt b/tree-sitter-ggsql/test/corpus/basic.txt index 881519acf..6652e9dd2 100644 --- a/tree-sitter-ggsql/test/corpus/basic.txt +++ b/tree-sitter-ggsql/test/corpus/basic.txt @@ -890,7 +890,7 @@ DRAW point (identifier (bare_identifier)))) name: (aesthetic_name))))) - (visualise_from + (single_source_from source: (qualified_name (identifier (bare_identifier)))) @@ -1138,7 +1138,7 @@ DRAW bar MAPPING category AS x, total AS y (query (visualise_statement (visualise_keyword) - (visualise_from + (single_source_from source: (qualified_name (identifier (bare_identifier)))) @@ -1664,7 +1664,7 @@ DRAW point MAPPING x AS x (bare_identifier)))))))))))) (visualise_statement (visualise_keyword) - (visualise_from + (single_source_from source: (qualified_name (identifier (bare_identifier)))) @@ -3110,7 +3110,7 @@ VISUALISE FROM data DRAW point (number)))))) (visualise_statement (visualise_keyword) - (visualise_from + (single_source_from (qualified_name (identifier (bare_identifier)))) @@ -3133,7 +3133,7 @@ VISUALISE FROM data DRAW point (insert_statement))) (visualise_statement (visualise_keyword) - (visualise_from + (single_source_from (qualified_name (identifier (bare_identifier)))) @@ -3156,7 +3156,7 @@ VISUALISE FROM data DRAW point (update_statement))) (visualise_statement (visualise_keyword) - (visualise_from + (single_source_from (qualified_name (identifier (bare_identifier)))) @@ -3179,7 +3179,7 @@ VISUALISE FROM data DRAW point (delete_statement))) (visualise_statement (visualise_keyword) - (visualise_from + (single_source_from (qualified_name (identifier (bare_identifier)))) @@ -3205,7 +3205,7 @@ VISUALISE FROM data DRAW point (insert_statement))) (visualise_statement (visualise_keyword) - (visualise_from + (single_source_from (qualified_name (identifier (bare_identifier)))) @@ -4536,3 +4536,36 @@ DRAW point SETTING bounds => [null, 1, Inf, -Inf], limit => inf (bare_identifier))) value: (parameter_value (infinity)))))))) + +================================================================================ +TABULATE bare keyword +================================================================================ + +SELECT 1 TABULATE + +-------------------------------------------------------------------------------- + +(query + (sql_portion + (sql_statement + (select_statement + (select_body + (number))))) + (tabulate_statement + (tabulate_keyword))) + +================================================================================ +TABULATE FROM single source +================================================================================ + +TABULATE FROM sales + +-------------------------------------------------------------------------------- + +(query + (tabulate_statement + (tabulate_keyword) + (single_source_from + source: (qualified_name + (identifier + (bare_identifier))))))
idname1