Skip to content

Commit

Permalink
materializations: don't try to drop nullability if the projection has…
Browse files Browse the repository at this point in the history
… a default value

In e852e80, materializations were made aware of the possibility for default values in the
materialized columns, and that we should not try to drop "not null" constraints on columns like
this, under the assumption that the destination system is fine with us sending a "null" since it
will automatically be populated (in a sense) with that default value.

This didn't take into consideration the fact that Flow projections can also have default values, and
in this case we will never see a "null" for these fields from the Flow runtime, and instead will
always see that default value. So here too we do not need to drop nullability constraints - the
materialized columns can stay "not null" and as long as there is that default value annotation
things will work fine.

This is particularly important for materializing nullable collection keys with default values. This
is allowed (see 5d53313 as well), and some systems like postgres consider primary key fields
non-nullable. Prior to this commit, the connector would try to drop a "not null" constraint on a
primary key column that was created from a projection with a default value if the materialized
column didn't have a default value. We don't currently create columns with default values based on
projections (though maybe we will, later) so this would cause the materialization to fail the next
time it tries to do an `Apply`. But now, the materialization will not try to drop a "not null" on a
materialized key field as long as the projection has a default value.
  • Loading branch information
williamhbaker committed Apr 8, 2024
1 parent 1316eb9 commit edfda36
Showing 1 changed file with 6 additions and 4 deletions.
10 changes: 6 additions & 4 deletions materialize-boilerplate/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,10 +178,12 @@ func ApplyChanges(ctx context.Context, req *pm.Request_Apply, applier Applier, i
}

newRequired := projection.Inference.Exists == pf.Inference_MUST && !slices.Contains(projection.Inference.Types, pf.JsonTypeNull)
if !existingField.Nullable && !newRequired && !existingField.HasDefault {
// The existing field is not nullable and does not have a default value, but
// the proposed projection for the field is nullable. The existing field
// will need to be modified to be made nullable.
newlyNullable := !existingField.Nullable && !newRequired
projectionHasDefault := projection.Inference.DefaultJson != nil
if newlyNullable && !existingField.HasDefault && !projectionHasDefault {
// The field has newly been made nullable and neither the existing field nor
// the projection has a default value. The existing field will need to be
// modified to be made nullable since it may need to hold null values now.
params.NewlyNullableFields = append(params.NewlyNullableFields, existingField)
}
} else {
Expand Down

0 comments on commit edfda36

Please sign in to comment.