-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DBZ-8302: Treat oracle negative timestamp values as -infinity. #5996
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Bue-von-hon <[email protected]> Co-authored-by: chungeun-choi <[email protected]> Co-authored-by: sean-k1 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dongwook-chan Thanks for the PR. Could you please reformat the code so the tests are executed. lso could you please add a test case for the new feature? Thanks
debezium-connector-jdbc/pom.xml
Outdated
@@ -202,6 +202,7 @@ | |||
<dependency> | |||
<groupId>com.oracle.database.jdbc</groupId> | |||
<artifactId>ojdbc11</artifactId> | |||
<version>23.6.0.24.10</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why it is necessary to override the version here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, my bad! I removed the version tag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jpechane
I also added config variable ORACLE_INFINITY_CONVERTER_ENABLE
for the feature,
but have trouble accessing it from convertInfinityTimestampValue(). Could you give some advice on this? And I would like some advice on running integration test too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @dongwook-chan do you intend to add test cases?
We were hoping to create a PR on this issue but found that the repo is archived.
Below is the PR that we attempted to create.
We want to refer to config from ZonedTimestampType.
But it takes a lot of depth to reach config from this class.
So we would you to provide some advice on this issue pls.
https://github.com/dongwook-chan/debezium-connector-jdbc/blob/2e2aa669c2a4f50ba85196d5cf8f4115a5155c28/src/main/java/io/debezium/connector/jdbc/dialect/mysql/ZonedTimestampType.java#L37-L39
We tried to run integration test on jdbc sink connector but failed.
So if you have any instructions on this please provide.