From 09bd1cc7bfa91a8f2d8c306f83ce3abb75ed1865 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Wed, 16 Sep 2026 12:45:12 +0200 Subject: [PATCH 1/8] Add TABULATE SPAN clause grammar and AST construction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SPAN groups columns under one spanner: `SPAN label ACROSS col, ... [SETTING ...]`, where label is a string (possibly empty) or NULL (suppress the cell, keep the grouping) — mirrors label_assignment's own string/NULL value shape. tabulate_statement now takes a repeated tab_clause (LABEL and SPAN, any order), the same "any order, repeated" shape viz_clause gives VISUALISE's own clauses. Table gains a `spans: Vec` field and Spanner::validate_settings, built the same way a GeomTrait declares default_params() — a static name/default/constraint list validated through the existing validate_parameter — so SPAN's SETTING keys (gather, level) get the same type checking layer settings already do. Co-Authored-By: Claude Sonnet 5 --- src/lib.rs | 2 +- src/parser/builder.rs | 108 ++++++++++++++++-- src/table/mod.rs | 101 ++++++++++++++++- tree-sitter-ggsql/CLAUDE.md | 2 +- tree-sitter-ggsql/grammar.js | 33 +++++- tree-sitter-ggsql/test/corpus/basic.txt | 141 +++++++++++++++++++++++- 6 files changed, 370 insertions(+), 17 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 0ff8b1b3..f0765166 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -72,7 +72,7 @@ pub use plot::{ // Re-export the parse-time Plot/Table result and the Table stub pub use spec::Spec; -pub use table::Table; +pub use table::{Spanner, Table}; // Re-export aesthetic classification utilities pub use plot::aesthetic::{ diff --git a/src/parser/builder.rs b/src/parser/builder.rs index bc5591e6..ce7ba04c 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, Spec, Table}; +use crate::{GgsqlError, Result, Spanner, Spec, Table}; use std::collections::HashMap; use tree_sitter::Node; @@ -354,7 +354,7 @@ fn build_tabulate_statement(node: &Node, source: &SourceTree) -> Result { table.source = Some(parse_data_source(&source_node, source)); } } - "label_clause" => { + "tab_clause" => { process_tab_clause(&child, source, &mut table)?; } _ => {} @@ -365,20 +365,53 @@ fn build_tabulate_statement(node: &Node, source: &SourceTree) -> Result
{ } /// Process a table clause node -// A single arm today, deliberately: this mirrors process_viz_clause's match -// shape so a second TABULATE clause (FACET/SCALE) only needs a new arm. -#[allow(clippy::single_match)] fn process_tab_clause(node: &Node, source: &SourceTree, table: &mut Table) -> Result<()> { - match node.kind() { - "label_clause" => { - table.labels = build_labels(node, source)?; + let mut cursor = node.walk(); + for child in node.children(&mut cursor) { + match child.kind() { + "label_clause" => { + table.labels = build_labels(&child, source)?; + } + "span_clause" => { + table.spans.push(build_span_clause(&child, source)?); + } + _ => {} } - _ => {} } Ok(()) } +/// Build a Spanner from a span_clause node: SPAN label ACROSS col, ... [SETTING ...] +fn build_span_clause(node: &Node, source: &SourceTree) -> Result { + let label_node = node.child_by_field_name("label").ok_or_else(|| { + GgsqlError::ParseError("Missing 'label' field in SPAN clause".to_string()) + })?; + let label = match label_node.kind() { + "string" => Some(parse_string_node(&label_node, source)), + "null_literal" => None, + _ => { + return Err(GgsqlError::ParseError(format!( + "SPAN label must be a string or null, got: {}", + label_node.kind() + ))); + } + }; + + let columns = source.find_texts(node, "(span_columns (identifier) @col)"); + + let settings = match source.find_node(node, "(setting_clause) @s") { + Some(setting_node) => parse_setting_clause(&setting_node, source)?, + None => Parameters::new(), + }; + + Ok(Spanner { + label, + columns, + settings, + }) +} + /// Process a visualization clause node fn process_viz_clause(node: &Node, source: &SourceTree, spec: &mut Plot) -> Result<()> { let mut cursor = node.walk(); @@ -1343,6 +1376,63 @@ mod tests { assert!(matches!(specs[1], Spec::Table(_))); } + #[test] + fn test_tabulate_span_basic() { + let specs = parse_test_specs("TABULATE FROM sales SPAN 'Pretty Name' ACROSS foo, bar, baz") + .unwrap(); + let table = specs[0].as_table().expect("expected a Table spec"); + + assert_eq!(table.spans.len(), 1); + assert_eq!(table.spans[0].label, Some("Pretty Name".to_string())); + assert_eq!(table.spans[0].columns, vec!["foo", "bar", "baz"]); + assert!(table.spans[0].settings.is_empty()); + } + + #[test] + fn test_tabulate_span_null_label_suppresses_the_cell() { + let specs = parse_test_specs("TABULATE FROM sales SPAN NULL ACROSS foo, bar").unwrap(); + let table = specs[0].as_table().expect("expected a Table spec"); + + assert_eq!(table.spans[0].label, None); + } + + #[test] + fn test_tabulate_span_empty_label_is_distinct_from_null() { + let specs = parse_test_specs("TABULATE FROM sales SPAN '' ACROSS foo, bar").unwrap(); + let table = specs[0].as_table().expect("expected a Table spec"); + + assert_eq!(table.spans[0].label, Some(String::new())); + } + + #[test] + fn test_tabulate_span_with_setting() { + let specs = + parse_test_specs("TABULATE FROM sales SPAN 'W' ACROSS foo SETTING width => '40%'") + .unwrap(); + let table = specs[0].as_table().expect("expected a Table spec"); + + assert_eq!( + table.spans[0].settings.get("width"), + Some(&ParameterValue::String("40%".to_string())) + ); + } + + #[test] + fn test_tabulate_multiple_spans_and_label_in_any_order() { + let specs = parse_test_specs( + "TABULATE FROM sales LABEL id => 'ID' SPAN 'A' ACROSS foo, bar SPAN 'B' ACROSS baz", + ) + .unwrap(); + let table = specs[0].as_table().expect("expected a Table spec"); + + assert_eq!(table.labels.labels.get("id"), Some(&Some("ID".to_string()))); + assert_eq!(table.spans.len(), 2); + assert_eq!(table.spans[0].label, Some("A".to_string())); + assert_eq!(table.spans[0].columns, vec!["foo", "bar"]); + assert_eq!(table.spans[1].label, Some("B".to_string())); + assert_eq!(table.spans[1].columns, vec!["baz"]); + } + // ======================================== // PROJECT Property Validation Tests // ======================================== diff --git a/src/table/mod.rs b/src/table/mod.rs index a88e81f2..4c7e9425 100644 --- a/src/table/mod.rs +++ b/src/table/mod.rs @@ -7,7 +7,9 @@ use serde::{Deserialize, Serialize}; -use crate::plot::Labels; +use crate::plot::{ + validate_parameter, DefaultParamValue, Labels, ParamConstraint, ParamDefinition, Parameters, +}; use crate::DataSource; /// Complete ggsql table specification. @@ -24,6 +26,11 @@ pub struct Table { /// unchanged, just keyed by column name instead of aesthetic name. An /// empty `Labels` means no overrides at all. pub labels: Labels, + /// Column spanners (from `TABULATE SPAN`), one per `SPAN` clause written + /// — a query with several `SPAN` clauses produces one `Spanner` each, + /// not one `Spanner` with several groups (`SPAN` itself never bundles + /// more than one group per clause). + pub spans: Vec, } impl Table { @@ -32,6 +39,7 @@ impl Table { Self { source: None, labels: Labels::default(), + spans: Vec::new(), } } } @@ -41,3 +49,94 @@ impl Default for Table { Self::new() } } + +/// One `SPAN` clause: a named group of columns rendered as one spanner cell +/// above them. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct Spanner { + /// Display text for the spanner cell. `None` is `SPAN NULL` — suppress + /// the spanner cell but keep the column grouping (e.g. for `settings` + /// that apply to the group regardless of whether it has a visible + /// label). `Some(String::new())` is `SPAN ''` — a present but blank + /// cell. Mirrors `label_assignment`'s own string/NULL value shape. + pub label: Option, + /// The columns this spanner covers, in the order written. + pub columns: Vec, + /// `SETTING` parameters for this spanner (e.g. `width => '40%'`). + pub settings: Parameters, +} + +/// `SETTING` parameters `SPAN` accepts — one static name/default/constraint +/// list, mirroring how a `GeomTrait` declares `default_params()`, so +/// `Spanner::validate_settings` doesn't hand-roll its own type checks per key. +const SPAN_PARAMS: &[ParamDefinition] = &[ + ParamDefinition { + name: "gather", + default: DefaultParamValue::Boolean(true), + constraint: ParamConstraint::boolean(), + }, + ParamDefinition { + name: "level", + // No default: absence means "assign automatically" (see + // assign_spanner_levels), not "assume some fixed level". + default: DefaultParamValue::Null, + constraint: ParamConstraint::count(1.0), + }, +]; + +impl Spanner { + /// Validate `settings` against `SPAN_PARAMS`, mirroring + /// `Layer::validate_settings`. + pub fn validate_settings(&self) -> Result<(), String> { + let valid: Vec<&str> = SPAN_PARAMS.iter().map(|p| p.name).collect(); + + for (name, value) in &self.settings { + // Key isn't in SPAN_PARAMS at all. + let Some(param) = SPAN_PARAMS.iter().find(|p| p.name == name.as_str()) else { + return Err(format!( + "SPAN setting should be {}, not '{}'", + crate::or_list_quoted(&valid, '\''), + name + )); + }; + // Known key, but wrong shape (e.g. `gather` not a boolean). + validate_parameter(name, value, ¶m.constraint)?; + } + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::plot::ParameterValue; + + fn spanner_with_settings(settings: Parameters) -> Spanner { + Spanner { + label: Some(String::new()), + columns: vec!["a".to_string()], + settings, + } + } + + #[test] + fn validate_settings_accepts_valid_gather_and_level() { + let mut settings = Parameters::new(); + settings.insert("gather".to_string(), ParameterValue::Boolean(false)); + settings.insert("level".to_string(), ParameterValue::Number(2.0)); + + assert!(spanner_with_settings(settings).validate_settings().is_ok()); + } + + #[test] + fn validate_settings_rejects_an_unknown_key() { + let mut settings = Parameters::new(); + settings.insert( + "width".to_string(), + ParameterValue::String("40%".to_string()), + ); + + assert!(spanner_with_settings(settings).validate_settings().is_err()); + } +} diff --git a/tree-sitter-ggsql/CLAUDE.md b/tree-sitter-ggsql/CLAUDE.md index c3162435..02ad2b58 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(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. +`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 still a **work in progress**: it recognises the keyword, an optional `single_source_from` (the same single-source `FROM` rule VISUALISE uses), and a repeated `tab_clause` — `LABEL` and `SPAN`, in any order, the same "any order, repeated" shape `viz_clause` gives VISUALISE's own clauses. Only `LABEL` is wired into the typed `Table` AST by [`/src/parser/builder.rs`](../src/parser/builder.rs) so far — `SPAN` parses but isn't consumed yet. (Revisit this paragraph's level of detail once most TABULATE clauses exist — describing two clauses inline is fine for now, but won't scale the way VISUALISE's own clause list needed a dedicated table instead.) 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 d36b5e21..2589905d 100644 --- a/tree-sitter-ggsql/grammar.js +++ b/tree-sitter-ggsql/grammar.js @@ -669,15 +669,46 @@ module.exports = grammar({ ))), // TABULATE — still incomplete, more clauses expected as Table grows. + // LABEL and SPAN clauses may repeat and appear in any order after the + // source, the same "any order, repeated" shape viz_clause gives VISUALISE + // — only single_source_from is fixed in position. tabulate_statement: $ => prec.dynamic(1, seq( $.tabulate_keyword, optional($.single_source_from), - optional($.label_clause), + repeat($.tab_clause) )), // TABULATE keyword as explicit high-precedence token (mirrors visualise_keyword) tabulate_keyword: $ => token(prec(10, caseInsensitive("TABULATE"))), + // All the TABULATE clauses (mirrors viz_clause's role for VISUALISE). + tab_clause: $ => choice( + $.label_clause, + $.span_clause, + ), + + // SPAN — groups columns under one spanner cell. Multiple spanners repeat + // the whole clause (SPAN ... SPAN ...), the same model DRAW/SCALE use for + // more than one instance — not a comma list inside one SPAN. + span_clause: $ => seq( + caseInsensitive('SPAN'), + // Mandatory: a string sets the cell's text (possibly '', a + // present-but-blank cell); NULL suppresses the cell while still + // grouping the columns (e.g. for a shared SETTING like width). + // Reuses label_assignment's value shape. + field('label', choice($.string, $.null_literal)), + caseInsensitive('ACROSS'), + $.span_columns, + optional($.setting_clause) + ), + + // The group of columns this SPAN covers — same plain identifier-list + // shape as partition_columns/facet_vars/project_aesthetics. + span_columns: $ => seq( + $.identifier, + repeat(seq(',', $.identifier)) + ), + // 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 dc6d7d81..8afc0403 100644 --- a/tree-sitter-ggsql/test/corpus/basic.txt +++ b/tree-sitter-ggsql/test/corpus/basic.txt @@ -4585,9 +4585,142 @@ TABULATE FROM sales LABEL id => 'ID' source: (qualified_name (identifier (bare_identifier)))) - (label_clause - (label_assignment - name: (label_type + (tab_clause + (label_clause + (label_assignment + name: (label_type + (identifier + (bare_identifier))) + value: (string)))))) + +================================================================================ +TABULATE SPAN clause +================================================================================ + +TABULATE FROM sales SPAN 'Pretty Name' ACROSS foo, bar, baz + +-------------------------------------------------------------------------------- + +(query + (tabulate_statement + (tabulate_keyword) + (single_source_from + source: (qualified_name + (identifier + (bare_identifier)))) + (tab_clause + (span_clause + label: (string) + (span_columns + (identifier + (bare_identifier)) + (identifier + (bare_identifier)) + (identifier + (bare_identifier))))))) + +================================================================================ +TABULATE SPAN clause with a single column +================================================================================ + +TABULATE FROM sales SPAN 'A' ACROSS foo + +-------------------------------------------------------------------------------- + +(query + (tabulate_statement + (tabulate_keyword) + (single_source_from + source: (qualified_name + (identifier + (bare_identifier)))) + (tab_clause + (span_clause + label: (string) + (span_columns + (identifier + (bare_identifier))))))) + +================================================================================ +TABULATE SPAN clause with a NULL label +================================================================================ + +TABULATE FROM sales SPAN NULL ACROSS foo, bar + +-------------------------------------------------------------------------------- + +(query + (tabulate_statement + (tabulate_keyword) + (single_source_from + source: (qualified_name + (identifier + (bare_identifier)))) + (tab_clause + (span_clause + label: (null_literal) + (span_columns + (identifier + (bare_identifier)) + (identifier + (bare_identifier))))))) + +================================================================================ +TABULATE SPAN clause with a SETTING +================================================================================ + +TABULATE FROM sales SPAN 'W' ACROSS foo SETTING width => '40%' + +-------------------------------------------------------------------------------- + +(query + (tabulate_statement + (tabulate_keyword) + (single_source_from + source: (qualified_name + (identifier + (bare_identifier)))) + (tab_clause + (span_clause + label: (string) + (span_columns (identifier (bare_identifier))) - value: (string))))) + (setting_clause + (parameter_assignment + name: (parameter_name + (identifier + (bare_identifier))) + value: (parameter_value + (string)))))))) + +================================================================================ +TABULATE LABEL then SPAN clause, any order +================================================================================ + +TABULATE FROM sales LABEL id => 'ID' SPAN 'A' ACROSS foo, bar + +-------------------------------------------------------------------------------- + +(query + (tabulate_statement + (tabulate_keyword) + (single_source_from + source: (qualified_name + (identifier + (bare_identifier)))) + (tab_clause + (label_clause + (label_assignment + name: (label_type + (identifier + (bare_identifier))) + value: (string)))) + (tab_clause + (span_clause + label: (string) + (span_columns + (identifier + (bare_identifier)) + (identifier + (bare_identifier))))))) From ea51b1cb3cf6bcdeab78331630fa377b93ba9c65 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Wed, 16 Sep 2026 13:59:59 +0200 Subject: [PATCH 2/8] Resolve TABULATE SPAN into positioned spanner cells MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires SPAN into table resolution: build_cells validates each spanner's settings, reorders columns so gather-enabled spanners become contiguous (gt's tab_spanner(gather = TRUE) default), assigns each spanner a header row (explicit SETTING level pins directly, others greedily bump past any column-set intersection, then levels are compacted), and RLE-builds one TableCell per contiguous run of a spanner's columns — fragmenting a non-contiguous spanner into several cells. NULL-labeled spanners are filtered out before level assignment so they group columns without consuming a header row. Spanner-specific resolution (reordering, level assignment, cell construction) lives in the new sibling module execute/table_spanner.rs rather than growing table.rs further, mirroring how Plot's own resolution logic is split across schema.rs/casting.rs/layer.rs/scale.rs/position.rs/ cte.rs. validate() now also runs Spanner::validate_settings per SPAN clause, the same way layer SETTINGs are validated, so a bad SPAN setting is caught before execution. Co-Authored-By: Claude Sonnet 5 --- src/CLAUDE.md | 2 +- src/execute/mod.rs | 2 + src/execute/table.rs | 326 +++++++++++++++++++---- src/execute/table_spanner.rs | 500 +++++++++++++++++++++++++++++++++++ src/table/mod.rs | 8 +- src/validate.rs | 24 +- 6 files changed, 800 insertions(+), 62 deletions(-) create mode 100644 src/execute/table_spanner.rs diff --git a/src/CLAUDE.md b/src/CLAUDE.md index 3ee6a254..82d02e53 100644 --- a/src/CLAUDE.md +++ b/src/CLAUDE.md @@ -24,7 +24,7 @@ src/ │ ├── 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) +├── table/ AST for TABULATE, parallel to plot/ (source, labels, spans) ├── reader/ Reader trait + drivers (DuckDB, SQLite, ODBC, Snowflake, …) ├── execute/ Pipeline that turns Plot + Reader → ResolvedPlot ├── writer/ Writer trait + Vega-Lite implementation (see writer/vegalite/CLAUDE.md) diff --git a/src/execute/mod.rs b/src/execute/mod.rs index 456251f4..eef00362 100644 --- a/src/execute/mod.rs +++ b/src/execute/mod.rs @@ -10,6 +10,7 @@ //! - `layer`: Layer query building, data transforms, and stat application //! - `scale`: Scale creation, resolution, type coercion, and OOB handling //! - `table`: Table (TABULATE) resolution +//! - `table_spanner`: `TABULATE SPAN` resolution, called from `table` mod casting; mod cte; @@ -18,6 +19,7 @@ mod position; mod scale; mod schema; mod table; +mod table_spanner; // Re-export public API pub use casting::TypeRequirement; diff --git a/src/execute/table.rs b/src/execute/table.rs index 1b01a34d..0bbb9914 100644 --- a/src/execute/table.rs +++ b/src/execute/table.rs @@ -2,18 +2,19 @@ //! //! 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`, plus (as `Table` grows headings/spanners/footnotes) -//! resolving that data into positioned `TableCell`s. As those concerns grow -//! they're expected to split into sibling files here, the way Plot's own +//! produces `body`, plus resolving that data into positioned `TableCell`s. +//! `SPAN`-specific resolution (column reordering, header-row assignment) +//! lives in the sibling `table_spanner` module, the way Plot's own //! resolution logic is split across `schema.rs`/`casting.rs`/`layer.rs`/ //! `scale.rs`/`position.rs`/`cte.rs` rather than left in one file. +use super::table_spanner::{create_spanners, reorder_table_columns}; use crate::array_util::value_to_string; use crate::parser::{self, SourceTree}; use crate::plot::Labels; use crate::reader::{Reader, ResolvedTable}; use crate::validate::{validate, ValidationWarning}; -use crate::{DataFrame, GgsqlError, Result, Spec}; +use crate::{DataFrame, GgsqlError, Result, Spec, Table}; /// Resolve a TABULATE query into a `ResolvedTable`. /// @@ -57,13 +58,33 @@ pub fn resolve_table_with_reader(query: &str, reader: &dyn Reader) -> Result Result> { + for (idx, spanner) in table.spans.iter().enumerate() { + spanner + .validate_settings() + .map_err(|e| GgsqlError::ValidationError(format!("SPAN {}: {}", idx + 1, e)))?; + } + + let columns = create_table_columns(df, &table.labels); + let columns = reorder_table_columns(columns, &table.spans)?; + let spanners = create_spanners(&columns, &table.spans)?; let column_labels = create_column_labels(&columns); - let table_body = create_body(&df, &columns); - let cells = compose_cells(column_labels, table_body); + let header = compose_header(spanners, column_labels); + let table_body = create_body(df, &columns); + let cells = rowbind_cells(header, table_body); validate_overlaps(&cells)?; - Ok(ResolvedTable::new(table, cells, sql, warnings)) + Ok(cells) } /// One column's identity within a table layout: its source name and resolved @@ -76,21 +97,18 @@ pub fn resolve_table_with_reader(query: &str, reader: &dyn Reader) -> Result NULL` empties the label; -/// `LABEL col => 'text'` sets it to `text`. +/// order — `reorder_table_columns` is what may reorder this list +/// afterward, not this function. `labels` (from a `TABULATE LABEL` clause) +/// is the one authority for a column's label. fn create_table_columns(df: &DataFrame, labels: &Labels) -> Vec { df.get_column_names() .into_iter() @@ -108,8 +126,9 @@ fn create_table_columns(df: &DataFrame, labels: &Labels) -> Vec { /// Build one `ColumnLabel` cell per column, numbered from `top == 0`, in /// `columns`' order. /// -/// Row numbering here is local to this function alone — `compose_cells` -/// is what decides where this sits relative to the body, not this function. +/// Row numbering here is local to this function alone — `compose_header`/ +/// `rowbind_cells` are what decide where this sits relative to spanners and +/// the body, not this function. fn create_column_labels(columns: &[TableColumn]) -> Vec { columns .iter() @@ -156,46 +175,63 @@ fn create_body(df: &DataFrame, columns: &[TableColumn]) -> Vec { cells } -/// Compose a column-label row and a body into one layout: a pure function of -/// its two arguments, with no `DataFrame`/SQL knowledge of its own. Shifts -/// `body` down by however many rows `column_labels` occupies — today always -/// one, but this is what lets the two stay ignorant of each other's size. As -/// `Table` grows (headings/spanners/footnotes), more of these intermediate -/// composers are expected alongside this one (e.g. for a header or footer), -/// each combining a subset of parts the same way. -fn compose_cells(column_labels: Vec, mut body: Vec) -> Vec { - let row_offset = column_labels +/// Stack `bottom` below `top`, offsetting `bottom` down by whatever row +/// extent `top` actually occupies — a pure function of its two arguments, +/// with no `DataFrame`/SQL knowledge of its own, and no assumption about +/// either side's row count. R's `rbind()` for already-positioned cells: +/// every part of the table (spanners, column labels, body, ...) gets +/// stacked together with the same primitive rather than each combination +/// hardcoding its own offset arithmetic. +fn rowbind_cells(top: Vec, mut bottom: Vec) -> Vec { + if top.is_empty() { + return bottom; + } + if bottom.is_empty() { + return top; + } + + let row_offset = top .iter() .map(|cell| cell.bottom) .max() .map_or(0, |bottom| bottom + 1); - for cell in &mut body { + for cell in &mut bottom { cell.offset_rows(row_offset); } - let mut cells = column_labels; - cells.extend(body); + let mut cells = top; + cells.extend(bottom); cells } +/// Stack spanner rows above column labels — the header half of a table's +/// layout. Still just one `rowbind_cells` call today, but kept as its own +/// named step since a stubhead (another header part) is expected to join it. +fn compose_header(spanners: Vec, column_labels: Vec) -> Vec { + rowbind_cells(spanners, column_labels) +} + /// Check that no two cells in a resolved layout claim the same grid position. /// /// Walks every cell's full footprint (`top..=bottom` × `left..=right`, not -/// just its corners) into a set of occupied positions, erroring as soon as a -/// position is claimed twice. `O(total cell area)` rather than the O(n²) cost -/// of comparing every pair of cells — cheap for the common case (one 1x1 -/// cell per data value, so area == cell count) and only grows with the +/// just its corners) into a map of occupied positions to the `TableCellKind` +/// that claimed each one, erroring — naming both kinds involved — as soon as +/// a position is claimed twice. `O(total cell area)` rather than the O(n²) +/// cost of comparing every pair of cells — cheap for the common case (one +/// 1x1 cell per data value, so area == cell count) and only grows with the /// footprint spanning cells actually cover, not with `cells.len()` squared. fn validate_overlaps(cells: &[TableCell]) -> Result<()> { - let mut occupied = std::collections::HashSet::new(); + let mut occupied: std::collections::HashMap<(usize, usize), TableCellKind> = + std::collections::HashMap::new(); for cell in cells { for row in cell.top..=cell.bottom { for col in cell.left..=cell.right { - if !occupied.insert((row, col)) { + if let Some(existing_kind) = occupied.insert((row, col), cell.kind) { return Err(GgsqlError::ValidationError(format!( - "Table layout has more than one cell at row {row}, column {col}" + "Table layout has a {existing_kind} cell and a {} cell clashing at row {row}, column {col}", + cell.kind ))); } } @@ -226,6 +262,20 @@ pub enum TableCellKind { ColumnLabel, /// A data value (gt's `body`). Body, + /// A spanner cell, grouping several columns under one label (`TABULATE + /// SPAN`). + Spanner, +} + +impl std::fmt::Display for TableCellKind { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let text = match self { + TableCellKind::ColumnLabel => "column label", + TableCellKind::Body => "body", + TableCellKind::Spanner => "spanner", + }; + write!(f, "{text}") + } } /// A single positioned cell within a resolved table layout. @@ -273,12 +323,37 @@ impl TableCell { self.left += cols; self.right += cols; } + + /// Whether this cell belongs in a table's header (`ColumnLabel`, + /// `Spanner`) rather than its body (`Body`) — lets a writer pick `/ (each omitted when empty), and renders each row through the new render_row: cells sharing a top walk every column position, emitting a colspan attribute via TableCell::width() and filling any gap (a spanner not reaching every column, or one fragmented by another spanner's occupancy) with a synthesized empty cell of the same kind, so a row always reaches the table's full column count. Co-Authored-By: Claude Sonnet 5 --- src/writer/html.rs | 172 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 151 insertions(+), 21 deletions(-) diff --git a/src/writer/html.rs b/src/writer/html.rs index 8faa3cab..ccc2780a 100644 --- a/src/writer/html.rs +++ b/src/writer/html.rs @@ -1,9 +1,12 @@ //! A minimal HTML table writer. //! //! Renders a `ResolvedTable`'s cells as a bare `
` + /// vs `` (or an equivalent) off the cell itself, without matching on + /// `TableCellKind` at every call site. + pub fn is_header(&self) -> bool { + matches!( + self.kind, + TableCellKind::ColumnLabel | TableCellKind::Spanner + ) + } + + /// This cell's width in columns — `right - left + 1`, since both bounds + /// are inclusive. + pub fn width(&self) -> usize { + self.right - self.left + 1 + } + + /// This cell's height in rows — `bottom - top + 1`, since both bounds + /// are inclusive. No caller yet; kept alongside `width` for symmetry. + pub fn height(&self) -> usize { + self.bottom - self.top + 1 + } } #[cfg(test)] mod layout_tests { use super::*; use crate::df; + use crate::plot::{ParameterValue, Parameters}; + use crate::Spanner; fn column(name: &str, label: &str) -> TableColumn { TableColumn { @@ -311,6 +386,28 @@ mod layout_tests { assert_eq!(columns[2].label, "extra"); // absent: kept as-is } + fn spanner_with(columns: &[&str], label: Option<&str>, settings: Parameters) -> Spanner { + Spanner { + label: label.map(str::to_string), + columns: columns.iter().map(|s| s.to_string()).collect(), + settings, + } + } + + fn spanner(columns: &[&str]) -> Spanner { + spanner_with(columns, Some(""), Parameters::new()) + } + + fn spanner_with_setting(columns: &[&str], key: &str, value: ParameterValue) -> Spanner { + let mut settings = Parameters::new(); + settings.insert(key.to_string(), value); + spanner_with(columns, Some(""), settings) + } + + fn labeled_spanner(columns: &[&str], label: &str) -> Spanner { + spanner_with(columns, Some(label), Parameters::new()) + } + #[test] fn create_column_labels_builds_one_cell_per_column_at_row_zero() { let columns = vec![column("id", "id"), column("name", "name")]; @@ -392,14 +489,14 @@ mod layout_tests { } #[test] - fn compose_cells_shifts_the_body_below_a_single_label_row() { + fn rowbind_cells_shifts_the_bottom_below_a_single_top_row() { let column_labels = vec![cell(TableCellKind::ColumnLabel, 0, 0, "id")]; let body = vec![ cell(TableCellKind::Body, 0, 0, "1"), cell(TableCellKind::Body, 1, 1, "2"), ]; - let cells = compose_cells(column_labels, body); + let cells = rowbind_cells(column_labels, body); assert_eq!(cells.len(), 3); assert_eq!(cells[0].kind, TableCellKind::ColumnLabel); @@ -412,19 +509,39 @@ mod layout_tests { } #[test] - fn compose_cells_offsets_by_the_label_rows_actual_extent_not_a_hardcoded_one() { + fn rowbind_cells_offsets_by_the_top_rows_actual_extent_not_a_hardcoded_one() { // Nothing produces a multi-row column-label section today, but - // `compose_cells` computes the offset from `column_labels` itself - // rather than assuming exactly one row — pin that down directly. + // `rowbind_cells` computes the offset from `top` itself rather than + // assuming exactly one row — pin that down directly. let column_labels = vec![cell(TableCellKind::ColumnLabel, 0, 1, "id")]; let body = vec![cell(TableCellKind::Body, 0, 0, "1")]; - let cells = compose_cells(column_labels, body); + let cells = rowbind_cells(column_labels, body); assert_eq!(cells[1].top, 2); assert_eq!(cells[1].bottom, 2); } + #[test] + fn rowbind_cells_returns_bottom_unchanged_when_top_is_empty() { + let body = vec![cell(TableCellKind::Body, 0, 0, "1")]; + + let cells = rowbind_cells(Vec::new(), body); + + assert_eq!(cells.len(), 1); + assert_eq!(cells[0].top, 0); + } + + #[test] + fn rowbind_cells_returns_top_unchanged_when_bottom_is_empty() { + let column_labels = vec![cell(TableCellKind::ColumnLabel, 0, 0, "id")]; + + let cells = rowbind_cells(column_labels, Vec::new()); + + assert_eq!(cells.len(), 1); + assert_eq!(cells[0].top, 0); + } + fn cell_at( kind: TableCellKind, top: usize, @@ -462,9 +579,8 @@ mod layout_tests { ]; let error = validate_overlaps(&cells).unwrap_err(); - assert!( - matches!(error, GgsqlError::ValidationError(msg) if msg.contains("row 0, column 0")) - ); + assert!(matches!(error, GgsqlError::ValidationError(msg) + if msg.contains("row 0, column 0") && msg.contains("body cell and a body cell"))); } #[test] @@ -475,10 +591,101 @@ mod layout_tests { // disjoint labels/body can create on their own. let cells = vec![ cell_at(TableCellKind::ColumnLabel, 0, 0, 0, 1), - cell_at(TableCellKind::Body, 0, 0, 1, 1), + cell_at(TableCellKind::Spanner, 0, 0, 1, 1), ]; - assert!(validate_overlaps(&cells).is_err()); + let error = validate_overlaps(&cells).unwrap_err(); + assert!(matches!(error, GgsqlError::ValidationError(msg) + if msg.contains("column label cell and a spanner cell"))); + } + + #[test] + fn table_cell_kind_display_names_all_three_variants() { + assert_eq!(TableCellKind::ColumnLabel.to_string(), "column label"); + assert_eq!(TableCellKind::Body.to_string(), "body"); + assert_eq!(TableCellKind::Spanner.to_string(), "spanner"); + } + + #[test] + fn build_cells_composes_spanners_labels_and_body_together() { + let frame = df! { + "a" => vec![1i32], + "b" => vec![2i32], + } + .unwrap(); + let mut table = Table::new(); + table.spans = vec![labeled_spanner(&["a", "b"], "G")]; + + let cells = build_cells(&frame, &table).unwrap(); + + let spanner_cell = cells + .iter() + .find(|c| c.kind == TableCellKind::Spanner) + .unwrap(); + assert_eq!(spanner_cell.top, 0); + assert_eq!(spanner_cell.left, 0); + assert_eq!(spanner_cell.right, 1); + assert_eq!(spanner_cell.content, "G"); + + assert!(cells + .iter() + .filter(|c| c.kind == TableCellKind::ColumnLabel) + .all(|c| c.top == 1)); + assert!(cells + .iter() + .filter(|c| c.kind == TableCellKind::Body) + .all(|c| c.top == 2)); + } + + #[test] + fn build_cells_reorders_columns_for_a_spanner_before_building_labels_and_body() { + let frame = df! { + "a" => vec![1i32], + "x" => vec![9i32], + "b" => vec![2i32], + } + .unwrap(); + let mut table = Table::new(); + table.spans = vec![labeled_spanner(&["a", "b"], "G")]; + + let cells = build_cells(&frame, &table).unwrap(); + + let mut label_cells: Vec<_> = cells + .iter() + .filter(|c| c.kind == TableCellKind::ColumnLabel) + .collect(); + label_cells.sort_by_key(|c| c.left); + let label_order: Vec<&str> = label_cells.iter().map(|c| c.content.as_str()).collect(); + assert_eq!(label_order, vec!["a", "b", "x"]); + + let mut body_cells: Vec<_> = cells + .iter() + .filter(|c| c.kind == TableCellKind::Body) + .collect(); + body_cells.sort_by_key(|c| c.left); + let body_order: Vec<&str> = body_cells.iter().map(|c| c.content.as_str()).collect(); + assert_eq!(body_order, vec!["1", "2", "9"]); + } + + #[test] + fn build_cells_surfaces_a_genuine_spanner_overlap_as_an_error() { + // (a,b) auto-assigns level 1; (b,c) is explicitly pinned to level 1 + // too, and neither reordering nor level assignment resolves that — + // build_cells should surface this via validate_overlaps, not + // silently produce a broken layout. + let frame = df! { + "a" => vec![1i32], + "b" => vec![2i32], + "c" => vec![3i32], + } + .unwrap(); + let mut table = Table::new(); + table.spans = vec![ + spanner(&["a", "b"]), + spanner_with_setting(&["b", "c"], "level", ParameterValue::Number(1.0)), + ]; + + assert!(build_cells(&frame, &table).is_err()); } } @@ -529,11 +736,26 @@ mod tests { 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. + // VISUALISE's FROM. let reader = reader_with_sales(); let result = resolve_table_with_reader("TABULATE VISUALISE FROM sales DRAW point", &reader); assert!(result.is_err()); } + + #[test] + fn test_tabulate_label_applies_under_the_tab_clause_wrapper() { + // label_clause is nested under tab_clause in the grammar (so LABEL + // and SPAN can appear in any order) — build_tabulate_statement must + // unwrap it, not match "label_clause" as a direct child. + let reader = reader_with_sales(); + let resolved = + resolve_table_with_reader("TABULATE FROM sales LABEL id => 'ID'", &reader).unwrap(); + + let label_cell = resolved + .cells() + .iter() + .find(|c| c.kind == TableCellKind::ColumnLabel && c.left == 0) + .unwrap(); + assert_eq!(label_cell.content, "ID"); + } } diff --git a/src/execute/table_spanner.rs b/src/execute/table_spanner.rs new file mode 100644 index 00000000..31767e69 --- /dev/null +++ b/src/execute/table_spanner.rs @@ -0,0 +1,500 @@ +//! `TABULATE SPAN` resolution: column reordering (`gather`) and header-row +//! (level) assignment for spanners, called from `table::build_cells`. + +use super::table::TableColumn; +use crate::{GgsqlError, Result, Spanner, TableCell, TableCellKind}; + +/// Reorder `columns` so every `gather`-enabled spanner's members become +/// contiguous, folding spanners in `spans`' order (declaration order) — +/// mirrors gt's `tab_spanner(gather = TRUE)`, which is the default there and +/// here. `SETTING gather => false` opts a spanner out of this entirely, +/// leaving its columns wherever they land. +/// +/// This only ever touches column *order* — it says nothing about which +/// spanner ends up on which header row. Two spanners can both gather +/// successfully and still need separate rows (their column ranges can +/// overlap even once each is individually contiguous); that's level +/// assignment, a separate, later concern this function doesn't address. +/// +/// Trusts `gather`'s type without checking it — `Spanner::validate_settings` +/// (called upstream, in both the standalone `validate()` path and +/// `build_cells`, mirroring `Layer::validate_settings`) already rejected a +/// non-boolean value before this ever runs. "Does this spanner name a real +/// column" is a different kind of check (referential, needs `columns` +/// itself as context, not just the setting's shape) and stays inline in +/// `gather_columns`. +pub(crate) fn reorder_table_columns( + mut columns: Vec, + spans: &[Spanner], +) -> Result> { + for span in spans { + let gather = span + .settings + .get("gather") + .and_then(|value| value.as_bool()) + .unwrap_or(true); + + if gather { + columns = gather_columns(columns, &span.columns)?; + } + } + + Ok(columns) +} + +/// Move `members` (in the order given) to sit contiguously where the first +/// of them currently is, pulling the rest in around it — the minimal move +/// that unifies them, leaving every other column's relative order untouched. +/// Errors if `members` names a column not present in `columns` at all. +fn gather_columns(columns: Vec, members: &[String]) -> Result> { + let Some(anchor) = members.first() else { + return Ok(columns); + }; + + let anchor_index = columns + .iter() + .position(|c| &c.name == anchor) + .ok_or_else(|| { + GgsqlError::ValidationError(format!("SPAN references unknown column '{anchor}'")) + })?; + let insertion_index = columns[..anchor_index] + .iter() + .filter(|c| !members.contains(&c.name)) + .count(); + + let mut remainder = Vec::with_capacity(columns.len()); + let mut by_name: std::collections::HashMap = + std::collections::HashMap::new(); + for column in columns { + if members.contains(&column.name) { + by_name.insert(column.name.clone(), column); + } else { + remainder.push(column); + } + } + + let mut result: Vec = remainder.drain(..insertion_index).collect(); + for name in members { + let column = by_name.remove(name).ok_or_else(|| { + GgsqlError::ValidationError(format!("SPAN references unknown column '{name}'")) + })?; + result.push(column); + } + result.extend(remainder); + + Ok(result) +} + +/// Assign each spanner a 1-indexed level (header row), matching gt's model: +/// an explicit `SETTING level => N` pins a spanner there directly — no +/// conflict check, even against another spanner already at that level; +/// `validate_overlaps` catches a genuine clash once real cells exist, the +/// same way it catches any other overlapping `TableCell`. A spanner with no +/// explicit level is assigned greedily instead (see the `None` arm below). +/// +/// Levels are then compacted to remove gaps a mix of explicit levels can +/// leave behind (e.g. 1, 3, 4 → 1, 2, 3) — a spanner's level only matters +/// relative to the others, not its literal number, so gaps would just waste +/// header rows. +/// +/// Infallible: `level`'s type/shape is already checked by +/// `Spanner::validate_settings` before this ever runs. +fn assign_spanner_levels(spans: &[Spanner]) -> Vec { + let mut levels: Vec = Vec::with_capacity(spans.len()); + + for span in spans { + let level = match span.settings.get("level") { + Some(value) => value + .as_number() + .expect("Spanner::validate_settings already checked 'level' is a number") + as usize, + None => { + // Bump the candidate level while it's shared with an + // already-assigned spanner whose columns intersect this + // one's — any overlap (crossing, nesting, or identical sets) + // would render as two rectangles claiming the same column, + // so "intersects at all" is the only test needed. + let mut candidate = 1; + while spans.iter().zip(&levels).any(|(other, &other_level)| { + other_level == candidate + && other.columns.iter().any(|c| span.columns.contains(c)) + }) { + candidate += 1; + } + candidate + } + }; + levels.push(level); + } + + // Compact: gaps left by explicit levels (e.g. 1, 3, 4) collapse to a + // dense range (1, 2, 3), inline rather than a separate helper since + // nothing else needs this in isolation. + let mut distinct = levels.clone(); + distinct.sort_unstable(); + distinct.dedup(); + + levels + .into_iter() + .map(|level| distinct.binary_search(&level).unwrap() + 1) + .collect() +} + +/// Build one `TableCell` per contiguous run of a spanner's columns, calling +/// `assign_spanner_levels` itself. Numbered locally from `top == 0`, the +/// same convention `create_column_labels`/`create_body` use; stitching +/// these rows above column labels and body is a separate, later step. +pub(crate) fn create_spanners( + columns: &[TableColumn], + spans: &[Spanner], +) -> Result> { + if spans.is_empty() { + return Ok(Vec::new()); + } + + // Filter out spanners with `null` labels. They don't contribute to cells + // so their level is irrellevant and shouldn't affect other levels. + let spans: Vec = spans + .iter() + .filter(|s| s.label.is_some()) + .cloned() + .collect(); + + let levels = assign_spanner_levels(&spans); + let max_level = levels.iter().copied().max().unwrap_or(0); + + let mut cells = Vec::new(); + + for (span, &level) in spans.iter().zip(&levels) { + let label = span + .label + .as_ref() + .expect("spans is filtered to only Some(label) spanners"); + // Row 0 is topmost. Level 1 is bottom-most. + let row = max_level - level; + + // We use run length encoding to find 'runs' of columns belonging to span. + // If span has disjoint columns, these are multiple runs. + let mut run_start = None; + for (index, column) in columns.iter().enumerate() { + // Does column belong to span? + let in_span = span.columns.contains(&column.name); + match (in_span, run_start) { + // Found column in span, initiate new run + (true, None) => run_start = Some(index), + // Column not in span, end run and push cell + (false, Some(start)) => { + cells.push(TableCell { + kind: TableCellKind::Spanner, + top: row, + bottom: row, + left: start, + right: index - 1, + content: label.clone(), + }); + run_start = None; + } + _ => {} + } + } + // Started but not ended: last column + if let Some(start) = run_start { + cells.push(TableCell { + kind: TableCellKind::Spanner, + top: row, + bottom: row, + left: start, + right: columns.len() - 1, + content: label.clone(), + }); + } + } + + Ok(cells) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::plot::{ParameterValue, Parameters}; + + fn column(name: &str, label: &str) -> TableColumn { + TableColumn { + name: name.to_string(), + label: label.to_string(), + } + } + + fn names(columns: &[TableColumn]) -> Vec<&str> { + columns.iter().map(|c| c.name.as_str()).collect() + } + + fn spanner_with(columns: &[&str], label: Option<&str>, settings: Parameters) -> Spanner { + Spanner { + label: label.map(str::to_string), + columns: columns.iter().map(|s| s.to_string()).collect(), + settings, + } + } + + fn spanner(columns: &[&str]) -> Spanner { + spanner_with(columns, Some(""), Parameters::new()) + } + + fn spanner_with_setting(columns: &[&str], key: &str, value: ParameterValue) -> Spanner { + let mut settings = Parameters::new(); + settings.insert(key.to_string(), value); + spanner_with(columns, Some(""), settings) + } + + fn labeled_spanner(columns: &[&str], label: &str) -> Spanner { + spanner_with(columns, Some(label), Parameters::new()) + } + + fn null_spanner(columns: &[&str]) -> Spanner { + spanner_with(columns, None, Parameters::new()) + } + + #[test] + fn gather_columns_moves_members_next_to_the_anchor_pulling_from_the_right() { + let columns = vec![ + column("x", "x"), + column("a", "a"), + column("y", "y"), + column("b", "b"), + column("z", "z"), + column("c", "c"), + column("w", "w"), + ]; + + let result = gather_columns( + columns, + &["a".to_string(), "b".to_string(), "c".to_string()], + ) + .unwrap(); + + assert_eq!(names(&result), vec!["x", "a", "b", "c", "y", "z", "w"]); + } + + #[test] + fn gather_columns_uses_declared_order_not_original_order() { + // Members are scattered as C, A, B originally, but declared as + // [A, B, C] — gathering must produce A, B, C (declared order), not + // C, A, B (original relative order). + let columns = vec![ + column("c", "c"), + column("x", "x"), + column("a", "a"), + column("y", "y"), + column("b", "b"), + ]; + + let result = gather_columns( + columns, + &["a".to_string(), "b".to_string(), "c".to_string()], + ) + .unwrap(); + + assert_eq!(names(&result), vec!["x", "a", "b", "c", "y"]); + } + + #[test] + fn gather_columns_is_a_no_op_when_already_contiguous() { + let columns = vec![column("a", "a"), column("b", "b"), column("c", "c")]; + + let result = gather_columns(columns, &["a".to_string(), "b".to_string()]).unwrap(); + + assert_eq!(names(&result), vec!["a", "b", "c"]); + } + + #[test] + fn gather_columns_errors_on_unknown_column() { + let columns = vec![column("a", "a"), column("b", "b")]; + + let result = gather_columns(columns, &["a".to_string(), "nope".to_string()]); + + assert!(result.is_err()); + } + + #[test] + fn reorder_table_columns_defaults_gather_to_true() { + let columns = vec![ + column("a", "a"), + column("x", "x"), + column("b", "b"), + column("y", "y"), + ]; + let spans = vec![spanner(&["a", "b"])]; + + let result = reorder_table_columns(columns, &spans).unwrap(); + + assert_eq!(names(&result), vec!["a", "b", "x", "y"]); + } + + #[test] + fn reorder_table_columns_skips_gather_false() { + let columns = vec![ + column("a", "a"), + column("x", "x"), + column("b", "b"), + column("y", "y"), + ]; + let spans = vec![spanner_with_setting( + &["a", "b"], + "gather", + ParameterValue::Boolean(false), + )]; + + let result = reorder_table_columns(columns, &spans).unwrap(); + + assert_eq!(names(&result), vec!["a", "x", "b", "y"]); + } + + #[test] + fn reorder_table_columns_folds_spanners_in_declaration_order() { + let columns = vec![ + column("a", "a"), + column("x", "x"), + column("b", "b"), + column("y", "y"), + ]; + // Second span's gather runs against the *result* of the first, not + // the original order. + let spans = vec![spanner(&["a", "b"]), spanner(&["b", "y"])]; + + let result = reorder_table_columns(columns, &spans).unwrap(); + + assert_eq!(names(&result), vec!["a", "b", "y", "x"]); + } + + #[test] + fn assign_spanner_levels_needs_three_levels_for_mutually_crossing_spanners() { + let spans = vec![ + spanner(&["a", "b"]), + spanner(&["b", "c"]), + spanner(&["a", "c"]), + ]; + + let levels = assign_spanner_levels(&spans); + + assert_eq!(levels, vec![1, 2, 3]); + } + + #[test] + fn assign_spanner_levels_pins_explicit_level_without_a_conflict_check() { + // (a,b) auto-assigns to level 1; (b,c) is explicitly pinned to level + // 1 too, despite conflicting with (a,b) — this function doesn't + // error on that, validate_overlaps does once real cells exist. + let spans = vec![ + spanner(&["a", "b"]), + spanner_with_setting(&["b", "c"], "level", ParameterValue::Number(1.0)), + ]; + + let levels = assign_spanner_levels(&spans); + + assert_eq!(levels, vec![1, 1]); + } + + #[test] + fn assign_spanner_levels_compacts_gaps_left_by_explicit_levels() { + let spans = vec![ + spanner_with_setting(&["a"], "level", ParameterValue::Number(1.0)), + spanner_with_setting(&["b"], "level", ParameterValue::Number(3.0)), + spanner_with_setting(&["c"], "level", ParameterValue::Number(4.0)), + ]; + + let levels = assign_spanner_levels(&spans); + + assert_eq!(levels, vec![1, 2, 3]); + } + + #[test] + fn create_spanners_builds_one_cell_per_disjoint_spanner_on_the_same_row() { + let columns = vec![ + column("a", "a"), + column("b", "b"), + column("c", "c"), + column("d", "d"), + ]; + let spans = vec![ + labeled_spanner(&["a", "b"], "G1"), + labeled_spanner(&["c", "d"], "G2"), + ]; + + let cells = create_spanners(&columns, &spans).unwrap(); + + assert_eq!(cells.len(), 2); + assert_eq!(cells[0].top, 0); + assert_eq!(cells[0].bottom, 0); + assert_eq!(cells[0].left, 0); + assert_eq!(cells[0].right, 1); + assert_eq!(cells[0].content, "G1"); + assert_eq!(cells[1].left, 2); + assert_eq!(cells[1].right, 3); + assert_eq!(cells[1].content, "G2"); + assert!(cells.iter().all(|c| c.kind == TableCellKind::Spanner)); + } + + #[test] + fn create_spanners_puts_crossing_spanners_on_separate_rows() { + let columns = vec![column("a", "a"), column("b", "b"), column("c", "c")]; + let spans = vec![ + labeled_spanner(&["a", "b"], "G1"), + labeled_spanner(&["b", "c"], "G2"), + ]; + + let cells = create_spanners(&columns, &spans).unwrap(); + + // Level 1 (G1, closest to the columns) is the bottom spanner row — + // the higher local row number, since row 0 is the topmost row. + let g1 = cells.iter().find(|c| c.content == "G1").unwrap(); + let g2 = cells.iter().find(|c| c.content == "G2").unwrap(); + assert_eq!(g1.top, 1); + assert_eq!(g1.left, 0); + assert_eq!(g1.right, 1); + assert_eq!(g2.top, 0); + assert_eq!(g2.left, 1); + assert_eq!(g2.right, 2); + } + + #[test] + fn create_spanners_fragments_a_non_contiguous_spanner_into_multiple_cells() { + let columns = vec![column("a", "a"), column("x", "x"), column("b", "b")]; + let spans = vec![labeled_spanner(&["a", "b"], "G")]; + + let cells = create_spanners(&columns, &spans).unwrap(); + + assert_eq!(cells.len(), 2); + assert!(cells.iter().all(|c| c.content == "G")); + assert_eq!(cells[0].left, 0); + assert_eq!(cells[0].right, 0); + assert_eq!(cells[1].left, 2); + assert_eq!(cells[1].right, 2); + } + + #[test] + fn create_spanners_skips_null_labeled_spanners_and_their_levels() { + // If the NULL spanner participated in level assignment, it would + // cross (a,b) and (b,c) and force "G" to level 2 — filtering it out + // first means "G" is the only spanner left, so it stays at level 1. + let columns = vec![column("a", "a"), column("b", "b"), column("c", "c")]; + let spans = vec![null_spanner(&["a", "b"]), labeled_spanner(&["b", "c"], "G")]; + + let cells = create_spanners(&columns, &spans).unwrap(); + + assert_eq!(cells.len(), 1); + assert_eq!(cells[0].top, 0); + assert_eq!(cells[0].content, "G"); + assert_eq!(cells[0].left, 1); + assert_eq!(cells[0].right, 2); + } + + #[test] + fn create_spanners_returns_empty_for_no_spanners() { + let columns = vec![column("a", "a"), column("b", "b")]; + + let cells = create_spanners(&columns, &[]).unwrap(); + + assert!(cells.is_empty()); + } +} diff --git a/src/table/mod.rs b/src/table/mod.rs index 4c7e9425..f0cae157 100644 --- a/src/table/mod.rs +++ b/src/table/mod.rs @@ -1,9 +1,9 @@ //! 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. Still minimal: `source` (from `TABULATE FROM`) and `labels` -//! (from `TABULATE LABEL`) are populated so far. +//! Defines the typed `Table` structure that represents parsed `TABULATE` +//! statements, parallel to how `plot` defines `Plot` for `VISUALISE` +//! statements: `source` (from `TABULATE FROM`), `labels` (from `TABULATE +//! LABEL`), and `spans` (from `TABULATE SPAN`) are populated so far. use serde::{Deserialize, Serialize}; diff --git a/src/validate.rs b/src/validate.rs index b15fe7a5..596240ec 100644 --- a/src/validate.rs +++ b/src/validate.rs @@ -283,11 +283,11 @@ 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. + // `sql_part` already reflects `Table.source`: `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" @@ -296,6 +296,20 @@ pub fn validate(query: &str) -> Result { }); } + // Validate each SPAN's SETTING parameters (unknown key, or wrong shape + // for a recognized one) — mirrors the per-layer settings validation + // above. + if let Some(table) = tables.first() { + for (idx, spanner) in table.spans.iter().enumerate() { + if let Err(e) = spanner.validate_settings() { + errors.push(ValidationError { + message: format!("SPAN {}: {}", idx + 1, e), + location: None, + }); + } + } + } + Ok(Validated { sql: sql_part, visual: viz_part, From 73f613b60a8515a9a0d433845b7199bcbfffb696 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Wed, 16 Sep 2026 14:03:27 +0200 Subject: [PATCH 3/8] Render spanner rows with colspan in HtmlWriter write_table now buckets cells by is_header() into header/body row groups, wrapping them in
` — no styling, no -//! 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 +//! footnotes, since `Table` has no fields to describe those yet. Spanner +//! rows are rendered (as `colspan`, one `` per level, above the column +//! labels), but a column with no spanner at a given level gets a blank +//! filler cell rather than a `rowspan`-merged one — see `render_row`. 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`, which works directly off //! a `DataFrame` rather than resolved `TableCell`s. @@ -12,7 +15,7 @@ use std::collections::HashMap; use crate::util::escape_html; use crate::writer::{Writer, WriterOptions}; -use crate::{DataFrame, GgsqlError, Plot, Result, TableCell, TableCellKind}; +use crate::{DataFrame, GgsqlError, Plot, Result, TableCell}; /// Renders a resolved table as a bare HTML `
`. Does not support plots. #[derive(Debug, Default)] @@ -47,37 +50,122 @@ impl Writer for HtmlWriter { } fn write_table(&self, cells: &[TableCell]) -> Result { - let mut column_labels: Vec<&TableCell> = cells + let ncol = cells .iter() - .filter(|cell| cell.kind == TableCellKind::ColumnLabel) - .collect(); - column_labels.sort_by_key(|cell| cell.left); + .map(|cell| cell.right) + .max() + .map_or(0, |r| r + 1); + // BTreeMap because essentially Vec> but compact for missing rows + let mut header_rows: BTreeMap> = BTreeMap::new(); let mut body_rows: BTreeMap> = BTreeMap::new(); - for cell in cells.iter().filter(|cell| cell.kind == TableCellKind::Body) { - body_rows.entry(cell.top).or_default().push(cell); + + // Sorting Hat: Hmmm... Yes... BODY ROW!!! *applause* + for cell in cells { + if cell.is_header() { + header_rows.entry(cell.top).or_default().push(cell); + } else { + body_rows.entry(cell.top).or_default().push(cell); + } } - let mut html = String::from("
\n\n"); - for cell in column_labels { - html.push_str(&format!("", escape_html(&cell.content))); + let mut html = String::from("
{}
\n"); + + if !header_rows.is_empty() { + html.push_str("\n"); + for (_, row) in header_rows { + html.push_str(&render_row(row, ncol)); + } + html.push_str("\n"); } - html.push_str("\n\n\n"); - for (_, mut row) in body_rows { - row.sort_by_key(|cell| cell.left); - html.push_str(""); - for cell in row { - html.push_str(&format!("", escape_html(&cell.content))); + if !body_rows.is_empty() { + html.push_str("\n"); + for (_, row) in body_rows { + html.push_str(&render_row(row, ncol)); } - html.push_str("\n"); + html.push_str("\n"); } - html.push_str("\n
{}
"); + + html.push_str(""); Ok(html) } } +/// Render one `TableCell` as an HTML tag — ``/`` from +/// `cell.is_header()`, with a `colspan` attribute only when the cell +/// actually covers more than one column. +fn render_cell(cell: &TableCell) -> String { + let tag = if cell.is_header() { "th" } else { "td" }; + let colspan = cell.width(); + let attrs = if colspan > 1 { + format!(" colspan=\"{colspan}\"") + } else { + String::new() + }; + format!("<{tag}{attrs}>{}", escape_html(&cell.content)) +} + +/// Render one row — cells sharing a `top` — as `...`, walking every +/// column position from `0..ncol` and padding any gap with a synthesized +/// empty cell of the same kind. +fn render_row(mut cells: Vec<&TableCell>, ncol: usize) -> String { + // write_table's only caller never hands this an empty `cells` (an entry + // is only ever created already holding a cell), but an empty row has no + // `kind` to synthesize fillers with, so this returns early rather than + // assume that guarantee holds forever. + if cells.is_empty() { + return String::new(); + } + + // The walk below depends on `left`-ascending order. + cells.sort_by_key(|cell| cell.left); + let kind = cells[0].kind; + let mut html = String::from(""); + + // `ncol` comes from the whole table, not this row's own cells — a row + // that doesn't reach the last column must still pad out to it. + let mut col = 0; + let mut i = 0; + while col < ncol { + // Check if column has cell + if i < cells.len() && cells[i].left == col { + html.push_str(&render_cell(cells[i])); + col = cells[i].right + 1; + i += 1; + } else { + // A gap (a spanner not reaching every column, or one fragmented + // by another spanner's occupancy) renders through `render_cell` + // too, so one place decides which tag a `TableCellKind` gets, + // not two. + let filler = TableCell { + kind, + top: cells[0].top, + bottom: cells[0].top, + left: col, + right: col, + content: String::new(), + }; + html.push_str(&render_cell(&filler)); + col += 1; + } + } + + html.push_str("\n"); + html +} + +#[cfg(test)] +mod render_row_tests { + use super::*; + + #[test] + fn render_row_returns_empty_string_for_no_cells() { + assert_eq!(render_row(Vec::new(), 3), ""); + } +} + #[cfg(test)] #[cfg(feature = "duckdb")] mod tests { @@ -105,6 +193,48 @@ mod tests { assert!(!html.contains("a")); } + #[test] + fn test_write_table_renders_a_spanner_row_with_colspan_and_fills_gaps() { + let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); + reader + .execute_sql( + "CREATE TABLE sales AS SELECT * FROM (VALUES (1, 'a', 10)) AS t(id, name, amount)", + ) + .unwrap(); + let spec = reader + .execute("TABULATE FROM sales SPAN 'Info' ACROSS id, name") + .unwrap(); + + let writer = HtmlWriter::new(); + let html = writer.render(&spec).unwrap(); + + // The spanner covers id+name (colspan 2), with an empty header cell + // filling the gap above "amount", which has no spanner over it. + assert!(html.contains("Info")); + assert!(html.contains("id")); + assert!(html.contains("name")); + assert!(html.contains("amount")); + // The spanner row renders above the column-label row. + assert!(html.find("Info").unwrap() < html.find("id").unwrap()); + } + + #[test] + fn test_write_table_omits_tbody_for_a_zero_row_result() { + let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); + reader + .execute_sql("CREATE TABLE sales AS SELECT * FROM (VALUES (1, 'a')) AS t(id, name)") + .unwrap(); + let spec = reader + .execute("SELECT * FROM sales WHERE 1 = 0 TABULATE") + .unwrap(); + + let writer = HtmlWriter::new(); + let html = writer.render(&spec).unwrap(); + + assert!(html.contains("")); + assert!(!html.contains("")); + } + #[test] fn test_write_plot_is_unsupported() { let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); From 5cc886055a6fd24833531a27d925003a8fbd2cf1 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Wed, 16 Sep 2026 14:26:42 +0200 Subject: [PATCH 4/8] Merge repeated TABULATE LABEL clauses and suppress an all-NULL label row MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit process_tab_clause was overwriting table.labels on every LABEL clause instead of merging, unlike the VISUALISE-side handler for the same node — a second LABEL clause silently dropped every override from the first. create_column_labels now also returns no cells at all when every column's label is empty (only reachable via LABEL col => NULL, or => '', on every column, since an unlabeled column keeps its non-empty name), so a wholly suppressed label row is omitted entirely rather than rendered as a row of blank header cells. Co-Authored-By: Claude Sonnet 5 --- src/execute/table.rs | 28 ++++++++++++++++++++++++++++ src/parser/builder.rs | 18 +++++++++++++++++- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/execute/table.rs b/src/execute/table.rs index 0bbb9914..91474eb5 100644 --- a/src/execute/table.rs +++ b/src/execute/table.rs @@ -130,6 +130,14 @@ fn create_table_columns(df: &DataFrame, labels: &Labels) -> Vec { /// `rowbind_cells` are what decide where this sits relative to spanners and /// the body, not this function. fn create_column_labels(columns: &[TableColumn]) -> Vec { + // The only way every column's label ends up empty is `LABEL col => NULL` + // (or `=> ''`) on every column, since an unlabeled column keeps its + // (non-empty) name — so a wholly suppressed row omits the row entirely + // rather than rendering a row of blank header cells. + if columns.iter().all(|c| c.label.is_empty()) { + return Vec::new(); + } + columns .iter() .enumerate() @@ -687,6 +695,26 @@ mod layout_tests { assert!(build_cells(&frame, &table).is_err()); } + + #[test] + fn build_cells_omits_the_column_label_row_when_every_label_is_null() { + let frame = df! { + "id" => vec![1i32], + "name" => vec!["a".to_string()], + } + .unwrap(); + let mut table = Table::new(); + table.labels.labels.insert("id".to_string(), None); + table.labels.labels.insert("name".to_string(), None); + + let cells = build_cells(&frame, &table).unwrap(); + + assert!(!cells.iter().any(|c| c.kind == TableCellKind::ColumnLabel)); + assert!(cells + .iter() + .filter(|c| c.kind == TableCellKind::Body) + .all(|c| c.top == 0)); + } } #[cfg(test)] diff --git a/src/parser/builder.rs b/src/parser/builder.rs index ce7ba04c..c1edba5a 100644 --- a/src/parser/builder.rs +++ b/src/parser/builder.rs @@ -370,7 +370,10 @@ fn process_tab_clause(node: &Node, source: &SourceTree, table: &mut Table) -> Re for child in node.children(&mut cursor) { match child.kind() { "label_clause" => { - table.labels = build_labels(&child, source)?; + let new_labels = build_labels(&child, source)?; + for (key, value) in new_labels.labels { + table.labels.labels.insert(key, value); + } } "span_clause" => { table.spans.push(build_span_clause(&child, source)?); @@ -1417,6 +1420,19 @@ mod tests { ); } + #[test] + fn test_tabulate_repeated_label_clauses_merge_rather_than_overwrite() { + let specs = + parse_test_specs("TABULATE FROM sales LABEL id => 'ID' LABEL name => 'Name'").unwrap(); + let table = specs[0].as_table().expect("expected a Table spec"); + + assert_eq!(table.labels.labels.get("id"), Some(&Some("ID".to_string()))); + assert_eq!( + table.labels.labels.get("name"), + Some(&Some("Name".to_string())) + ); + } + #[test] fn test_tabulate_multiple_spans_and_label_in_any_order() { let specs = parse_test_specs( From 10a9503118ee50f87fa6850437d094f1cc188848 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Wed, 16 Sep 2026 15:52:08 +0200 Subject: [PATCH 5/8] Document TABULATE and SPAN New doc/syntax/clause/tabulate.qmd covers TABULATE's FROM/LABEL/SPAN clauses, wired into the navbar and syntax sidebar in _quarto.yml. ggsql.xml gains TabulateClause and SpanClause highlighting contexts, mirroring how DRAW/PLACE/SCALE/PROJECT/FACET/LABEL/VISUALISE already switch contexts on their own keyword. TABULATE is reachable from every existing context (a new TABULATE statement can start after any VISUALISE clause), but SPAN is only reachable from TabulateClause, SpanClause and LabelClause, since it's a TABULATE-only clause and TABULATE is mutually exclusive with VISUALISE within one statement. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 7 +++ doc/_quarto.yml | 4 ++ doc/ggsql.xml | 76 +++++++++++++++++++++++++++ doc/syntax/clause/tabulate.qmd | 93 ++++++++++++++++++++++++++++++++++ 4 files changed, 180 insertions(+) create mode 100644 doc/syntax/clause/tabulate.qmd diff --git a/CHANGELOG.md b/CHANGELOG.md index 52f30f2d..5d79b83f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,13 @@ - `ggsql-jupyter` accepts `--version`. +- (WIP) New `TABULATE` clause renders a query result as a table instead of a + plot. Tables are rendered via new `html` writer. `TABULATE` supports the + following clauses: + - `LABEL` overrides the display of column labels + - `SPAN` groups columns under a shared header cell (spanner). + + ### Changed - The wasm bundle draws plots with ggsql's own renderer instead of emitting Vega-Lite. A query is executed in the browser and drawn straight to SVG, so diff --git a/doc/_quarto.yml b/doc/_quarto.yml index e19046fe..bb4d3b9f 100644 --- a/doc/_quarto.yml +++ b/doc/_quarto.yml @@ -53,6 +53,8 @@ website: href: syntax/clause/project.qmd - text: "`LABEL`" href: syntax/clause/label.qmd + - text: "`TABULATE`" + href: syntax/clause/tabulate.qmd - text: Gallery href: gallery/index.qmd - href: faq.qmd @@ -106,6 +108,8 @@ website: href: syntax/clause/project.qmd - text: "`LABEL`" href: syntax/clause/label.qmd + - text: "`TABULATE`" + href: syntax/clause/tabulate.qmd - section: Layers contents: - section: Types diff --git a/doc/ggsql.xml b/doc/ggsql.xml index 26955ec7..32c07b42 100644 --- a/doc/ggsql.xml +++ b/doc/ggsql.xml @@ -97,6 +97,8 @@ LABEL VISUALISE VISUALIZE + TABULATE + SPAN @@ -110,6 +112,7 @@ RENAMING TO VIA + ACROSS @@ -397,6 +400,7 @@ + @@ -444,6 +448,7 @@ + @@ -484,6 +489,7 @@ + @@ -523,6 +529,7 @@ + @@ -569,6 +576,7 @@ + @@ -612,6 +620,7 @@ + @@ -654,6 +663,8 @@ + + @@ -676,6 +687,70 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -691,6 +766,7 @@ + diff --git a/doc/syntax/clause/tabulate.qmd b/doc/syntax/clause/tabulate.qmd new file mode 100644 index 00000000..2d19e913 --- /dev/null +++ b/doc/syntax/clause/tabulate.qmd @@ -0,0 +1,93 @@ +--- +title: "Output a table instead of a plot with `TABULATE`" +--- + + + +The `TABULATE` clause marks the beginning of a ggsql table declaration — the plain-table counterpart to [`VISUALISE`](visualise.qmd). Instead of building a plot, it renders the query result directly as a table. As with `VISUALISE`, it demarcates where the regular SQL query ends: everything before `TABULATE` is standard SQL sent to the backend, and if it ends with a `SELECT`, that result becomes the table's data unless overridden by `FROM`. + +`TABULATE` currently only produces output through the HTML writer — every other writer (Vega-Lite, SVG, PDF, `.hep`, the raster writers) supports plots only and errors if given a `TABULATE` query. + +## Clause syntax +`TABULATE` doesn't require any additional clauses. + +```ggsql +TABULATE FROM + LABEL => , ... + SPAN ACROSS , ... + SETTING => , ... +``` + +`LABEL` and `SPAN` may each appear more than once, in any order, after the source. + +### `FROM` +```ggsql +FROM +``` + +Same shape as [`VISUALISE`'s `FROM`](visualise.qmd#from): a bare identifier names a CTE or table already available in the backend, and a string names a file path the backend can read directly. `FROM` is optional if the query already ends with a `SELECT`; combining both (a trailing `SELECT` *and* `TABULATE FROM`) is an error, since it would leave two candidate data sources. + +### `LABEL` +```ggsql +LABEL => , ... +``` + +Overrides the display header text for one or more columns. Unlike `VISUALISE`'s `LABEL` (which is keyed by aesthetic), `TABULATE`'s `LABEL` is keyed by column name, and there is no equivalent to `VISUALISE`'s automatic labelling logic — a column with no `LABEL` entry keeps its own name as its header. + +Give `NULL` instead of a string to blank a column's header while keeping the column itself: + +```ggsql +TABULATE FROM sales + LABEL revenue => 'Revenue ($)', region => NULL +``` + +If every column ends up with a blank header this way, the header row is omitted entirely rather than rendered as a row of empty cells. + +### `SPAN` +```ggsql +SPAN ACROSS , ... + SETTING => , ... +``` + +Groups columns under one spanner cell, drawn in a row above the column labels: + +```ggsql +TABULATE FROM sales + SPAN 'Q1' ACROSS jan, feb, mar +``` + +The label is mandatory and can be a string (`''` is a present-but-blank cell) or `NULL`, which suppresses the spanner cell itself while still grouping the columns — useful when the only reason for the `SPAN` is a shared `SETTING`. Each `SPAN` clause defines exactly one spanner; give several `SPAN` clauses for several spanners, the same way multiple `DRAW` clauses give multiple layers. + +Spanners whose column sets overlap (but aren't identical) are placed on separate header rows automatically, closest-to-the-columns first. + +#### `SETTING` +```ggsql +SETTING => , ... +``` + +* `gather` (boolean, default `true`): moves the spanner's columns to sit contiguously, pulling them next to the first-listed member without disturbing any other column's relative order. Set to `false` to leave the columns exactly where they are. +* `level` (whole number, no default): pins the spanner to a specific header row (`1` is the row closest to the column labels). If omitted, a level is assigned automatically, high enough to avoid every other spanner it shares a column with. + +### Examples + +```{ggsql} +SELECT * FROM ggsql:penguins LIMIT 5 + +TABULATE +``` + + + +```{ggsql} +SELECT * FROM ggsql:penguins LIMIT 5 + +TABULATE + SPAN 'Bill' ACROSS bill_len, bill_dep + LABEL + bill_len => 'Length', + bill_dep => 'Depth' +``` From a44828d6baaed3fb84a7b5978b46d4c4b1270c31 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Wed, 16 Sep 2026 16:46:13 +0200 Subject: [PATCH 6/8] Teach HtmlWriter to render and consume rowspan cells MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit render_cell now emits a rowspan attribute alongside colspan, and render_row takes the set of columns a rowspan from an earlier row already occupies, skipping them (no cell, no filler) rather than double-rendering. occupied_columns_per_row computes that set for every row in one pass over cells. write_table also now checks that every header cell sits above every body cell, a requirement of splitting output into separate / blocks that isn't a general TableCell invariant. Nothing in the resolution pipeline produces a multi-row TableCell yet — this is writer-side groundwork, exercised directly with hand-built cells, ahead of compose_header actually merging a spanner-less column's label into a rowspan. Co-Authored-By: Claude Sonnet 5 --- src/writer/html.rs | 164 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 141 insertions(+), 23 deletions(-) diff --git a/src/writer/html.rs b/src/writer/html.rs index ccc2780a..5e97bf55 100644 --- a/src/writer/html.rs +++ b/src/writer/html.rs @@ -3,15 +3,17 @@ //! Renders a `ResolvedTable`'s cells as a bare `` — no styling, no //! footnotes, since `Table` has no fields to describe those yet. Spanner //! rows are rendered (as `colspan`, one `` per level, above the column -//! labels), but a column with no spanner at a given level gets a blank -//! filler cell rather than a `rowspan`-merged one — see `render_row`. 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`, which works directly off -//! a `DataFrame` rather than resolved `TableCell`s. +//! labels); `render_cell`/`render_row` can also render a `rowspan` cell, +//! though nothing in the resolution pipeline produces one yet, so a column +//! with no spanner at a given level still gets a blank filler cell rather +//! than a merged one. 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`, which works +//! directly off a `DataFrame` rather than resolved `TableCell`s. use std::collections::BTreeMap; use std::collections::HashMap; +use std::collections::HashSet; use crate::util::escape_html; use crate::writer::{Writer, WriterOptions}; @@ -69,20 +71,41 @@ impl Writer for HtmlWriter { } } + // Not a general TableCell invariant — just what this writer's split + // into two blocks requires. + if let (Some(&max_header_row), Some(&min_body_row)) = + (header_rows.keys().next_back(), body_rows.keys().next()) + { + if max_header_row >= min_body_row { + return Err(GgsqlError::WriterError(format!( + "HtmlWriter renders headers and body as separate / blocks, so \ + every header cell must sit above every body cell; found a header row at \ + {max_header_row} at or below a body row at {min_body_row}" + ))); + } + } + + let nrow = cells + .iter() + .map(|cell| cell.bottom) + .max() + .map_or(0, |r| r + 1); + let occupied = occupied_columns_per_row(cells, nrow); + let mut html = String::from("
\n"); if !header_rows.is_empty() { html.push_str("\n"); - for (_, row) in header_rows { - html.push_str(&render_row(row, ncol)); + for (row, row_cells) in header_rows { + html.push_str(&render_row(row_cells, ncol, &occupied[row])); } html.push_str("\n"); } if !body_rows.is_empty() { html.push_str("\n"); - for (_, row) in body_rows { - html.push_str(&render_row(row, ncol)); + for (row, row_cells) in body_rows { + html.push_str(&render_row(row_cells, ncol, &occupied[row])); } html.push_str("\n"); } @@ -94,23 +117,44 @@ impl Writer for HtmlWriter { } /// Render one `TableCell` as an HTML tag — `...`, walking every /// column position from `0..ncol` and padding any gap with a synthesized -/// empty cell of the same kind. -fn render_row(mut cells: Vec<&TableCell>, ncol: usize) -> String { +/// empty cell of the same kind. `occupied` (from `occupied_columns_per_row`) +/// names the columns a rowspan from an earlier row already claims — those +/// get neither a cell nor a filler here. +fn render_row(mut cells: Vec<&TableCell>, ncol: usize, occupied: &HashSet) -> String { // write_table's only caller never hands this an empty `cells` (an entry // is only ever created already holding a cell), but an empty row has no // `kind` to synthesize fillers with, so this returns early rather than @@ -129,8 +173,9 @@ fn render_row(mut cells: Vec<&TableCell>, ncol: usize) -> String { let mut col = 0; let mut i = 0; while col < ncol { - // Check if column has cell - if i < cells.len() && cells[i].left == col { + if occupied.contains(&col) { + col += 1; + } else if i < cells.len() && cells[i].left == col { html.push_str(&render_cell(cells[i])); col = cells[i].right + 1; i += 1; @@ -157,12 +202,85 @@ fn render_row(mut cells: Vec<&TableCell>, ncol: usize) -> String { } #[cfg(test)] -mod render_row_tests { +mod render_tests { use super::*; + use crate::TableCellKind; + + fn cell( + kind: TableCellKind, + top: usize, + bottom: usize, + left: usize, + right: usize, + ) -> TableCell { + TableCell { + kind, + top, + bottom, + left, + right, + content: String::new(), + } + } #[test] fn render_row_returns_empty_string_for_no_cells() { - assert_eq!(render_row(Vec::new(), 3), ""); + assert_eq!(render_row(Vec::new(), 3, &HashSet::new()), ""); + } + + #[test] + fn render_cell_emits_rowspan_when_height_is_greater_than_one() { + let c = cell(TableCellKind::ColumnLabel, 0, 1, 0, 0); + assert_eq!(render_cell(&c), ""); + } + + #[test] + fn render_row_skips_a_column_occupied_by_a_rowspan_from_above() { + let a = cell(TableCellKind::ColumnLabel, 1, 1, 0, 0); + let c = cell(TableCellKind::ColumnLabel, 1, 1, 2, 2); + let occupied = HashSet::from([1]); + + let row = render_row(vec![&a, &c], 3, &occupied); + + assert_eq!(row, "\n"); + } + + #[test] + fn write_table_errors_when_a_header_cell_is_not_above_every_body_cell() { + let cells = vec![ + cell(TableCellKind::Body, 0, 0, 0, 0), + cell(TableCellKind::ColumnLabel, 0, 0, 1, 1), + ]; + + assert!(HtmlWriter::new().write_table(&cells).is_err()); + } + + #[test] + fn write_table_renders_a_rowspan_cell_once_and_skips_it_in_the_next_row() { + // Column 1 has no spanner, so its label rowspans up through row 0 + // instead of row 0 getting a blank filler there. + let cells = vec![ + cell(TableCellKind::Spanner, 0, 0, 0, 0), + cell(TableCellKind::ColumnLabel, 0, 1, 1, 1), + cell(TableCellKind::ColumnLabel, 1, 1, 0, 0), + cell(TableCellKind::Body, 2, 2, 0, 0), + cell(TableCellKind::Body, 2, 2, 1, 1), + ]; + + let html = HtmlWriter::new().write_table(&cells).unwrap(); + + assert_eq!( + html, + "
`/`` from -/// `cell.is_header()`, with a `colspan` attribute only when the cell -/// actually covers more than one column. +/// `cell.is_header()`, with a `colspan`/`rowspan` attribute only when the +/// cell actually spans more than one column/row. fn render_cell(cell: &TableCell) -> String { let tag = if cell.is_header() { "th" } else { "td" }; let colspan = cell.width(); - let attrs = if colspan > 1 { - format!(" colspan=\"{colspan}\"") - } else { - String::new() - }; + let rowspan = cell.height(); + let mut attrs = String::new(); + if colspan > 1 { + attrs.push_str(&format!(" colspan=\"{colspan}\"")); + } + if rowspan > 1 { + attrs.push_str(&format!(" rowspan=\"{rowspan}\"")); + } format!("<{tag}{attrs}>{}", escape_html(&cell.content)) } +/// Per row, the column positions already covered by a cell that started in +/// an earlier row and hasn't ended yet (a rowspan declared above that row) — +/// those get no cell and no filler when the row is rendered, since the +/// earlier cell already claims that grid position. +fn occupied_columns_per_row(cells: &[TableCell], nrow: usize) -> Vec> { + let mut occupied = vec![HashSet::new(); nrow]; + for cell in cells { + // Excludes `top` itself — that row renders the cell normally. + let spanned_rows = (cell.top + 1)..=cell.bottom; + for occupied_row in &mut occupied[spanned_rows] { + occupied_row.extend(cell.left..=cell.right); + } + } + occupied +} + /// Render one row — cells sharing a `top` — as `
\n\ + \n\ + \n\ + \n\ + \n\ + \n\ + \n\ + \n\ +
" + ); } } From ecd87c915572afbde51478f291ebf83539c9dab7 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Wed, 16 Sep 2026 17:37:03 +0200 Subject: [PATCH 7/8] Stretch an unspanned column's label over its header gap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit compose_header now grows a column's label cell upward into every consecutive spanner row above it that has no spanner covering that column, stopping at the first row that does — a rowspan on the label instead of a separate blank filler cell for that gap. Deliberately diverges from gt: gt only ever stretches into the single row immediately above the labels, even when rows further up are also empty for that column. Verified directly against gt's own output. Co-Authored-By: Claude Sonnet 5 --- src/execute/table.rs | 147 +++++++++++++++++++++++++++++++++++++++++-- src/writer/html.rs | 9 ++- 2 files changed, 146 insertions(+), 10 deletions(-) diff --git a/src/execute/table.rs b/src/execute/table.rs index 91474eb5..f7a27c90 100644 --- a/src/execute/table.rs +++ b/src/execute/table.rs @@ -214,10 +214,61 @@ fn rowbind_cells(top: Vec, mut bottom: Vec) -> Vec, column_labels: Vec) -> Vec { - rowbind_cells(spanners, column_labels) + let num_spanner_rows = spanners.iter().map(|c| c.bottom).max().map_or(0, |r| r + 1); + let header = rowbind_cells(spanners, column_labels); + stretch_unspanned_column_labels(header, num_spanner_rows) +} + +/// Grow a column's label cell upward into every consecutive spanner row +/// above it (starting from the row closest to the labels) that has no +/// spanner covering that column, stopping at the first one that does — +/// turning those rows into a `rowspan` on the label instead of separate +/// blank filler cells. Deliberately diverges from gt here: gt only ever +/// stretches into the single row immediately above the labels, even when +/// rows further up are also empty for that column (verified directly +/// against gt's own output). +fn stretch_unspanned_column_labels( + mut header: Vec, + num_spanner_rows: usize, +) -> Vec { + if num_spanner_rows == 0 { + return header; + } + + let ncol = header.iter().map(|c| c.right).max().map_or(0, |r| r + 1); + let mut stretch_depth = vec![0usize; ncol]; + for (column, depth) in stretch_depth.iter_mut().enumerate() { + for row in (0..num_spanner_rows).rev() { + let covered = header.iter().any(|c| { + c.kind == TableCellKind::Spanner + && c.top == row + && c.left <= column + && column <= c.right + }); + if covered { + break; + } + *depth += 1; + } + } + + for label in header + .iter_mut() + .filter(|c| c.kind == TableCellKind::ColumnLabel) + { + // Assumes every ColumnLabel cell is exactly one column wide (true of + // everything create_column_labels produces) — a wider one would need + // its own stretch depth reconciled across its whole span, not just + // `left`. + label.top -= stretch_depth[label.left]; + } + + header } /// Check that no two cells in a resolved layout claim the same grid position. @@ -518,9 +569,9 @@ mod layout_tests { #[test] fn rowbind_cells_offsets_by_the_top_rows_actual_extent_not_a_hardcoded_one() { - // Nothing produces a multi-row column-label section today, but // `rowbind_cells` computes the offset from `top` itself rather than - // assuming exactly one row — pin that down directly. + // assuming exactly one row — pin that down directly, independent of + // whichever caller happens to produce a multi-row `top`. let column_labels = vec![cell(TableCellKind::ColumnLabel, 0, 1, "id")]; let body = vec![cell(TableCellKind::Body, 0, 0, "1")]; @@ -550,6 +601,92 @@ mod layout_tests { assert_eq!(cells[0].top, 0); } + #[test] + fn compose_header_stretches_an_unspanned_columns_label_over_the_gap() { + // "G" covers a, b (columns 0, 1) at the one spanner row; c has no + // spanner at all. + let spanners = vec![cell_at(TableCellKind::Spanner, 0, 0, 0, 1)]; + let column_labels = vec![ + cell_at(TableCellKind::ColumnLabel, 0, 0, 0, 0), + cell_at(TableCellKind::ColumnLabel, 0, 0, 1, 1), + cell_at(TableCellKind::ColumnLabel, 0, 0, 2, 2), + ]; + + let header = compose_header(spanners, column_labels); + + let a = header + .iter() + .find(|c| c.kind == TableCellKind::ColumnLabel && c.left == 0) + .unwrap(); + let c = header + .iter() + .find(|c| c.kind == TableCellKind::ColumnLabel && c.left == 2) + .unwrap(); + assert_eq!((a.top, a.bottom), (1, 1)); + assert_eq!((c.top, c.bottom), (0, 1)); + } + + #[test] + fn compose_header_stops_stretching_at_the_first_row_that_covers_the_column() { + // "Outer" (row 0, the far level) covers both a and b; "Inner" (row + // 1, next to the labels) covers only a. b's row-1 gap is contiguous + // with the labels, so it stretches by one row — but row 0 already + // covers b, so the stretch must stop there, not skip past it. + let spanners = vec![ + cell_at(TableCellKind::Spanner, 0, 0, 0, 1), + cell_at(TableCellKind::Spanner, 1, 1, 0, 0), + ]; + let column_labels = vec![ + cell_at(TableCellKind::ColumnLabel, 0, 0, 0, 0), + cell_at(TableCellKind::ColumnLabel, 0, 0, 1, 1), + ]; + + let header = compose_header(spanners, column_labels); + + let a = header + .iter() + .find(|c| c.kind == TableCellKind::ColumnLabel && c.left == 0) + .unwrap(); + let b = header + .iter() + .find(|c| c.kind == TableCellKind::ColumnLabel && c.left == 1) + .unwrap(); + assert_eq!((a.top, a.bottom), (2, 2)); + assert_eq!((b.top, b.bottom), (1, 2)); + } + + #[test] + fn compose_header_stretches_across_every_row_when_none_of_them_cover_the_column() { + // "G" (row 1) covers only a; "H" (row 0) covers only b; c has no + // spanner at either level, so its label absorbs both rows. + let spanners = vec![ + cell_at(TableCellKind::Spanner, 0, 0, 1, 1), + cell_at(TableCellKind::Spanner, 1, 1, 0, 0), + ]; + let column_labels = vec![ + cell_at(TableCellKind::ColumnLabel, 0, 0, 0, 0), + cell_at(TableCellKind::ColumnLabel, 0, 0, 1, 1), + cell_at(TableCellKind::ColumnLabel, 0, 0, 2, 2), + ]; + + let header = compose_header(spanners, column_labels); + + let c = header + .iter() + .find(|cell| cell.kind == TableCellKind::ColumnLabel && cell.left == 2) + .unwrap(); + assert_eq!((c.top, c.bottom), (0, 2)); + } + + #[test] + fn compose_header_does_nothing_when_there_are_no_spanners() { + let column_labels = vec![cell_at(TableCellKind::ColumnLabel, 0, 0, 0, 0)]; + + let header = compose_header(Vec::new(), column_labels); + + assert_eq!((header[0].top, header[0].bottom), (0, 0)); + } + fn cell_at( kind: TableCellKind, top: usize, diff --git a/src/writer/html.rs b/src/writer/html.rs index 5e97bf55..1769b960 100644 --- a/src/writer/html.rs +++ b/src/writer/html.rs @@ -312,7 +312,7 @@ mod tests { } #[test] - fn test_write_table_renders_a_spanner_row_with_colspan_and_fills_gaps() { + fn test_write_table_renders_a_spanner_row_with_colspan() { let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap(); reader .execute_sql( @@ -326,12 +326,11 @@ mod tests { let writer = HtmlWriter::new(); let html = writer.render(&spec).unwrap(); - // The spanner covers id+name (colspan 2), with an empty header cell - // filling the gap above "amount", which has no spanner over it. - assert!(html.contains("Info")); + // "amount" has no spanner, so its label stretches up into the + // spanner row (rowspan) instead of a blank filler cell there. + assert!(html.contains("Infoamount")); assert!(html.contains("id")); assert!(html.contains("name")); - assert!(html.contains("amount")); // The spanner row renders above the column-label row. assert!(html.find("Info").unwrap() < html.find("id").unwrap()); } From b85c1b8f643465bd1c94ae8469994c1e5125acad Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Thu, 17 Sep 2026 10:49:08 +0200 Subject: [PATCH 8/8] Align spanner level assignment and add SPAN id references to gt assign_spanner_levels now matches gt's real resolve_spanner_level() rule: a spanner's level is one more than the highest level of any already-assigned spanner it shares a column with, rather than the lowest level free of overlap. Confirmed against gt's own source; it can use more header rows than strictly necessary for a chain of pairwise-but-not-all conflicting spanners, which is deliberate parity with gt, not a compaction gap. New SETTING id => 'foo' on a SPAN clause lets a later SPAN's ACROSS list reference an earlier spanner by id, folding its columns in alongside any literal ones (SPAN 'H1' ACROSS q1, apr after SPAN 'Q1' ... SETTING id => 'q1'). Table::resolve_spanner_ids expands these (only backward references resolve; a forward reference or typo falls through to the existing unknown-column error) and rejects a duplicate id; it lives on Table rather than in execute::table_spanner so validate() can call it without depending on execute. A separate check, folded into create_spanners since it needs the real column list, rejects an id that collides with an actual column name. Co-Authored-By: Claude Sonnet 5 --- doc/syntax/clause/tabulate.qmd | 11 +++ src/execute/table.rs | 9 ++- src/execute/table_spanner.rs | 135 +++++++++++++++++++++++++++++---- src/table/mod.rs | 50 ++++++++++++ src/validate.rs | 7 ++ 5 files changed, 197 insertions(+), 15 deletions(-) diff --git a/doc/syntax/clause/tabulate.qmd b/doc/syntax/clause/tabulate.qmd index 2d19e913..3c6c3bad 100644 --- a/doc/syntax/clause/tabulate.qmd +++ b/doc/syntax/clause/tabulate.qmd @@ -71,6 +71,17 @@ SETTING => , ... * `gather` (boolean, default `true`): moves the spanner's columns to sit contiguously, pulling them next to the first-listed member without disturbing any other column's relative order. Set to `false` to leave the columns exactly where they are. * `level` (whole number, no default): pins the spanner to a specific header row (`1` is the row closest to the column labels). If omitted, a level is assigned automatically, high enough to avoid every other spanner it shares a column with. +* `id` (string, no default): names this spanner so a *later* `SPAN` clause can fold its columns into a new one. + +Reference an `id` in place of a column name in a later `SPAN`'s `ACROSS` list to include everything the earlier spanner covers: + +```ggsql +TABULATE FROM sales + SPAN 'Q1' ACROSS jan, feb, mar SETTING id => 'q1' + SPAN 'H1' ACROSS q1, apr, may, jun +``` + +`H1` ends up covering `jan, feb, mar, apr, may, jun`. Only an `id` declared by an *earlier* `SPAN` clause is recognised — referencing one declared later, or a typo, is treated as an unknown column, the same error as any other bad `ACROSS` entry. Every `id` must be unique across the whole query, and can't collide with an actual column name. ### Examples diff --git a/src/execute/table.rs b/src/execute/table.rs index f7a27c90..c4df3f5a 100644 --- a/src/execute/table.rs +++ b/src/execute/table.rs @@ -7,6 +7,8 @@ //! lives in the sibling `table_spanner` module, the way Plot's own //! resolution logic is split across `schema.rs`/`casting.rs`/`layer.rs`/ //! `scale.rs`/`position.rs`/`cte.rs` rather than left in one file. +//! `Table::resolve_spanner_ids` is the exception — it needs no `DataFrame`, +//! so it lives on `Table` itself, reachable from `validate()` too. use super::table_spanner::{create_spanners, reorder_table_columns}; use crate::array_util::value_to_string; @@ -74,10 +76,13 @@ fn build_cells(df: &DataFrame, table: &Table) -> Result> { .validate_settings() .map_err(|e| GgsqlError::ValidationError(format!("SPAN {}: {}", idx + 1, e)))?; } + let spans = table + .resolve_spanner_ids() + .map_err(GgsqlError::ValidationError)?; let columns = create_table_columns(df, &table.labels); - let columns = reorder_table_columns(columns, &table.spans)?; - let spanners = create_spanners(&columns, &table.spans)?; + let columns = reorder_table_columns(columns, &spans)?; + let spanners = create_spanners(&columns, &spans)?; let column_labels = create_column_labels(&columns); let header = compose_header(spanners, column_labels); let table_body = create_body(df, &columns); diff --git a/src/execute/table_spanner.rs b/src/execute/table_spanner.rs index 31767e69..95b0497f 100644 --- a/src/execute/table_spanner.rs +++ b/src/execute/table_spanner.rs @@ -4,6 +4,23 @@ use super::table::TableColumn; use crate::{GgsqlError, Result, Spanner, TableCell, TableCellKind}; +/// Check that no SPAN's `id` collides with an actual column name. A +/// duplicate `id` across spanners is already rejected by +/// `Table::resolve_spanner_ids`, which has no access to real column names — +/// `create_spanners` calls this once it does. +fn check_spanner_id_column_collision(spans: &[Spanner], columns: &[TableColumn]) -> Result<()> { + for span in spans { + if let Some(id) = span.settings.get("id").and_then(|v| v.as_str()) { + if columns.iter().any(|c| c.name == id) { + return Err(GgsqlError::ValidationError(format!( + "SPAN id '{id}' collides with an existing column name" + ))); + } + } + } + Ok(()) +} + /// Reorder `columns` so every `gather`-enabled spanner's members become /// contiguous, folding spanners in `spans`' order (declaration order) — /// mirrors gt's `tab_spanner(gather = TRUE)`, which is the default there and @@ -109,19 +126,20 @@ fn assign_spanner_levels(spans: &[Spanner]) -> Vec { .expect("Spanner::validate_settings already checked 'level' is a number") as usize, None => { - // Bump the candidate level while it's shared with an - // already-assigned spanner whose columns intersect this - // one's — any overlap (crossing, nesting, or identical sets) - // would render as two rectangles claiming the same column, - // so "intersects at all" is the only test needed. - let mut candidate = 1; - while spans.iter().zip(&levels).any(|(other, &other_level)| { - other_level == candidate - && other.columns.iter().any(|c| span.columns.contains(c)) - }) { - candidate += 1; - } - candidate + // One more than the highest level of any already-assigned + // spanner whose columns intersect this one's — matches gt's + // own `resolve_spanner_level()`. Can use more levels than + // strictly necessary for a chain of pairwise-but-not-all + // conflicting spanners, since it never revisits a lower + // level once something deeper claims a shared column. + spans + .iter() + .zip(&levels) + .filter(|(other, _)| other.columns.iter().any(|c| span.columns.contains(c))) + .map(|(_, &level)| level) + .max() + .unwrap_or(0) + + 1 } }; levels.push(level); @@ -151,6 +169,7 @@ pub(crate) fn create_spanners( if spans.is_empty() { return Ok(Vec::new()); } + check_spanner_id_column_collision(spans, columns)?; // Filter out spanners with `null` labels. They don't contribute to cells // so their level is irrellevant and shouldn't affect other levels. @@ -380,6 +399,22 @@ mod tests { assert_eq!(levels, vec![1, 2, 3]); } + #[test] + fn assign_spanner_levels_pushes_a_chained_conflict_past_a_free_level() { + // X(a,b) and Z(c,d) share no column and could both sit at level 1, + // but Z conflicts with Y(b,c), which conflicts with X — matching + // gt, Z gets pushed to level 3 rather than reusing X's level 1. + let spans = vec![ + spanner(&["a", "b"]), + spanner(&["b", "c"]), + spanner(&["c", "d"]), + ]; + + let levels = assign_spanner_levels(&spans); + + assert_eq!(levels, vec![1, 2, 3]); + } + #[test] fn assign_spanner_levels_pins_explicit_level_without_a_conflict_check() { // (a,b) auto-assigns to level 1; (b,c) is explicitly pinned to level @@ -497,4 +532,78 @@ mod tests { assert!(cells.is_empty()); } + + #[test] + fn create_spanners_rejects_a_spanner_id_that_collides_with_a_column() { + let columns = vec![column("a", "a"), column("b", "b")]; + let mut settings = Parameters::new(); + settings.insert("id".to_string(), ParameterValue::String("a".to_string())); + let spans = vec![spanner_with(&["a", "b"], Some("G"), settings)]; + + assert!(create_spanners(&columns, &spans).is_err()); + } + + fn spanner_with_id(columns: &[&str], id: &str) -> Spanner { + let mut settings = Parameters::new(); + settings.insert("id".to_string(), ParameterValue::String(id.to_string())); + spanner_with(columns, Some(""), settings) + } + + fn table_with_spans(spans: Vec) -> crate::Table { + crate::Table { + spans, + ..crate::Table::new() + } + } + + #[test] + fn resolve_spanner_ids_expands_a_reference_to_an_earlier_spanners_columns() { + let table = table_with_spans(vec![ + spanner_with_id(&["a", "b"], "x"), + spanner(&["x", "c"]), + ]); + + let resolved = table.resolve_spanner_ids().unwrap(); + + assert_eq!(resolved[1].columns, vec!["a", "b", "c"]); + } + + #[test] + fn resolve_spanner_ids_leaves_a_forward_reference_unresolved() { + // "x" is declared after it's referenced here — left as a literal + // string, to be caught downstream as an unknown column. + let table = table_with_spans(vec![ + spanner(&["x", "c"]), + spanner_with_id(&["a", "b"], "x"), + ]); + + let resolved = table.resolve_spanner_ids().unwrap(); + + assert_eq!(resolved[0].columns, vec!["x", "c"]); + } + + #[test] + fn resolve_spanner_ids_rejects_a_duplicate_id() { + let table = table_with_spans(vec![ + spanner_with_id(&["a"], "dup"), + spanner_with_id(&["b"], "dup"), + ]); + + assert!(table.resolve_spanner_ids().is_err()); + } + + #[test] + fn resolve_spanner_ids_resolves_transitively() { + // z -> y -> x: z references y, which already expanded its own + // reference to x by the time z is processed. + let table = table_with_spans(vec![ + spanner_with_id(&["a", "b"], "x"), + spanner_with_id(&["x", "c"], "y"), + spanner(&["y", "d"]), + ]); + + let resolved = table.resolve_spanner_ids().unwrap(); + + assert_eq!(resolved[2].columns, vec!["a", "b", "c", "d"]); + } } diff --git a/src/table/mod.rs b/src/table/mod.rs index f0cae157..74380b6e 100644 --- a/src/table/mod.rs +++ b/src/table/mod.rs @@ -50,6 +50,49 @@ impl Default for Table { } } +impl Table { + /// Expand a SPAN's `ACROSS` entry that names an earlier spanner's `id` + /// into that spanner's own columns — `SPAN 'Y' ACROSS x_id, c` (after + /// `SPAN 'X' ACROSS a, b SETTING id => 'x_id'`) resolves to columns a, + /// b, c for `Y`. Only ids from earlier spans are recognised; an id + /// declared later, or a genuine typo, is left as a literal string and + /// caught downstream as an unknown column, the same as any other bad + /// reference. + /// + /// Lives here rather than alongside its sibling spanner-resolution + /// steps in `execute::table_spanner` because it operates on `Spanner` + /// alone, with no `DataFrame` involved — `validate()` calls it directly + /// to catch a duplicate id without depending on `execute`. + pub fn resolve_spanner_ids(&self) -> Result, String> { + let mut resolved: Vec = Vec::with_capacity(self.spans.len()); + let mut ids: std::collections::HashMap = std::collections::HashMap::new(); + + for span in &self.spans { + let columns = span + .columns + .iter() + .flat_map(|entry| match ids.get(entry) { + Some(&idx) => resolved[idx].columns.clone(), + None => vec![entry.clone()], + }) + .collect(); + + let mut resolved_span = span.clone(); + resolved_span.columns = columns; + + if let Some(id) = resolved_span.settings.get("id").and_then(|v| v.as_str()) { + if ids.insert(id.to_string(), resolved.len()).is_some() { + return Err(format!("Duplicate SPAN id '{id}'")); + } + } + + resolved.push(resolved_span); + } + + Ok(resolved) + } +} + /// One `SPAN` clause: a named group of columns rendered as one spanner cell /// above them. #[derive(Debug, Clone, Serialize, Deserialize)] @@ -82,6 +125,13 @@ const SPAN_PARAMS: &[ParamDefinition] = &[ default: DefaultParamValue::Null, constraint: ParamConstraint::count(1.0), }, + ParamDefinition { + name: "id", + // No default: absence means this spanner has no id and can't be + // referenced by a later one's ACROSS list. + default: DefaultParamValue::Null, + constraint: ParamConstraint::string(), + }, ]; impl Spanner { diff --git a/src/validate.rs b/src/validate.rs index 596240ec..c554d3d5 100644 --- a/src/validate.rs +++ b/src/validate.rs @@ -308,6 +308,13 @@ pub fn validate(query: &str) -> Result { }); } } + + if let Err(e) = table.resolve_spanner_ids() { + errors.push(ValidationError { + message: e, + location: None, + }); + } } Ok(Validated {