Failed PostgreSQL Source update causes invalid state and won't refresh

[matthelm]

We were recently adding new sub-sources to an existing PostgreSQL source and had a bumpy experience as follows:

• The deploy adding the sub-sources failed with the following error: ERROR: table raw.organization__c not found in source (SQLSTATE XX000). We realized we had a typo in our sub-source names.
• We deployed a revert, removing the new sub-sources, but were surprised to see that it also failed. ERROR: table raw.organization__c not found in source (SQLSTATE XX000).
• Looking at the Materialize instance we confirmed that the sub-sources hadn't been created.
• Looking at Pulumi's state we found that the sub-sources were listed, which seemed incorrect.
• We tried a pulumi refresh but it surprisingly reported there was no changes.
• We fixed the issue by manually editing the state, removing the sub-sources and the error message.

This leaves us suspecting two potential bugs with the PostgreSQL source resource type:

1. A failed attempt to add sub-sources still adds them to the state
2. A pulumi refresh doesn't reload the sub-sources

[benesch]

Something seems funny with the way the Terraform provider is rigged up on this. We provide the tables parameter as input...

https://github.com/MaterializeInc/i2/blob/bdfb53146ea264957bbb13d868041fdaa3d5e885/i2/odw/resource.py#L677

but it's actually this subsource parameter that is updated on read:

https://github.com/MaterializeInc/terraform-provider-materialize/blob/6b3637a64db28c90d1766e4b3c35172cf68ece84/pkg/resources/resource_source_postgres.go#L67C31-L67C31

terraform-provider-materialize/pkg/resources/resource_source.go

Lines 61 to 80 in 6b3637a

	// Subsources
	deps, err := materialize.ListDependencies(meta.(*sqlx.DB), utils.ExtractId(i), "source")
	if err != nil {
	return diag.FromErr(err)
	}
	depMaps := []map[string]interface{}{}
	for _, dep := range deps {
	depMap := map[string]interface{}{}
	depMap["name"] = dep.ObjectName.String
	depMap["schema_name"] = dep.SchemaName.String
	depMap["database_name"] = dep.DatabaseName.String
	depMaps = append(depMaps, depMap)
	}
	if err := d.Set("subsource", depMaps); err != nil {
	return diag.FromErr(err)
	}

That subsource parameter is totally ignored by sourcePostgresUpdate, which explains why the pulumi refresh had no effect. We can limit our investigation to this code in sourcePostgresUpdate, which handles addition/removal of subsources based on changes to the tables parameter:

terraform-provider-materialize/pkg/resources/resource_source_postgres.go

Lines 203 to 224 in 6b3637a

	if d.HasChange("table") {
	ot, nt := d.GetChange("table")
	addTables := materialize.DiffTableStructs(nt.([]interface{}), ot.([]interface{}))
	dropTables := materialize.DiffTableStructs(ot.([]interface{}), nt.([]interface{}))
	if len(addTables) > 0 {
	var colDiff []string
	if d.HasChange("text_columns") {
	oc, nc := d.GetChange("text_columns")
	colDiff = diffTextColumns(nc.([]interface{}), oc.([]interface{}))
	}
	if err := b.AddSubsource(addTables, colDiff); err != nil {
	return diag.FromErr(err)
	}
	}
	if len(dropTables) > 0 {
	if err := b.DropSubsource(dropTables); err != nil {
	return diag.FromErr(err)
	}
	}
	}

I think the symptoms we saw today are well explained by this code. The first update added some new tables to the state. The resulting update failed because the tables did not exist upstream. The second update then removed those tables from the state. The provider logic computed dropTables as the removed tables and then attempted to drop those subsources from Materialize—even though those subsources never got created! The only way out was to force edit the state to remove those tables, to make the state reflect reality without going through the codepath that runs DROP SUBSOURCE.

I think the way out here is to teach sourceRead to update the tables state instead of the ignored subsources state. I also think the PostgreSQL source resource should not support FOR SCHEMAS (...) or FOR ALL TABLES. That's basically never what you want with Terraform, because that will expand out to a list of tables in your state that cannot be automatically reflected in your Terraform source code, and therefore will cause a spurious diff on your next plan.

If a user does want to do FOR SCHEMAS or FOR ALL TABLES with Terraform, IMO the correct thing for them to do is to use the PostgreSQL provider's data source to read out the tables in the upstream PostgreSQL database in Terraform, and then use a Terraform loop to convert the upstream schemas and tables into table nested resources.

[dehume]

Not supporting FOR SCHEMAS and FOR ALL TABLES makes this much easier to implement. Will implement a unique read for the Postgres resource.