Update incremental table execution to avoid create or replace #1712
Conversation
| }${options.length > 0 ? `OPTIONS(${options.join(",")})` : ""}as ${table.query}`; | ||
| let statement = "create "; | ||
| if (table.enumType !== dataform.TableType.INCREMENTAL) { | ||
| statement += "or replace "; |
There was a problem hiding this comment.
From the bug - I think we still want to create or replace in the case that it is incremental AND it's not projected AND it's full refresh?
There was a problem hiding this comment.
We should also add a test for this case
There was a problem hiding this comment.
What is projected? Have added the full refresh.
This is tested, but not obviously, across
dataform/tests/api/api.spec.ts
Line 463 in c55ca6b
There was a problem hiding this comment.
Ah right! Done.
I had to update the protected logic, such as removing the error that we test for here
dataform/tests/api/api.spec.ts
Line 109 in c55ca6b
There was a problem hiding this comment.
Right, sorry I think this has got really confusing, a few thoughts.
- leave this method as
createOrReplaceand don't pass in run options or do all that logic. - Add a
createIfNotExistsmethod next to it (they can share some private impl if you want, but I wouldn't bother) - Update the logic in publishTasks, only for the incremental tables, to do what you want to do and call
createIfNotExistsby default, andcreateOrReplaceonly when!protected && fullRefresh
The original method was clear what it did, but now you've moved this logic into it, I'd say it's much harder to follow what's going on in publishTasks.
| @@ -74,10 +74,6 @@ export class Builder { | |||
| tableMetadata: dataform.ITableMetadata, | |||
| runConfig: dataform.IRunConfig | |||
| ) { | |||
| if (table.protected && this.runConfig.fullRefresh) { | |||
Fixes #1561