[Feature] Add support for constraints in databricks_sql_table resource

[wiatrak2]

Changes

Hi!

I'd like to extend the databricks_sql_table resource with constraints feature.

Within this pull request I extend the table's resource by allowing user to specify PRIMARY KEY and FOREIGN KEY constraints. I haven't implemented the CHECK constraint, as this one is more tricky and is not allowed during table creation - it can only be added later in the table's lifecycle. If the proposed changed are accepted, I can try to add the implementation for CHECK, as well as cover other constraint options.

I'm aware that the amount of implemented tests may not be satisfying. If this is the case - please let me know and guide my, what kind of tests should be added.

Tests

• make test run locally
• relevant change in docs/ folder
• covered with integration tests in internal/acceptance
• relevant acceptance tests are passing
• using Go SDK

[alexott]

there is a #4155 in progress - can you coordinate with that user to avoid duplications?

[wiatrak2]

oh, indeed! I see the other user decided on slightly different approach to configuration of the constraint. And seems that the syntax is not compliant with Databricks SQL, maybe that's why there are some test failures. I will ask in that PR if we can somehow cooperate :)

otherwise, is there something I could do to proceed with the feature?

[wiatrak2]

there is a #4155 in progress - can you coordinate with that user to avoid duplications?

seems the feature should be developed within this PR - I'm open to review and suggestions

[hshahconsulting]

Could you add functionality for the "RELY" optimization of Primary Keys?

ALTER TABLE customer ADD PRIMARY KEY (c_customer_sk) RELY

Source: https://www.databricks.com/blog/primary-key-and-foreign-key-constraints-are-ga-and-now-enable-faster-queries

[hshahconsulting]

Would like the RELY optimization support for primary keys, please

[wiatrak2]

Sure, optional configuration of a constraint with RELY added

[tanmay-db]

Thank you @wiatrak2 for the PR, it looks good, I left some comments, can you please take a look?

[tanmay-db]

This would work because we are essentially checking for case insensitivity and ignoring the mapping here:

	if alias, ok := columnTypeAliases[caseInsensitiveColumnType]; ok {
		return alias
	}

We shouldn't use getColumnType here.

[tanmay-db]

Should we make this as part of the ConstraintInfo struct that we do for getWrappedConstraintName as it will only be used for each constraint info and internally uses ConstraintInfo. getWrappedConstraintName, getWrappedKeyColumnNames and Rely?

We do the same for SqlTableInfo. serializeColumnInfos, serializeColumnInfo

[tanmay-db]

Since we only support PRIMARY KEY and FOREIGN KEY, we should fail client side if some other value is used.

[github-actions]

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/terraform

Inputs:

• PR number: 4205
• Commit SHA: 22bdf56ecbc89dd531962fb762a58212ecf0e6f2

Checks will be approved automatically on success.

[wiatrak2]

Hi @tanmay-db

Thank you for your review. I adjusted the implementation to your comments:

• regarding CustomizeSchema method - I misunderstood the functionality. Now, I decided to remove any entry for constraints. If you have any suggestions - please let me know
• I changed the serialize functions to be part of ConstraintInfo struct
• I strongly agree, that providing any other Constraint type than primary/foreign key on table creation phase should trigger some error, hence I decided to rearrange the statement composing function a bit to handle some error. Moreover, validation on constraint type during table creation allowed to implement the CHECK type as well, that would be consistent do Databricks way of work - i.e. it's not allowed for CREATE TABLE, but you can add it later.

Please let me know what you think about these recent changes

[nkvuong]

@wiatrak2 there are CRUD APIs for PK/FK Table Constraints - https://docs.databricks.com/api/workspace/tableconstraints/create, does it make sense to have them as separate resources?

wdyt @tanmay-db ?