File-based CDK: fix schemas merge for nullable object types

[askarpets]

What

Fix for https://github.com/airbytehq/oncall/issues/4948

How

Skip choosing wider type if a key is present in schema1 and schema2, but has a type of null in schema2

Review guide

1. schema_helpers.py
2. test_schema_helpers.py

User Impact

No

Can this PR be safely reverted and rolled back?

• YES 💚
• NO ❌

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
airbyte-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 1, 2024 11:21am

[natikgadzhi]

To clarify, would it just make that if the second schema has a field and it's type is null, that null type won't be added to the existing schema's type?

Why would second schema's field type be null only?

[askarpets]

To clarify, would it just make that if the second schema has a field and it's type is null, that null type won't be added to the existing schema's type?

@natikgadzhi that field with null type will be added if it's not in the schema yet

Why would second schema's field type be null only?

This is what I found during investigation of https://github.com/airbytehq/oncall/issues/4948

[clnoll]

@roman-yermilov-gl the code change looks good, but can you explain how this solves the problem in the ticket? I think a code comment with this info would be helpful too.

[askarpets]

@roman-yermilov-gl the code change looks good, but can you explain how this solves the problem in the ticket? I think a code comment with this info would be helpful too.

@clnoll the user is facing an issue when running the discover command. I found that some of their files have records like below:

record 1: {"a": {"some_key": "some_value"}}
record 2: {"a": null}

Because of that, the schema could not be merged due to this logic, so I decided to not pass fields with null values to the _choose_wider_type method if their keys are already in the merged schema. Does that make sense?

[clnoll]

@askarpets, sorry I tagged Roman instead of you!

Thanks for the explanation, it makes sense to me. Can you please add a comment to the code about this bug?

[askarpets]

Added, thanks for the review @clnoll!