Skip to content

Nicer hints and errors in rill query#9118

Merged
begelundmuller merged 2 commits intomainfrom
begelundmuller/better-rill-query
Mar 25, 2026
Merged

Nicer hints and errors in rill query#9118
begelundmuller merged 2 commits intomainfrom
begelundmuller/better-rill-query

Conversation

@begelundmuller
Copy link
Copy Markdown
Contributor

@begelundmuller begelundmuller commented Mar 25, 2026

Checklist:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

@begelundmuller begelundmuller requested a review from pjain1 March 25, 2026 10:25
@begelundmuller begelundmuller self-assigned this Mar 25, 2026
Comment thread cli/cmd/query/query.go

// Print rows with warning if the default limit was reached.
ch.PrintQueryResponse(res)
if !cmd.Flags().Changed("limit") && len(res.Data) == limit {
Copy link
Copy Markdown
Member

@pjain1 pjain1 Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: very rare case but what if there were actually only 100 rows in the dataset, may be not worth handling.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'm okay with not handling that since the caller didn't specify an explicit limit and this is only a warning. If the caller retries with an explicit limit, the warning goes away :)

@begelundmuller begelundmuller merged commit ec3deb4 into main Mar 25, 2026
18 of 19 checks passed
@begelundmuller begelundmuller deleted the begelundmuller/better-rill-query branch March 25, 2026 15:02
begelundmuller added a commit that referenced this pull request Mar 30, 2026
* Nicer hints and errors in `rill query`

* QA
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.

2 participants