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

Update debug task to make the debug query an adapter method #2751

Closed
PedroMiguelFigueiredo opened this issue Sep 11, 2020 · 6 comments · Fixed by #2817
Closed

Update debug task to make the debug query an adapter method #2751

PedroMiguelFigueiredo opened this issue Sep 11, 2020 · 6 comments · Fixed by #2817
Labels
good_first_issue Straightforward + self-contained changes, good for new contributors!

Comments

@PedroMiguelFigueiredo
Copy link

PedroMiguelFigueiredo commented Sep 11, 2020

The existing code is here:
https://github.com/fishtown-analytics/dbt/blob/fa8a4f2020655c60bdb66270616f5654e168a729/core/dbt/task/debug.py#L337

But that won't work on some third party adapters. One example is Oracle, which requires a from clause in any select statement.

Instead, this should be an adapter method on the base adapter, named debug_query() that takes no arguments. Then adapter plugins will be able to override the method with an appropriate debug method. For example, on Oracle, it might be select 1 from dual.

@jtcohen6
Copy link
Contributor

Hey @PedroMiguelFigueiredo, could you say a bit more here?

@beckjake
Copy link
Contributor

@jtcohen6 it looks like on oracle, selects require a FROM clause and their solution to the obvious problems that causes is a special identifier named dual that's globally accessible.

If that is the case, we probably should have an adapter.debug_query() method that defaults to adapter.execute("select 1 as id"). Then the author of the oracle plugin (or other databases with similar restrictions) can add their oracle-only special case behavior.

@jtcohen6
Copy link
Contributor

Ahh that makes a lot of sense, and sounds good as an approach

@fabrice-etanchaud
Copy link

fabrice-etanchaud commented Sep 15, 2020

Hi all !

Yes, we had the same idea here in the dbt-oracle repo :
Error on dbt debug

This would be great, and maybe this could be circonveined just in SQLAdapter and OracleAdapter.

By chance, the dremio adapter will not need it ;-), as dremio is calcite based :

dbt adapter for dremio

dbt's community is simply great !

@beckjake beckjake added the good_first_issue Straightforward + self-contained changes, good for new contributors! label Sep 15, 2020
@beckjake beckjake changed the title This won't work with Oracle. Must have (...) FROM DUAL; Update debug task to make the debug query an adapter method Sep 15, 2020
@zmac12
Copy link
Contributor

zmac12 commented Oct 3, 2020

Submitted a PR prematurely, my apologies. I'd like to resolve this issue! As far as inputting a custom debug sql statement for the new method in the base debug task, will that need to pull from the profile passed to the task?

@jtcohen6
Copy link
Contributor

jtcohen6 commented Oct 5, 2020

@zmac12 I think you had the right start in #2811, I'll comment over there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good_first_issue Straightforward + self-contained changes, good for new contributors!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants