Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Inline limit in SQL sent from
dbt show
#8641Inline limit in SQL sent from
dbt show
#8641Changes from 9 commits
40d6db1
d77bfe2
0cafc07
29445b4
a8d01ee
a2ace01
1de5677
1ebfd74
7c118f7
81f68e6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens here if
limit = -1
(get all rows)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It returns all rows! There is a test for that case: https://github.com/dbt-labs/dbt-core/pull/8641/files#diff-6cd0c2e8c5c04d1c5e93c3f8916b21d8b9f626d060b4858d62de0f88ea16158aR31
the show task passes limit as
None
to this macro for any negative value of the limit cli arg: https://github.com/dbt-labs/dbt-core/pull/8641/files#diff-a210be47df8fb8ce0478d0ab960cb5901d7b550c9cd02d0b13222bbd258a5b0cR25There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: do we need the alias here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep! here's what happens without:
Check warning on line 28 in core/dbt/task/show.py
Codecov / codecov/patch
core/dbt/task/show.py#L27-L28
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no longer passing limit through self.adapter.execute or making use of fetchmany, so closing #8471 as part of this PR as well