Remove extra semicolon in insert_by_period materialization

[sean-rose]

This is a:

• bug fix PR with no breaking changes — please ensure the base branch is master
• new functionality — please ensure the base branch is the latest dev/ branch
• a breaking change — please ensure the base branch is the latest dev/ branch

Description & motivation

This removes an extra semicolon in the insert_by_period materialization that causes the initial materialization (or rematerialized due to a full refresh) to fail immediately after creating the empty table when running in Snowflake with a "cannot unpack non-iterable NoneType object" error.

Checklist

• I have verified that these changes work locally on the following warehouses (Note: it's okay if you do not have access to all warehouses, this helps us understand what has been covered)
  • BigQuery
  • Postgres
  • Redshift
  • Snowflake
• I have "dispatched" any new macro(s) so non-core adapters can also use them (e.g. the star() source)
• I have updated the README.md (if applicable)
• I have added tests & descriptions to my models (and macros if applicable)
• I have added an entry to CHANGELOG.md

[joellabes]

Hey @sean-rose, thanks for opening this! You're caught up in Jeremy's consistent-ification of Snowflake semicolons in dbt 0.21.0.

This is in a grey area. The current version of dbt-utils has [">=0.20.0", "<0.22.0"] as its dbt Core version boundary, and the semicolon change happened in the middle of that (0.21). Merging this change would be a breaking change for all Redshift users, and Snowflake users (if any) still on 0.20.x.

Because the insert_by_period materialisation only claims compatibility with Redshift, and we're reviewing how it should play with the wider ecosystem at the moment (see #410 (comment) and dbt-labs/dbt-core#4174), I'm going to close this issue without merging it I'm sorry.

In the meantime, you can apply these changes to your project by making your own copy of the materialisation, or consider using the SF-specific variant in #410

[sean-rose]

Hey @sean-rose, thanks for opening this! You're caught up in Jeremy's consistent-ification of Snowflake semicolons in dbt 0.21.0.

This is in a grey area. The current version of dbt-utils has [">=0.20.0", "<0.22.0"] as its dbt Core version boundary, and the semicolon change happened in the middle of that (0.21). Merging this change would be a breaking change for all Redshift users, and Snowflake users (if any) still on 0.20.x.

@joellabes as far as I can tell that is not the case.

In dbt 0.20.0 the create_table_as macro generated SQL statements that already end with a semicolon for both Redshift and Snowflake, and that hasn't changed as of 0.21.latest for both Redshift and Snowflake.

So at least as of the dbt core versions dbt_utils currently supports, the extra semicolon being removed here shouldn't constitute a breaking change for any of those versions for Redshift or Snowflake.

There's even another place in the insert_by_period materialization where create_table_as is called without adding a trailing semicolon, and that's been working fine.

And for Snowflake the issue this is addressing didn't start with dbt 0.21.0, it was already an issue before that.

Because the insert_by_period materialisation only claims compatibility with Redshift, and we're reviewing how it should play with the wider ecosystem at the moment (see #410 (comment) and dbt-labs/dbt-core#4174), I'm going to close this issue without merging it I'm sorry.

I did notice #410, and if that ends up being merged by all means disregard this one. But it seemed like #410 might not make the cut for the next version, and if that's the case then I think having this fix would be very helpful for all Snowflake users trying to make use of insert_by_period. While there are certainly other issues with getting it working on Snowflake, there are workarounds for those. This issue is the one unavoidable hard failure I've run into, and the fix is straightforward.

So if #410 doesn't land for 0.7.4 would you consider reopening this for inclusion in 0.7.4?

[joellabes]

So at least as of the dbt core versions dbt_utils currently supports, the extra semicolon being removed here shouldn't constitute a breaking change for any of those versions for Redshift or Snowflake.

OK! this was over-enthusiastic pattern-matching on my part then; sorry about that! Thanks for doing the detective work to set me straight.

In that case, I'm happy to include this in 0.7.4, which I suspect will be coming out any day now.