Fix issue where string dtype singleton elsize was accidentally changed (#73)

* Fix issue where string dtype singleton elsize was accidentally changed

* Update apt before installing deps

Author: peytondmurray

.github/workflows/ubuntu_build.yml

@@ -69,8 +69,9 @@ jobs:
         echo "*** cat /etc/odbc.ini"
         cat /etc/odbc.ini
 
-    - name: Install libicu dependency
+    - name: Update apt and install libicu dependency
       run: |
+        sudo apt update
         sudo apt install -y libicu-dev
 
     - name: Install ODBC driver for SQL Server

src/npcontainer.cpp

@@ -728,21 +728,30 @@ int coerce_column_desc_types(column_desc &cd, bool unicode, PyArray_Descr *descr
     Py_INCREF(descr);
 
     switch (descr->type_num) {
+        // Since we modify the elsize for string and unicode types, we need to create an entirely
+        // new descr (to be sure we aren't modifying the global numpy unicode/bytestring descr
+        // singletons)
         case NPY_STRING:
             cd.sql_c_type_ = SQL_C_BINARY;
+            cd.npy_type_descr_ = PyArray_DescrNew(descr);
+            Py_DECREF(descr);
+
             PyDataType_SET_ELSIZE(
-                descr,
+                cd.npy_type_descr_,
                 static_cast<npy_int>(cd.sql_size_)
             );
-            cd.element_buffer_size_ = PyDataType_ELSIZE(descr);
+            cd.element_buffer_size_ = PyDataType_ELSIZE(cd.npy_type_descr_);
             break;
         case NPY_UNICODE:
             cd.sql_c_type_ = SQL_C_WCHAR;
+            cd.npy_type_descr_ = PyArray_DescrNew(descr);
+            Py_DECREF(descr);
+
             PyDataType_SET_ELSIZE(
-                descr,
+                cd.npy_type_descr_,
                 static_cast<npy_int>(cd.sql_size_)
             );
-            cd.element_buffer_size_ = PyDataType_ELSIZE(descr);
+            cd.element_buffer_size_ = PyDataType_ELSIZE(cd.npy_type_descr_);
             break;
         case NPY_INT8:
             cd.sql_c_type_ = SQL_C_STINYINT;
@@ -953,7 +962,7 @@ map_column_desc_types(column_desc &cd, bool unicode)
         case SQL_BINARY:
         case SQL_VARBINARY:
         case SQL_LONGVARBINARY:
-            dtype = PyArray_DescrFromType(NPY_STRING);
+            dtype = PyArray_DescrNewFromType(NPY_STRING);
             if (dtype != NULL) {
                 // Set the element size for numpy
                 PyDataType_SET_ELSIZE(
@@ -1163,6 +1172,7 @@ query_desc::translate_types(bool use_unicode, PyObject *target_dtypes, int &unsu
                     Py_XDECREF(descr);
                     return -1;
                 }
+
                 // coerce_column_desc_types takes ownership of descr; no Py_DECREF needed
                 unsupported_fields += coerce_column_desc_types(column, use_unicode, descr);
             }

tests/test_numpy.py

@@ -19,7 +19,7 @@ def connection() -> npyodbc.Connection:
 
 @pytest.fixture(scope="module")
 def cursor(connection) -> npyodbc.Cursor:
-    """Return a cursor for a SQLite3 database."""
+    """Return a new cursor for a SQLite3 database."""
     return connection.cursor()
 
 
@@ -34,11 +34,13 @@ def cleanup(cursor: npyodbc.Cursor):
     cursor : npyodbc.Cursor
         Cursor for the database to clean up
     """
-    for table in cursor.execute('SELECT tbl_name FROM sqlite_schema;').fetchall():
+    for table in cursor.execute(
+        "SELECT tbl_name FROM sqlite_schema UNION ALL "
+        "SELECT tbl_name FROM sqlite_temp_schema;"
+    ).fetchall():
         cursor.execute(f'DROP TABLE {table[0]};')
     assert len(cursor.execute('SELECT tbl_name FROM sqlite_schema;').fetchall()) == 0
 
-
 def assert_result_close(a: np.ndarray, b: np.ndarray, dtype: Optional[np.dtype] = None):
     """Assert that a is close to b.
 
@@ -374,3 +376,41 @@ def test_null_strings(cursor):
     assert_array_equal(res['f_isnull'], [False, True])
 
     cleanup(cursor)
+
+
+@pytest.mark.parametrize(
+    "dtype", [
+        "<S256",
+        "<U256"
+    ]
+)
+def test_string_dtype_itemsize_corruption(cursor, dtype):
+    """Test that the string and unicode dtype singletons don't get modified by npyodbc.
+
+    This happens if the PyArray_Descr for the string or unicode dtype has its itemsize
+    modified; all modifications should happen on _copies_ of that singleton dtype, i.e.
+    we should be calling
+
+        PyArray_DescrNewFromType(NPY_STRING) // <-- Returns a copy of the singleton
+
+    rather than
+
+        PyArray_DescrFromType(NPY_STRING) // <-- Returns the singleton string dtype
+    """
+    cursor.execute("""
+        create temp table binary_type(
+            binary_type_n BLOB,
+            binary_type   BLOB NOT NULL
+        )
+    """)
+
+    cursor.execute("insert into binary_type values (1, 2)")
+    cursor.execute("insert into binary_type values (NULL, 5)")
+    cursor.execute("select * from binary_type")
+
+    itemsize_before = np.array(["abcdef"], dtype=dtype).itemsize
+    _ = cursor.fetchdictarray()
+    itemsize_after = np.array(["abcdef"], dtype=dtype).itemsize
+
+    assert itemsize_before == itemsize_after
+    cleanup(cursor)