Xuanwo commented on code in PR #27:
URL: https://github.com/apache/paimon-rust/pull/27#discussion_r1711330789


##########
crates/paimon/src/spec/schema_change.rs:
##########
@@ -264,185 +271,181 @@ mod tests {
 
     #[test]
     fn test_schema_change_serialize_deserialize() {
-        // SchemaChange: SetOption
-        let schema_change =
-            SchemaChange::set_option("snapshot.time-retained".to_string(), 
"2h".to_string());
-        let json = serde_json::to_string(&schema_change).unwrap();
-        let schema_change = 
serde_json::from_str::<SchemaChange>(&json).unwrap();
-        assert_eq!(
-            schema_change,
-            SchemaChange::set_option("snapshot.time-retained".to_string(), 
"2h".to_string())
-        );
-
-        // SchemaChange: RemoveOption
-        let schema_change = 
SchemaChange::remove_option("compaction.max.file-num".to_string());
-        let json = serde_json::to_string(&schema_change).unwrap();
-        let schema_change = 
serde_json::from_str::<SchemaChange>(&json).unwrap();
-        assert_eq!(
-            schema_change,
-            SchemaChange::remove_option("compaction.max.file-num".to_string())
-        );
-
-        // SchemaChange: UpdateComment
-        let schema_change = 
SchemaChange::update_comment(Some("table.comment".to_string()));
-        let json = serde_json::to_string(&schema_change).unwrap();
-        let schema_change = 
serde_json::from_str::<SchemaChange>(&json).unwrap();
-        assert_eq!(
-            schema_change,
-            SchemaChange::update_comment(Some("table.comment".to_string()))
-        );
-
-        // SchemaChange: AddColumn
-        let schema_change =
-            SchemaChange::add_column("col1".to_string(), 
DataType::Int(IntType::new()));
-        let json = serde_json::to_string(&schema_change).unwrap();
-        let schema_change = 
serde_json::from_str::<SchemaChange>(&json).unwrap();
-        assert_eq!(
-            schema_change,
-            SchemaChange::add_column("col1".to_string(), 
DataType::Int(IntType::new()))
-        );
-
-        // SchemaChange: AddColumn with description
-        let schema_change = SchemaChange::add_column_with_description(
-            "col1".to_string(),
-            DataType::Int(IntType::new()),
-            "col1_description".to_string(),
-        );
-        let json = serde_json::to_string(&schema_change).unwrap();
-        let schema_change = 
serde_json::from_str::<SchemaChange>(&json).unwrap();
-        assert_eq!(
-            schema_change,
-            SchemaChange::add_column_with_description(
-                "col1".to_string(),
-                DataType::Int(IntType::new()),
-                "col1_description".to_string(),
-            )
-        );
-
-        // SchemaChange: AddColumn with description and column_move
-        let schema_change = 
SchemaChange::add_column_with_description_and_column_move(
-            "col1".to_string(),
-            DataType::Int(IntType::new()),
-            "col1_description".to_string(),
-            ColumnMove::move_first("col1_first".to_string()),
-        );
-        let json = serde_json::to_string(&schema_change).unwrap();
-        let schema_change = 
serde_json::from_str::<SchemaChange>(&json).unwrap();
-        assert_eq!(
-            schema_change,
-            SchemaChange::add_column_with_description_and_column_move(
-                "col1".to_string(),
-                DataType::Int(IntType::new()),
-                "col1_description".to_string(),
-                ColumnMove::move_first("col1_first".to_string()),
-            )
-        );
-
-        // SchemaChange: RenameColumn
-        let schema_change =
-            SchemaChange::rename_column("col3".to_string(), 
"col3_new_name".to_string());
-        let json = serde_json::to_string(&schema_change).unwrap();
-        let schema_change = 
serde_json::from_str::<SchemaChange>(&json).unwrap();
-        assert_eq!(
-            schema_change,
-            SchemaChange::rename_column("col3".to_string(), 
"col3_new_name".to_string())
-        );
-
-        // SchemaChange: DropColumn
-        let schema_change = SchemaChange::drop_column("col1".to_string());
-        let json = serde_json::to_string(&schema_change).unwrap();
-        let schema_change = 
serde_json::from_str::<SchemaChange>(&json).unwrap();
-        assert_eq!(schema_change, 
SchemaChange::drop_column("col1".to_string()));
-
-        // SchemaChange: UpdateColumnType
-        let schema_change = SchemaChange::update_column_type(
-            "col14".to_string(),
-            DataType::Double(DoubleType::new()),
-        );
-        let json = serde_json::to_string(&schema_change).unwrap();
-        let schema_change = 
serde_json::from_str::<SchemaChange>(&json).unwrap();
-        assert_eq!(
-            schema_change,
-            SchemaChange::update_column_type(
-                "col14".to_string(),
-                DataType::Double(DoubleType::new()),
-            )
-        );
-
-        // SchemaChange: UpdateColumnPosition
-        let schema_change =
-            
SchemaChange::update_column_position(ColumnMove::move_first("col4_first".to_string()));
-        let json = serde_json::to_string(&schema_change).unwrap();
-        let schema_change = 
serde_json::from_str::<SchemaChange>(&json).unwrap();
-        assert_eq!(
-            schema_change,
-            
SchemaChange::update_column_position(ColumnMove::move_first("col4_first".to_string()))
-        );
-
-        // SchemaChange: UpdateColumnNullability
-        let schema_change = 
SchemaChange::update_column_nullability("col4".to_string(), false);
-        let json = serde_json::to_string(&schema_change).unwrap();
-        let schema_change = 
serde_json::from_str::<SchemaChange>(&json).unwrap();
-        assert_eq!(
-            schema_change,
-            SchemaChange::update_column_nullability("col4".to_string(), false)
-        );
-
-        // SchemaChange: UpdateColumnsNullability
-        let schema_change = SchemaChange::update_columns_nullability(
-            vec!["col5".to_string(), "f2".to_string()],
-            false,
-        );
-        let json = serde_json::to_string(&schema_change).unwrap();
-        let schema_change = 
serde_json::from_str::<SchemaChange>(&json).unwrap();
-        assert_eq!(
-            schema_change,
-            SchemaChange::update_columns_nullability(
-                vec!["col5".to_string(), "f2".to_string()],
-                false
-            )
-        );
-
-        // SchemaChange: UpdateColumnComment
-        let schema_change =
-            SchemaChange::update_column_comment("col4".to_string(), "col4 
field".to_string());
-        let json = serde_json::to_string(&schema_change).unwrap();
-        let schema_change = 
serde_json::from_str::<SchemaChange>(&json).unwrap();
-        assert_eq!(
-            schema_change,
-            SchemaChange::update_column_comment("col4".to_string(), "col4 
field".to_string())
-        );
-        // SchemaChange: UpdateColumnsComment
-        let schema_change = SchemaChange::update_columns_comment(
-            vec!["col5".to_string(), "f1".to_string()],
-            "col5 f1 field".to_string(),
-        );
-        let json = serde_json::to_string(&schema_change).unwrap();
-        let schema_change = 
serde_json::from_str::<SchemaChange>(&json).unwrap();
-        assert_eq!(
-            schema_change,
-            SchemaChange::update_columns_comment(
-                vec!["col5".to_string(), "f1".to_string()],
-                "col5 f1 field".to_string()
-            )
-        );
+        let json_data = r#"
+        [
+          {
+            "setOption": {
+              "key": "snapshot.time-retained",
+              "value": "2h"
+            }
+          },
+          {
+            "removeOption": {
+              "key": "compaction.max.file-num"
+            }
+          },
+          {
+            "updateComment": {
+              "comment": "table.comment"
+            }
+          },
+          {
+            "addColumn": {
+              "fieldName": "col1",
+              "dataType": {
+                "Int": {
+                  "nullable": true
+                }
+              },
+              "description": "col1_description",
+              "move": {
+                "fieldName": "col1_first",
+                "referencedFieldName": null,
+                "type": "FIRST"
+              }
+            }
+          },
+          {
+            "renameColumn": {
+              "fieldName": "col3",
+              "newName": "col3_new_name"
+            }
+          },
+          {
+            "dropColumn": {
+              "fieldName": "col1"
+            }
+          },
+          {
+            "updateColumnType": {
+              "fieldName": "col14",
+              "dataType": {
+                "Double": {
+                  "nullable": true
+                }
+              }
+            }
+          },
+          {
+            "updateColumnPosition": {
+              "move": {
+                "fieldName": "col4_first",
+                "referencedFieldName": null,
+                "type": "FIRST"
+              }
+            }
+          },
+          {
+            "updateColumnNullability": {
+              "fieldName": [
+                "col5",
+                "f2"
+              ],
+              "nullable": false
+            }
+          },
+          {
+            "updateColumnComment": {
+              "fieldNames": [
+                "col5",
+                "f1"
+              ],
+              "newDescription": "col5 f1 field"
+            }
+          }
+        ]"#;
+
+        let schema_changes: Vec<SchemaChange> = 
serde_json::from_str(json_data).unwrap();
+
+        for change in schema_changes {

Review Comment:
   Hi, `SchemaChange` has implemented `Eq`, so we don't need to check it this 
way. Instead, we can:
   
   ```rust
   assert_eq!(
       schema_changes, 
       vec![
           SchemaChange::SetOption {
               key: "snapshot.time-retained".to_string, 
               value: "2h".to_string()
           },
           ...
       ]);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to