Improvements on the sending update/upsert statements

**New features:** * Implemented ability to send update/upsert requests with field names when schema is disabled (`fetch_schema=False`) and when fields are not found in the schema (good example of this case is using json path like `data.inner1.inner2.key1` as a key) **Bug fixes:** * Fixed issue with not being able to send Decimals in update statements. Now there are no extra checks - any payload is sent directly to Tarantool (fixes #34) **Other changes** * Fixed tests failing on modern Tarantool in the SQL queries. * Remove from ci/cd testing on mac os python 3.7 * Added Tarantool 3 to CI testing
igorcoding · May 5, 2024 · b6c9c66 · b6c9c66
1 parent 2277115
commit b6c9c66
Show file tree

Hide file tree

Showing 14 changed files with 269 additions and 98 deletions.
diff --git a/.github/workflows/actions.yaml b/.github/workflows/actions.yaml
@@ -8,40 +8,44 @@ jobs:
       matrix:
         os: [ ubuntu-latest, macos-latest ]
         python-version: ['3.7', '3.8', '3.9', '3.10', '3.11', '3.12', 'pypy3.10']
-        tarantool: ['1.10', '2']
+        tarantool: ['1.10', '2', '3']
         exclude:
           - os: macos-latest
             tarantool: '1.10'
+          - os: macos-latest
+            tarantool: '2'
+          - os: macos-latest
+            python-version: '3.7'
           - python-version: 'pypy3.10'
             tarantool: '1.10'
 
     runs-on: ${{ matrix.os }}
 
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
         with:
           submodules: recursive
       - name: Set up Python ${{ matrix.python-version }}
-        uses: actions/setup-python@v4
+        uses: actions/setup-python@v5
         with:
           python-version: ${{ matrix.python-version }}
-      - name: Install Tarantool ${{ matrix.tarantool }}
+
+      - name: Install Tarantool ${{ matrix.tarantool }} on Ubuntu
+        if: matrix.os == 'ubuntu-latest'
         run: |
-          if [ "$RUNNER_OS" == "Linux" ]; then
-            curl -L https://tarantool.io/nTmSHOX/release/${{ matrix.tarantool }}/installer.sh | bash
-            sudo apt-get -y install tarantool
-          elif [ "$RUNNER_OS" == "macOS" ]; then
-            brew install tarantool
-          else
-            echo "$RUNNER_OS not supported"
-            exit 1
-          fi
+          curl -L https://tarantool.io/nTmSHOX/release/${{ matrix.tarantool }}/installer.sh | bash
+          sudo apt-get -y install tarantool
+
+      - name: Install Tarantool ${{ matrix.tarantool }} on MacOS
+        if: matrix.os == 'macos-latest'
+        run: brew install tarantool
+
       - name: Install dependencies
         run: |
           python -m pip install --upgrade pip setuptools wheel coveralls
       - name: Run tests
         run: |
-          if [[ "$RUNNER_OS" == "Linux" && ${{ matrix.python-version }} == "3.12" && ${{ matrix.tarantool }} == "2" ]]; then
+          if [[ "$RUNNER_OS" == "Linux" && ${{ matrix.python-version }} == "3.12" && ${{ matrix.tarantool }} == "3" ]]; then
               make build && make test
               make clean && make debug && make coverage
               # coveralls
@@ -62,11 +66,11 @@ jobs:
       - test
 
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
         with:
           submodules: recursive
 
-      - uses: actions/setup-python@v4
+      - uses: actions/setup-python@v5
 
       - name: Install cibuildwheel
         run: python -m pip install --upgrade cibuildwheel
@@ -92,19 +96,19 @@ jobs:
         id: get_tag
         run: echo ::set-output name=TAG::${GITHUB_REF/refs\/tags\//}
       - run: echo "Current tag is ${{ steps.get_tag.outputs.TAG }}"
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
         with:
           submodules: recursive
       - name: Set up Python
-        uses: actions/setup-python@v4
+        uses: actions/setup-python@v5
         with:
           python-version: '3.12'
 
       - name: Install dependencies
         run: |
           python -m pip install --upgrade pip setuptools wheel twine build
 
-      - uses: actions/download-artifact@v3
+      - uses: actions/download-artifact@v4
         with:
           name: wheels
           path: wheels
@@ -133,12 +137,12 @@ jobs:
     needs:
       - test
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
         with:
           submodules: recursive
 
       - name: Set up Python
-        uses: actions/setup-python@v4
+        uses: actions/setup-python@v5
         with:
           python-version: '3.12'
 

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,17 @@
 # Changelog
 
+## v2.2.0
+**New features:**
+* Implemented ability to send update/upsert requests with field names when schema is disabled (`fetch_schema=False`) and when fields are not found in the schema (good example of this case is using json path like `data.inner1.inner2.key1` as a key)
+
+**Bug fixes:**
+* Fixed issue with not being able to send Decimals in update statements. Now there are no extra checks - any payload is sent directly to Tarantool (fixes [#34](https://github.com/igorcoding/asynctnt/issues/34))
+
+**Other changes**
+* Fixed tests failing on modern Tarantool in the SQL queries.
+* Removed from ci/cd testing on macOS python 3.7
+* Added Tarantool 3 to CI Testing
+
 ## v2.1.0
 **Breaking changes:**
 * Dropped support for Python 3.6

diff --git a/Makefile b/Makefile
@@ -45,7 +45,7 @@ mypy:
 	$(PYTHON) -m mypy --enable-error-code ignore-without-code .
 
 ruff:
-	$(PYTHON) -m ruff .
+	$(PYTHON) -m ruff check .
 
 style-check:
 	$(PYTHON) -m black --check --diff .

diff --git a/asynctnt/__init__.py b/asynctnt/__init__.py
@@ -16,4 +16,4 @@
     TarantoolTuple,
 )
 
-__version__ = "2.1.0"
+__version__ = "2.2.0"
diff --git a/asynctnt/iproto/requests/update.pyx b/asynctnt/iproto/requests/update.pyx
@@ -23,11 +23,17 @@ cdef char *encode_update_ops(WriteBuffer buffer,
 
         uint32_t extra_length
 
+        bint field_encode_as_str
         uint64_t field_no
+        char *field_str_c
+        ssize_t field_str_len
         object field_no_obj
 
         uint32_t splice_position, splice_offset
 
+    field_encode_as_str = 0
+    field_str_c = NULL
+
     begin = NULL
 
     if operations is not None:
@@ -60,22 +66,27 @@ cdef char *encode_update_ops(WriteBuffer buffer,
             raise TypeError(
                 'Operation type must of a str or bytes type')
 
+        cpython.bytes.PyBytes_AsStringAndSize(str_temp, &op_str_c,
+                                              &op_str_len)
+
         field_no_obj = operation[1]
         if isinstance(field_no_obj, int):
             field_no = <int> field_no_obj
         elif isinstance(field_no_obj, str):
             if space.metadata is not None:
-                field_no = <int> space.metadata.id_by_name(field_no_obj)
+                field_no = <int> space.metadata.id_by_name_safe(field_no_obj)
+                if field_no == -1:
+                    field_encode_as_str = 1
             else:
-                raise TypeError(
-                    'Operation field_no must be int as there is '
-                    'no format declaration in space {}'.format(space.sid))
+                field_encode_as_str = 1
+
+            if field_encode_as_str:
+                str_temp = encode_unicode_string(field_no_obj, buffer._encoding)
+                cpython.bytes.PyBytes_AsStringAndSize(str_temp, &field_str_c, &field_str_len)
         else:
             raise TypeError(
                 'Operation field_no must be of either int or str type')
 
-        cpython.bytes.PyBytes_AsStringAndSize(str_temp, &op_str_c,
-                                              &op_str_len)
         op = <char> 0
         if op_str_len == 1:
             op = op_str_c[0]
@@ -85,28 +96,9 @@ cdef char *encode_update_ops(WriteBuffer buffer,
                 or op == tarantool.IPROTO_OP_AND \
                 or op == tarantool.IPROTO_OP_XOR \
                 or op == tarantool.IPROTO_OP_OR \
-                or op == tarantool.IPROTO_OP_DELETE:
-            op_argument = operation[2]
-            if not isinstance(op_argument, int):
-                raise TypeError(
-                    'int argument required for '
-                    'Arithmetic and Delete operations'
-                )
-            # mp_sizeof_array(3)
-            # + mp_sizeof_str(1)
-            # + mp_sizeof_uint(field_no)
-            extra_length = 1 + 2 + mp_sizeof_uint(field_no)
-            p = begin = buffer._ensure_allocated(p, extra_length)
-
-            p = mp_encode_array(p, 3)
-            p = mp_encode_str(p, op_str_c, 1)
-            p = mp_encode_uint(p, field_no)
-            buffer._length += (p - begin)
-            p = buffer.mp_encode_obj(p, op_argument)
-        elif op == tarantool.IPROTO_OP_INSERT \
+                or op == tarantool.IPROTO_OP_DELETE \
+                or op == tarantool.IPROTO_OP_INSERT \
                 or op == tarantool.IPROTO_OP_ASSIGN:
-            op_argument = operation[2]
-
             # mp_sizeof_array(3)
             # + mp_sizeof_str(1)
             # + mp_sizeof_uint(field_no)
@@ -115,13 +107,19 @@ cdef char *encode_update_ops(WriteBuffer buffer,
 
             p = mp_encode_array(p, 3)
             p = mp_encode_str(p, op_str_c, 1)
-            p = mp_encode_uint(p, field_no)
+            if field_str_c == NULL:
+                p = mp_encode_uint(p, field_no)
+            else:
+                p = mp_encode_str(p, field_str_c, field_str_len)
+
             buffer._length += (p - begin)
+
+            op_argument = operation[2]
             p = buffer.mp_encode_obj(p, op_argument)
 
         elif op == tarantool.IPROTO_OP_SPLICE:
             if op_len < 5:
-                raise IndexError(
+                raise ValueError(
                     'Splice operation must have length of 5, '
                     'but got: {}'.format(op_len)
                 )
@@ -146,7 +144,10 @@ cdef char *encode_update_ops(WriteBuffer buffer,
 
             p = mp_encode_array(p, 5)
             p = mp_encode_str(p, op_str_c, 1)
-            p = mp_encode_uint(p, field_no)
+            if field_str_c == NULL:
+                p = mp_encode_uint(p, field_no)
+            else:
+                p = mp_encode_str(p, field_str_c, field_str_len)
             p = mp_encode_uint(p, splice_position)
             p = mp_encode_uint(p, splice_offset)
             buffer._length += (p - begin)

diff --git a/asynctnt/iproto/schema.pxd b/asynctnt/iproto/schema.pxd
@@ -21,6 +21,7 @@ cdef public class Metadata [object C_Metadata, type C_Metadata_Type]:
     cdef inline void add(self, int id, Field field)
     cdef inline str name_by_id(self, int i)
     cdef inline int id_by_name(self, str name) except *
+    cdef inline int id_by_name_safe(self, str name) except*
 
 
 cdef class SchemaIndex:

diff --git a/asynctnt/iproto/schema.pyx b/asynctnt/iproto/schema.pyx
@@ -51,6 +51,15 @@ cdef class Metadata:
             raise KeyError('Field \'{}\' not found'.format(name))
         return <int> <object> fld
 
+    cdef inline int id_by_name_safe(self, str name) except *:
+        cdef:
+            PyObject *fld
+
+        fld = cpython.dict.PyDict_GetItem(self.name_id_map, name)
+        if fld == NULL:
+            return -1
+        return <int> <object> fld
+
     cdef inline int len(self):
         return <int> cpython.list.PyList_GET_SIZE(self.fields)
 

diff --git a/pyproject.toml b/pyproject.toml
@@ -120,15 +120,15 @@ skip_glob = [
 
 
 [tool.ruff]
-select = [
+lint.select = [
     "E",  # pycodestyle errors
     "W",  # pycodestyle warnings
     "F",  # pyflakes
     # "I",  # isort
     "C",  # flake8-comprehensions
     "B",  # flake8-bugbear
 ]
-ignore = [
+lint.ignore = [
     "E501",  # line too long, handled by black
     "B008",  # do not perform function calls in argument defaults
     "C901",  # too complex

diff --git a/tests/files/app.lua b/tests/files/app.lua
@@ -51,6 +51,14 @@ local function bootstrap()
         types = {}
     }
 
+    function b:sql_space_name(space_name)
+        if self:check_version({3, 0}) then
+            return space_name
+        else
+            return space_name:upper()
+        end
+    end
+
     function b:check_version(expected)
         return check_version(expected, self.tarantool_ver)
     end
@@ -197,16 +205,22 @@ function truncate()
     _truncate(box.space.tester)
     _truncate(box.space.no_schema_space)
 
-    if box.space.SQL_SPACE ~= nil then
-        box.execute('DELETE FROM sql_space')
-    end
-
-    if box.space.SQL_SPACE_AUTOINCREMENT ~= nil then
-        box.execute('DELETE FROM sql_space_autoincrement')
-    end
-
-    if box.space.SQL_SPACE_AUTOINCREMENT_MULTIPLE ~= nil then
-        box.execute('DELETE FROM sql_space_autoincrement_multiple')
+    local sql_spaces = {
+        'sql_space',
+        'sql_space_autoincrement',
+        'sql_space_autoincrement_multiple',
+    }
+    for _, sql_space in ipairs(sql_spaces) do
+        local variants = {
+            sql_space,
+            sql_space:upper(),
+        }
+
+        for _, variant in ipairs(variants) do
+            if box.space[variant] ~= nil then
+                box.execute('DELETE FROM ' .. variant)
+            end
+        end
     end
 
     _truncate(box.space.tester_ext_dec)

diff --git a/tests/test_common.py b/tests/test_common.py
@@ -123,7 +123,9 @@ async def test__parse_numeric_map_keys(self):
     async def test__read_buffer_reallocate_ok(self):
         await self.tnt_reconnect(initial_read_buffer_size=1)
 
-        p, cmp = get_complex_param(encoding=self.conn.encoding)
+        p, cmp = get_complex_param(
+            encoding=self.conn.encoding, replace_bin=self.conn.version < (3, 0)
+        )
         try:
             res = await self.conn.call("func_param", [p])
         except Exception as e:

diff --git a/tests/test_op_call.py b/tests/test_op_call.py
@@ -79,12 +79,16 @@ async def test__call_args_tuple(self):
             self.fail(e)
 
     async def test__call_complex_param(self):
-        p, cmp = get_complex_param(encoding=self.conn.encoding)
+        p, cmp = get_complex_param(
+            encoding=self.conn.encoding, replace_bin=self.conn.version < (3, 0)
+        )
         res = await self.conn.call("func_param", [p])
         self.assertDictEqual(res[0][0], cmp, "Body ok")
 
     async def test__call_complex_param_bare(self):
-        p, cmp = get_complex_param(encoding=self.conn.encoding)
+        p, cmp = get_complex_param(
+            encoding=self.conn.encoding, replace_bin=self.conn.version < (3, 0)
+        )
         cmp = [cmp]
         res = await self.conn.call("func_param_bare", [p])
         if not self.has_new_call():
@@ -177,12 +181,16 @@ async def test__call16_args_tuple(self):
             self.fail(e)
 
     async def test__call16_complex_param(self):
-        p, cmp = get_complex_param(encoding=self.conn.encoding)
+        p, cmp = get_complex_param(
+            encoding=self.conn.encoding, replace_bin=self.conn.version < (3, 0)
+        )
         res = await self.conn.call("func_param", [p])
         self.assertDictEqual(res[0][0], cmp, "Body ok")
 
     async def test__call16_complex_param_bare(self):
-        p, cmp = get_complex_param(encoding=self.conn.encoding)
+        p, cmp = get_complex_param(
+            encoding=self.conn.encoding, replace_bin=self.conn.version < (3, 0)
+        )
         res = await self.conn.call16("func_param_bare", [p])
         self.assertDictEqual(res[0][0], cmp, "Body ok")