Snowflake-Labs / terraform-provider-snowflake

Terraform provider for managing Snowflake accounts
https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest
MIT License
549 stars 420 forks source link

Table constraint of eligible snowflake feature - "primary_key" is deprecated: Reason: "Definitions of primary key constraint to create on table" #2415

Open acakovacevic opened 9 months ago

acakovacevic commented 9 months ago

Terraform CLI and Provider Versions

Visual Studio Code Version: 1.85.2 (user setup) Commit: 8b3775030ed1a69b13e4f4c628c612102e30a681 Date: 2024-01-18T06:40:10.514Z Electron: 25.9.7 ElectronBuildId: 26354273 Chromium: 114.0.5735.289 Node.js: 18.15.0 V8: 11.4.183.29-electron.0 OS: Windows_NT x64 10.0.19045

Terraform Configuration

terraform {
  required_providers {
    snowflake = {
      source = "Snowflake-Labs/snowflake"
      version = "0.84.1"
    }
}

Expected Behavior

To be able to use snowflake the same way as in other snowflake tools. Example: Snowsight - Table definition Table Definition create or replace TRANSIENT TABLE DEV_CLIENTDATA.DWU.TESTTABLE ( COL1 VARCHAR(32) NOT NULL, COL2 VARCHAR(32), EMAIL VARCHAR(256), ZIP_CODE VARCHAR(32), BATCH_ID NUMBER(38,0) NOT NULL, V VARIANT NOT NULL, FILENAME VARCHAR(512) NOT NULL, FILE_ROW_NUMBER NUMBER(12,0) NOT NULL, TMS TIMESTAMP_NTZ(9) NOT NULL, constraint TESTTABLE_PK primary key (FILENAME, FILE_ROW_NUMBER) );

Actual Behavior

Not to deprecated snowflake supported syntax

Steps to Reproduce

open terraform in Visual Studio Code

How much impact is this issue causing?

Low

Logs

No response

Additional Information

No response

sfc-gh-asawicki commented 9 months ago

Hey @acakovacevic. Thanks for reaching out to us.

You can define the primary key in the snowflake_table_constraint resource (docs).

We will still discuss the resource design before going to V1. Separation is sometimes necessary, though.

acakovacevic commented 9 months ago

I know about snowflake_table_constraint resource and using it would lead to unnatural and very poor terraform design. Separation is never necessary and terraform whould follow snowsight design. It is way easier to manage constraints within table definition using snowflake_table resource.

sfc-gh-swinkler commented 9 months ago

Hello @acakovacevic! Thank you for your thoughtful and constructive criticism, I hope to shed some light on why this decision was made.

To start with, the primary_key attribute was deprecated from the current table resources mainly because it was a flawed design. It wasn't correctly named and couldn't support other types of constraints, so would have to be deprecated no matter what other decision was made That doesn't mean we wouldn't consider adding table constraints back to the snowflake_table resource in the future, but it wouldn't look the same as it did before. Specifically, I would like table constraints to look like:

Snippet 1: What is possible in the future

resource "snowflake_table" "tb" {
 table_constraints = [
  {
   // table constraint definition
  }, 
  {
   // second table constraint definition
  }
 ]
}

However, there are also some limitations with how we can represent Terraform configuration schema in the Terraform plugin SDK v2 we are using, which won't be remedied until we upgrade to the new Terraform plugin framework. in the current plugin SDK, we would only be able to do the following:

Snippet 2: What is possible Now

resource "snowflake_table" "tb" {
 table_constraint {
  // table constraint definition
 }
  table_constraint {
  // table constraint definition
 }
  table_constraint {
  // table constraint definition
 }
}

What I do not want to do is implement a complex patch to add table constraints with what is possible now, only to have to deprecate it and introduce another breaking change again in the future. We are well aware of how breaking changes impact our customers, and we try to make as few of them as possible. This is also why tags were deprecated from most resources, as we would have to do much the same thing.

Furthermore, there are good reasons for creating a snowflake_table_constraint resource, although they might not be obvious at first glance. I would first point to the precedent of splitting complex resources along API lines, like what was done with the aws_s3_bucket resource. This greatly reduces complexity and makes it easier for users to understand and utilize the resource. Especially one as complex as tables. Smaller, more concise Terraform configs are also easier to validate and perform diff calculations on (although this is not strictly a limitation). Finally, although you are using the snowflake_table resource, it is important to recognize that many other customers are not. A snowflake_table_constraint resource allows these other customers to manage table constraints even without having Terraform manage their tables. Note that this reasoning is also why we have a snowflake_object_parameter resource, despite the fact that object parameters can also be specified in resources.