-
Notifications
You must be signed in to change notification settings - Fork 609
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): support mapping json of pg to jsonb in rw in pg-cdc #20444
base: main
Are you sure you want to change the base?
Conversation
70e0703
to
2f5b328
Compare
8678878
to
1156c89
Compare
Note that
Similarly, |
} | ||
Ok(JsonbVal::from(Value::from_text(raw)?)) | ||
Ok(match *ty { | ||
Type::JSON => JsonbVal::from(Value::from_text(raw)?), |
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.
Let's acknowledge in comments that treating JSON
as JSONB
can be wrong in special cases.
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.
Added. Will also add it in doc.
@@ -207,7 +207,7 @@ impl PostgresExternalTable { | |||
SeaType::Boolean => Ok(PgType::BOOL), | |||
SeaType::Point => Ok(PgType::POINT), | |||
SeaType::Uuid => Ok(PgType::UUID), | |||
SeaType::JsonBinary => Ok(PgType::JSONB), | |||
SeaType::Json | SeaType::JsonBinary => Ok(PgType::JSONB), |
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.
How is this used in RisingWave? I see CHAR_ARRAY
and TIMETZ_ARRAY
here which do not exist in RisingWave.
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.
This part of codes are used to load the schema of the table to be connected via source or sink.
timestamptz[]
should be supported in RW?
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.
I meant it should be either:
// mapping into RisingWave supported data types
SeaType::Char(_) | SeaType::Varchar(_) => Ok(PgType::VARCHAR),
SeaType::Json | SeaType::JsonBinary => Ok(PgType::JSONB)
or
// mapping into PostgreSQL data types, regardless of RisingWave support
SeaType::Varchar(_) => Ok(PgType::VARCHAR),
SeaType::Char(_) => Ok(PgType::CHAR),
SeaType::JsonBinary => Ok(PgType::JSONB),
SeaType::Json => Ok(PgType::JSON),
But after this PR it is:
SeaType::Varchar(_) => Ok(PgType::VARCHAR),
SeaType::Char(_) => Ok(PgType::CHAR),
SeaType::Json | SeaType::JsonBinary => Ok(PgType::JSONB)
That's why I am asking how is this function used / what is its responsibility.
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
As titled
Checklist
Documentation
Release note