Adding new unique key column - fixes #322

[the4thamigo-uk]

Summary

Proposed solution to support adding new columns that are also added to the unique_key.

[CLAassistant]

All committers have signed the CLA.

[the4thamigo-uk]

fixes #322

[BentsiLeviav]

Hi @the4thamigo-uk
Thank you for your contribution!

This #332 was opened a few days ago and handles schema changes. Generally, support in the sync_all_columns option was introduced.
Could you check if it solves your issue?

Thanks

[the4thamigo-uk]

@BentsiLeviav Ah yes I noticed the legacy fallback, but I was aiming to do a minimal fix. Great that there is a better solution.

I suppose if the PR passes a test which is equivalent to https://github.com/ClickHouse/dbt-clickhouse/pull/323/files#diff-f2e636456ac1c2366b5fd10e6beaf3560a946882645f7cee6ba0e0501bf301e3 then it would be fine.

[BentsiLeviav]

I see that the table creation passes successfully, but is there a chance there is a mistake in this assertion?
After running the test the table looks like this:

[the4thamigo-uk]

I see that the table creation passes successfully, but is there a chance there is a mistake in this assertion? After running the test the table looks like this:

Yes its interesting to know what the behaviour should be I suppose. I guess you are suggesting row 4 should not be present?

[the4thamigo-uk]

HMm when I run it I dont see your row 4?

$ pytest -k test_append_unique_key
==================================================================================================================== test session starts =====================================================================================================================
platform linux -- Python 3.12.3, pytest-8.3.2, pluggy-1.5.0
configfile: pytest.ini
testpaths: tests/integration, #, name, per, convention
collected 159 items / 158 deselected / 1 selected                                                                                                                                                                                                            

tests/integration/adapter/incremental/test_schema_change.py 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PDB set_trace (IO-capturing turned off) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> /home/xxx/venatus/dbt-clickhouse/tests/integration/adapter/incremental/test_schema_change.py(90)test_append_unique_key()
-> assert result[0][2] == 0
(Pdb) result
[(0, 1, 0), (1, 2, 0), (2, 3, 4), (3, 4, 5), (4, 5, 6)]

[BentsiLeviav]

@the4thamigo-uk
What ClickHouse version do you use?

[the4thamigo-uk]

@the4thamigo-uk What ClickHouse version do you use?

24.3.4.147 - but would be surprised if it makes a difference...

[BentsiLeviav]

That is really weird. Did you test it with the changes contributed at #332 ?
At the beginning, the table has the following data:

and the incremental query:

select
    number as col_1,
    number + 1 as col_2,
    number + 2 as col_3
from numbers(2, 3)

yields this:

So the append of these 2 groups would be 6 rows:

Anyway, the error mentioned in #322 doesn't appear anymore. I'm closing this PR and the issue, feel free to comment/reopen if needed or if something is wrong.

[the4thamigo-uk]

Yeah weird... anyway, the incremental schema changes is much better. Thanks @canbekley