Skip to content
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

[CT-2428] dbt show should include --limit in compiled query #7390

Closed
jtcohen6 opened this issue Apr 18, 2023 · 1 comment · Fixed by #7545
Closed

[CT-2428] dbt show should include --limit in compiled query #7390

jtcohen6 opened this issue Apr 18, 2023 · 1 comment · Fixed by #7545
Assignees
Labels
bug Something isn't working
Milestone

Comments

@jtcohen6
Copy link
Contributor

jtcohen6 commented Apr 18, 2023

dbt show --limit will limit the number of rows of data previewed on the CLI.

Currently, dbt show applies a limit after it's already run the query and loaded the results into memory:

adapter_response, execute_result = self.adapter.execute(
compiled_node.compiled_code, fetch=True
)

I would expect the query templated by dbt show to include limit {limit}.

Otherwise, dbt could run a query that returns 10k rows, load all of them into memory (in an agate.Table), and only then filter down to the default (5). My sense is that risks significant slowdown, and OOM issues.


Acceptance criteria:

  • Instead of just directly executing compiled_node.compiled_code, template it into a subquery that includes limit
  • In order to write the subquery in a cross-database-compatible way, we should either
    • Use a (Jinja) macro or materialization to template the subquery with limit
    • Use an adapter (Python) method — much easier to implement, and probably sufficient

Out of scope: Supporting BigQuery's no-cost "preview" API (bq head), which would require us to do a diff check / cache validation on the logical/applied state of this model. (logical = current SQL, applied = most recently materialized) Let's keep thinking about if there are ways to support that in the future. Update: Opened a separate issue for this: #7391.

@jtcohen6 jtcohen6 added bug Something isn't working Team:Execution labels Apr 18, 2023
@jtcohen6 jtcohen6 added this to the v1.5.x milestone Apr 18, 2023
@github-actions github-actions bot changed the title dbt show should include --limit in compiled query [CT-2428] dbt show should include --limit in compiled query Apr 18, 2023
@iknox-fa
Copy link
Contributor

@jtcohen6 Per BLG we'd actually like to implement this in the base adapter (option 2). It strikes a good balance between doing all-the-things (supporting bq head and any other adapter specific logic we might want to use) and doing the quick-and-dirty version where we edit the sql just for the show command. It's been pointed accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants