Skip to content

[source-postgres] Fix issue #6857 - #53649

Merged
Wenqi Hu (burakku) merged 1 commit into
masterfrom
wenqi-bug-fix
Feb 19, 2025
Merged

Wenqi Hu (burakku) merged 1 commit into
masterfrom
wenqi-bug-fix

Conversation

@burakku

@burakku Wenqi Hu (burakku) commented Feb 12, 2025 •

Copy link
Copy Markdown
Contributor

What

Fixes https://github.com/airbytehq/oncall/issues/6857

We have issue in #6857 about the data conversion error

Currently in our data type converter for debezium, we check if the value is null, if it is and column is non-nullable, we return the default value directly without conversion. Otherwise, return null as is.

Two issues here:

we should convert the default value as well then return it
it appears that we found null value in non-nullable column

How

This PR check if the column is nullable, if so, return null as is
if not, we feed the default value to conversion logic and return the result.

Review guide

User Impact

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

@vercel

vercel Bot commented Feb 12, 2025 •

Copy link
Copy Markdown

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 Feb 19, 2025 7:01am

@burakku Wenqi Hu (burakku) changed the title [source pg] Debug pg ts [source pg] Fix issue #6857 Feb 13, 2025
@burakku
Wenqi Hu (burakku) marked this pull request as ready for review February 13, 2025 20:39
@burakku
Wenqi Hu (burakku) requested a review from a team as a code owner February 13, 2025 20:39
@burakku

Wenqi Hu (burakku) commented Feb 13, 2025 •

Copy link
Copy Markdown
Contributor Author

/bump-version type=patch changelog="Fix issue that column default value did not get converted"

Bump Version job started... Check job output.

✅ Changes applied successfully. (640ffed62459fedea28305b371e1efde449897d2)

@theyueli Yue Li (theyueli) changed the title [source pg] Fix issue #6857 [source-postgres] Fix issue #6857 Feb 18, 2025
registration.register(SchemaBuilder.string().optional(), x -> {
if (x == null) {
return DebeziumConverterUtils.convertDefaultValue(field);
return field.isOptional() ? null : getNumberConvertedValue(field.defaultValue());

@theyueli Yue Li (theyueli) Feb 18, 2025 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand we need to convert the default value if field.isOptional returns false.

Could you explain why do we replace

DebeziumConverterUtils.convertDefaultValue(field); with getNumberConvertedValue(field.defaultValue()) ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually you are right, we should use DebeziumConverterUtils. I think we are misuse isOptional as if it's nullable. I will take another look

@theyueli Yue Li (theyueli) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix... ideally i'm wondering if we could factor out the structure

if (x == null) {
return field.isOptional() ? null : getXXXTypeConverted(field.defaultValue());
}

I was thinking if we could use

DebeziumConverterUtils.convertDefaultValue(); to replace the getXXXTypeConverted call above, but it seems you are getting rid of the Debezium utility... (and I was wondering why in the comment above I posted)

This branch was successfully deployed

1 active deployment
Preview — f6aea164 Deployed Feb 19, 2025 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/mysql connectors/source/postgres

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants