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

deprecate sql_where #744

Closed
drewbanin opened this issue Apr 26, 2018 · 4 comments
Closed

deprecate sql_where #744

drewbanin opened this issue Apr 26, 2018 · 4 comments
Milestone

Comments

@drewbanin
Copy link
Contributor

drewbanin commented Apr 26, 2018

See also:

The sql_where config isn't actually useful and people shouldn't use it. We should:

@cmcarthur cmcarthur added this to the 0.10.2 milestone Apr 26, 2018
@jthandy jthandy added this to the Stephen Girard milestone Oct 25, 2018
@drewbanin drewbanin modified the milestones: Stephen Girard, Grace Kelly Nov 2, 2018
@drewbanin drewbanin assigned drewbanin and unassigned drewbanin Nov 14, 2018
@dbt-labs dbt-labs deleted a comment from cmcarthur Nov 14, 2018
@drewbanin
Copy link
Contributor Author

drewbanin commented Nov 14, 2018

@jthandy I'm thinking about a better way to handle incremental models. I think that the general pattern in place currently is a good one, but we could make it a lot simpler with a macro.

I figure we can provide this is_incremental() macro in dbt. The macro can then be used in projects as shown below:

{% macro is_incremental() %}
    {#-- do not run introspective queries in parsing #}
    {% if not execute %}
        {{ return(False) }}
    {% else %}
        {% set relation = adapter.get_relation(this.schema, this.table) %}
        {{ return(relation is not none and not flags.FULL_REFRESH) }}
    {% endif %}
{% endmacro %}
-- models/some_model.sql

{{ config(materialized='incremental') }}

select * from public.events

{% if is_incremental() %}
    where dvce_created_tstamp > (select max(dvce_created_tstamp) from {{ this }})
{% endif %}

This is just some sugar over the existing implementation, but it also helps dbt avoid doing any queries at parse-time

Very happy to iterate on this -- keen to hear your thoughts!

cc @clrcrl

@drewbanin drewbanin added the help_wanted Trickier changes, with a clear starting point, good for previous/experienced contributors label Nov 14, 2018
@jthandy
Copy link
Member

jthandy commented Nov 15, 2018

i just wanted to get people to stop using a bad feature we built in 2016 so this is beyond my wildest dreams :D I have literally nothing to add to this--love it.

@clrcrl
Copy link
Contributor

clrcrl commented Nov 15, 2018

So as I mentioned to @drewbanin in person, I also love this!
The only thing I don't love is the terminology is_incremental – I would expect that to be true for any model which has materialized == 'incremental'.
I don't have a better term to offer at present... is_incremental_run? Maybe? Eh.

@drewbanin
Copy link
Contributor Author

I'm with you @clrcrl -- is_incremental() is kind of funky. I played around with partial_run() or insert_new() or apply_filter() but all of those are contextless IMO. I think we should just pick one -- if users have strong opinions, they can alias the name with their own macro :)

@drewbanin drewbanin removed the help_wanted Trickier changes, with a clear starting point, good for previous/experienced contributors label Dec 5, 2018
drewbanin added a commit that referenced this issue Dec 5, 2018
…mental-models

(fixes #744) deprecate sql_where and provide an alternative
iknox-fa pushed a commit that referenced this issue Feb 8, 2022
automatic commit by git-black, original commits:
  80232ff
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

No branches or pull requests

4 participants