diff --git a/ggsql-cli/src/main.rs b/ggsql-cli/src/main.rs index e27fdb5a..2ca80fca 100644 --- a/ggsql-cli/src/main.rs +++ b/ggsql-cli/src/main.rs @@ -430,8 +430,8 @@ fn render_spec(spec: ResolvedSpec, args: &RenderArgs, writer: &WriterSpec) { ResolvedSpec::Table(table) => { if args.verbose { eprintln!("\nQuery executed:"); - eprintln!(" Rows: {}", table.body().height()); - eprintln!(" Columns: {}", table.body().width()); + eprintln!(" Rows: {}", table.nrow()); + eprintln!(" Columns: {}", table.ncol()); } } } diff --git a/ggsql-jupyter/src/executor.rs b/ggsql-jupyter/src/executor.rs index 6e79b2ba..613680e2 100644 --- a/ggsql-jupyter/src/executor.rs +++ b/ggsql-jupyter/src/executor.rs @@ -294,14 +294,14 @@ impl QueryExecutor { ResolvedSpec::Table(table) => { tracing::info!( "Query executed: {} rows, {} cols", - table.body().height(), - table.body().width() + table.nrow(), + table.ncol() ); for warning in table.warnings() { tracing::warn!("{}", warning.message); } - let html = HtmlWriter::new().write_table(table.table(), table.body())?; + let html = HtmlWriter::new().write_table(table.cells())?; tracing::debug!("Generated HTML table: {} chars", html.len()); Ok(ExecutionResult::Table { html }) diff --git a/src/doc/API.md b/src/doc/API.md index c9449059..83397de3 100644 --- a/src/doc/API.md +++ b/src/doc/API.md @@ -408,9 +408,9 @@ pub trait Writer { /// Check whether a plot can be rendered by this writer, without rendering it fn validate_plot(&self, spec: &Plot) -> Result<()>; - /// Render a resolved table and its body data. Defaults to an "unsupported" - /// error; only `HtmlWriter` overrides it as of this writing. - fn write_table(&self, table: &Table, body: &DataFrame) -> Result { .. } + /// Render a resolved table's cells. Defaults to an "unsupported" error; + /// only `HtmlWriter` overrides it as of this writing. + fn write_table(&self, cells: &[TableCell]) -> Result { .. } /// Render a `ResolvedSpec` from `reader.execute()` — the usual entry point. /// Dispatches to `write_plot`/`write_table` depending on the variant. diff --git a/src/execute/mod.rs b/src/execute/mod.rs index 9ada5a9a..456251f4 100644 --- a/src/execute/mod.rs +++ b/src/execute/mod.rs @@ -23,7 +23,7 @@ mod table; pub use casting::TypeRequirement; pub use cte::CteDefinition; pub use schema::TypeInfo; -pub use table::resolve_table_with_reader; +pub use table::{resolve_table_with_reader, TableCell, TableCellKind}; use crate::naming; use crate::parser; diff --git a/src/execute/table.rs b/src/execute/table.rs index 8c6ebb3f..1b01a34d 100644 --- a/src/execute/table.rs +++ b/src/execute/table.rs @@ -2,12 +2,18 @@ //! //! 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`. +//! 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 +//! resolution logic is split across `schema.rs`/`casting.rs`/`layer.rs`/ +//! `scale.rs`/`position.rs`/`cte.rs` rather than left in one file. +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::{GgsqlError, Result, Spec}; +use crate::{DataFrame, GgsqlError, Result, Spec}; /// Resolve a TABULATE query into a `ResolvedTable`. /// @@ -50,9 +56,430 @@ pub fn resolve_table_with_reader(query: &str, reader: &dyn Reader) -> Result NULL` empties the label; +/// `LABEL col => 'text'` sets it to `text`. +fn create_table_columns(df: &DataFrame, labels: &Labels) -> Vec { + df.get_column_names() + .into_iter() + .map(|name| { + let label = match labels.labels.get(&name) { + None => name.clone(), + Some(None) => String::new(), + Some(Some(label)) => label.clone(), + }; + TableColumn { name, label } + }) + .collect() +} + +/// 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. +fn create_column_labels(columns: &[TableColumn]) -> Vec { + columns + .iter() + .enumerate() + .map(|(index, column)| TableCell { + kind: TableCellKind::ColumnLabel, + top: 0, + bottom: 0, + left: index, + right: index, + content: column.label.clone(), + }) + .collect() +} + +/// Build one `Body` cell per `DataFrame` value, numbered from `top == 0`, in +/// `columns`' order rather than `df`'s raw column order — the same seam +/// `create_column_labels` uses, so the two stay in sync under a future +/// reordering. Looks each column up in `df` **by name**, not position, since +/// `columns` may already be reordered relative to `df` by the time this runs. +fn create_body(df: &DataFrame, columns: &[TableColumn]) -> Vec { + let mut cells = Vec::new(); + + for (index, column) in columns.iter().enumerate() { + // Looked up once per column, outside the row loop: `DataFrame::column` + // is an `O(ncol)` scan over the schema, so doing this per row instead + // would cost `O(nrow * ncol)` lookups rather than `O(ncol)`. + let array = df + .column(&column.name) + .expect("TableColumn.name always names a column of df"); + + for row in 0..df.height() { + cells.push(TableCell { + kind: TableCellKind::Body, + top: row, + bottom: row, + left: index, + right: index, + content: value_to_string(array, row), + }); + } + } + + 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 + .iter() + .map(|cell| cell.bottom) + .max() + .map_or(0, |bottom| bottom + 1); + + for cell in &mut body { + cell.offset_rows(row_offset); + } + + let mut cells = column_labels; + cells.extend(body); + cells +} + +/// 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 +/// footprint spanning cells actually cover, not with `cells.len()` squared. +fn validate_overlaps(cells: &[TableCell]) -> Result<()> { + let mut occupied = std::collections::HashSet::new(); + + for cell in cells { + for row in cell.top..=cell.bottom { + for col in cell.left..=cell.right { + if !occupied.insert((row, col)) { + return Err(GgsqlError::ValidationError(format!( + "Table layout has more than one cell at row {row}, column {col}" + ))); + } + } + } + } + + Ok(()) +} + +// ============================================================================= +// Public API: TableCell +// ============================================================================= + +/// What role a `TableCell` plays in the table's layout. +/// +/// Naming follows R's gt package (`column_labels`, `body`, ...), since ggsql's +/// table grammar is expected to keep drawing on its part vocabulary as more +/// of it (spanners, stub, footnotes, source notes) gets built out here. +/// +/// Lets a writer tell cells apart (e.g. `` vs ``) without relying on +/// position — a column label is a `ColumnLabel` cell, not "whatever's in row +/// 0". A caption is expected to become a `TableCell` too once `Table` can +/// resolve one (still just text with a position, spanning the full width) — +/// not added yet, since nothing produces one today. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum TableCellKind { + /// A column label (gt's `column_labels`). + ColumnLabel, + /// A data value (gt's `body`). + Body, +} + +/// A single positioned cell within a resolved table layout. +/// +/// Parallel to `PreparedData` on the Plot side (an intermediate resolution +/// type, not the final `ResolvedTable` envelope) — but there is no Plot-side +/// equivalent to the shape itself, since Plot resolves at `DataFrame` +/// granularity, not per-cell. +/// +/// Position is an inclusive grid rectangle: `top`/`bottom` are row indices, +/// `left`/`right` are column indices, 0-based, inclusive on both ends. A +/// non-spanning cell has `top == bottom` and `left == right`. Colspan/rowspan +/// and adjacency helpers beyond `offset_rows`/`offset_cols` are expected to +/// live elsewhere and account for the inclusive convention themselves, +/// rather than each caller doing `+ 1` arithmetic against these fields +/// directly. Style/formatting fields are deliberately not included yet — add +/// them once a feature needs them. +#[derive(Debug, Clone)] +pub struct TableCell { + /// What role this cell plays (column label, body, ...). + pub kind: TableCellKind, + /// Top row index (inclusive). + pub top: usize, + /// Bottom row index (inclusive). + pub bottom: usize, + /// Left column index (inclusive). + pub left: usize, + /// Right column index (inclusive). + pub right: usize, + /// The cell's text content. + pub content: String, +} + +impl TableCell { + /// Shift this cell down by `rows`, moving `top` and `bottom` together so + /// a spanning cell keeps its height. + pub fn offset_rows(&mut self, rows: usize) { + self.top += rows; + self.bottom += rows; + } + + /// Shift this cell right by `cols`, moving `left` and `right` together + /// so a spanning cell keeps its width. + pub fn offset_cols(&mut self, cols: usize) { + self.left += cols; + self.right += cols; + } +} + +#[cfg(test)] +mod layout_tests { + use super::*; + use crate::df; + + fn column(name: &str, label: &str) -> TableColumn { + TableColumn { + name: name.to_string(), + label: label.to_string(), + } + } + + #[test] + fn create_table_columns_resolves_default_suppress_and_override() { + let frame = df! { + "id" => vec![1i32], + "name" => vec!["a".to_string()], + "extra" => vec![true], + } + .unwrap(); + + let mut labels = Labels::default(); + labels + .labels + .insert("id".to_string(), Some("ID".to_string())); + labels.labels.insert("name".to_string(), None); + // "extra" has no entry at all: no LABEL clause mentioned it. + + let columns = create_table_columns(&frame, &labels); + + assert_eq!(columns[0].name, "id"); + assert_eq!(columns[0].label, "ID"); // overridden + assert_eq!(columns[1].label, ""); // explicitly suppressed + assert_eq!(columns[2].label, "extra"); // absent: kept as-is + } + + #[test] + fn create_column_labels_builds_one_cell_per_column_at_row_zero() { + let columns = vec![column("id", "id"), column("name", "name")]; + + let labels = create_column_labels(&columns); + + assert_eq!(labels.len(), 2); + assert_eq!(labels[0].kind, TableCellKind::ColumnLabel); + assert_eq!(labels[0].top, 0); + assert_eq!(labels[0].bottom, 0); + assert_eq!(labels[0].left, 0); + assert_eq!(labels[0].right, 0); + assert_eq!(labels[0].content, "id"); + assert_eq!(labels[1].left, 1); + assert_eq!(labels[1].right, 1); + assert_eq!(labels[1].content, "name"); + } + + #[test] + fn create_body_numbers_rows_from_zero() { + let frame = df! { + "id" => vec![1i32, 2], + "name" => vec!["a".to_string(), "b".to_string()], + } + .unwrap(); + let columns = create_table_columns(&frame, &Labels::default()); + + let body = create_body(&frame, &columns); + + assert_eq!(body.len(), 4); + assert!(body.iter().all(|cell| cell.kind == TableCellKind::Body)); + // Column 0 ("id"): both rows, before column 1 starts — cells are + // pushed column-major, not row-major (see create_body's inline + // comment on why `array` is looked up once per column). + assert_eq!(body[0].top, 0); + assert_eq!(body[0].bottom, 0); + assert_eq!(body[0].left, 0); + assert_eq!(body[0].content, "1"); + assert_eq!(body[1].top, 1); + assert_eq!(body[1].bottom, 1); + assert_eq!(body[1].left, 0); + assert_eq!(body[1].content, "2"); + // Column 1 ("name") + assert_eq!(body[2].top, 0); + assert_eq!(body[2].left, 1); + assert_eq!(body[2].content, "a"); + assert_eq!(body[3].top, 1); + assert_eq!(body[3].left, 1); + assert_eq!(body[3].content, "b"); + } + + #[test] + fn create_body_looks_up_columns_by_name_not_position() { + // `columns` reordered relative to `frame`'s own column order — + // `create_body` must follow `columns`, not `df`'s raw position, for + // spanner-driven reordering to actually reach the body. + let frame = df! { + "id" => vec![1i32], + "name" => vec!["a".to_string()], + } + .unwrap(); + let columns = vec![column("name", "name"), column("id", "id")]; + + let body = create_body(&frame, &columns); + + assert_eq!(body[0].content, "a"); // "name" column, placed first + assert_eq!(body[1].content, "1"); // "id" column, placed second + } + + fn cell(kind: TableCellKind, top: usize, bottom: usize, content: &str) -> TableCell { + TableCell { + kind, + top, + bottom, + left: 0, + right: 0, + content: content.to_string(), + } + } + + #[test] + fn compose_cells_shifts_the_body_below_a_single_label_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); + + assert_eq!(cells.len(), 3); + assert_eq!(cells[0].kind, TableCellKind::ColumnLabel); + assert_eq!(cells[0].top, 0); + assert_eq!(cells[1].kind, TableCellKind::Body); + assert_eq!(cells[1].top, 1); + assert_eq!(cells[1].bottom, 1); + assert_eq!(cells[2].top, 2); + assert_eq!(cells[2].bottom, 2); + } + + #[test] + fn compose_cells_offsets_by_the_label_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. + 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); + + assert_eq!(cells[1].top, 2); + assert_eq!(cells[1].bottom, 2); + } + + fn cell_at( + kind: TableCellKind, + top: usize, + bottom: usize, + left: usize, + right: usize, + ) -> TableCell { + TableCell { + kind, + top, + bottom, + left, + right, + content: String::new(), + } + } + + #[test] + fn validate_overlaps_accepts_a_disjoint_layout() { + let cells = vec![ + cell_at(TableCellKind::ColumnLabel, 0, 0, 0, 0), + cell_at(TableCellKind::ColumnLabel, 0, 0, 1, 1), + cell_at(TableCellKind::Body, 1, 1, 0, 0), + cell_at(TableCellKind::Body, 1, 1, 1, 1), + ]; + + assert!(validate_overlaps(&cells).is_ok()); + } + + #[test] + fn validate_overlaps_rejects_two_cells_at_the_same_position() { + let cells = vec![ + cell_at(TableCellKind::Body, 0, 0, 0, 0), + cell_at(TableCellKind::Body, 0, 0, 0, 0), + ]; + + let error = validate_overlaps(&cells).unwrap_err(); + assert!( + matches!(error, GgsqlError::ValidationError(msg) if msg.contains("row 0, column 0")) + ); + } + + #[test] + fn validate_overlaps_rejects_a_spanning_cell_overlapping_a_later_one() { + // A cell spanning columns 0..=1 on row 0 overlapping a second cell + // that only touches column 1 on the same row — the shape a spanner + // bug or a bad spanner declaration would produce, not something + // 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), + ]; + + assert!(validate_overlaps(&cells).is_err()); + } } #[cfg(test)] @@ -75,8 +502,8 @@ mod tests { let resolved = resolve_table_with_reader("TABULATE FROM sales", &reader).unwrap(); assert_eq!(resolved.sql(), "SELECT * FROM sales"); - assert_eq!(resolved.body().height(), 3); - assert_eq!(resolved.body().width(), 2); + assert_eq!(resolved.nrow(), 3); + assert_eq!(resolved.ncol(), 2); } #[test] @@ -88,10 +515,7 @@ mod tests { resolve_table_with_reader("SELECT * FROM sales TABULATE", &reader).unwrap(); assert_eq!(from_only.sql(), select_then_tabulate.sql()); - assert_eq!( - from_only.body().height(), - select_then_tabulate.body().height() - ); + assert_eq!(from_only.nrow(), select_then_tabulate.nrow()); } #[test] diff --git a/src/lib.rs b/src/lib.rs index 99f11a16..0ff8b1b3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -89,6 +89,11 @@ pub use util::{and_list, and_list_quoted, or_list, or_list_quoted}; // DataFrame abstraction (wraps Arrow RecordBatch) pub use dataframe::DataFrame; +// Re-export the resolved table layout Writer::write_table needs — the +// Table-side counterpart to DataFrame, not to the plot:: AST vocabulary +// above, since Table has no specification vocabulary of its own yet. +pub use execute::{TableCell, TableCellKind}; + /// Main library error type #[derive(thiserror::Error, Debug)] pub enum GgsqlError { diff --git a/src/parser/builder.rs b/src/parser/builder.rs index ea1aa53d..bc5591e6 100644 --- a/src/parser/builder.rs +++ b/src/parser/builder.rs @@ -244,7 +244,7 @@ pub fn build_ast(source: &SourceTree) -> Result> { ) } "tabulate_statement" => { - let table = build_tabulate_statement(&stmt_node, source); + let table = build_tabulate_statement(&stmt_node, source)?; (table.source.is_some(), "TABULATE", Spec::Table(table)) } other => { @@ -343,19 +343,40 @@ fn build_visualise_statement(node: &Node, source: &SourceTree) -> Result { } /// Build a single Table from a tabulate_statement node -fn build_tabulate_statement(node: &Node, source: &SourceTree) -> Table { +fn build_tabulate_statement(node: &Node, source: &SourceTree) -> Result { let mut table = Table::new(); let mut cursor = node.walk(); for child in node.children(&mut cursor) { - if child.kind() == "single_source_from" { - if let Some(source_node) = child.child_by_field_name("source") { - table.source = Some(parse_data_source(&source_node, source)); + match child.kind() { + "single_source_from" => { + if let Some(source_node) = child.child_by_field_name("source") { + table.source = Some(parse_data_source(&source_node, source)); + } } + "label_clause" => { + process_tab_clause(&child, source, &mut table)?; + } + _ => {} } } - table + Ok(table) +} + +/// 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)?; + } + _ => {} + } + + Ok(()) } /// Process a visualization clause node diff --git a/src/plot/main.rs b/src/plot/main.rs index 3c0b1ffa..bfa4c351 100644 --- a/src/plot/main.rs +++ b/src/plot/main.rs @@ -74,7 +74,7 @@ pub struct Plot { } /// Text labels (from LABELS clause) -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Default)] pub struct Labels { /// Label assignments (label type → text, None = suppress) pub labels: HashMap>, diff --git a/src/reader/mod.rs b/src/reader/mod.rs index 23891e6f..fe99230c 100644 --- a/src/reader/mod.rs +++ b/src/reader/mod.rs @@ -37,7 +37,7 @@ use crate::execute::{prepare_data_with_reader, resolve_table_with_reader}; use crate::parser::{self, SourceTree}; use crate::plot::{CastTargetType, Plot}; use crate::validate::{validate, ValidationWarning}; -use crate::{naming, DataFrame, GgsqlError, Result, Spec, Table}; +use crate::{naming, DataFrame, GgsqlError, Result, Spec, Table, TableCell}; // ============================================================================= // SQL Dialect @@ -710,16 +710,13 @@ pub struct Metadata { pub struct ResolvedTable { /// The resolved table specification pub(crate) table: Table, - // PROVISIONAL, NOT A FINAL DESIGN DECISION: a plain `DataFrame` is enough - // to design the execution plumbing against, but this was never settled - // as the real representation. It will very likely need to become a - // table-specific intermediate representation once real table writers - // exist (e.g. an HTML/gt-style writer) and we know what they actually - // need `body` to carry. Don't build on this shape assuming it's final. - /// The data resolved from `table.source` (or the main SQL if there was no - /// TABULATE FROM) - pub(crate) body: DataFrame, - /// The SQL query that was executed to produce `body` + /// The resolved layout: one cell per column label and per data value, + /// resolved from `table.source` (or the main SQL if there was no + /// TABULATE FROM). See `TableCell` for the position/kind conventions. + /// `nrow()`/`ncol()` are computed from this rather than stored + /// separately, so there's one source of truth for the table's shape. + pub(crate) cells: Vec, + /// The SQL query that was executed to produce `cells` pub(crate) sql: String, /// Validation warnings from preparation pub(crate) warnings: Vec, diff --git a/src/reader/spec.rs b/src/reader/spec.rs index 6cd9dec1..f35ed489 100644 --- a/src/reader/spec.rs +++ b/src/reader/spec.rs @@ -5,7 +5,7 @@ use std::collections::HashMap; use crate::naming; use crate::plot::Plot; use crate::validate::ValidationWarning; -use crate::{DataFrame, Table}; +use crate::{DataFrame, Table, TableCell}; use super::{Metadata, ResolvedPlot, ResolvedSpec, ResolvedTable}; @@ -115,13 +115,13 @@ impl ResolvedTable { /// Create a new ResolvedTable. pub(crate) fn new( table: Table, - body: DataFrame, + cells: Vec, sql: String, warnings: Vec, ) -> Self { Self { table, - body, + cells, sql, warnings, } @@ -132,14 +132,29 @@ impl ResolvedTable { &self.table } - /// Get the resolved body data. See the PROVISIONAL note on the `body` - /// field in `reader::mod` — this accessor's return type will likely - /// change once real table writers exist. - pub fn body(&self) -> &DataFrame { - &self.body + /// Get the resolved layout: one cell per column label and per data value. + pub fn cells(&self) -> &[TableCell] { + &self.cells } - /// The SQL query that was executed to produce `body`. + /// Number of data rows (not counting the column-label row), computed + /// from `cells`. The column-label row is always `bottom == 0`, so it + /// only determines this max when there are no data rows, where it + /// correctly gives `0`. + pub fn nrow(&self) -> usize { + self.cells.iter().map(|cell| cell.bottom).max().unwrap_or(0) + } + + /// Number of columns, computed from `cells`. + pub fn ncol(&self) -> usize { + self.cells + .iter() + .map(|cell| cell.right) + .max() + .map_or(0, |right| right + 1) + } + + /// The SQL query that was executed to produce `cells`. pub fn sql(&self) -> &str { &self.sql } diff --git a/src/table/mod.rs b/src/table/mod.rs index 75198a78..a88e81f2 100644 --- a/src/table/mod.rs +++ b/src/table/mod.rs @@ -2,11 +2,12 @@ //! //! This module will define the typed `Table` structure that represents parsed //! `TABULATE` statements, parallel to how `plot` defines `Plot` for `VISUALISE` -//! statements. It is currently minimal: only `source` (from `TABULATE FROM`) -//! is populated so far. +//! statements. Still minimal: `source` (from `TABULATE FROM`) and `labels` +//! (from `TABULATE LABEL`) are populated so far. use serde::{Deserialize, Serialize}; +use crate::plot::Labels; use crate::DataSource; /// Complete ggsql table specification. @@ -18,12 +19,20 @@ pub struct Table { /// `Plot`, there are no layers to hold a per-layer source override, so /// this is the only place a `TABULATE`'s data source can come from. pub source: Option, + /// Column display labels (from `TABULATE LABEL`). Reuses `plot::Labels` + /// as-is — the same "name → text, None = suppress" shape applies + /// unchanged, just keyed by column name instead of aesthetic name. An + /// empty `Labels` means no overrides at all. + pub labels: Labels, } impl Table { /// Create a new empty Table. pub fn new() -> Self { - Self { source: None } + Self { + source: None, + labels: Labels::default(), + } } } diff --git a/src/writer/html.rs b/src/writer/html.rs index 9d362e60..8faa3cab 100644 --- a/src/writer/html.rs +++ b/src/writer/html.rs @@ -1,19 +1,18 @@ //! A minimal HTML table writer. //! -//! Renders a `ResolvedTable`'s body as a bare `
` — no styling, no -//! headings/spanners/footnotes, since `Table` has no fields to describe -//! those yet. This is a stub to prove the Table → writer plumbing end to -//! end, not the real grammar-of-tables output; it deliberately does not -//! reuse `ggsql-jupyter`'s existing `dataframe_to_html`, since that's built -//! around `DataFrame` specifically, and `ResolvedTable.body`'s type is -//! itself still provisional (see the note on that field). - +//! Renders a `ResolvedTable`'s cells as a bare `
` — 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 +//! `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 crate::array_util::value_to_string; use crate::util::escape_html; use crate::writer::{Writer, WriterOptions}; -use crate::{DataFrame, GgsqlError, Plot, Result, Table}; +use crate::{DataFrame, GgsqlError, Plot, Result, TableCell, TableCellKind}; /// Renders a resolved table as a bare HTML `
`. Does not support plots. #[derive(Debug, Default)] @@ -47,21 +46,29 @@ impl Writer for HtmlWriter { )) } - fn write_table(&self, _table: &Table, body: &DataFrame) -> Result { + fn write_table(&self, cells: &[TableCell]) -> Result { + let mut column_labels: Vec<&TableCell> = cells + .iter() + .filter(|cell| cell.kind == TableCellKind::ColumnLabel) + .collect(); + column_labels.sort_by_key(|cell| cell.left); + + 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); + } + let mut html = String::from("
\n\n"); - for name in body.get_column_names() { - html.push_str(&format!("", escape_html(&name))); + for cell in column_labels { + html.push_str(&format!("", escape_html(&cell.content))); } html.push_str("\n\n\n"); - let columns = body.get_columns(); - for row in 0..body.height() { + for (_, mut row) in body_rows { + row.sort_by_key(|cell| cell.left); html.push_str(""); - for column in columns { - html.push_str(&format!( - "", - escape_html(&value_to_string(column, row)) - )); + for cell in row { + html.push_str(&format!("", escape_html(&cell.content))); } html.push_str("\n"); } diff --git a/src/writer/mod.rs b/src/writer/mod.rs index ac6091a6..c19b040a 100644 --- a/src/writer/mod.rs +++ b/src/writer/mod.rs @@ -29,7 +29,7 @@ //! without knowing which writer they picked. use crate::reader::ResolvedSpec; -use crate::{DataFrame, GgsqlError, Plot, Result, Table}; +use crate::{DataFrame, GgsqlError, Plot, Result, TableCell}; use std::collections::HashMap; pub mod options; @@ -164,25 +164,30 @@ pub trait Writer { /// Ok(()) if the spec is compatible, otherwise an error fn validate_plot(&self, spec: &Plot) -> Result<()>; - /// Generate output from a resolved table specification and its body data + /// Generate output from a resolved table's cells /// /// The table-side counterpart to `write_plot()`. Defaults to rejecting /// every table, so a writer that only supports Plot output (every writer, /// as of this writing) needs no changes; a writer that does support /// tables overrides this instead. /// + /// Unlike `write_plot`, there is no AST parameter: `Table` (the parsed + /// `TABULATE` spec) has nothing left that a writer needs by the time + /// `cells` exists — its only field (`source`) is already consumed + /// building `cells`. If `Table` grows something a writer genuinely needs + /// that isn't itself expressible as a cell, add it back then. + /// /// # Arguments /// - /// * `table` - The parsed TABULATE specification - /// * `body` - The resolved data (see the PROVISIONAL note on - /// `ResolvedTable.body` — this parameter's type may change) + /// * `cells` - The resolved table layout — see `TableCell` for the + /// position/kind conventions /// /// # Errors /// /// Returns `GgsqlError::WriterError` if this writer doesn't support /// tables, or output generation fails. - fn write_table(&self, table: &Table, body: &DataFrame) -> Result { - let _ = (table, body); + fn write_table(&self, cells: &[TableCell]) -> Result { + let _ = cells; Err(GgsqlError::WriterError( "this writer does not support tables".to_string(), )) @@ -219,7 +224,7 @@ pub trait Writer { fn render(&self, spec: &ResolvedSpec) -> Result { match spec { ResolvedSpec::Plot(plot) => self.write_plot(plot.plot(), plot.data()), - ResolvedSpec::Table(table) => self.write_table(table.table(), table.body()), + ResolvedSpec::Table(table) => self.write_table(table.cells()), } } } diff --git a/tree-sitter-ggsql/grammar.js b/tree-sitter-ggsql/grammar.js index 293d0f56..d36b5e21 100644 --- a/tree-sitter-ggsql/grammar.js +++ b/tree-sitter-ggsql/grammar.js @@ -668,13 +668,11 @@ module.exports = grammar({ caseInsensitive("VISUALIZE") ))), - // TABULATE — placeholder for tabular output, parallel to VISUALISE: an - // optional FROM right after the keyword, same single_source_from as - // visualise_statement (no joins, no comma list). No other clauses yet: - // the Table AST it builds has no fields to populate. + // TABULATE — still incomplete, more clauses expected as Table grows. tabulate_statement: $ => prec.dynamic(1, seq( $.tabulate_keyword, optional($.single_source_from), + optional($.label_clause), )), // TABULATE keyword as explicit high-precedence token (mirrors visualise_keyword) diff --git a/tree-sitter-ggsql/test/corpus/basic.txt b/tree-sitter-ggsql/test/corpus/basic.txt index 6652e9dd..dc6d7d81 100644 --- a/tree-sitter-ggsql/test/corpus/basic.txt +++ b/tree-sitter-ggsql/test/corpus/basic.txt @@ -4569,3 +4569,25 @@ TABULATE FROM sales source: (qualified_name (identifier (bare_identifier)))))) + +================================================================================ +TABULATE LABEL clause +================================================================================ + +TABULATE FROM sales LABEL id => 'ID' + +-------------------------------------------------------------------------------- + +(query + (tabulate_statement + (tabulate_keyword) + (single_source_from + source: (qualified_name + (identifier + (bare_identifier)))) + (label_clause + (label_assignment + name: (label_type + (identifier + (bare_identifier))) + value: (string)))))
{}{}
{}{}