Airbtye platform should validate that not all PKs are null, if stream has at least one PK

[evantahler]

The Airbyte platform should fail the sync if a record is found with all null primary keys.

In #31926, we decided that if there are multiple PKs in the stream, that means the source is attempting to describe a composite primary key. Any of the keys can be null, but at lease one of them must be non-null.

The platform should fail the sync if a record is found which has all primary-key columns as null. This works for the degenerate case of 1 PK. If there is no primary key for the stream this check should be skipped.

[evantahler]

This is related to #31926

[evantahler]

@davinchia this is ready to be worked on now

[davinchia]

Note:

• what is the performance impact of this?
• @evantahler can we confirm, the primary key is something we always expect to be present? just want to make sure there aren't cases where the primary key isn't present and we need to account for that.

[evantahler]

There are sources/streams which do not have primary any primary keys. These streams should (1) always be full refresh and (2) not be subject to this check. I updated the title to make this more clear

[davinchia]

thanks.

for a stream configured with primary keys, is it problematic if we enforce this heavily? e.g. are we worried about the case where a user wants to use a field as a primary key, and this field is actually missing in some records.

it sounds like we aren't - wanted to double check.

[evantahler]

are we worried about the case where a user wants to use a field as a primary key, and this field is actually missing in some records.

That's exactly what this check should detect and fail the sync. Then, there are some next steps:

• The source has bad data and the user can fix it up (in DB or File sources)
• The source connector is presenting a bad schema and/or the API isn't as reliable as we think it is. The source should be changed to only allow full-refresh syncs for this stream (and remove the PK)

[benmoriceau]

@evantahler Just to be sure that I am doing the right thing. My understanding is that this error will be qualified as a Source error, am I right?

[evantahler]

Yep! Not having the catalog correct is a source issue

[benmoriceau]

@evantahler The fix has been merged, I'll start to progressive rollout on Wednesday 29th. I will start with 1%, then 10% on the same day and I'm aiming for 100% on Thursday.