Skip to content

Preserve native query and query-file multiplicity - #553

Open
sdairs wants to merge 1 commit into
stack/469-select-client-versionfrom
stack/470-client-query-multiplicity
Open

Preserve native query and query-file multiplicity#553
sdairs wants to merge 1 commit into
stack/469-select-client-versionfrom
stack/470-client-query-multiplicity

Conversation

@sdairs

@sdairs sdairs commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • preserve repeatable --query and --queries-file values in native client argv order
  • reject mixed inline/file sources at clap time and guard repeated queries on pre-23.9 clients
  • document the native contract and cover empty values plus -- pass-through with fake children

Tests

  • cargo test -p clickhousectl --test local_client_selectors_test
  • cargo test -p clickhousectl clickhouse_client
  • cargo test -p clickhousectl
  • cargo fmt --all --check
  • cargo clippy -p clickhousectl --all-targets -- -D warnings

Closes #470

@sdairs

sdairs commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

Carry-over review from PR #506 (old PR for #470). This PR is the more complete implementation: it is the only one that implements issue #470's final acceptance criterion (version check so the wrapper doesn't promise an unsupported shape — the <23.9.1.1854 gate in crates/clickhousectl/src/local/mod.rs:~324-337, correctly placed after resolution for both named and direct paths, with boundary tests at 23.9.1.1853/1854). #506 punted on this with manual source inspection. Preferred over #506.

Carry-over analysis of the one substantive comment on #506: Cursor Bugbot claimed the single-flag multitoken --queries-file forwarding was wrong; sdairs refuted it with ClientBase.cpp source citations across 5 versions (multitoken(), so one flag with many paths is native-valid) while noting "Repeated flags may also parse, but one flag per file is not required." This PR adopts one-flag-per-path (mod.rs:~311-313), which is inside sdairs' validated envelope, and proves forwarding of that shape with a fake child. No adverse carry-over.

Two minor regressions vs #506 to decide on:

  1. The wrapper no longer accepts multi-path-after-one-flag input (--queries-file a.sql b.sql, via num_args = 1..). The issue's failing examples used repeated flags, so acceptance criteria are still met — but confirm the narrowing is intentional.
  2. Preserve native query and query-file multiplicity in local client #506 had a help-text assertion test; this PR does not. Consider adding one.

Note: the 23.9.1.1854 floor is sourced only via the linked changelog + pinned tests (plausible; repeated files predate repeatable queries). Fine, but keep the citation in the README as-is.

CI note: local postgres edge cases fails on this branch but identically on its stack base; the diff touches no Postgres code — inherited from #547's script drift, not this PR.

@sdairs
sdairs force-pushed the stack/470-client-query-multiplicity branch from 074bff2 to 63343f8 Compare August 26, 2026 18:58
@sdairs
sdairs force-pushed the stack/470-client-query-multiplicity branch 2 times, most recently from 5fc6498 to 7daf66b Compare August 26, 2026 19:03
@sdairs
sdairs force-pushed the stack/470-client-query-multiplicity branch from 7daf66b to 941aef9 Compare August 26, 2026 19:07
@sdairs
sdairs force-pushed the stack/470-client-query-multiplicity branch from 941aef9 to f3d5b9a Compare August 26, 2026 19:10
@sdairs
sdairs force-pushed the stack/470-client-query-multiplicity branch 2 times, most recently from 4e32889 to 7ff256e Compare August 26, 2026 19:20
@sdairs
sdairs force-pushed the stack/470-client-query-multiplicity branch 2 times, most recently from f0cc128 to d54adf4 Compare August 26, 2026 19:28
@sdairs
sdairs force-pushed the stack/470-client-query-multiplicity branch from d54adf4 to fa8b1a4 Compare August 26, 2026 19:31
@sdairs
sdairs force-pushed the stack/470-client-query-multiplicity branch from fa8b1a4 to bf8533f Compare August 26, 2026 19:34
@sdairs
sdairs force-pushed the stack/470-client-query-multiplicity branch from bf8533f to 5cbd879 Compare August 26, 2026 19:36
@sdairs
sdairs force-pushed the stack/470-client-query-multiplicity branch from 5cbd879 to 8dacb50 Compare August 26, 2026 20:10
@sdairs
sdairs force-pushed the stack/470-client-query-multiplicity branch from 8dacb50 to 173af49 Compare August 26, 2026 20:30
@sdairs
sdairs force-pushed the stack/470-client-query-multiplicity branch 4 times, most recently from edefe37 to c9a28aa Compare August 26, 2026 20:44
@sdairs
sdairs force-pushed the stack/470-client-query-multiplicity branch from c9a28aa to 07b7496 Compare August 26, 2026 20:49
@sdairs
sdairs force-pushed the stack/470-client-query-multiplicity branch from 07b7496 to 92d1899 Compare August 26, 2026 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Preserve native query and query-file multiplicity in local client

1 participant