alter_column_type - Not working for column change

[visch]

To replicate setup an incremental model.

1. Setup the your table to include a column like Varchar(50), alter the column to include varchar (5000) sql below
2. Run your model, which will trigger the alter_column_type macro
3. It will fail

For 1. above

USE DBName
ALTER TABLE extract.tablename
ALTER COLUMN COLUMNNAME varchar(5000);

For 3. above

    alter table "DBName"."extract"."COLUMNNAME " add column "COLUMNNAME __dbt_alter" character varying(5000);
    update "DBName"."extract"."COLUMNNAME " set "COLUMNNAME __dbt_alter" = "DEPARTMENTID";
    alter table "DBName"."extract"."COLUMNNAME " drop column "COLUMNNAME " cascade;
    alter table "DBName"."extract"."COLUMNNAME " rename column "COLUMNNAME __dbt_alter" to "DEPARTMENTID"
    
    
2021-07-19 12:33:09.231848 (Thread-3): Database error: ('42000', "[42000] [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Incorrect syntax near the keyword 'column'. (156) (SQLExecDirectW); [42000] [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Incorrect syntax near the keyword 'cascade'. (156); [42000] [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Incorrect syntax near 'rename'. (102)")

[dataders]

@visch do you think this might be related to microsoft/dbt-synapse#63? I almost wonder if there's an issue with sqlserver__alter_column_type not just synapse__alter_column_type.

[visch]

Dove in

https://github.com/dbt-labs/dbt/blob/develop/core/dbt/include/global_project/macros/adapters/common.sql#L163-L179

Seems to be where the SQL is coming from.

The sql for mssql
should be alter table "DBName"."extract"."COLUMNNAME " drop column "COLUMNNAME " on delete cascade;

I'm not certain that you actually want to cascade on delete, but this seems to be the syntax issue.

It looks like that's the syntax most warehouses use. See https://docs.aws.amazon.com/redshift/latest/dg/r_ALTER_TABLE.html vs https://docs.microsoft.com/en-us/sql/t-sql/statements/alter-table-table-constraint-transact-sql?view=sql-server-ver15

The fix in https://github.com/dbt-msft/dbt-synapse/pull/64/files looks like it's be valid for MSSQL as well, might be worth putting it in this repo instead of synapse's 🤷

[visch]

@visch do you think this might be related to dbt-msft/dbt-synapse#63? I almost wonder if there's an issue with sqlserver__alter_column_type not just synapse__alter_column_type.

short answer, yes :D

[dataders]

so looking at things. it seems the dbt-sqlserver is making use of the dbt-core's default__alter_column_type macro, which it shouldn't be. So the addition of synapse__alter_column_type in microsoft/dbt-synapse#64, should really be added here as sqlserver__alter_column_type (which dbt-synapse will make use of no problem). @dlarsen5 does that make sense to you?

[dlarsen5]

@swanderz yep that makes sense, should I rename the macro and make a PR to dbt-sqlserver instead?

[dataders]

@swanderz yep that makes sense, should I rename the macro and make a PR to dbt-sqlserver instead?

that'd be great -- thanks @dlarsen5!

[dataders]

@dlarsen5 friendly bump. I hope to release dbt-sqlserver and dbt-synapse v0.21.0 soon. if you don't have a chance to do this in the next week, I'll just copy your logic from your old PR into this one.

[dlarsen5]

@swanderz pushed a MR but I don't have a local sqlserver to test the changes against so not sure if it works/fixes the incremental issue for synapse

[dataders]

Thanks @dlarsen5! I'll test it myself next week.
Closing in favor of #169