From 6b4b015683a936b271e7106b045d499b6e7f7cef Mon Sep 17 00:00:00 2001 From: Noah Muldavin Date: Tue, 1 Sep 2026 14:27:31 -0700 Subject: [PATCH 1/2] feat(ddsql)!: remove dead interval and offset flags --- docs/COMMANDS.md | 2 +- src/commands/ddsql.rs | 2 -- src/main.rs | 14 +++++------- src/test_commands.rs | 51 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 58 insertions(+), 11 deletions(-) diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index fcec3a1c..d4c896ea 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) +- **ddsql** - DDSQL queries and discovery (table, spec, schema). Paginate deterministic, ordered results with `OFFSET n LIMIT m` in SQL. Control granularity with the query-wide `--from`/`--to` window or per-source table-function timestamp arguments; a `WHERE` time filter only filters existing buckets. - **symdb** - Symbol Database queries (search scopes, probe locations) ### Monitoring & Alerting diff --git a/src/commands/ddsql.rs b/src/commands/ddsql.rs index 7024d53c..f532bf5b 100644 --- a/src/commands/ddsql.rs +++ b/src/commands/ddsql.rs @@ -784,9 +784,7 @@ pub async fn table( query: &str, from: &str, to: &str, - _interval: Option, limit: Option, - _offset: Option, ) -> Result<()> { let query = resolve_query(query)?; let rows = execute_ddsql_query(cfg, &query, from, to, limit.map(i64::from)).await?; diff --git a/src/main.rs b/src/main.rs index 037ba186..3429829d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1143,6 +1143,11 @@ enum Commands { /// pup ddsql schema tables --query ec2 --limit 100 /// pup ddsql schema columns --table-id public.aws.ec2_instance /// + /// PAGINATION AND GRANULARITY: + /// Write OFFSET n LIMIT m in SQL to paginate deterministic, ordered results. + /// Set the query-wide time window with --from/--to. Override it per source with + /// table-function timestamp arguments. A WHERE time filter does not change source granularity. + /// /// AUTHENTICATION: /// All ddsql commands support OAuth2 (via 'pup auth login') or API key + Application key. #[command(verbatim_doc_comment)] @@ -4526,12 +4531,8 @@ enum DdsqlActions { help = "End time. Accepts the same formats as --from (e.g., now)" )] to: String, - #[arg(long, help = "Aggregation interval in milliseconds (default: 60000)")] - interval: Option, #[arg(long, default_value_t = 50, help = "Maximum number of rows to return")] limit: i32, - #[arg(long, help = "Number of rows to skip (for pagination)")] - offset: Option, }, /// Print DDSQL reference guidance from the editor tooling Spec, @@ -17059,12 +17060,9 @@ async fn main_inner() -> anyhow::Result<()> { query, from, to, - interval, limit, - offset, } => { - commands::ddsql::table(&cfg, &query, &from, &to, interval, Some(limit), offset) - .await?; + commands::ddsql::table(&cfg, &query, &from, &to, Some(limit)).await?; } DdsqlActions::Spec => { commands::ddsql::spec(&cfg).await?; diff --git a/src/test_commands.rs b/src/test_commands.rs index 6d874011..4b08a3f7 100644 --- a/src/test_commands.rs +++ b/src/test_commands.rs @@ -556,6 +556,57 @@ fn test_ddsql_table_query_accepts_explicit_stdin_marker() { } } +#[test] +fn test_ddsql_table_accepts_limit() { + use clap::Parser; + + let cli = crate::Cli::try_parse_from([ + "pup", "ddsql", "table", "--query", "SELECT 1", "--limit", "5000", + ]) + .expect("ddsql table should accept a row limit"); + + 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_rejects_removed_interval_and_offset_flags() { + for (flag, value) in [("--interval", "60000"), ("--offset", "5")] { + let err = crate::Cli::command() + .try_get_matches_from(["pup", "ddsql", "table", "--query", "SELECT 1", flag, value]) + .expect_err("removed ddsql table flag should not parse"); + + assert_eq!(err.kind(), clap::error::ErrorKind::UnknownArgument); + assert!(err.to_string().contains(flag)); + } +} + +#[test] +fn test_ddsql_help_omits_removed_interval_and_offset_flags() { + let mut command = crate::Cli::command(); + let ddsql = command + .find_subcommand_mut("ddsql") + .expect("ddsql command should exist"); + let ddsql_help = ddsql.render_long_help().to_string(); + let table_help = ddsql + .find_subcommand_mut("table") + .expect("ddsql table command should exist") + .render_long_help() + .to_string(); + + for removed_flag in ["--interval", "--offset"] { + assert!(!ddsql_help.contains(removed_flag)); + assert!(!table_help.contains(removed_flag)); + } + assert!(ddsql_help.contains("OFFSET n LIMIT m")); + assert!(ddsql_help.contains("--from/--to")); +} + #[test] fn test_ddsql_table_query_requires_explicit_value() { let result = crate::Cli::command().try_get_matches_from(["pup", "ddsql", "table", "--query"]); From 92caf5a8fbef0a5f3f98cae7dbde9c69b79c57a3 Mon Sep 17 00:00:00 2001 From: Noah Muldavin Date: Thu, 3 Sep 2026 14:57:31 -0700 Subject: [PATCH 2/2] fix(ddsql): align row limit guidance with API --- docs/COMMANDS.md | 2 +- src/main.rs | 14 ++++++++++--- src/test_commands.rs | 47 ++++++++++++++------------------------------ 3 files changed, 27 insertions(+), 36 deletions(-) diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index d4c896ea..fcec3a1c 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). Paginate deterministic, ordered results with `OFFSET n LIMIT m` in SQL. Control granularity with the query-wide `--from`/`--to` window or per-source table-function timestamp arguments; a `WHERE` time filter only filters existing buckets. +- **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 3429829d..023d654c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1143,10 +1143,14 @@ enum Commands { /// pup ddsql schema tables --query ec2 --limit 100 /// pup ddsql schema columns --table-id public.aws.ec2_instance /// - /// PAGINATION AND GRANULARITY: + /// PAGINATION: /// Write OFFSET n LIMIT m in SQL to paginate deterministic, ordered results. + /// Results will be capped at 5000 rows. Extend this up to 10000 with --limit; + /// paginate to retrieve more than 10000 rows. + /// + /// TIME WINDOW: /// Set the query-wide time window with --from/--to. Override it per source with - /// table-function timestamp arguments. A WHERE time filter does not change source granularity. + /// table-function timestamp arguments. A WHERE time filter does not change the source window. /// /// AUTHENTICATION: /// All ddsql commands support OAuth2 (via 'pup auth login') or API key + Application key. @@ -4531,7 +4535,11 @@ enum DdsqlActions { help = "End time. Accepts the same formats as --from (e.g., now)" )] to: String, - #[arg(long, default_value_t = 50, help = "Maximum number of rows to return")] + #[arg( + long, + default_value_t = 5000, + help = "API response row cap (1-10000); use SQL LIMIT to bound query results" + )] limit: i32, }, /// Print DDSQL reference guidance from the editor tooling diff --git a/src/test_commands.rs b/src/test_commands.rs index 4b08a3f7..70ce8fb6 100644 --- a/src/test_commands.rs +++ b/src/test_commands.rs @@ -557,13 +557,11 @@ fn test_ddsql_table_query_accepts_explicit_stdin_marker() { } #[test] -fn test_ddsql_table_accepts_limit() { +fn test_ddsql_table_uses_api_row_limit_default() { use clap::Parser; - let cli = crate::Cli::try_parse_from([ - "pup", "ddsql", "table", "--query", "SELECT 1", "--limit", "5000", - ]) - .expect("ddsql table should accept a row limit"); + let cli = crate::Cli::try_parse_from(["pup", "ddsql", "table", "--query", "SELECT 1"]) + .expect("ddsql table should parse"); match cli.command { crate::Commands::Ddsql { action } => match action { @@ -575,36 +573,21 @@ fn test_ddsql_table_accepts_limit() { } #[test] -fn test_ddsql_table_rejects_removed_interval_and_offset_flags() { - for (flag, value) in [("--interval", "60000"), ("--offset", "5")] { - let err = crate::Cli::command() - .try_get_matches_from(["pup", "ddsql", "table", "--query", "SELECT 1", flag, value]) - .expect_err("removed ddsql table flag should not parse"); - - assert_eq!(err.kind(), clap::error::ErrorKind::UnknownArgument); - assert!(err.to_string().contains(flag)); - } -} +fn test_ddsql_table_accepts_limit_override() { + use clap::Parser; -#[test] -fn test_ddsql_help_omits_removed_interval_and_offset_flags() { - let mut command = crate::Cli::command(); - let ddsql = command - .find_subcommand_mut("ddsql") - .expect("ddsql command should exist"); - let ddsql_help = ddsql.render_long_help().to_string(); - let table_help = ddsql - .find_subcommand_mut("table") - .expect("ddsql table command should exist") - .render_long_help() - .to_string(); + let cli = crate::Cli::try_parse_from([ + "pup", "ddsql", "table", "--query", "SELECT 1", "--limit", "10000", + ]) + .expect("ddsql table should accept a row limit override"); - for removed_flag in ["--interval", "--offset"] { - assert!(!ddsql_help.contains(removed_flag)); - assert!(!table_help.contains(removed_flag)); + match cli.command { + crate::Commands::Ddsql { action } => match action { + crate::DdsqlActions::Table { limit, .. } => assert_eq!(limit, 10000), + _ => panic!("expected DdsqlActions::Table"), + }, + _ => panic!("expected Commands::Ddsql"), } - assert!(ddsql_help.contains("OFFSET n LIMIT m")); - assert!(ddsql_help.contains("--from/--to")); } #[test]