feat: add schema change #27
Conversation
| /// | ||
| /// Reference: <https://github.com/apache/paimon/blob/release-0.8.2/paimon-core/src/main/java/org/apache/paimon/schema/SchemaChange.java#L36> | ||
| #[derive(Debug, PartialEq, Eq, Deserialize, Serialize)] | ||
| pub enum SchemaChange { |
There was a problem hiding this comment.
I think we don't need to specify types for every enum value. It's unlikely that we'll use SetOption without SchemaChange.
How about embedding the struct directly in this enum?
pub enum SchemaChange {
SetOption {
key: String,
value: String,
}
...
}| // specific language governing permissions and limitations | ||
| // under the License. | ||
|
|
||
| use crate::spec::DataTypeRoot; |
There was a problem hiding this comment.
DataTypeRoot has been removed. Please merge with main, thanks!
| /// Reference: <https://github.com/apache/paimon/blob/release-0.8.2/paimon-core/src/main/java/org/apache/paimon/schema/SchemaChange.java#L410> | ||
| #[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] | ||
| #[serde(rename_all = "camelCase")] | ||
| pub struct Move { |
There was a problem hiding this comment.
Move is a keyword in rust, how about naming it ColumnMove and ColumnMoveType?
There was a problem hiding this comment.
Move is a keyword in rust, how about naming it
ColumnMoveandColumnMoveType?
OK, I will do it.
| use crate::spec::{DataType, IntType}; | ||
|
|
||
| #[test] | ||
| fn test_set_option() { |
There was a problem hiding this comment.
Would you like to add a test against real string?
There was a problem hiding this comment.
Would you like to add a test against real string?
Are these tests necessary? If they are not, I will move out and add serialize/deserialize tests for the schema_change.
There was a problem hiding this comment.
If they are not, I will move out and add
serialize/deserializetests for theschema_change.
That makes more sense.
|
|
||
| let schema_changes: Vec<SchemaChange> = serde_json::from_str(json_data).unwrap(); | ||
|
|
||
| for change in schema_changes { |
There was a problem hiding this comment.
Hi, SchemaChange has implemented Eq, so we don't need to check it this way. Instead, we can:
assert_eq!(
schema_changes,
vec![
SchemaChange::SetOption {
key: "snapshot.time-retained".to_string,
value: "2h".to_string()
},
...
]);
|
Hi, @SteNicholas @Aitozi, this PR is happy to go. |
close: #25