Alter doc (#1876)

### What problem does this PR solve? Add http api for alter. Fix pysdk test. Add alter document. Fix replay add/drop column. Fix cleanup before do full checkpoint when restart. ### Type of change - [x] Bug Fix (non-breaking change which fixes an issue) - [x] New Feature (non-breaking change which adds functionality) - [x] Documentation Update --------- Co-authored-by: Jin Hai <[email protected]>
infiniflow · Sep 18, 2024 · 8157588 · 8157588
1 parent 5f3023e
commit 8157588
Show file tree

Hide file tree

Showing 35 changed files with 473 additions and 254 deletions.
diff --git a/docs/references/pysdk_api_reference.md b/docs/references/pysdk_api_reference.md
@@ -1901,6 +1901,52 @@ table_object.output(["num", "body", "vec", "sparse_column", "year", "tensor", "_
 
 ---
 
+## add_columns
+
+```python
+table_object.add_columns(column_defs)
+```
+
+### Parameters
+
+#### column_defs: `dict[str, dict[str, Any]]`, *Required*
+
+A dictionary defining the columns to add. Each key in the dictionary is a column name (`str`), with a corresponding 'value' dictionary defining the column's data type and default value. See the description of `create_table()`'s `columns_definition` for all available settings. 
+
+:::caution NOTE
+You must specify a default value each time you add a column.
+:::
+
+
+#### Examples
+```python
+# Add an integer column and a varchar column at once
+table_obj.add_columns({"new_column_name1": {"type": "integer", "default": 0}, "new_column_name2": {"type": "varchar", "default": ""}})
+```
+
+---
+
+# drop_columns
+
+```python
+table_object.drop_columns(column_names)
+```
+
+### Parameters
+
+#### column_names: `list[str]`, *Required*
+
+A list of strings representing the names of the columns to drop.
+
+#### Examples
+
+```python
+# Remove two columns at once
+table_object.drop_columns(["column_name1", "column_name2"])
+```
+
+---
+
 ## to_result
 
 

diff --git a/python/infinity_http.py b/python/infinity_http.py
@@ -25,6 +25,7 @@ class infinity_http:
 
     def disconnect(self):
         print("disconnect")
+        return database_result(error_code=ErrorCode.OK)
 
     def set_up_header(self, param=[], tp={}):
         header = {}
@@ -101,6 +102,14 @@ def raise_exception(self, resp, expect={}):
         logging.debug("----------------------------------------------")
         return
 
+    def get_database_result(self, resp, expect={}):
+        try:
+            self.raise_exception(resp, expect)
+            return database_result(error_code=ErrorCode.OK)
+        except InfinityException as e:
+            print(e)
+            return database_result(error_code=e.error_code)
+
     # database
     def create_database(self, db_name, opt=ConflictType.Error):
         url = f"databases/{db_name}"
@@ -507,6 +516,28 @@ def explain(self, ExplainType = ExplainType.Physical):
                     message = message + res[k] + "\n"
         return message
 
+    def add_columns(self, columns_definition = {}):
+        url = f"databases/{self.database_name}/tables/{self.table_name}/columns"
+        h = self.set_up_header(["accept", "content-type"])
+        fields = []
+        for col in columns_definition:
+            tmp = {"name": col}
+            for param_name in columns_definition[col]:
+                tmp[param_name.lower()] = columns_definition[col][param_name]
+            fields.append(tmp)
+        d = self.set_up_data([], {"fields": fields})
+        r = self.request(url, "put", h, d)
+        return self.get_database_result(r)
+
+    def drop_columns(self, column_name: list[str] | str):
+        if isinstance(column_name, str):
+            column_name = [column_name]
+        url = f"databases/{self.database_name}/tables/{self.table_name}/columns"
+        h = self.set_up_header(["accept", "content-type"])
+        d = self.set_up_data([], {"column_names": column_name})
+        r = self.request(url, "delete", h, d)
+        return self.get_database_result(r)
+
     def output(
         self,
         output=[],

diff --git a/python/infinity_sdk/infinity/remote_thrift/table.py b/python/infinity_sdk/infinity/remote_thrift/table.py
@@ -398,12 +398,12 @@ def optimize(self, index_name: str, opt_params: dict[str, str]):
         opt_options.opt_params = [ttypes.InitParameter(k, v) for k, v in opt_params.items()]
         return self._conn.optimize(db_name=self._db_name, table_name=self._table_name, optimize_opt=opt_options)
 
-    def add_columns(self, column_def_dict: dict):
-        column_defs = []
-        for index, (column_name, column_info) in enumerate(column_def_dict.items()):
+    def add_columns(self, column_defs: dict):
+        column_defs_list = []
+        for index, (column_name, column_info) in enumerate(column_defs.items()):
             check_valid_name(column_name, "Column")
-            get_ordinary_info(column_info, column_defs, column_name, index)
-        return self._conn.add_columns(db_name=self._db_name, table_name=self._table_name, column_defs=column_defs)
+            get_ordinary_info(column_info, column_defs_list, column_name, index)
+        return self._conn.add_columns(db_name=self._db_name, table_name=self._table_name, column_defs=column_defs_list)
 
     def drop_columns(self, column_names: list[str] | str):
         if isinstance(column_names, str):

diff --git a/python/restart_test/test_alter.py b/python/restart_test/test_alter.py
@@ -176,7 +176,7 @@ def part1(infinity_obj):
             res = table_obj.drop_columns(["c2"])
             assert res.error_code == ErrorCode.OK
 
-            time.sleep(3)  # wait for the data to be cleaned up
+            time.sleep(4)  # wait for the data to be cleaned up
 
             dropped_column_dirs = pathlib.Path(data_dir).rglob("1.col*")
             # find the column file with the column idx = 1
@@ -186,4 +186,19 @@ def part1(infinity_obj):
             column_dirs = pathlib.Path(data_dir).rglob("*[02].col")
             assert len(list(column_dirs)) == 2
 
+            res = table_obj.drop_columns(["c3"])
+            assert res.error_code == ErrorCode.OK
+
         part1()
+
+        @decorator
+        def part2(infinity_obj):
+            db_obj = infinity_obj.get_database("default_db")
+            table_obj = db_obj.get_table(table_name)
+
+            dropped_column_dirs = pathlib.Path(data_dir).rglob("2.col*")
+            assert len(list(dropped_column_dirs)) == 0
+
+            db_obj.drop_table(table_name)
+
+        part2()
diff --git a/python/restart_test/test_cleanup.py b/python/restart_test/test_cleanup.py
@@ -39,14 +39,17 @@ def test_cleanuped_data(
         uri = common_values.TEST_LOCAL_HOST
         infinity_runner.clear()
         table_name = "test_cleanup"
+        table_name2 = "test2_cleanup"
 
         decorator = infinity_runner_decorator_factory(config, uri, infinity_runner)
 
         @decorator
         def part1(infinity_obj):
             db_obj = infinity_obj.get_database("default_db")
             db_obj.drop_table(table_name, ConflictType.Ignore)
+            db_obj.drop_table(table_name2, ConflictType.Ignore)
             table_obj = db_obj.create_table(table_name, columns, ConflictType.Error)
+            table_obj2 = db_obj.create_table(table_name2, columns, ConflictType.Error)
 
             insert_n = 100
             data_gen = data_gen_factory(insert_n)
@@ -56,6 +59,7 @@ def part1(infinity_obj):
                 for column_name, column_data in zip(columns.keys(), data):
                     data_line[column_name] = column_data
                 table_obj.insert([data_line])
+                table_obj2.insert([data_line])
 
             db_obj.drop_table(table_name, ConflictType.Error)
 
@@ -65,16 +69,14 @@ def part1(infinity_obj):
             dropped_dirs = pathlib.Path(data_dir).rglob(f"*{table_name}*")
             assert len(list(dropped_dirs)) == 0
 
+            db_obj.drop_table(table_name2, ConflictType.Error)
+
         part1()
 
         @decorator
         def part2(infinity_obj):
-            db_obj = infinity_obj.get_database("default_db")
-            try:
-                table_obj = db_obj.get_table(table_name)
-                assert False
-            except infinity.InfinityException as e:
-                assert e.error_code == infinity.ErrorCode.TABLE_NOT_EXIST
+            dropped_dirs = pathlib.Path(data_dir).rglob(f"*{table_name2}*")
+            assert len(list(dropped_dirs)) == 0
 
         part2()
 
@@ -101,7 +103,7 @@ def test_cleanuped_index(
         data_gen_factory,
     ):
         config = "test/data/config/restart_test/test_cleanup/1.toml"
-        data_dir = "var/infinity/data"
+        data_dir = "/var/infinity/data"
         uri = common_values.TEST_LOCAL_HOST
         infinity_runner.clear()
         table_name = "test_cleanup_index"

diff --git a/python/test_pysdk/test_alter.py b/python/test_pysdk/test_alter.py
@@ -19,26 +19,27 @@
 
 class TestInfinity:
     @pytest.fixture(autouse=True)
-    def setup_and_teardown(self, local_infinity, http):
+    def setup_and_teardown(self, local_infinity, http, suffix):
         if local_infinity:
             module = importlib.import_module("infinity_embedded.index")
             globals()["index"] = module
             self.uri = common_values.TEST_LOCAL_PATH
             self.infinity_obj = infinity_embedded.connect(self.uri)
+        elif http:
+            self.uri = common_values.TEST_LOCAL_HOST
+            self.infinity_obj = infinity_http()
         else:
             self.uri = common_values.TEST_LOCAL_HOST
             self.infinity_obj = infinity.connect(self.uri)
-        if http:
-            self.infinity_obj = infinity_http()
 
+        self.suffix = suffix
         yield
 
         res = self.infinity_obj.disconnect()
         assert res.error_code == infinity.ErrorCode.OK
 
-    @pytest.mark.skip(reason="conflict with other")
     def test_simple_add_columns(self):
-        table_name = "test_add_column"
+        table_name = "test_add_column" + self.suffix
         db_obj = self.infinity_obj.get_database("default_db")
         db_obj.drop_table(table_name, ConflictType.Ignore)
         table_obj = db_obj.create_table(
@@ -92,9 +93,8 @@ def test_simple_add_columns(self):
 
         db_obj.drop_table(table_name)
 
-    @pytest.mark.skip(reason="conflict with other")
     def test_simple_drop_columns(self):
-        table_name = "test_drop_column"
+        table_name = "test_drop_column" + self.suffix
         db_obj = self.infinity_obj.get_database("default_db")
         db_obj.drop_table(table_name, ConflictType.Ignore)
         table_obj = db_obj.create_table(
@@ -144,9 +144,8 @@ def test_simple_drop_columns(self):
 
         db_obj.drop_table(table_name)
 
-    @pytest.mark.skip(reason="conflict with other")
     def test_add_drop_column_with_index(self):
-        table_name = "test_add_drop_column_with_index"
+        table_name = "test_add_drop_column_with_index" + self.suffix
         db_obj = self.infinity_obj.get_database("default_db")
         db_obj.drop_table(table_name, ConflictType.Ignore)
         table_obj = db_obj.create_table(

diff --git a/python/test_pysdk/test_basic.py b/python/test_pysdk/test_basic.py
@@ -36,11 +36,12 @@ def setup(self, local_infinity, http):
             globals()["index"] = module
             self.uri = common_values.TEST_LOCAL_PATH
             self.infinity_obj = infinity_embedded.connect(self.uri)
+        elif http:
+            self.uri = common_values.TEST_LOCAL_HOST
+            self.infinity_obj = infinity_http()
         else:
             self.uri = common_values.TEST_LOCAL_HOST
             self.infinity_obj = infinity.connect(self.uri)
-        if http:
-            self.infinity_obj = infinity_http()
 
     def teardown(self):
         res = self.infinity_obj.disconnect()

diff --git a/python/test_pysdk/test_condition.py b/python/test_pysdk/test_condition.py
@@ -23,11 +23,12 @@ def setup(self, local_infinity, http):
         if local_infinity:
             self.uri = common_values.TEST_LOCAL_PATH
             self.infinity_obj = infinity_embedded.connect(self.uri)
+        elif http:
+            self.uri = common_values.TEST_LOCAL_HOST
+            self.infinity_obj = infinity_http()
         else:
             self.uri = common_values.TEST_LOCAL_HOST
             self.infinity_obj = infinity.connect(self.uri)
-        if http:
-            self.infinity_obj = infinity_http()
         assert self.infinity_obj
 
     def teardown(self):

diff --git a/python/test_pysdk/test_connection_pool.py b/python/test_pysdk/test_connection_pool.py
@@ -25,11 +25,12 @@ def setup(self, local_infinity, http):
         if local_infinity:
             self.uri = common_values.TEST_LOCAL_PATH
             self.infinity_obj = infinity_embedded.connect(self.uri)
+        elif http:
+            self.uri = common_values.TEST_LOCAL_HOST
+            self.infinity_obj = infinity_http()
         else:
             self.uri = common_values.TEST_LOCAL_HOST
             self.infinity_obj = infinity.connect(self.uri)
-        if http:
-            self.infinity_obj = infinity_http()
         assert self.infinity_obj
 
     def teardown(self):

diff --git a/python/test_pysdk/test_convert.py b/python/test_pysdk/test_convert.py
@@ -28,11 +28,12 @@ def setup(self, local_infinity, http):
             globals()['InfinityException'] = func
             self.uri = common_values.TEST_LOCAL_PATH
             self.infinity_obj = infinity_embedded.connect(self.uri)
+        elif http:
+            self.uri = common_values.TEST_LOCAL_HOST
+            self.infinity_obj = infinity_http()
         else:
             self.uri = common_values.TEST_LOCAL_HOST
             self.infinity_obj = infinity.connect(self.uri)
-        if http:
-            self.infinity_obj = infinity_http()
         assert self.infinity_obj
 
     def teardown(self):

diff --git a/python/test_pysdk/test_database.py b/python/test_pysdk/test_database.py
@@ -35,12 +35,13 @@ def setup_class(request, local_infinity, http):
         globals()['InfinityException'] = func
         uri = common_values.TEST_LOCAL_PATH
         request.cls.infinity_obj = infinity_embedded.connect(uri)
+    elif http:
+        uri = common_values.TEST_LOCAL_HOST
+        request.cls.infinity_obj = infinity_http()
     else:
         uri = common_values.TEST_LOCAL_HOST
         request.cls.infinity_obj = infinity.connect(uri)
     request.cls.uri = uri
-    if http:
-        request.cls.infinity_obj = infinity_http()
     yield
     request.cls.infinity_obj.disconnect()
 

diff --git a/python/test_pysdk/test_delete.py b/python/test_pysdk/test_delete.py
@@ -37,12 +37,13 @@ def setup_class(request, local_infinity, http):
         globals()['InfinityException'] = func
         uri = common_values.TEST_LOCAL_PATH
         request.cls.infinity_obj = infinity_embedded.connect(uri)
+    elif http:
+        uri = common_values.TEST_LOCAL_HOST
+        request.cls.infinity_obj = infinity_http()
     else:
         uri = common_values.TEST_LOCAL_HOST
         request.cls.infinity_obj = infinity.connect(uri)
     request.cls.uri = uri
-    if http:
-        request.cls.infinity_obj = infinity_http()
     yield
     request.cls.infinity_obj.disconnect()
 

diff --git a/python/test_pysdk/test_explain.py b/python/test_pysdk/test_explain.py
@@ -30,11 +30,12 @@ def setup(self, local_infinity, http):
             globals()['ExplainType'] = func
             self.uri = common_values.TEST_LOCAL_PATH
             self.infinity_obj = infinity_embedded.connect(self.uri)
+        elif http:
+           self.uri = common_values.TEST_LOCAL_HOST
+           self.infinity_obj = infinity_http()
         else:
             self.uri = common_values.TEST_LOCAL_HOST
             self.infinity_obj = infinity.connect(self.uri)
-        if http:
-           self.infinity_obj = infinity_http()
         assert self.infinity_obj
 
     def teardown(self):

diff --git a/python/test_pysdk/test_export.py b/python/test_pysdk/test_export.py
@@ -37,12 +37,13 @@ def setup_class(request, local_infinity, http):
         globals()['InfinityException'] = func
         uri = common_values.TEST_LOCAL_PATH
         request.cls.infinity_obj = infinity_embedded.connect(uri)
+    elif http:
+        uri = common_values.TEST_LOCAL_HOST
+        request.cls.infinity_obj = infinity_http()
     else:
         uri = common_values.TEST_LOCAL_HOST
         request.cls.infinity_obj = infinity.connect(uri)
     request.cls.uri = uri
-    if http:
-        request.cls.infinity_obj = infinity_http()
     yield
     request.cls.infinity_obj.disconnect()