From 09ed16cf900feb8332b3453f6ab84086c9034cdb Mon Sep 17 00:00:00 2001 From: Noah Muldavin Date: Tue, 1 Sep 2026 13:55:39 -0700 Subject: [PATCH 1/3] feat(ddsql)!: remove time-series command Remove the redundant DDSQL time-series CLI path and direct users to table --limit 5000 for the former row-limit default. Reject the removed subcommand explicitly and remove it from help, docs, and the parity harness. --- docs/COMMANDS.md | 4 ++-- scripts/test_harness.py | 6 ------ src/commands/ddsql.rs | 13 ------------- src/main.rs | 34 +--------------------------------- src/test_commands.rs | 26 +++++++++++++++++++------- 5 files changed, 22 insertions(+), 61 deletions(-) diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 1c93dd53..296e7712 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -29,7 +29,7 @@ pup [options] # Nested commands | monitors | list, get, create, update, delete, search, diff | src/commands/monitors.rs | ✅ | | dashboards | list, get, create, update, diff, delete, url, annotations (list, get-page, create, update, delete) | src/commands/dashboards.rs, src/commands/annotations.rs | ✅ | | dbm | samples (search) | src/commands/dbm.rs | ✅ | -| ddsql | table, time-series, spec, schema (tables, columns) | src/commands/ddsql.rs | ✅ | +| ddsql | table, spec, schema (tables, columns) | src/commands/ddsql.rs | ✅ | | debugger | probes (list, get, create, delete, watch) | src/commands/debugger.rs | ✅ | | slos | list, get, create, update, diff, delete, status | src/commands/slos.rs | ✅ | | incidents | list, get, attachments, settings, handles, postmortem-templates | src/commands/incidents.rs | ✅ | @@ -177,7 +177,7 @@ pup infrastructure hosts list - **traces** - APM spans metrics (list, get, create, update, delete) - **rum** - Real User Monitoring (apps, metrics, retention-filters, sessions) - **events** - Infrastructure events (post, list, search, get) -- **ddsql** - DDSQL queries and discovery (table, time-series, spec, schema) +- **ddsql** - DDSQL queries and discovery (table, spec, schema); use `pup ddsql table --limit 5000` for the former 5,000-row default - **symdb** - Symbol Database queries (search scopes, probe locations) ### Monitoring & Alerting diff --git a/scripts/test_harness.py b/scripts/test_harness.py index 3faa6a8e..b99bccf0 100755 --- a/scripts/test_harness.py +++ b/scripts/test_harness.py @@ -1494,12 +1494,6 @@ def status(self) -> str: "category": "auth_required", "expect_json": True, }, - { - "label": "ddsql time-series", - "args": ["ddsql", "time-series", "--query=SELECT 1"], - "category": "auth_required", - "expect_json": True, - }, # ── integrations (new) ──────────────────────────────────────────────── { "label": "integrations list", diff --git a/src/commands/ddsql.rs b/src/commands/ddsql.rs index 68830ef3..1674186d 100644 --- a/src/commands/ddsql.rs +++ b/src/commands/ddsql.rs @@ -793,19 +793,6 @@ pub async fn table( formatter::output(cfg, &rows) } -pub async fn time_series( - cfg: &Config, - query: &str, - from: &str, - to: &str, - _interval: Option, - limit: i32, -) -> Result<()> { - let query = resolve_query(query)?; - let rows = execute_ddsql_query(cfg, &query, from, to, Some(i64::from(limit))).await?; - formatter::output(cfg, &rows) -} - /// Transform a DDSQL columnar response into a row-based JSON array. /// /// Each column is `{"name": "col1", "values": ["a", "b"]}` under the diff --git a/src/main.rs b/src/main.rs index 79534ab5..03b8bfb9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1131,7 +1131,6 @@ enum Commands { /// /// COMMANDS: /// table Execute query and return table data (supports -o json/yaml/table/csv) - /// time-series Execute query and return time series data /// spec Print DDSQL reference guidance used by the editor tooling /// schema Discover DDSQL tables and columns /// @@ -1139,7 +1138,7 @@ enum Commands { /// pup ddsql table --query "SELECT * FROM reference_tables.offices_ips LIMIT 5" /// pup ddsql table --query "SELECT * FROM reference_tables.offices_ips" -o csv > results.csv /// cat query.sql | pup ddsql table --query - -o table - /// pup ddsql time-series --query "SELECT timestamp, value, tags->'host' AS host FROM dd.metrics_timeseries('avg:system.cpu.user{*} by {host}')" --from 1h + /// pup ddsql table --query "SELECT timestamp, value, tags->'host' AS host FROM dd.metrics_timeseries('avg:system.cpu.user{*} by {host}')" --from 1h --limit 5000 /// pup ddsql spec /// pup ddsql schema tables --query ec2 --limit 100 /// pup ddsql schema columns --table-id public.aws.ec2_instance @@ -4531,28 +4530,6 @@ enum DdsqlActions { #[arg(long, help = "Number of rows to skip (for pagination)")] offset: Option, }, - /// Execute DDSQL query and return time series data - #[command(name = "time-series")] - TimeSeries { - #[arg( - long, - allow_hyphen_values = true, - help = "DDSQL query string, or use --query - to read from stdin" - )] - query: String, - #[arg(long, default_value = "1h", help = "Start time")] - from: String, - #[arg(long, default_value = "now", help = "End time")] - to: String, - #[arg(long, help = "Aggregation interval in milliseconds (default: 60000)")] - interval: Option, - #[arg( - long, - default_value_t = 5000, - help = "Maximum number of rows to return" - )] - limit: i32, - }, /// Print DDSQL reference guidance from the editor tooling Spec, /// Discover DDSQL tables and columns @@ -17038,15 +17015,6 @@ async fn main_inner() -> anyhow::Result<()> { commands::ddsql::table(&cfg, &query, &from, &to, interval, Some(limit), offset) .await?; } - DdsqlActions::TimeSeries { - query, - from, - to, - interval, - limit, - } => { - commands::ddsql::time_series(&cfg, &query, &from, &to, interval, limit).await?; - } DdsqlActions::Spec => { commands::ddsql::spec(&cfg).await?; } diff --git a/src/test_commands.rs b/src/test_commands.rs index e5f18fd0..f9d099b1 100644 --- a/src/test_commands.rs +++ b/src/test_commands.rs @@ -530,18 +530,30 @@ fn test_ddsql_table_query_accepts_explicit_stdin_marker() { } #[test] -fn test_ddsql_time_series_query_accepts_explicit_stdin_marker() { +fn test_ddsql_time_series_is_rejected() { + let err = crate::Cli::command() + .try_get_matches_from(["pup", "ddsql", "time-series", "--query", "SELECT 1"]) + .expect_err("removed ddsql time-series command should not parse"); + + assert_eq!(err.kind(), clap::error::ErrorKind::InvalidSubcommand); + assert!(err + .to_string() + .contains("unrecognized subcommand 'time-series'")); +} + +#[test] +fn test_ddsql_table_accepts_former_row_limit() { use clap::Parser; - let cli = crate::Cli::try_parse_from(["pup", "ddsql", "time-series", "--query", "-"]) - .expect("ddsql time-series --query - should parse"); + let cli = crate::Cli::try_parse_from([ + "pup", "ddsql", "table", "--query", "SELECT 1", "--limit", "5000", + ]) + .expect("ddsql table should accept the former 5000-row default explicitly"); match cli.command { crate::Commands::Ddsql { action } => match action { - crate::DdsqlActions::TimeSeries { query, .. } => { - assert_eq!(query, "-"); - } - _ => panic!("expected DdsqlActions::TimeSeries"), + crate::DdsqlActions::Table { limit, .. } => assert_eq!(limit, 5000), + _ => panic!("expected DdsqlActions::Table"), }, _ => panic!("expected Commands::Ddsql"), } From d0224c7eecd829a969ab406acd326e23728b00e3 Mon Sep 17 00:00:00 2001 From: Noah Muldavin Date: Tue, 1 Sep 2026 14:37:20 -0700 Subject: [PATCH 2/3] docs(ddsql): rely on table row limit default Remove the explicit 5,000-row replacement guidance and its dedicated parse test per review feedback. --- docs/COMMANDS.md | 2 +- src/main.rs | 2 +- src/test_commands.rs | 18 ------------------ 3 files changed, 2 insertions(+), 20 deletions(-) diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 296e7712..cd3ebd91 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -177,7 +177,7 @@ pup infrastructure hosts list - **traces** - APM spans metrics (list, get, create, update, delete) - **rum** - Real User Monitoring (apps, metrics, retention-filters, sessions) - **events** - Infrastructure events (post, list, search, get) -- **ddsql** - DDSQL queries and discovery (table, spec, schema); use `pup ddsql table --limit 5000` for the former 5,000-row default +- **ddsql** - DDSQL queries and discovery (table, spec, schema) - **symdb** - Symbol Database queries (search scopes, probe locations) ### Monitoring & Alerting diff --git a/src/main.rs b/src/main.rs index 03b8bfb9..2b78cfe2 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1138,7 +1138,7 @@ enum Commands { /// pup ddsql table --query "SELECT * FROM reference_tables.offices_ips LIMIT 5" /// pup ddsql table --query "SELECT * FROM reference_tables.offices_ips" -o csv > results.csv /// cat query.sql | pup ddsql table --query - -o table - /// pup ddsql table --query "SELECT timestamp, value, tags->'host' AS host FROM dd.metrics_timeseries('avg:system.cpu.user{*} by {host}')" --from 1h --limit 5000 + /// pup ddsql table --query "SELECT timestamp, value, tags->'host' AS host FROM dd.metrics_timeseries('avg:system.cpu.user{*} by {host}')" --from 1h /// pup ddsql spec /// pup ddsql schema tables --query ec2 --limit 100 /// pup ddsql schema columns --table-id public.aws.ec2_instance diff --git a/src/test_commands.rs b/src/test_commands.rs index f9d099b1..85e1e64f 100644 --- a/src/test_commands.rs +++ b/src/test_commands.rs @@ -541,24 +541,6 @@ fn test_ddsql_time_series_is_rejected() { .contains("unrecognized subcommand 'time-series'")); } -#[test] -fn test_ddsql_table_accepts_former_row_limit() { - use clap::Parser; - - let cli = crate::Cli::try_parse_from([ - "pup", "ddsql", "table", "--query", "SELECT 1", "--limit", "5000", - ]) - .expect("ddsql table should accept the former 5000-row default explicitly"); - - match cli.command { - crate::Commands::Ddsql { action } => match action { - crate::DdsqlActions::Table { limit, .. } => assert_eq!(limit, 5000), - _ => panic!("expected DdsqlActions::Table"), - }, - _ => panic!("expected Commands::Ddsql"), - } -} - #[test] fn test_ddsql_table_query_requires_explicit_value() { let result = crate::Cli::command().try_get_matches_from(["pup", "ddsql", "table", "--query"]); From 9e100c7f1278ba67468ee93687aab94805bf5f46 Mon Sep 17 00:00:00 2001 From: Cody Lee Date: Thu, 3 Sep 2026 06:57:47 -0500 Subject: [PATCH 3/3] Remove deprecated test for ddsql time-series command Removed test for ddsql time-series command that was deprecated. --- src/test_commands.rs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/test_commands.rs b/src/test_commands.rs index 85e1e64f..d7c4053e 100644 --- a/src/test_commands.rs +++ b/src/test_commands.rs @@ -529,18 +529,6 @@ fn test_ddsql_table_query_accepts_explicit_stdin_marker() { } } -#[test] -fn test_ddsql_time_series_is_rejected() { - let err = crate::Cli::command() - .try_get_matches_from(["pup", "ddsql", "time-series", "--query", "SELECT 1"]) - .expect_err("removed ddsql time-series command should not parse"); - - assert_eq!(err.kind(), clap::error::ErrorKind::InvalidSubcommand); - assert!(err - .to_string() - .contains("unrecognized subcommand 'time-series'")); -} - #[test] fn test_ddsql_table_query_requires_explicit_value() { let result = crate::Cli::command().try_get_matches_from(["pup", "ddsql", "table", "--query"]);