-
Notifications
You must be signed in to change notification settings - Fork 59
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
[ADAP-1030] [Regression] Back-to-back hooks with a transaction using CTAS + DML fails in 1.5+ #663
Comments
For additional context, this also fails if you call the macro twice within the model (you'll need to have run the project at least once so the table exists, but you get the idea). So it doesn't seem related specifically to hooks. |
Running with |
Final context, this happens even if I make it not bounded by a transaction block. So I really don't know what's going on here... |
For context, dbt-redshift switched from @rlh1994 do you think it is similar to what is described here? Do you know if this is only happening in cross-database scenarios? Or does it also happen in single database scenarios? What happens if you try a greatly simplified version using a permanent table (that isn't dropped at the end)? {% macro transaction_macro() %}
{% if execute %}
create table if not exists trans_temp_table as (
select 1 as test_col
);
delete from {{target.schema}}.test_table where test_col in (select test_col from trans_temp_table);
insert into {{target.schema}}.test_table (select * from trans_temp_table);
{% endif %}
{% endmacro %} |
I'm not 100% sure if it is the same - it's not entirely clear if the OID reference it can't find is the temporary table, or the main table. It's also single database only, no cross-database stuff here. I had to alter your code slightly, as redshift only seems to let you use
So this does work, and it also works in a transaction interestingly enough. I've re-added the drop statement in and that also works, but the temporary table and dropping seems to be where the issue is! i.e. this still works:
I think it's not ideal, and I'm not sure I entirely trust transactions now being run with one command at a time, but at least it gives us a fix we can implement that should resolve any issues people are seeing... |
Okay, glad that you found a workaround! I'm wondering if there is some kind of interaction effect with the way Do either temporary or permanent tables work if you pass a unique postfix when you call the {% macro transaction_macro(hook_num=0) %}
{% set trans_temp_table = "trans_temp_table_" ~ hook_num %}
{% if execute %}
begin transaction;
create temporary table if not exists {{ trans_temp_table }} (
test_col int
);
truncate table {{ trans_temp_table }};
insert into {{ trans_temp_table }} (test_col) values (1);
delete from {{target.schema}}.test_table where test_col in (select test_col from {{ trans_temp_table }});
insert into {{target.schema}}.test_table (select * from {{ trans_temp_table }});
drop table {{ trans_temp_table }};
end transaction;
{% endif %}
{% endmacro %} on-run-end:
- "{{ transaction_macro(1) }}"
- "{{ transaction_macro(2) }}" |
I'm not sure what I was doing yesterday but it does seem to work with temporary tables using this method regardless of a suffix, sorry. That seems to suggest that the Even testing removing the DML on the temp table (I wondered if the implicit commit from a truncate was doing something) seems to keep it working, so the only difference is how the temp table is created. Working:
OID error:
|
Glad we've honed in on it! Based on your testing, this kind of CTAS raises that OID error when it's within back-to-back post-hooks regardless if the table is temporary or not. It requires the following:
I'm going to label this as "help wanted". |
Is this a regression in a recent version of dbt-redshift?
Current Behavior
So this is a weird one, I am struggling to really debug the issue and work out why it is happening, let alone how to fix it even with a workaround. I have a macro (used in a post hook) that creates a temp table, and uses this table to delete from then insert into a fixed table (based off a model). We use this in our packages to update our manifest based on the run outcomes, but I have simplified it for this report.
Prior to version 1.5, this worked, and importantly this worked on multiple threads AND when the hook is called multiple times. In reality the hook shouldn't need to be called multiple times, but it can happen if there is user error or something else going on. Setting this to a single thread does not solve the issue.
What now happens is we get an
could not open relation with OID 3251611
error on the running of the second hook. I have tried making the temp table not temporary, I have tried adding a commit inside the transaction block, outside the transaction block. I have tried dropping the temporary table before the transaction starts, and inside the start of the transaction, and i have tried addingautocommit: false
to my profile - none of it seems to work.I have confirmed this works on version 1.4.0, so it must be related to the change in connection method.
Expected/Previous Behavior
The post hooks would be complete successfully. I'd even be fine with a workaround that allows it to still work, I am out of ideas.
Steps To Reproduce
test_table.sql
content:transaction_macro.sql
content:dbt run --target redshift
. Compare if you run on dbt 1.4.0 that it is successful.Relevant log output
Environment
Additional Context
It's maybe related to this #521 and possibly also this #501 ?
The text was updated successfully, but these errors were encountered: