When using incremental tables, the column data types are invalidly updated

[gshank]

resolves #276

Original issue in dbt-labs/dbt-core#10362

Problem

In dbt-postgres, contract changes were applied to temporary tables, causing original tables to be changed to match.

Solution

If a contract exists, do not modify the columns of incremental tables.

Checklist

• I have read the contributing guide and understand what's expected of me
• I have run this code in development, and it appears to resolve the stated issue
• This PR includes tests, or tests are not required/relevant for this PR
• This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc.) or this PR has already received feedback and approval from Product or DX

[github-actions]

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

[mikealfare]

@gshank I read through the original issue and I'm not clear on how this resolves that issue. If I understand this change correctly, it looks like we're doing this:

1. Remove the __repr__ to use the default from dataclasses
2. Only add columns when there is no contract enforcement

I don't see how 1. would affect anything. And 2. removes adjusting columns entirely for contract-enforced models instead of correcting the type. From this PR, it seems like there's an assumption that models with a contract will never change. Is that true? What happens if the user deliberately adds a column, and updates the contract to reflect the new column? Is that something we don't support?

[gshank]

I removed the repr only because it caused a lot of confusion when debugging, because it wasn't adequately displaying the contents of the class and the default worked better. It's not related to the fix.

The problem that this fixes is that the intermediate tables have different data types than the actual incremental table, and that is causing the actual incremental table to have the data types updated from valid (contract enforced) data types to data types that do not match the contract.

It's not totally clear to me what the point of "expand_column_data_types" is supposed to be, so I tried to limit the change to just what was necessary to fix this bug.

[mikealfare]

I removed the repr only because it caused a lot of confusion when debugging, because it wasn't adequately displaying the contents of the class and the default worked better. It's not related to the fix.

That's what I suspected, and I'm fine with that.

The problem that this fixes is that the intermediate tables have different data types than the actual incremental table, and that is causing the actual incremental table to have the data types updated from valid (contract enforced) data types to data types that do not match the contract.

I see how this is happening too, and agree that it shouldn't be.

It's not totally clear to me what the point of "expand_column_data_types" is supposed to be, so I tried to limit the change to just what was necessary to fix this bug.

I could be totally off here, but I believe the point of that is to account for the scenario where the model is updated to include new columns. Say the old model is select a, b, c from my_source. The resulting incremental table would have three columns. But then we need to update the model to be select a, b, c, d from my_source because [reasons]. We need to run an alter statement on the incremental table before adding the new records so that it can accept column d. This scenario could happen whether or not the model is contract-enforced. The contract-enforced scenario just requires that the user update the contract as well when this happens. Assuming my interpretation is correct, I believe this change would inadvertently lock folks into a fixed incremental schema when using a contract, or require a full refresh of the table any time a column is added.

After further discussion, restricting data type changes on existing columns on contract-enforced models feels intuitive.