-
Notifications
You must be signed in to change notification settings - Fork 590
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
feat(cdc): allow decimal to rw_int256 and varchar #16346
Conversation
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
c7d4944
to
ccc2589
Compare
a006ada
to
db6c98f
Compare
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.
LGTM, thanks!
Also kindly mention @bestmike007 |
...onnector-service/src/main/java/com/risingwave/connector/source/common/PostgresValidator.java
Show resolved
Hide resolved
037c7e8
to
be55d48
Compare
107 POSITIVE_INFINITY |
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.
Supposed to be NaN
and Infinity
?
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.
Yes. Trying to resolve this issue. If it's too non-trivial, I'll open another PR to fix it. Besides, I also find that in our shared source cdc, the conversion from pg-numeric
to rw-numeric
will also lose Infinity
and -Infinity
. Will fix it soon.
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.
🫠Too complicated to fix, as it requires us to know the types in upstream tables when parsing the data in JSON payload. Otherwise, we cannot distinguish whether a string is converted from a numeric or a real varchar.
So in this PR, when mapping pg-numeric to varchar, NaN will be NAN
, inf will be POSITIVE_INFINITY
, and -inf will be NEGATIVE_INFINITY
. At least these behavior is aligned among backfiling and normal data updates. Do you think that's acceptable?
For the bug in the conversion from pg-numeric to rw-numeric, I'll create a PR later to resolve it.
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.
Create a issue here to record.
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.
LGTM
be55d48
to
9d4577a
Compare
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Resolve #15977. With this PR,
numeric
in PG can be converted intorw_int256
orvarchar
implicitly through CDC.The only thing user need to do is when create table, mark the data type of the column corresponding to numeric as
rw_int256
orvarchar
.One thing worth mentioning is that the numerics with decimal parts will be considered as NULL if the column is to be converted to
rw_int256
.Currently, it's only for postgres. We can add support for mysql later if requested.
Checklist
./risedev check
(or alias,./risedev c
)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.