From 31ac5a7a2d0145420e3fd930b0eedd45be3f0136 Mon Sep 17 00:00:00 2001 From: Jahnvi Thakkar Date: Thu, 17 Sep 2026 21:21:01 +0530 Subject: [PATCH 1/7] REFACTOR: Keep fetchmany column metadata native and call-local Remove native metadata dictionary roundtrips while preserving eager Unicode names and fresh per-call descriptions. Add behavior and profiling regression coverage. Performance acceptance remains unresolved after the bounded local study. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 4 + mssql_python/pybind/ddbc_bindings.cpp | 117 +++++-- tests/test_040_fetch_native_metadata.py | 387 ++++++++++++++++++++++++ 3 files changed, 480 insertions(+), 28 deletions(-) create mode 100644 tests/test_040_fetch_native_metadata.py diff --git a/CHANGELOG.md b/CHANGELOG.md index aab046b3a..eee11e3d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), does not change the default provider or ship any Rust driver binaries. ### Changed +- `fetchmany()` keeps freshly described column types and sizes in call-local native + metadata instead of round-tripping them through Python dictionaries. Column names + retain eager Unicode conversion; public column descriptions and fetch behavior + are unchanged. - `mssql-python` now depends on `mssql-python-rs==0.1.0` for `mssql_py_core` instead of embedding files owned by that separately published distribution. - **GH-769 deprecation policy:** The misplaced `GetInfoConstants` members diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 35a3b6da4..3a7ed8a67 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -3022,9 +3022,51 @@ SQLSMALLINT SQLNumResultCols_wrap(SqlHandlePtr statementHandle) { return columnCount; } -// Wrap SQLDescribeCol -SQLRETURN SQLDescribeCol_wrap(SqlHandlePtr StatementHandle, py::list& ColumnMetadata) { - PERF_TIMER("SQLDescribeCol_wrap"); +namespace { + +struct FetchColumnMetadata { + py::object name; + SQLSMALLINT dataType; + SQLULEN columnSize; + SQLSMALLINT decimalDigits; + SQLSMALLINT nullable; +}; + +py::dict GetFetchColumnMetadata(const py::list& columns, size_t index) { + return columns[index].cast(); +} + +const FetchColumnMetadata& GetFetchColumnMetadata( + const std::vector& columns, size_t index) { + return columns.at(index); +} + +SQLSMALLINT GetFetchColumnType(const py::dict& column) { + return column["DataType"].cast(); +} + +SQLSMALLINT GetFetchColumnType(const FetchColumnMetadata& column) { + return column.dataType; +} + +SQLULEN GetFetchColumnSize(const py::dict& column) { + return column["ColumnSize"].cast(); +} + +SQLULEN GetFetchColumnSize(const FetchColumnMetadata& column) { + return column.columnSize; +} + +std::string GetFetchColumnName(const py::dict& column) { + return column["ColumnName"].cast(); +} + +std::string GetFetchColumnName(const FetchColumnMetadata& column) { + return column.name.cast(); +} + +template +SQLRETURN DescribeColumns(SqlHandlePtr StatementHandle, AppendColumn&& appendColumn) { LOG("SQLDescribeCol: Getting column descriptions for statement_handle=%p", (void*)StatementHandle->get()); if (!SQLDescribeCol_ptr) { @@ -3052,14 +3094,12 @@ SQLRETURN SQLDescribeCol_wrap(SqlHandlePtr StatementHandle, py::list& ColumnMeta &ColumnSize, &DecimalDigits, &Nullable); if (SQL_SUCCEEDED(retcode)) { - // Append a named py::dict to ColumnMetadata - // TODO: Should we define a struct for this task instead of dict? - ColumnMetadata.append( - py::dict("ColumnName"_a = dupeSqlWCharAsUtf16Le( - ColumnName, std::min(static_cast(NameLength), - (sizeof(ColumnName) / sizeof(SQLWCHAR)) - 1)), - "DataType"_a = DataType, "ColumnSize"_a = ColumnSize, - "DecimalDigits"_a = DecimalDigits, "Nullable"_a = Nullable)); + // Own the name and preserve eager UTF-16 conversion, including codec errors. + auto name = py::cast(dupeSqlWCharAsUtf16Le( + ColumnName, std::min(static_cast(NameLength), + (sizeof(ColumnName) / sizeof(SQLWCHAR)) - 1))); + appendColumn(FetchColumnMetadata{ + std::move(name), DataType, ColumnSize, DecimalDigits, Nullable}); } else { return retcode; } @@ -3067,6 +3107,19 @@ SQLRETURN SQLDescribeCol_wrap(SqlHandlePtr StatementHandle, py::list& ColumnMeta return SQL_SUCCESS; } +} // namespace + +// Wrap SQLDescribeCol +SQLRETURN SQLDescribeCol_wrap(SqlHandlePtr StatementHandle, py::list& ColumnMetadata) { + PERF_TIMER("SQLDescribeCol_wrap"); + return DescribeColumns(StatementHandle, [&](FetchColumnMetadata column) { + ColumnMetadata.append( + py::dict("ColumnName"_a = column.name, "DataType"_a = column.dataType, + "ColumnSize"_a = column.columnSize, "DecimalDigits"_a = column.decimalDigits, + "Nullable"_a = column.nullable)); + }); +} + SQLRETURN SQLSpecialColumns_wrap(SqlHandlePtr StatementHandle, SQLSMALLINT identifierType, const py::object& catalogObj, const py::object& schemaObj, const std::u16string& table, SQLSMALLINT scope, @@ -3999,16 +4052,17 @@ SQLRETURN SQLFetchScroll_wrap(SqlHandlePtr StatementHandle, SQLSMALLINT FetchOri // For column in the result set, binds a buffer to retrieve column data // TODO: Move to anonymous namespace, since it is not used outside this file -SQLRETURN SQLBindColums(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& columnNames, +template +SQLRETURN SQLBindColums(SQLHSTMT hStmt, ColumnBuffers& buffers, const Metadata& columnNames, SQLUSMALLINT numCols, int fetchSize, int charCtype = SQL_C_WCHAR) { PERF_TIMER("SQLBindColums"); SQLRETURN ret = SQL_SUCCESS; const bool useWideChar = (charCtype == SQL_C_WCHAR); // Bind columns based on their data types for (SQLUSMALLINT col = 1; col <= numCols; col++) { - auto columnMeta = columnNames[col - 1].cast(); - SQLSMALLINT dataType = columnMeta["DataType"].cast(); - SQLULEN columnSize = columnMeta["ColumnSize"].cast(); + const auto& columnMeta = GetFetchColumnMetadata(columnNames, col - 1); + SQLSMALLINT dataType = GetFetchColumnType(columnMeta); + SQLULEN columnSize = GetFetchColumnSize(columnMeta); switch (dataType) { case SQL_CHAR: @@ -4140,7 +4194,7 @@ SQLRETURN SQLBindColums(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& column buffers.indicators[col - 1].data()); break; default: - std::string columnName = columnMeta["ColumnName"].cast(); + std::string columnName = GetFetchColumnName(columnMeta); std::ostringstream errorString; errorString << "Unsupported data type for column - " << columnName.c_str() << ", Type - " << dataType << ", column ID - " << col; @@ -4149,7 +4203,7 @@ SQLRETURN SQLBindColums(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& column break; } if (!SQL_SUCCEEDED(ret)) { - std::string columnName = columnMeta["ColumnName"].cast(); + std::string columnName = GetFetchColumnName(columnMeta); std::ostringstream errorString; errorString << "Failed to bind column - " << columnName.c_str() << ", Type - " << dataType << ", column ID - " << col; @@ -4163,7 +4217,8 @@ SQLRETURN SQLBindColums(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& column // Fetch rows in batches // TODO: Move to anonymous namespace, since it is not used outside this file -SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& columnNames, +template +SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, const Metadata& columnNames, py::list& rows, SQLUSMALLINT numCols, SQLULEN& numRowsFetched, const std::vector& lobColumns, const std::string& charEncoding = "utf-16le", @@ -4213,9 +4268,9 @@ SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& colum { PERF_TIMER("FetchBatchData::cache_column_metadata"); for (SQLUSMALLINT col = 0; col < numCols; col++) { - const auto& columnMeta = columnNames[col].cast(); - columnInfos[col].dataType = columnMeta["DataType"].cast(); - columnInfos[col].columnSize = columnMeta["ColumnSize"].cast(); + const auto& columnMeta = GetFetchColumnMetadata(columnNames, col); + columnInfos[col].dataType = GetFetchColumnType(columnMeta); + columnInfos[col].columnSize = GetFetchColumnSize(columnMeta); columnInfos[col].isLob = std::find(lobColumns.begin(), lobColumns.end(), col + 1) != lobColumns.end(); columnInfos[col].processedColumnSize = columnInfos[col].columnSize; @@ -4504,8 +4559,8 @@ SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& colum break; } default: { - const auto& columnMeta = columnNames[col - 1].cast(); - std::string columnName = columnMeta["ColumnName"].cast(); + const auto& columnMeta = GetFetchColumnMetadata(columnNames, col - 1); + std::string columnName = GetFetchColumnName(columnMeta); std::ostringstream errorString; errorString << "Unsupported data type for column - " << columnName.c_str() << ", Type - " << dataType << ", column ID - " << col; @@ -4650,18 +4705,24 @@ SQLRETURN FetchMany_wrap(SqlHandlePtr StatementHandle, py::list& rows, int fetch SQLSMALLINT numCols = SQLNumResultCols_wrap(StatementHandle); // Retrieve column metadata - py::list columnNames; - ret = SQLDescribeCol_wrap(StatementHandle, columnNames); + std::vector columnNames; + ret = DescribeColumns(StatementHandle, [&](FetchColumnMetadata column) { + columnNames.push_back(std::move(column)); + }); if (!SQL_SUCCEEDED(ret)) { LOG("FetchMany_wrap: Failed to get column descriptions - SQLRETURN=%d", ret); return ret; } + if (numCols < 0 || columnNames.size() != static_cast(numCols)) { + LOG("FetchMany_wrap: Column metadata count does not match result column count"); + ThrowStdException("Column metadata count does not match result column count"); + } std::vector lobColumns; for (SQLSMALLINT i = 0; i < numCols; i++) { - auto colMeta = columnNames[i].cast(); - SQLSMALLINT dataType = colMeta["DataType"].cast(); - SQLULEN columnSize = colMeta["ColumnSize"].cast(); + const auto& colMeta = GetFetchColumnMetadata(columnNames, i); + SQLSMALLINT dataType = GetFetchColumnType(colMeta); + SQLULEN columnSize = GetFetchColumnSize(colMeta); if (IsLobOrVariantColumn(dataType, columnSize)) { lobColumns.push_back(i + 1); // 1-based diff --git a/tests/test_040_fetch_native_metadata.py b/tests/test_040_fetch_native_metadata.py new file mode 100644 index 000000000..eb46976e4 --- /dev/null +++ b/tests/test_040_fetch_native_metadata.py @@ -0,0 +1,387 @@ +"""Call-local fetch metadata must preserve public descriptions and fetch state.""" + +import datetime as dt +import os +from pathlib import Path +import subprocess +import sys +import textwrap +from decimal import Decimal +from uuid import UUID + +import pytest + +import mssql_python +from mssql_python import ddbc_bindings + + +@pytest.fixture +def metadata_cursor(conn_str): + with mssql_python.connect(conn_str) as connection: + with connection.cursor() as cursor: + yield cursor + + +def _query(columns, count=20): + values = ",".join(f"({i})" for i in range(1, 21)) + return ( + f"SELECT {','.join(columns)} FROM (VALUES {values}) AS source(id) " + f"WHERE id<={count} ORDER BY id" + ) + + +def _assert_rows(rows, expected): + assert [tuple(row) for row in rows] == expected + assert [[type(value) for value in row] for row in rows] == [ + [type(value) for value in row] for row in expected + ] + + +def _describe(cursor): + result = [] + assert ddbc_bindings.DDBCSQLDescribeCol(cursor.hstmt, result) == 0 + for column in result: + assert set(column) == {"ColumnName", "DataType", "ColumnSize", "DecimalDigits", "Nullable"} + assert type(column["ColumnName"]) is str + for key in ("DataType", "ColumnSize", "DecimalDigits", "Nullable"): + assert type(column[key]) is int + return result + + +@pytest.mark.parametrize("width", [3, 24]) +@pytest.mark.parametrize("size", [None, 1, 10, 1000, "varied"]) +@pytest.mark.parametrize("count", [0, 20]) +def test_fetchmany_shape_sizes_and_eof(metadata_cursor, width, size, count): + cursor = metadata_cursor + expressions = ["id", "CONVERT(NVARCHAR(30),N'row')", "CONVERT(FLOAT,id)*0.25"] + columns = [f"{expressions[i % 3]} AS c{i}" for i in range(width)] + cursor.execute(_query(columns, count)) + assert cursor.arraysize == 1 + description = cursor.description + assert all(len(column) == 7 for column in description) + assert [column[0] for column in description] == [f"c{i}" for i in range(width)] + metadata = _describe(cursor) + output = [] + iteration = 0 + while True: + fetch_size = (1, 10, 3, 1000)[iteration % 4] if size == "varied" else size + batch = cursor.fetchmany() if fetch_size is None else cursor.fetchmany(fetch_size) + assert cursor.description == description + if not batch: + break + output.extend(batch) + iteration += 1 + _assert_rows(output, [(i, "row", i * 0.25) * (width // 3) for i in range(1, count + 1)]) + assert _describe(cursor) == metadata + assert cursor.fetchmany(1) == [] + assert cursor.fetchone() is None + assert cursor.fetchall() == [] + + +@pytest.mark.parametrize("method", ["fetchmany", "fetchall", "arrow_batch"]) +def test_public_metadata_names_and_fields(metadata_cursor, method): + cursor = metadata_cursor + names = [ + "duplicate", + "duplicate", + "\u03a9\u540d", + "emoji_\U0001f600", + "bracket]name", + "x" * 128, + ] + columns = [f"CONVERT(INT,id) AS [{name.replace(']', ']]')}]" for name in names] + cursor.execute(_query(columns, 1)) + metadata = _describe(cursor) + assert metadata == [ + {"ColumnName": name, "DataType": 4, "ColumnSize": 10, "DecimalDigits": 0, "Nullable": 1} + for name in names + ] + assert [column[0] for column in cursor.description] == names + if method == "arrow_batch": + pytest.importorskip("pyarrow") + batch = cursor.arrow_batch(1) + assert batch.schema.names == names + assert [column.to_pylist() for column in batch.columns] == [[1]] * len(names) + else: + rows = cursor.fetchmany(1) if method == "fetchmany" else cursor.fetchall() + _assert_rows(rows, [(1,) * len(names)]) + assert _describe(cursor) == metadata + + +_TYPES = [ + ("INT", "7", 7), + ("SMALLINT", "-7", -7), + ("BIGINT", "2147483649", 2147483649), + ("TINYINT", "255", 255), + ("BIT", "1", True), + ("REAL", "1.5", 1.5), + ("FLOAT", "2.25", 2.25), + ("DECIMAL(20,4)", "123.4500", Decimal("123.4500")), + ("NUMERIC(28,8)", "-0.125", Decimal("-0.125")), + ("MONEY", "4.25", Decimal("4.25")), + ("DATE", "'2001-02-03'", dt.date(2001, 2, 3)), + ("TIME(7)", "'12:34:56.1234567'", dt.time(12, 34, 56, 123456)), + ("DATETIME2(7)", "'2001-02-03T12:34:56.1234567'", dt.datetime(2001, 2, 3, 12, 34, 56, 123456)), + ("DATETIME", "'2001-02-03T12:34:56'", dt.datetime(2001, 2, 3, 12, 34, 56)), + ( + "DATETIMEOFFSET(7)", + "'2001-02-03T12:34:56.1234567+05:30'", + dt.datetime(2001, 2, 3, 12, 34, 56, 123456, dt.timezone(dt.timedelta(minutes=330))), + ), + ( + "UNIQUEIDENTIFIER", + "'12345678-1234-5678-1234-567812345678'", + UUID("12345678-1234-5678-1234-567812345678"), + ), + ("VARCHAR(20)", "'ascii'", "ascii"), + ("CHAR(8)", "'ascii'", "ascii "), + ("NVARCHAR(30)", "N'\u03a9\U0001f600'", "\u03a9\U0001f600"), + ("NCHAR(5)", "N'\u03a9'", "\u03a9 "), + ("VARBINARY(10)", "0x00010200", b"\x00\x01\x02\x00"), + ("BINARY(4)", "0x00010203", b"\x00\x01\x02\x03"), + ("VARCHAR(1)", "''", ""), + ("NVARCHAR(1)", "N''", ""), +] + + +@pytest.mark.parametrize("size", [1, 10, 1000]) +def test_fetchmany_typed_nulls_and_values(metadata_cursor, size): + columns = [ + f"CASE WHEN id%3=0 THEN CAST(NULL AS {sqltype}) " + f"ELSE CAST({literal} AS {sqltype}) END AS c{i}" + for i, (sqltype, literal, _) in enumerate(_TYPES) + ] + cursor = metadata_cursor + cursor.execute(_query(columns)) + description = cursor.description + metadata = _describe(cursor) + assert len(metadata) == 24 + output = [] + while batch := cursor.fetchmany(size): + output.extend(batch) + values = tuple(value for _, _, value in _TYPES) + _assert_rows(output, [(None,) * 24 if i % 3 == 0 else values for i in range(1, 21)]) + assert cursor.description == description + assert _describe(cursor) == metadata + + +def test_reexecute_and_nextset_change_shape(metadata_cursor): + cursor = metadata_cursor + for _ in range(3): + cursor.execute("SELECT 1 AS first_name; SELECT N'new' AS second_name, 2 AS extra") + _assert_rows(cursor.fetchmany(1), [(1,)]) + assert cursor.nextset() + assert [col[0] for col in cursor.description] == ["second_name", "extra"] + _assert_rows(cursor.fetchmany(10), [("new", 2)]) + assert not cursor.nextset() + cursor.execute("SELECT CAST(3.5 AS DECIMAL(6,2)) AS replacement") + assert _describe(cursor)[0]["ColumnName"] == "replacement" + _assert_rows(cursor.fetchmany(), [(Decimal("3.50"),)]) + + +def test_converter_changes_on_execute_and_live_decoding(metadata_cursor): + cursor = metadata_cursor + connection = cursor.connection + cursor.execute(_query(["id", "CAST('ascii' AS VARCHAR(12)) AS txt"], 4)) + _assert_rows(cursor.fetchmany(1), [(1, "ascii")]) + calls = [] + + def convert(value): + calls.append(value) + return value + 100 + + connection.add_output_converter(mssql_python.SQL_INTEGER, convert) + connection.setdecoding(mssql_python.SQL_CHAR, "utf-8", mssql_python.SQL_CHAR) + cursor.execute(_query(["id", "CAST('ascii' AS VARCHAR(12)) AS txt"], 4)) + _assert_rows(cursor.fetchmany(1), [(101, "ascii")]) + assert calls == [1] + connection.setdecoding(mssql_python.SQL_CHAR, "latin1", mssql_python.SQL_CHAR) + _assert_rows(cursor.fetchmany(1), [(102, "ascii")]) + assert calls == [1, 2] + connection.remove_output_converter(mssql_python.SQL_INTEGER) + cursor.execute(_query(["id", "CAST('ascii' AS VARCHAR(12)) AS txt"], 1)) + _assert_rows(cursor.fetchmany(1), [(1, "ascii")]) + connection.setdecoding(mssql_python.SQL_CHAR) + cursor.execute(_query(["id", "CAST('ascii' AS VARCHAR(12)) AS txt"], 1)) + _assert_rows(cursor.fetchmany(10), [(1, "ascii")]) + + +@pytest.mark.parametrize("size", [1, 10]) +def test_fetchmany_lob_and_xml_typed_nulls(metadata_cursor, size): + cursor = metadata_cursor + columns = [ + "CASE WHEN id%2=0 THEN CAST(NULL AS NVARCHAR(MAX)) ELSE " + "REPLICATE(CAST(N'x' AS NVARCHAR(MAX)),9001) END AS txt", + "CASE WHEN id%2=0 THEN CAST(NULL AS VARBINARY(MAX)) ELSE " + "CAST(REPLICATE(CAST('a' AS VARCHAR(MAX)),10003) AS VARBINARY(MAX)) END AS bin", + "CASE WHEN id%2=0 THEN CAST(NULL AS XML) ELSE CAST('value' AS XML) END AS xml", + ] + cursor.execute(_query(columns, 4)) + metadata = _describe(cursor) + rows = [] + while batch := cursor.fetchmany(size): + rows.extend(batch) + values = ("x" * 9001, b"a" * 10003, "value") + _assert_rows(rows, [values, (None, None, None), values, (None, None, None)]) + assert _describe(cursor) == metadata + + +def _isolated(script, tmp_path): + environment = dict(os.environ) + root = str(Path(mssql_python.__file__).resolve().parent.parent) + environment["PYTHONPATH"] = os.pathsep.join([root, environment.get("PYTHONPATH", "")]) + result = subprocess.run( + [sys.executable, "-c", textwrap.dedent(script)], + cwd=tmp_path, + env=environment, + capture_output=True, + text=True, + timeout=45, + ) + assert result.returncode == 0, result.stdout + result.stderr + + +def test_interleaving_movement_and_variant_freshness(tmp_path): + _isolated( + """ + import os + import gc + import mssql_python as db + with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: + with connection.cursor() as cursor: + query = "SELECT id FROM (VALUES(1),(2),(3),(4),(5),(6),(7)) s(id) ORDER BY id" + for _ in range(4): + cursor.execute(query) + assert cursor.fetchmany(1)[0][0] == 1 + gc.collect() + assert cursor.fetchone()[0] == 2 + cursor.scroll(1) + assert cursor.fetchmany(1)[0][0] == 4 + cursor.skip(1) + assert [tuple(row) for row in cursor.fetchall()] == [(6,), (7,)] + cursor.execute("CREATE TABLE #metadata_variant(id INT, v SQL_VARIANT)") + cursor.execute( + "INSERT INTO #metadata_variant VALUES " + "(1,CAST('abc' AS VARCHAR(3)))," + "(2,CAST('abcdefgh' AS VARCHAR(8)))," + "(3,CAST(REPLICATE('x',30) AS VARCHAR(30)))" + ) + for method in ("fetchmany", "fetchall"): + cursor.execute("SELECT v FROM #metadata_variant ORDER BY id") + assert cursor.fetchone()[0] == "abc" + if method == "fetchmany": + assert cursor.fetchmany(1)[0][0] == "abcdefgh" + assert cursor.fetchmany(1)[0][0] == "x"*30 + else: + assert [r[0] for r in cursor.fetchall()] == ["abcdefgh", "x"*30] + """, + tmp_path, + ) + + +def test_closure_and_error_recovery(tmp_path): + _isolated( + """ + import os + import mssql_python as db + from mssql_python import Cursor, InterfaceError, ProgrammingError, DatabaseError + with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: + with connection.cursor() as cursor: + cursor.execute("SELECT 1 AS c") + assert cursor.fetchmany(0) == [] + assert cursor.fetchmany(-1) == [] + assert cursor.fetchmany(1)[0][0] == 1 + try: + cursor.execute("SELECT invalid_column FROM (VALUES(1)) t(c)") + except DatabaseError: + pass + else: + raise AssertionError("invalid query did not raise") + cursor.execute("SELECT 2 AS changed") + assert cursor.fetchmany(1)[0][0] == 2 + try: + cursor.fetchmany(1) + except ProgrammingError: + pass + else: + raise AssertionError("closed cursor did not raise") + connection = db.connect(os.environ["DB_CONNECTION_STRING"]) + cursor = Cursor(connection) + cursor.execute("SELECT 1") + connection.close() + try: + cursor.fetchmany(1) + except (InterfaceError, ProgrammingError): + pass + else: + raise AssertionError("closed connection did not raise") + cursor.close() + """, + tmp_path, + ) + + +def test_malformed_column_name_fails_before_fetch(tmp_path): + _isolated( + """ + import os + import mssql_python as db + from mssql_python import ddbc_bindings as native + with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: + with connection.cursor() as cursor: + cursor.execute( + "DECLARE @s NVARCHAR(200) = N'SELECT 1 AS [' + " + "CAST(0x00D8 AS NVARCHAR(1)) + N']'; EXEC(@s)" + ) + for operation in ( + lambda: native.DDBCSQLDescribeCol(cursor.hstmt, []), + lambda: cursor.fetchmany(1), + ): + try: + operation() + except UnicodeDecodeError: + pass + else: + raise AssertionError("malformed UTF-16 column name did not raise") + assert native.DDBCSQLFetch(cursor.hstmt) == 0 + assert native.DDBCSQLFetch(cursor.hstmt) == 100 + """, + tmp_path, + ) + + +@pytest.mark.skipif( + not hasattr(ddbc_bindings, "profiling"), reason="requires native profiling instrumentation" +) +def test_fetchmany_avoids_python_description_roundtrip(tmp_path): + _isolated( + """ + import os + import mssql_python as db + from mssql_python import ddbc_bindings as native + with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: + with connection.cursor() as cursor: + cursor.execute("SELECT id FROM (VALUES(1),(2)) s(id) ORDER BY id") + native.profiling.reset() + native.profiling.enable() + try: + metadata = [] + native.DDBCSQLDescribeCol(cursor.hstmt, metadata) + finally: + native.profiling.disable() + assert native.profiling.get_stats()["ddbc::SQLDescribeCol_wrap"]["calls"] == 1 + assert len(metadata) == 1 + native.profiling.reset() + native.profiling.enable() + try: + assert cursor.fetchmany(1)[0][0] == 1 + assert cursor.fetchmany(1)[0][0] == 2 + assert cursor.fetchmany(1) == [] + finally: + native.profiling.disable() + stats = native.profiling.get_stats() + assert stats["ddbc::FetchMany_wrap"]["calls"] == 3 + assert stats.get("ddbc::SQLDescribeCol_wrap", {}).get("calls", 0) == 0 + """, + tmp_path, + ) From 252e9b6905526da323cd9690646de41cbdf02c23 Mon Sep 17 00:00:00 2001 From: Jahnvi Thakkar Date: Tue, 22 Sep 2026 14:52:42 +0530 Subject: [PATCH 2/7] PERF: Reuse stable native metadata within result sets Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 10 +- mssql_python/pybind/connection/connection.cpp | 24 + mssql_python/pybind/connection/connection.h | 1 + mssql_python/pybind/ddbc_bindings.cpp | 220 +++++++-- mssql_python/pybind/ddbc_bindings.h | 2 + mssql_python/pybind/result_metadata.hpp | 76 +++ tests/test_040_fetch_native_metadata.py | 439 +++++++++++++++++- 7 files changed, 722 insertions(+), 50 deletions(-) create mode 100644 mssql_python/pybind/result_metadata.hpp diff --git a/CHANGELOG.md b/CHANGELOG.md index e4ff8eecb..2cac01d4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,10 +57,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), does not change the default provider or ship any Rust driver binaries. ### Changed -- `fetchmany()` keeps freshly described column types and sizes in call-local native - metadata instead of round-tripping them through Python dictionaries. Column names - retain eager Unicode conversion; public column descriptions and fetch behavior - are unchanged. +- Fetches reuse owned native metadata for stable columns within a result set; + `fetchmany()` avoids the Python metadata-dictionary roundtrip. Re-execution, + result transitions and statement/connection cleanup invalidate this metadata. + Public descriptions stay fresh, name-validation timing is preserved, and + `sql_variant` columns retain per-row descriptions and per-value probes. + Fetch buffers, decoding settings and converted values are not cached. - DATE, TIME, and TIMESTAMP fetch conversion uses checked CPython constructors for the standard datetime types, while preserving cached substitute constructors, their positional arguments and exceptions, and fractional-second truncation. diff --git a/mssql_python/pybind/connection/connection.cpp b/mssql_python/pybind/connection/connection.cpp index 6960942fa..9c4304d2f 100644 --- a/mssql_python/pybind/connection/connection.cpp +++ b/mssql_python/pybind/connection/connection.cpp @@ -115,6 +115,7 @@ void Connection::connect(const py::dict& attrs_before) { void Connection::disconnect(bool rollbackBeforeDisconnect) { PERF_TIMER("Connection::disconnect"); + clearResultMetadata(); // Determine GIL state once, up front. disconnect() runs both from // pybind11-bound methods (GIL held) and from GIL-less destructor / shutdown // paths: Connection::~Connection() dropping the last shared_ptr, or teardown @@ -265,6 +266,24 @@ void Connection::checkError(SQLRETURN ret) const { } } +void Connection::clearResultMetadata() { + std::vector handles; + { + std::lock_guard lock(_childHandlesMutex); + handles.reserve(_childStatementHandles.size()); + for (const auto& weakHandle : _childStatementHandles) { + if (auto handle = weakHandle.lock()) { + handles.push_back(std::move(handle)); + } + } + } + // Releasing the last handle can acquire the connection cleanup gate. + // Keep that destruction outside the child-list lock. + for (const auto& handle : handles) { + handle->resultMetadata.clear(); + } +} + void Connection::commit() { PERF_TIMER("Connection::commit"); if (!_dbcHandle) { @@ -272,6 +291,7 @@ void Connection::commit() { } updateLastUsed(); LOG("Committing transaction"); + clearResultMetadata(); SQLRETURN ret; { // Release the GIL during the blocking SQLEndTran network round-trip. @@ -288,6 +308,7 @@ void Connection::rollback() { } updateLastUsed(); LOG("Rolling back transaction"); + clearResultMetadata(); SQLRETURN ret; { // Release the GIL during the blocking SQLEndTran network round-trip. @@ -302,6 +323,7 @@ void Connection::setAutocommit(bool enable) { if (!_dbcHandle) { ThrowStdException("Connection handle not allocated"); } + clearResultMetadata(); SQLINTEGER value = enable ? SQL_AUTOCOMMIT_ON : SQL_AUTOCOMMIT_OFF; LOG("Setting autocommit=%d", enable); SQLRETURN ret; @@ -395,6 +417,7 @@ SqlHandlePtr Connection::allocStatementHandle() { } SQLRETURN Connection::setAttribute(SQLINTEGER attribute, py::object value) { + clearResultMetadata(); LOG("Setting SQL attribute=%d", attribute); // SQLPOINTER ptr = nullptr; // SQLINTEGER length = 0; @@ -581,6 +604,7 @@ bool Connection::reset() { if (!_dbcHandle) { ThrowStdException("Connection handle not allocated"); } + clearResultMetadata(); LOG("Resetting connection via SQL_ATTR_RESET_CONNECTION"); // NOTE: SQL_ATTR_RESET_CONNECTION is a pool-checkin reset: it asks the // driver to wipe per-session state (temp tables, open cursors, SET diff --git a/mssql_python/pybind/connection/connection.h b/mssql_python/pybind/connection/connection.h index 0c66aed1e..f43613842 100644 --- a/mssql_python/pybind/connection/connection.h +++ b/mssql_python/pybind/connection/connection.h @@ -103,6 +103,7 @@ class Connection { void allocateDbcHandle(); void checkError(SQLRETURN ret) const; void applyAttrsBefore(const py::dict& attrs_before); + void clearResultMetadata(); std::u16string _connStr; bool _fromPool = false; diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 7fcbcf1f0..13dfc9719 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -1581,6 +1581,7 @@ void SqlHandle::markImplicitlyFreed() { _type); return; // Refuse to mark - let normal free() handle it } + resultMetadata.clear(); _implicitly_freed = true; } @@ -1597,6 +1598,7 @@ void SqlHandle::free() { SQLRETURN SqlHandle::freeHandle() { PERF_TIMER("SqlHandle::free"); + resultMetadata.clear(); bool pythonShuttingDown = is_python_finalizing(); bool skipDuringShutdown = _type == SQL_HANDLE_STMT || _type == SQL_HANDLE_DBC; #ifdef _WIN32 @@ -1637,6 +1639,7 @@ SQLRETURN SqlHandle::freeHandle() { } void SqlHandle::close_cursor() { + resultMetadata.clear(); if (is_python_finalizing()) { return; } @@ -1664,6 +1667,7 @@ void SqlHandle::close_cursor() { } void SqlHandle::cancel() { + resultMetadata.clear(); if (is_python_finalizing()) { return; } @@ -1698,6 +1702,7 @@ SQLRETURN SQLResetStmt_wrap(SqlHandlePtr statementHandle) { if (statementHandle->isImplicitlyFreed()) { return SQL_INVALID_HANDLE; } + statementHandle->resultMetadata.clear(); if (!SQLFreeStmt_ptr) { DriverLoader::getInstance().loadDriver(); } @@ -1719,6 +1724,7 @@ SQLRETURN SQLResetStmt_wrap(SqlHandlePtr statementHandle) { SQLRETURN SQLGetTypeInfo_Wrapper(SqlHandlePtr StatementHandle, SQLSMALLINT DataType) { PERF_TIMER("SQLGetTypeInfo_Wrapper"); + StatementHandle->resultMetadata.clear(); if (!SQLGetTypeInfo_ptr) { ThrowStdException("SQLGetTypeInfo function not loaded"); } @@ -1731,6 +1737,7 @@ SQLRETURN SQLGetTypeInfo_Wrapper(SqlHandlePtr StatementHandle, SQLSMALLINT DataT SQLRETURN SQLProcedures_wrap(SqlHandlePtr StatementHandle, const py::object& catalogObj, const py::object& schemaObj, const py::object& procedureObj) { PERF_TIMER("SQLProcedures_wrap"); + StatementHandle->resultMetadata.clear(); if (!SQLProcedures_ptr) { ThrowStdException("SQLProcedures function not loaded"); } @@ -1755,6 +1762,7 @@ SQLRETURN SQLForeignKeys_wrap(SqlHandlePtr StatementHandle, const py::object& pk const py::object& fkCatalogObj, const py::object& fkSchemaObj, const py::object& fkTableObj) { PERF_TIMER("SQLForeignKeys_wrap"); + StatementHandle->resultMetadata.clear(); if (!SQLForeignKeys_ptr) { ThrowStdException("SQLForeignKeys function not loaded"); } @@ -1787,6 +1795,7 @@ SQLRETURN SQLForeignKeys_wrap(SqlHandlePtr StatementHandle, const py::object& pk SQLRETURN SQLPrimaryKeys_wrap(SqlHandlePtr StatementHandle, const py::object& catalogObj, const py::object& schemaObj, const std::u16string& table) { PERF_TIMER("SQLPrimaryKeys_wrap"); + StatementHandle->resultMetadata.clear(); if (!SQLPrimaryKeys_ptr) { ThrowStdException("SQLPrimaryKeys function not loaded"); } @@ -1809,6 +1818,7 @@ SQLRETURN SQLStatistics_wrap(SqlHandlePtr StatementHandle, const py::object& cat const py::object& schemaObj, const std::u16string& table, SQLUSMALLINT unique, SQLUSMALLINT reserved) { PERF_TIMER("SQLStatistics_wrap"); + StatementHandle->resultMetadata.clear(); if (!SQLStatistics_ptr) { ThrowStdException("SQLStatistics function not loaded"); } @@ -1831,6 +1841,7 @@ SQLRETURN SQLColumns_wrap(SqlHandlePtr StatementHandle, const py::object& catalo const py::object& schemaObj, const py::object& tableObj, const py::object& columnObj) { PERF_TIMER("SQLColumns_wrap"); + StatementHandle->resultMetadata.clear(); if (!SQLColumns_ptr) { ThrowStdException("SQLColumns function not loaded"); } @@ -1946,6 +1957,7 @@ py::list SQLGetAllDiagRecords(SqlHandlePtr handle) { // Wrap SQLExecDirect SQLRETURN SQLExecDirect_wrap(SqlHandlePtr StatementHandle, const std::u16string& Query) { PERF_TIMER("SQLExecDirect_wrap"); + StatementHandle->resultMetadata.clear(); LOG("SQLExecDirect: Executing query directly - statement_handle=%p, " "query_length=%zu chars", (void*)StatementHandle->get(), Query.length()); @@ -1982,6 +1994,7 @@ SQLRETURN SQLTables_wrap(SqlHandlePtr StatementHandle, const std::u16string& cat const std::u16string& schema, const std::u16string& table, const std::u16string& tableType) { PERF_TIMER("SQLTables_wrap"); + StatementHandle->resultMetadata.clear(); if (!SQLTables_ptr) { LOG("SQLTables: Function pointer not initialized, loading driver"); DriverLoader::getInstance().loadDriver(); @@ -2028,6 +2041,7 @@ SQLRETURN SQLExecute_wrap(const SqlHandlePtr statementHandle, return SQL_INVALID_HANDLE; } + statementHandle->resultMetadata.clear(); SQLHANDLE hStmt = statementHandle->get(); // Configure forward-only / read-only cursor (matches slow path semantics). @@ -2828,6 +2842,7 @@ SQLRETURN SQLExecuteMany_wrap(const SqlHandlePtr statementHandle, const std::u16 std::vector& paramInfos, size_t paramSetSize, const py::dict& encodingSettings) { PERF_TIMER("SQLExecuteMany_wrap"); + statementHandle->resultMetadata.clear(); LOG("SQLExecuteMany: Starting batch execution - param_count=%zu, " "param_set_size=%zu", columnwise_params.size(), paramSetSize); @@ -3009,14 +3024,6 @@ SQLSMALLINT SQLNumResultCols_wrap(SqlHandlePtr statementHandle) { namespace { -struct FetchColumnMetadata { - py::object name; - SQLSMALLINT dataType; - SQLULEN columnSize; - SQLSMALLINT decimalDigits; - SQLSMALLINT nullable; -}; - py::dict GetFetchColumnMetadata(const py::list& columns, size_t index) { return columns[index].cast(); } @@ -3047,7 +3054,7 @@ std::string GetFetchColumnName(const py::dict& column) { } std::string GetFetchColumnName(const FetchColumnMetadata& column) { - return column.name.cast(); + return py::cast(column.name).cast(); } template @@ -3074,17 +3081,18 @@ SQLRETURN DescribeColumns(SqlHandlePtr StatementHandle, AppendColumn&& appendCol SQLSMALLINT DecimalDigits; SQLSMALLINT Nullable; - retcode = SQLDescribeCol_ptr(StatementHandle->get(), i, ColumnName, - sizeof(ColumnName) / sizeof(SQLWCHAR), &NameLength, &DataType, - &ColumnSize, &DecimalDigits, &Nullable); + { + PERF_TIMER("SQLDescribeCol::driver_call"); + retcode = SQLDescribeCol_ptr(StatementHandle->get(), i, ColumnName, + sizeof(ColumnName) / sizeof(SQLWCHAR), &NameLength, + &DataType, &ColumnSize, &DecimalDigits, &Nullable); + } if (SQL_SUCCEEDED(retcode)) { - // Own the name and preserve eager UTF-16 conversion, including codec errors. - auto name = py::cast(dupeSqlWCharAsUtf16Le( + auto name = dupeSqlWCharAsUtf16Le( ColumnName, std::min(static_cast(NameLength), - (sizeof(ColumnName) / sizeof(SQLWCHAR)) - 1))); - appendColumn(FetchColumnMetadata{ - std::move(name), DataType, ColumnSize, DecimalDigits, Nullable}); + (sizeof(ColumnName) / sizeof(SQLWCHAR)) - 1)); + appendColumn(std::move(name), DataType, ColumnSize, DecimalDigits, Nullable); } else { return retcode; } @@ -3092,17 +3100,57 @@ SQLRETURN DescribeColumns(SqlHandlePtr StatementHandle, AppendColumn&& appendCol return SQL_SUCCESS; } +SQLRETURN GetResultMetadata(const SqlHandlePtr& statement, SQLSMALLINT columnCount, + std::shared_ptr& metadata) { + const auto snapshot = statement->resultMetadata.snapshot(); + const bool matches = snapshot.metadata && columnCount >= 0 && + snapshot.metadata->columns.size() == static_cast(columnCount); + if (matches && snapshot.metadata->namesValidated) { + metadata = snapshot.metadata; + return SQL_SUCCESS; + } + auto pending = matches ? std::make_shared(*snapshot.metadata) + : std::make_shared(); + if (!matches) { + SQLRETURN ret = DescribeColumns( + statement, [&](std::u16string name, SQLSMALLINT type, SQLULEN size, + SQLSMALLINT digits, SQLSMALLINT nullable) { + // Preserve eager name validation before advancing the result set. + py::cast(name); + pending->columns.push_back( + {std::move(name), type, type == SQL_SS_VARIANT ? 0 : size, digits, nullable}); + }); + if (!SQL_SUCCEEDED(ret)) { + return ret; + } + } else { + // Row-wise fetches originally read names without decoding them. A later + // many/all fetch must still validate those names before its first advance. + for (const auto& column : pending->columns) { + py::cast(column.name); + } + } + pending->namesValidated = true; + statement->resultMetadata.publish(snapshot.generation, pending); + metadata = std::move(pending); + return SQL_SUCCESS; +} + } // namespace // Wrap SQLDescribeCol SQLRETURN SQLDescribeCol_wrap(SqlHandlePtr StatementHandle, py::list& ColumnMetadata) { PERF_TIMER("SQLDescribeCol_wrap"); - return DescribeColumns(StatementHandle, [&](FetchColumnMetadata column) { + SQLRETURN ret = SQL_ERROR; + ResultMetadataFailureGuard metadataFailure(StatementHandle->resultMetadata, ret); + ret = DescribeColumns(StatementHandle, [&](std::u16string name, SQLSMALLINT type, + SQLULEN size, SQLSMALLINT digits, + SQLSMALLINT nullable) { ColumnMetadata.append( - py::dict("ColumnName"_a = column.name, "DataType"_a = column.dataType, - "ColumnSize"_a = column.columnSize, "DecimalDigits"_a = column.decimalDigits, - "Nullable"_a = column.nullable)); + py::dict("ColumnName"_a = name, "DataType"_a = type, "ColumnSize"_a = size, + "DecimalDigits"_a = digits, "Nullable"_a = nullable)); }); + return ret; } SQLRETURN SQLSpecialColumns_wrap(SqlHandlePtr StatementHandle, SQLSMALLINT identifierType, @@ -3110,6 +3158,7 @@ SQLRETURN SQLSpecialColumns_wrap(SqlHandlePtr StatementHandle, SQLSMALLINT ident const std::u16string& table, SQLSMALLINT scope, SQLSMALLINT nullable) { PERF_TIMER("SQLSpecialColumns_wrap"); + StatementHandle->resultMetadata.clear(); if (!SQLSpecialColumns_ptr) { ThrowStdException("SQLSpecialColumns function not loaded"); } @@ -3138,8 +3187,13 @@ SQLRETURN SQLFetch_wrap(SqlHandlePtr StatementHandle) { } // Release the GIL during the blocking ODBC call - py::gil_scoped_release release; - return SQLFetch_ptr(StatementHandle->get()); + SQLRETURN ret = SQL_ERROR; + ResultMetadataFailureGuard metadataFailure(StatementHandle->resultMetadata, ret); + { + py::gil_scoped_release release; + ret = SQLFetch_ptr(StatementHandle->get()); + } + return ret; } // Non-static so it can be called from inline functions in header @@ -3342,24 +3396,58 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p SQLRETURN ret = SQL_SUCCESS; SQLHSTMT hStmt = StatementHandle->get(); - // Cache decimal separator to avoid repeated system calls + ResultMetadataFailureGuard metadataFailure(StatementHandle->resultMetadata, ret); + const auto snapshot = StatementHandle->resultMetadata.snapshot(); + // The separately exposed GetData entry point may request only a prefix. + const auto metadata = snapshot.metadata && snapshot.metadata->columns.size() >= colCount + ? snapshot.metadata + : nullptr; + auto pending = metadata ? nullptr : std::make_shared(); + bool complete = true; + if (pending) { + pending->columns.reserve(colCount); + } for (SQLSMALLINT i = 1; i <= colCount; ++i) { - SQLWCHAR columnName[256]; + SQLWCHAR uncachedColumnName[256]; + const SQLWCHAR* columnName = uncachedColumnName; SQLSMALLINT columnNameLen; SQLSMALLINT dataType; SQLULEN columnSize; SQLSMALLINT decimalDigits; SQLSMALLINT nullable; - ret = SQLDescribeCol_ptr(hStmt, i, columnName, sizeof(columnName) / sizeof(SQLWCHAR), - &columnNameLen, &dataType, &columnSize, &decimalDigits, &nullable); - if (!SQL_SUCCEEDED(ret)) { - LOG("SQLGetData: Error retrieving metadata for column %d - " - "SQLDescribeCol SQLRETURN=%d", - i, ret); - row.append(py::none()); - continue; + if (metadata && metadata->columns.at(i - 1).dataType != SQL_SS_VARIANT) { + const auto& column = metadata->columns.at(i - 1); + dataType = column.dataType; + columnSize = column.columnSize; + columnName = reinterpretU16stringAsSqlWChar(column.name); + ret = SQL_SUCCESS; + } else { + { + PERF_TIMER("SQLDescribeCol::driver_call"); + ret = SQLDescribeCol_ptr(hStmt, i, uncachedColumnName, + sizeof(uncachedColumnName) / sizeof(SQLWCHAR), + &columnNameLen, &dataType, &columnSize, &decimalDigits, + &nullable); + } + if (!SQL_SUCCEEDED(ret)) { + LOG("SQLGetData: Error retrieving metadata for column %d - " + "SQLDescribeCol SQLRETURN=%d", + i, ret); + complete = false; + row.append(py::none()); + continue; + } + if (pending) { + // Capture declared metadata before probing a variant's current value. + pending->columns.push_back({ + dupeSqlWCharAsUtf16Le( + uncachedColumnName, std::min(static_cast(columnNameLen), + std::size(uncachedColumnName) - 1)), + dataType, dataType == SQL_SS_VARIANT ? 0 : columnSize, decimalDigits, + nullable}); + } } // Preprocess sql_variant: detect underlying type to route to correct conversion logic @@ -3372,10 +3460,14 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p // SQLColAttribute(SQL_CA_SS_VARIANT_TYPE) to return the correct underlying C type. // Without this probe call, SQLColAttribute returns incorrect type codes. SQLLEN indicator; - ret = SQLGetData_ptr(hStmt, i, SQL_C_BINARY, NULL, 0, &indicator); + { + PERF_TIMER("sql_variant::null_probe"); + ret = SQLGetData_ptr(hStmt, i, SQL_C_BINARY, NULL, 0, &indicator); + } if (!SQL_SUCCEEDED(ret)) { LOG_ERROR("SQLGetData: Failed to probe sql_variant column %d - SQLRETURN=%d", i, ret); + complete = false; row.append(py::none()); continue; } @@ -3385,10 +3477,14 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p } // Now retrieve the underlying C type SQLLEN variantCType = 0; - ret = - SQLColAttribute_ptr(hStmt, i, SQL_CA_SS_VARIANT_TYPE, NULL, 0, NULL, &variantCType); + { + PERF_TIMER("sql_variant::subtype"); + ret = SQLColAttribute_ptr(hStmt, i, SQL_CA_SS_VARIANT_TYPE, NULL, 0, NULL, + &variantCType); + } if (!SQL_SUCCEEDED(ret)) { LOG_ERROR("SQLGetData: Failed to get sql_variant underlying type for column %d", i); + complete = false; row.append(py::none()); continue; } @@ -4035,6 +4131,14 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p ThrowStdException(errorString.str()); break; } + if (!SQL_SUCCEEDED(ret)) { + complete = false; + } + } + if (!complete) { + StatementHandle->resultMetadata.clear(); + } else if (pending && pending->columns.size() == colCount) { + StatementHandle->resultMetadata.publish(snapshot.generation, std::move(pending)); } return ret; } @@ -4055,7 +4159,8 @@ SQLRETURN SQLFetchScroll_wrap(SqlHandlePtr StatementHandle, SQLSMALLINT FetchOri SQLFreeStmt_ptr(StatementHandle->get(), SQL_UNBIND); // Perform scroll operation - SQLRETURN ret; + SQLRETURN ret = SQL_ERROR; + ResultMetadataFailureGuard metadataFailure(StatementHandle->resultMetadata, ret); { // Release the GIL during the blocking ODBC fetch py::gil_scoped_release release; @@ -4714,20 +4819,20 @@ SQLRETURN FetchMany_wrap(SqlHandlePtr StatementHandle, py::list& rows, int fetch // Issue #531: upgrade SQL_C_CHAR + utf-8 to SQL_C_WCHAR on Windows so the // driver does lossless UTF-16 conversion instead of returning ACP bytes. charCtype = EffectiveCharCtypeForFetch(charCtype, charEncoding); - SQLRETURN ret; + SQLRETURN ret = SQL_ERROR; + ResultMetadataFailureGuard metadataFailure(StatementHandle->resultMetadata, ret); SQLHSTMT hStmt = StatementHandle->get(); // Retrieve column count SQLSMALLINT numCols = SQLNumResultCols_wrap(StatementHandle); // Retrieve column metadata - std::vector columnNames; - ret = DescribeColumns(StatementHandle, [&](FetchColumnMetadata column) { - columnNames.push_back(std::move(column)); - }); + std::shared_ptr metadata; + ret = GetResultMetadata(StatementHandle, numCols, metadata); if (!SQL_SUCCEEDED(ret)) { LOG("FetchMany_wrap: Failed to get column descriptions - SQLRETURN=%d", ret); return ret; } + const auto& columnNames = metadata->columns; if (numCols < 0 || columnNames.size() != static_cast(numCols)) { LOG("FetchMany_wrap: Column metadata count does not match result column count"); ThrowStdException("Column metadata count does not match result column count"); @@ -4933,7 +5038,8 @@ SQLRETURN FetchArrowBatch_wrap(SqlHandlePtr StatementHandle, py::list& capsules, // An overly large fetch size doesn't seem to help performance int fetchSize = 64; - SQLRETURN ret; + SQLRETURN ret = SQL_ERROR; + ResultMetadataFailureGuard metadataFailure(StatementHandle->resultMetadata, ret); SQLHSTMT hStmt = StatementHandle->get(); // Retrieve column count SQLSMALLINT numCols = SQLNumResultCols_wrap(StatementHandle); @@ -5841,12 +5947,14 @@ SQLRETURN FetchAll_wrap(SqlHandlePtr StatementHandle, py::list& rows, // Issue #531: upgrade SQL_C_CHAR + utf-8 to SQL_C_WCHAR on Windows so the // driver does lossless UTF-16 conversion instead of returning ACP bytes. charCtype = EffectiveCharCtypeForFetch(charCtype, charEncoding); - SQLRETURN ret; + SQLRETURN ret = SQL_ERROR; + ResultMetadataFailureGuard metadataFailure(StatementHandle->resultMetadata, ret); SQLHSTMT hStmt = StatementHandle->get(); // Retrieve column count SQLSMALLINT numCols = SQLNumResultCols_wrap(StatementHandle); // Retrieve column metadata + const auto metadataSnapshot = StatementHandle->resultMetadata.snapshot(); py::list columnNames; ret = SQLDescribeCol_wrap(StatementHandle, columnNames); if (!SQL_SUCCEEDED(ret)) { @@ -5872,6 +5980,25 @@ SQLRETURN FetchAll_wrap(SqlHandlePtr StatementHandle, py::list& rows, LOG("FetchAll_wrap: LOB columns detected (%zu columns), using per-row " "SQLGetData path", lobColumns.size()); + if (numCols < 0 || columnNames.size() != static_cast(numCols)) { + LOG("FetchAll_wrap: Column metadata count does not match result column count"); + ThrowStdException("Column metadata count does not match result column count"); + } + // Keep the public-list setup for fetchall, but reuse its already-validated + // names/declared fields instead of describing stable columns on every row. + auto metadata = std::make_shared(); + metadata->namesValidated = true; + metadata->columns.reserve(numCols); + for (SQLSMALLINT i = 0; i < numCols; ++i) { + const auto column = GetFetchColumnMetadata(columnNames, i); + SQLSMALLINT type = GetFetchColumnType(column); + metadata->columns.push_back({ + column["ColumnName"].cast(), type, + type == SQL_SS_VARIANT ? 0 : GetFetchColumnSize(column), + column["DecimalDigits"].cast(), + column["Nullable"].cast()}); + } + StatementHandle->resultMetadata.publish(metadataSnapshot.generation, std::move(metadata)); while (true) { { // Release GIL during the blocking fetch @@ -5988,7 +6115,8 @@ SQLRETURN FetchOne_wrap(SqlHandlePtr StatementHandle, py::list& row, // Issue #531: upgrade SQL_C_CHAR + utf-8 to SQL_C_WCHAR on Windows so the // driver does lossless UTF-16 conversion instead of returning ACP bytes. charCtype = EffectiveCharCtypeForFetch(charCtype, charEncoding); - SQLRETURN ret; + SQLRETURN ret = SQL_ERROR; + ResultMetadataFailureGuard metadataFailure(StatementHandle->resultMetadata, ret); SQLHSTMT hStmt = StatementHandle->get(); // Unbind any columns from previous fetch operations (e.g., fetchmany) @@ -6020,6 +6148,7 @@ SQLRETURN FetchOne_wrap(SqlHandlePtr StatementHandle, py::list& row, // Wrap SQLMoreResults SQLRETURN SQLMoreResults_wrap(SqlHandlePtr StatementHandle) { PERF_TIMER("SQLMoreResults_wrap"); + StatementHandle->resultMetadata.clear(); LOG("SQLMoreResults_wrap: Check for more results"); if (!SQLMoreResults_ptr) { LOG("SQLMoreResults_wrap: Function pointer not initialized. Loading " @@ -6237,6 +6366,7 @@ PYBIND11_MODULE(ddbc_bindings, m) { m.def( "DDBCSQLSetStmtAttr", [](SqlHandlePtr stmt, SQLINTEGER attr, py::object value) { + stmt->resultMetadata.clear(); SQLPOINTER ptr_value; if (py::isinstance(value)) { // For integer attributes like SQL_ATTR_QUERY_TIMEOUT diff --git a/mssql_python/pybind/ddbc_bindings.h b/mssql_python/pybind/ddbc_bindings.h index 11c33d8d2..3706e6e9d 100644 --- a/mssql_python/pybind/ddbc_bindings.h +++ b/mssql_python/pybind/ddbc_bindings.h @@ -32,6 +32,7 @@ using py::literals::operator""_a; #include #include +#include "result_metadata.hpp" //------------------------------------------------------------------------------------------------- // SQL Server specific ODBC constants @@ -326,6 +327,7 @@ class SqlHandle { // thread-safe by spec (same assumption as the rest of the driver). std::unordered_map describeCache; void clearDescribeCache() { describeCache.clear(); } + ResultMetadataCache resultMetadata; private: // The caller must release the GIL before waiting for native cleanup. diff --git a/mssql_python/pybind/result_metadata.hpp b/mssql_python/pybind/result_metadata.hpp new file mode 100644 index 000000000..7daece3c7 --- /dev/null +++ b/mssql_python/pybind/result_metadata.hpp @@ -0,0 +1,76 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#pragma once + +#include +#include +#include +#include +#include +#include +#include + +struct FetchColumnMetadata { + std::u16string name; + SQLSMALLINT dataType; + SQLULEN columnSize; + SQLSMALLINT decimalDigits; + SQLSMALLINT nullable; +}; + +struct ResultMetadata { + std::vector columns; + bool namesValidated = false; +}; + +// Native-only ownership: cleanup/cancellation may run without the GIL. No ODBC, +// Python, or parent/child handle locks may be acquired while holding this mutex. +class ResultMetadataCache { + public: + struct Snapshot { + uint64_t generation; + std::shared_ptr metadata; + }; + + Snapshot snapshot() const { + std::lock_guard lock(mutex_); + return {generation_, metadata_}; + } + + void publish(uint64_t generation, std::shared_ptr metadata) { + std::lock_guard lock(mutex_); + if (generation == generation_) { + metadata_ = std::move(metadata); + } + } + + void clear() { + std::lock_guard lock(mutex_); + ++generation_; + metadata_.reset(); + } + + private: + mutable std::mutex mutex_; + uint64_t generation_ = 0; + std::shared_ptr metadata_; +}; + +class ResultMetadataFailureGuard { + public: + ResultMetadataFailureGuard(ResultMetadataCache& cache, const SQLRETURN& result) + : cache_(cache), result_(result), exceptions_(std::uncaught_exceptions()) {} + + ~ResultMetadataFailureGuard() { + if (std::uncaught_exceptions() > exceptions_ || + (!SQL_SUCCEEDED(result_) && result_ != SQL_NO_DATA)) { + cache_.clear(); + } + } + + private: + ResultMetadataCache& cache_; + const SQLRETURN& result_; + int exceptions_; +}; diff --git a/tests/test_040_fetch_native_metadata.py b/tests/test_040_fetch_native_metadata.py index eb46976e4..879ce4d92 100644 --- a/tests/test_040_fetch_native_metadata.py +++ b/tests/test_040_fetch_native_metadata.py @@ -1,4 +1,4 @@ -"""Call-local fetch metadata must preserve public descriptions and fetch state.""" +"""Native fetch metadata must preserve public descriptions and result-set state.""" import datetime as dt import os @@ -385,3 +385,440 @@ def test_fetchmany_avoids_python_description_roundtrip(tmp_path): """, tmp_path, ) + + +@pytest.mark.parametrize("method", ["one", "many"]) +def test_result_metadata_prepared_reexecution(metadata_cursor, method): + cursor = metadata_cursor + statement = cursor.hstmt + query = "SELECT CAST(? AS INT) AS n, CAST(? AS NVARCHAR(30)) AS text_value" + for value in range(4): + cursor.execute(query, (value, f"value-{value}")) + assert cursor.hstmt is statement + assert cursor.is_stmt_prepared[0] + profiling = hasattr(ddbc_bindings, "profiling") + if profiling: + ddbc_bindings.profiling.reset() + ddbc_bindings.profiling.enable() + try: + rows = [cursor.fetchone()] if method == "one" else cursor.fetchmany() + finally: + if profiling: + ddbc_bindings.profiling.disable() + _assert_rows(rows, [(value, f"value-{value}")]) + if profiling: + assert ( + ddbc_bindings.profiling.get_stats()["ddbc::SQLDescribeCol::driver_call"]["calls"] + == 2 + ) + assert cursor.fetchone() is None + cursor.execute("SELECT CAST(? AS DECIMAL(8,2)) AS amount", (Decimal("3.25"),)) + _assert_rows(cursor.fetchall(), [(Decimal("3.25"),)]) + + +def test_result_metadata_catalog_replacement(metadata_cursor): + cursor = metadata_cursor + for _ in range(2): + cursor.execute("SELECT 42 AS previous_column") + _assert_rows(cursor.fetchmany(), [(42,)]) + cursor.getTypeInfo(mssql_python.SQL_INTEGER) + description = cursor.description + assert len(description) > 1 + row = cursor.fetchone() + assert row is not None and len(row) == len(description) + assert row[1] == mssql_python.SQL_INTEGER + cursor.fetchall() + cursor.execute("SELECT N'replaced' AS new_column, 5 AS extra") + _assert_rows(cursor.fetchmany(), [("replaced", 5)]) + + +def test_result_metadata_native_reset_and_replacement(tmp_path): + _isolated( + """ + import os + import mssql_python as db + from mssql_python import ddbc_bindings as native + with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: + stmt = connection._conn.alloc_statement_handle() + try: + for _ in range(3): + assert native.DDBCSQLExecDirect(stmt, "SELECT 1 AS a") in (0, 1) + rows = [] + assert native.DDBCSQLFetchMany(stmt, rows, 1) in (0, 1) + assert rows == [[1]] + assert native.DDBCSQLResetStmt(stmt) in (0, 1) + assert native.DDBCSQLExecDirect( + stmt, "SELECT CAST(2 AS BIGINT) AS b, N'new' AS c" + ) in (0, 1) + row = [] + assert native.DDBCSQLFetchOne(stmt, row) in (0, 1) + assert row == [2, "new"] + stmt._close_cursor() + finally: + stmt.free() + """, + tmp_path, + ) + + +@pytest.mark.parametrize("operation", ["commit", "rollback", "autocommit"]) +def test_result_metadata_transaction_recovery(metadata_cursor, operation): + cursor = metadata_cursor + connection = cursor.connection + cursor.execute(_query(["id"], 3)) + _assert_rows(cursor.fetchmany(), [(1,)]) + if operation == "autocommit": + connection.autocommit = True + else: + getattr(connection, operation)() + cursor.execute("SELECT CAST(5.75 AS DECIMAL(8,2)) AS changed, N'text' AS extra") + _assert_rows(cursor.fetchall(), [(Decimal("5.75"), "text")]) + + +@pytest.mark.parametrize("operation", ["commit", "rollback", "autocommit"]) +def test_result_metadata_transaction_preserved_cursor(metadata_cursor, operation): + cursor = metadata_cursor + connection = cursor.connection + info = ( + mssql_python.SQL_CURSOR_ROLLBACK_BEHAVIOR + if operation == "rollback" + else mssql_python.SQL_CURSOR_COMMIT_BEHAVIOR + ) + if connection.getinfo(info) != 2: # SQL_CB_PRESERVE + pytest.skip("Driver does not preserve cursors; native helper coverage is required") + cursor.execute(_query(["id"], 3)) + _assert_rows([cursor.fetchone()], [(1,)]) + if operation == "autocommit": + connection.autocommit = True + else: + getattr(connection, operation)() + profiling = hasattr(ddbc_bindings, "profiling") + if profiling: + ddbc_bindings.profiling.reset() + ddbc_bindings.profiling.enable() + try: + _assert_rows([cursor.fetchone()], [(2,)]) + _assert_rows(cursor.fetchmany(), [(3,)]) + finally: + if profiling: + ddbc_bindings.profiling.disable() + if profiling: + assert ( + ddbc_bindings.profiling.get_stats()["ddbc::SQLDescribeCol::driver_call"]["calls"] == 1 + ) + + +def test_result_metadata_arrow_interleave(tmp_path): + pytest.importorskip("pyarrow") + _isolated( + """ + import gc + import os + import mssql_python as db + with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: + with connection.cursor() as cursor: + query = ("SELECT id, CAST(id AS BIGINT) AS big FROM " + "(VALUES(1),(2),(3),(4),(5),(6)) s(id) ORDER BY id") + for _ in range(3): + cursor.execute(query) + assert tuple(cursor.fetchone()) == (1, 1) + assert [tuple(r) for r in cursor.fetchmany(2)] == [(2, 2), (3, 3)] + batch = cursor.arrow_batch(1) + assert [c.to_pylist() for c in batch.columns] == [[4], [4]] + gc.collect() + assert tuple(cursor.fetchone()) == (5, 5) + assert [tuple(r) for r in cursor.fetchall()] == [(6, 6)] + assert cursor.fetchmany() == [] + """, + tmp_path, + ) + + +@pytest.mark.parametrize("method", ["one", "many", "all"]) +def test_result_metadata_variant_type_and_size_changes(metadata_cursor, method): + cursor = metadata_cursor + cursor.execute( + "CREATE TABLE #metadata_mixed_variant (id INT, v SQL_VARIANT, txt NVARCHAR(MAX))" + ) + cursor.execute( + "INSERT INTO #metadata_mixed_variant VALUES " + "(1,CAST(NULL AS SQL_VARIANT),N'first')," + "(2,CAST(CAST('abc' AS VARCHAR(3)) AS SQL_VARIANT),NULL)," + "(3,CAST(CAST('abcdefgh' AS VARCHAR(8)) AS SQL_VARIANT),N'third')," + "(4,CAST(CAST(17 AS INT) AS SQL_VARIANT),NULL)," + "(5,CAST(CAST(3.25 AS DECIMAL(8,2)) AS SQL_VARIANT),N'fifth')," + "(6,CAST(NULL AS SQL_VARIANT),NULL)," + "(7,CAST(CAST(0x010200 AS VARBINARY(3)) AS SQL_VARIANT),N'last')" + ) + cursor.execute("SELECT v, txt FROM #metadata_mixed_variant ORDER BY id") + if method == "one": + rows = list(cursor) + elif method == "many": + rows = [] + while batch := cursor.fetchmany(): + rows.extend(batch) + else: + rows = cursor.fetchall() + _assert_rows( + rows, + [ + (None, "first"), + ("abc", None), + ("abcdefgh", "third"), + (17, None), + (Decimal("3.25"), "fifth"), + (None, None), + (b"\x01\x02\x00", "last"), + ], + ) + + +def test_result_metadata_one_then_malformed_name_many_does_not_advance(tmp_path): + _isolated( + """ + import os + import mssql_python as db + from mssql_python import ddbc_bindings as native + with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: + with connection.cursor() as cursor: + cursor.execute( + "DECLARE @s NVARCHAR(300) = N'SELECT id AS [' + " + "CAST(0x00D8 AS NVARCHAR(1)) + " + "N'] FROM (VALUES(1),(2),(3)) s(id) ORDER BY id'; EXEC(@s)" + ) + # The low-level row path never decoded a supported column's name. + row = [] + assert native.DDBCSQLFetchOne(cursor.hstmt, row) in (0, 1) + assert row == [1] + for _ in range(2): + try: + native.DDBCSQLFetchMany(cursor.hstmt, [], 1) + except UnicodeDecodeError: + pass + else: + raise AssertionError("many accepted the malformed column name") + row = [] + profiling = hasattr(native, "profiling") + if profiling: + native.profiling.reset() + native.profiling.enable() + try: + assert native.DDBCSQLFetchOne(cursor.hstmt, row) in (0, 1) + finally: + if profiling: + native.profiling.disable() + assert row == [2] + if profiling: + assert native.profiling.get_stats()["ddbc::SQLDescribeCol::driver_call"]["calls"] == 1 + cursor.execute("SELECT 4 AS valid_name, N'recovered' AS text_value") + assert tuple(cursor.fetchone()) == (4, "recovered") + assert cursor.fetchmany() == [] + """, + tmp_path, + ) + + +@pytest.mark.skipif( + not hasattr(ddbc_bindings, "profiling"), reason="requires actual ODBC call instrumentation" +) +@pytest.mark.parametrize("method", ["one", "many"]) +def test_result_metadata_actual_description_counts(tmp_path, method): + _isolated( + f""" + import os + from decimal import Decimal + import mssql_python as db + from mssql_python import ddbc_bindings as native + with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: + with connection.cursor() as cursor: + columns = ",".join(f"id AS c{{i}}" for i in range(24)) + query = ("WITH n AS (SELECT TOP(10000) ROW_NUMBER() OVER " + "(ORDER BY a.object_id,b.object_id) AS id " + "FROM sys.all_objects a CROSS JOIN sys.all_objects b) " + f"SELECT {{columns}} FROM n ORDER BY id") + cursor.execute(query) + native.profiling.reset() + native.profiling.enable() + try: + for value in range(1, 10001): + row = cursor.fetchone() if {method!r} == "one" else cursor.fetchmany()[0] + assert tuple(row) == (value,) * 24 + assert cursor.fetchone() is None + assert cursor.fetchmany() == [] + finally: + native.profiling.disable() + stats = native.profiling.get_stats() + assert stats["ddbc::SQLDescribeCol::driver_call"]["calls"] == 24, stats + assert stats.get("ddbc::SQLDescribeCol_wrap", {{}}).get("calls", 0) == 0 + native.profiling.reset() + native.profiling.enable() + try: + metadata = [] + assert native.DDBCSQLDescribeCol(cursor.hstmt, metadata) in (0, 1) + finally: + native.profiling.disable() + assert len(metadata) == 24 + assert native.profiling.get_stats()["ddbc::SQLDescribeCol::driver_call"]["calls"] == 24 + cursor.execute( + "SELECT CAST(3 AS INT) AS changed, CAST(N'x' AS NVARCHAR(1)) AS text_value; " + "SELECT CAST(7.25 AS DECIMAL(8,2)) AS amount, " + "CAST(N'next long value' AS NVARCHAR(40)) AS name" + ) + assert tuple(cursor.fetchone()) == (3, "x") + assert cursor.nextset() + native.profiling.reset() + native.profiling.enable() + try: + assert tuple(cursor.fetchmany()[0]) == (Decimal("7.25"), "next long value") + assert cursor.fetchmany() == [] + finally: + native.profiling.disable() + assert native.profiling.get_stats()["ddbc::SQLDescribeCol::driver_call"]["calls"] == 2 + """, + tmp_path, + ) + + +@pytest.mark.skipif( + not hasattr(ddbc_bindings, "profiling"), reason="requires actual ODBC call instrumentation" +) +@pytest.mark.parametrize("method", ["one", "many", "all"]) +def test_result_metadata_variant_descriptions_remain_per_row(tmp_path, method): + _isolated( + f""" + import os + import mssql_python as db + from mssql_python import ddbc_bindings as native + with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: + with connection.cursor() as cursor: + cursor.execute( + "SELECT id, v FROM (VALUES " + "(1,CAST(NULL AS SQL_VARIANT))," + "(2,CAST('abc' AS SQL_VARIANT))," + "(3,CAST(17 AS SQL_VARIANT))) s(id,v) ORDER BY id" + ) + native.profiling.reset() + native.profiling.enable() + try: + if {method!r} == "one": + rows = list(cursor) + elif {method!r} == "all": + rows = cursor.fetchall() + else: + rows = [] + while batch := cursor.fetchmany(): + rows.extend(batch) + finally: + native.profiling.disable() + assert [tuple(row) for row in rows] == [(1,None),(2,"abc"),(3,17)] + stats = native.profiling.get_stats() + expected = 4 if {method!r} == "one" else 5 + assert stats["ddbc::SQLDescribeCol::driver_call"]["calls"] == expected, stats + assert stats["ddbc::sql_variant::null_probe"]["calls"] == 3, stats + assert stats["ddbc::sql_variant::subtype"]["calls"] == 2, stats + """, + tmp_path, + ) + + +@pytest.mark.parametrize("method", ["one", "many", "all"]) +def test_result_metadata_all_null_rows(tmp_path, method): + _isolated( + f""" + import os + import mssql_python as db + from mssql_python import ddbc_bindings as native + with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: + with connection.cursor() as cursor: + cursor.execute("SELECT CAST(NULL AS INT) AS scalar_null") + scalar_null = [] + scalar_status = native.DDBCSQLFetchOne(cursor.hstmt, scalar_null) + diagnostics = native.DDBCSQLGetAllDiagRecords(cursor.hstmt) + assert scalar_null == [None] + if scalar_status == 0: + assert diagnostics == [] + recovery_descriptions = 0 + else: + assert scalar_status == -1 + assert len(diagnostics) == 1 and "22002" in diagnostics[0][0], diagnostics + assert "Indicator variable required but not supplied" in diagnostics[0][1] + recovery_descriptions = 4 if {method!r} == "many" else 2 + values = ",".join(f"({{i}})" for i in range(1, 16)) + cursor.execute( + "SELECT CASE WHEN id%7=0 THEN NULL ELSE id END AS c0," + "CASE WHEN id%7=0 THEN CAST(NULL AS SQL_VARIANT) " + "WHEN id%3=0 THEN CAST(id AS SQL_VARIANT) " + "WHEN id%3=1 THEN CAST(N'row-'+CONVERT(NVARCHAR(12),id) AS SQL_VARIANT) " + "ELSE CAST(CONVERT(FLOAT,id)*0.25 AS SQL_VARIANT) END AS c1 " + f"FROM (VALUES{{values}}) s(id) ORDER BY id" + ) + profiling = hasattr(native, "profiling") + if profiling: + native.profiling.reset() + native.profiling.enable() + try: + if {method!r} == "one": + rows = list(cursor) + elif {method!r} == "all": + rows = cursor.fetchall() + else: + rows = [] + while batch := cursor.fetchmany(): + rows.extend(batch) + finally: + if profiling: + native.profiling.disable() + expected = [ + (None,None) if i%7==0 else (i,(i,f"row-{{i}}",i*0.25)[i%3]) + for i in range(1,16) + ] + assert [tuple(row) for row in rows] == expected + assert [[type(value) for value in row] for row in rows] == [ + [type(value) for value in row] for row in expected + ] + if profiling: + stats = native.profiling.get_stats() + # Only drivers/builds reporting a real scalar NULL error + # require the additional post-error cache repopulations. + expected_describes = (16 if {method!r} == "one" else 17) + recovery_descriptions + assert stats["ddbc::SQLDescribeCol::driver_call"]["calls"] == expected_describes, stats + assert stats["ddbc::sql_variant::null_probe"]["calls"] == 15 + assert stats["ddbc::sql_variant::subtype"]["calls"] == 13 + """, + tmp_path, + ) + + +def test_result_metadata_odbc_error_invalidates(tmp_path): + _isolated( + """ + import os + import mssql_python as db + from mssql_python import ddbc_bindings as native + with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: + with connection.cursor() as cursor: + cursor.execute("SELECT id FROM (VALUES(1),(2),(3)) s(id) ORDER BY id") + row = [] + assert native.DDBCSQLFetchOne(cursor.hstmt, row) in (0, 1) + assert row == [1] + assert native.DDBCSQLGetData( + cursor.hstmt, 2, [], "utf-16le", "utf-16le", db.SQL_WCHAR + ) == -1 + diagnostics = native.DDBCSQLGetAllDiagRecords(cursor.hstmt) + assert any("07009" in state for state, _ in diagnostics), diagnostics + profiling = hasattr(native, "profiling") + if profiling: + native.profiling.reset() + native.profiling.enable() + try: + row = [] + assert native.DDBCSQLFetchOne(cursor.hstmt, row) in (0, 1) + assert row == [2] + finally: + if profiling: + native.profiling.disable() + if profiling: + assert native.profiling.get_stats()["ddbc::SQLDescribeCol::driver_call"]["calls"] == 1 + """, + tmp_path, + ) From abd4f4b006f9bda5147d051fe4c047762eee1a10 Mon Sep 17 00:00:00 2001 From: Jahnvi Thakkar Date: Tue, 22 Sep 2026 19:58:48 +0530 Subject: [PATCH 3/7] REFACTOR: Extract child result metadata invalidation helper Preserve reserve-before-locking-weak-handles and release retained handles outside the child-list mutex. Expose the unchanged native-only algorithm for direct invariant coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- mssql_python/pybind/connection/connection.cpp | 16 +------------- mssql_python/pybind/result_metadata.hpp | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/mssql_python/pybind/connection/connection.cpp b/mssql_python/pybind/connection/connection.cpp index 9c4304d2f..de4eb45c0 100644 --- a/mssql_python/pybind/connection/connection.cpp +++ b/mssql_python/pybind/connection/connection.cpp @@ -267,21 +267,7 @@ void Connection::checkError(SQLRETURN ret) const { } void Connection::clearResultMetadata() { - std::vector handles; - { - std::lock_guard lock(_childHandlesMutex); - handles.reserve(_childStatementHandles.size()); - for (const auto& weakHandle : _childStatementHandles) { - if (auto handle = weakHandle.lock()) { - handles.push_back(std::move(handle)); - } - } - } - // Releasing the last handle can acquire the connection cleanup gate. - // Keep that destruction outside the child-list lock. - for (const auto& handle : handles) { - handle->resultMetadata.clear(); - } + ClearChildResultMetadata(_childHandlesMutex, _childStatementHandles); } void Connection::commit() { diff --git a/mssql_python/pybind/result_metadata.hpp b/mssql_python/pybind/result_metadata.hpp index 7daece3c7..0e08e28bd 100644 --- a/mssql_python/pybind/result_metadata.hpp +++ b/mssql_python/pybind/result_metadata.hpp @@ -9,6 +9,7 @@ #include #include #include +#include #include struct FetchColumnMetadata { @@ -57,6 +58,26 @@ class ResultMetadataCache { std::shared_ptr metadata_; }; +template +void ClearChildResultMetadata(std::mutex& childHandlesMutex, + const std::vector>& childHandles) { + std::vector> handles; + { + std::lock_guard lock(childHandlesMutex); + handles.reserve(childHandles.size()); + for (const auto& weakHandle : childHandles) { + if (auto handle = weakHandle.lock()) { + handles.push_back(std::move(handle)); + } + } + } + // Releasing the last handle can acquire the connection cleanup gate. + // Keep that destruction outside the child-list lock. + for (const auto& handle : handles) { + handle->resultMetadata.clear(); + } +} + class ResultMetadataFailureGuard { public: ResultMetadataFailureGuard(ResultMetadataCache& cache, const SQLRETURN& result) From c5fe1425854ee73ed4fac554e7919b3ee05bf4f1 Mon Sep 17 00:00:00 2001 From: Jahnvi Thakkar Date: Tue, 22 Sep 2026 19:59:03 +0530 Subject: [PATCH 4/7] CHORE: Harden native metadata regression coverage Require successful scalar NULL handling and exact describe counts. Add production-header cache, failure, concurrency, allocation and lifetime tests with active Release assertions, plus Windows/Linux/macOS CTest CI and test guidance. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/prompts/run-tests.prompt.md | 21 ++ .github/workflows/native-metadata-tests.yml | 49 +++++ tests/native/CMakeLists.txt | 31 +++ tests/native/allocation_failure.cpp | 32 +++ tests/native/result_metadata_tests.cpp | 208 ++++++++++++++++++++ tests/test_040_fetch_native_metadata.py | 16 +- 6 files changed, 345 insertions(+), 12 deletions(-) create mode 100644 .github/workflows/native-metadata-tests.yml create mode 100644 tests/native/CMakeLists.txt create mode 100644 tests/native/allocation_failure.cpp create mode 100644 tests/native/result_metadata_tests.cpp diff --git a/.github/prompts/run-tests.prompt.md b/.github/prompts/run-tests.prompt.md index da8bcfa88..2d6eb3a9b 100644 --- a/.github/prompts/run-tests.prompt.md +++ b/.github/prompts/run-tests.prompt.md @@ -122,6 +122,27 @@ Help the developer run tests to validate their changes. Follow this process base ## STEP 1: Choose What to Test +### Native metadata invariants (no database) + +The standalone CMake tests in `tests/native` exercise the production metadata +cache and child-handle invalidation helper without importing the Python package +or connecting to SQL Server. They require a C++17 compiler, CMake, and ODBC +headers (Windows SDK, `unixodbc-dev` on Linux, or `unixodbc` on macOS). +The Native Metadata Tests workflow runs them on Windows, Linux, and macOS. + +```bash +cmake -S tests/native -B build/native-metadata -DCMAKE_BUILD_TYPE=Release +cmake --build build/native-metadata --config Release --parallel 2 +ctest --test-dir build/native-metadata -C Release --output-on-failure +``` + +Assertions remain enabled in Release. Cases cover stale-generation rejection, +held snapshots, failure/EOF guards, concurrent invalidation, child isolation, +reserve failure before strong-reference acquisition, and last-owner destruction +outside the child-list lock. These native checks supplement, not replace, the +live transaction tests, which skip when the driver does not preserve cursors. +Native-only tests do not require the Python-test prerequisites above. + ### Test Categories | Category | Description | When to Use | diff --git a/.github/workflows/native-metadata-tests.yml b/.github/workflows/native-metadata-tests.yml new file mode 100644 index 000000000..a5887fbfa --- /dev/null +++ b/.github/workflows/native-metadata-tests.yml @@ -0,0 +1,49 @@ +name: Native Metadata Tests + +on: + pull_request: + types: [opened, reopened, synchronize, ready_for_review] + paths: + - 'mssql_python/pybind/result_metadata.hpp' + - 'mssql_python/pybind/connection/connection.cpp' + - 'tests/native/**' + - '.github/workflows/native-metadata-tests.yml' + push: + branches: [main] + paths: + - 'mssql_python/pybind/result_metadata.hpp' + - 'mssql_python/pybind/connection/connection.cpp' + - 'tests/native/**' + - '.github/workflows/native-metadata-tests.yml' + +permissions: + contents: read + +jobs: + native-metadata: + name: Native metadata (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + timeout-minutes: 10 + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest, macos-latest] + steps: + - name: Checkout + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + with: + persist-credentials: false + - name: Install ODBC headers (Linux) + if: runner.os == 'Linux' + run: | + sudo apt-get update + sudo apt-get install -y unixodbc-dev + - name: Install ODBC headers (macOS) + if: runner.os == 'macOS' + run: brew install unixodbc + - name: Configure + run: cmake -S tests/native -B build/native-metadata -DCMAKE_BUILD_TYPE=Release + - name: Build + run: cmake --build build/native-metadata --config Release --parallel 2 + - name: Test + run: ctest --test-dir build/native-metadata -C Release --output-on-failure diff --git a/tests/native/CMakeLists.txt b/tests/native/CMakeLists.txt new file mode 100644 index 000000000..83c8f3861 --- /dev/null +++ b/tests/native/CMakeLists.txt @@ -0,0 +1,31 @@ +cmake_minimum_required(VERSION 3.15) +project(mssql_python_native_tests LANGUAGES CXX) + +enable_testing() +find_package(Threads REQUIRED) + +add_executable(result_metadata_tests result_metadata_tests.cpp allocation_failure.cpp) +target_compile_features(result_metadata_tests PRIVATE cxx_std_17) +target_include_directories(result_metadata_tests PRIVATE ../../mssql_python/pybind) +target_link_libraries(result_metadata_tests PRIVATE Threads::Threads) + +if(WIN32) + target_compile_definitions(result_metadata_tests PRIVATE WIN32_LEAN_AND_MEAN NOMINMAX) +else() + find_path(ODBC_INCLUDE_DIR sql.h PATHS /opt/homebrew/include /usr/local/include) + if(NOT ODBC_INCLUDE_DIR) + message(FATAL_ERROR "ODBC headers are required: install unixodbc-dev or unixodbc.") + endif() + target_include_directories(result_metadata_tests PRIVATE "${ODBC_INCLUDE_DIR}") +endif() + +if(MSVC) + target_compile_options(result_metadata_tests PRIVATE /W4 /WX /UNDEBUG) +else() + target_compile_options(result_metadata_tests PRIVATE -Wall -Wextra -Werror -UNDEBUG) +endif() + +foreach(case_name IN ITEMS snapshots failures concurrent children allocation last_owner) + add_test(NAME result_metadata_${case_name} COMMAND result_metadata_tests ${case_name}) + set_tests_properties(result_metadata_${case_name} PROPERTIES TIMEOUT 20) +endforeach() diff --git a/tests/native/allocation_failure.cpp b/tests/native/allocation_failure.cpp new file mode 100644 index 000000000..bf9daab61 --- /dev/null +++ b/tests/native/allocation_failure.cpp @@ -0,0 +1,32 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#include +#include +#include + +struct TestHandle; +extern std::weak_ptr observedHandle; +extern bool failAllocation; +extern long ownersAtFailure; + +// Keep replacement allocation functions opaque to optimized test call sites. +void* operator new(std::size_t size) { + if (failAllocation) { + failAllocation = false; + ownersAtFailure = observedHandle.use_count(); + throw std::bad_alloc(); + } + if (void* memory = std::malloc(size ? size : 1)) { + return memory; + } + throw std::bad_alloc(); +} + +void operator delete(void* memory) noexcept { + std::free(memory); +} + +void operator delete(void* memory, std::size_t) noexcept { + std::free(memory); +} diff --git a/tests/native/result_metadata_tests.cpp b/tests/native/result_metadata_tests.cpp new file mode 100644 index 000000000..963608564 --- /dev/null +++ b/tests/native/result_metadata_tests.cpp @@ -0,0 +1,208 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#ifdef _WIN32 +#include +#endif +#include "result_metadata.hpp" + +#include +#include +#include +#include +#include +#include + +#ifdef NDEBUG +#error Native metadata tests require assertions, including Release builds. +#endif + +struct TestHandle { + ResultMetadataCache resultMetadata; + std::mutex* childMutex = nullptr; + + ~TestHandle() { + if (childMutex) { + bool acquired = false; + std::thread observer([&] { + acquired = childMutex->try_lock(); + if (acquired) { + childMutex->unlock(); + } + }); + observer.join(); + assert(acquired); + } + } +}; + +std::weak_ptr observedHandle; +bool failAllocation = false; +long ownersAtFailure = -1; + +static std::shared_ptr MakeMetadata() { + auto metadata = std::make_shared(); + metadata->columns.push_back({u"owned", SQL_INTEGER, 10, 0, 1}); + return metadata; +} + +static void Populate(ResultMetadataCache& cache) { + const auto snapshot = cache.snapshot(); + cache.publish(snapshot.generation, MakeMetadata()); +} + +static void TestSnapshots() { + ResultMetadataCache cache; + const auto initial = cache.snapshot(); + assert(!initial.metadata); + auto metadata = MakeMetadata(); + std::weak_ptr weak = metadata; + cache.publish(initial.generation, metadata); + auto held = cache.snapshot(); + assert(held.metadata == metadata); + cache.clear(); + assert(!cache.snapshot().metadata); + assert(cache.snapshot().generation != initial.generation); + + Populate(cache); + const auto replacement = cache.snapshot(); + cache.publish(initial.generation, metadata); + assert(cache.snapshot().metadata == replacement.metadata); + assert(held.metadata->columns.at(0).name == u"owned"); + metadata.reset(); + assert(!weak.expired()); + held.metadata.reset(); + assert(weak.expired()); +} + +static void TestFailures() { + ResultMetadataCache cache; + const SQLRETURN results[] = {SQL_SUCCESS, SQL_SUCCESS_WITH_INFO, SQL_NO_DATA, + SQL_ERROR, SQL_INVALID_HANDLE}; + for (SQLRETURN result : results) { + Populate(cache); + const auto before = cache.snapshot(); + { + ResultMetadataFailureGuard guard(cache, result); + } + const auto after = cache.snapshot(); + if (SQL_SUCCEEDED(result) || result == SQL_NO_DATA) { + assert(after.metadata == before.metadata); + assert(after.generation == before.generation); + } else { + assert(!after.metadata); + assert(after.generation != before.generation); + } + } + Populate(cache); + SQLRETURN result = SQL_SUCCESS; + try { + ResultMetadataFailureGuard guard(cache, result); + throw std::runtime_error("conversion failure"); + } catch (const std::runtime_error&) { + assert(!cache.snapshot().metadata); + } +} + +static void TestConcurrentInvalidation() { + ResultMetadataCache cache; + const auto metadata = MakeMetadata(); + std::thread invalidator([&] { + for (int i = 0; i < 1000; ++i) { + cache.clear(); + } + }); + for (int i = 0; i < 1000; ++i) { + const auto snapshot = cache.snapshot(); + cache.publish(snapshot.generation, metadata); + if (snapshot.metadata) { + assert(snapshot.metadata->columns.at(0).name == u"owned"); + } + } + invalidator.join(); + cache.clear(); + assert(!cache.snapshot().metadata); +} + +static void TestChildren() { + std::mutex childMutex; + auto first = std::make_shared(); + auto second = std::make_shared(); + auto unrelated = std::make_shared(); + std::vector> children{first, {}, second}; + Populate(first->resultMetadata); + Populate(second->resultMetadata); + Populate(unrelated->resultMetadata); + const auto held = first->resultMetadata.snapshot(); + ClearChildResultMetadata(childMutex, children); + assert(!first->resultMetadata.snapshot().metadata); + assert(!second->resultMetadata.snapshot().metadata); + assert(unrelated->resultMetadata.snapshot().metadata); + assert(held.metadata->columns.at(0).name == u"owned"); + ClearChildResultMetadata(childMutex, children); +} + +static void TestAllocationFailure() { + std::mutex childMutex; + auto owner = std::make_shared(); + observedHandle = owner; + std::vector> children{owner}; + Populate(owner->resultMetadata); + const auto before = owner->resultMetadata.snapshot(); + failAllocation = true; + try { + ClearChildResultMetadata(childMutex, children); + assert(false); + } catch (const std::bad_alloc&) { + assert(ownersAtFailure == 1); + assert(owner->resultMetadata.snapshot().metadata == before.metadata); + assert(childMutex.try_lock()); + childMutex.unlock(); + } + assert(!failAllocation); + ClearChildResultMetadata(childMutex, children); + assert(!owner->resultMetadata.snapshot().metadata); +} + +static void TestLastOwner() { + std::mutex childMutex; + auto owner = std::make_shared(); + owner->childMutex = &childMutex; + const std::weak_ptr weak = owner; + std::vector> children{owner}; + // Drop the external owner during invalidation, leaving only the helper's snapshot. + auto metadata = std::shared_ptr(new ResultMetadata, [&](auto* value) { + owner.reset(); + delete value; + }); + const auto generation = owner->resultMetadata.snapshot().generation; + owner->resultMetadata.publish(generation, std::move(metadata)); + ClearChildResultMetadata(childMutex, children); + assert(!owner && weak.expired()); +} + +int main(int argc, char** argv) { + if (argc != 2) { + std::fputs("Expected one native metadata test case\n", stderr); + return 2; + } + const char* name = argv[1]; + if (std::strcmp(name, "snapshots") == 0) { + TestSnapshots(); + } else if (std::strcmp(name, "failures") == 0) { + TestFailures(); + } else if (std::strcmp(name, "concurrent") == 0) { + TestConcurrentInvalidation(); + } else if (std::strcmp(name, "children") == 0) { + TestChildren(); + } else if (std::strcmp(name, "allocation") == 0) { + TestAllocationFailure(); + } else if (std::strcmp(name, "last_owner") == 0) { + TestLastOwner(); + } else { + std::fprintf(stderr, "Unknown native metadata test case: %s\n", name); + return 2; + } + std::printf("%s passed\n", name); + return 0; +} diff --git a/tests/test_040_fetch_native_metadata.py b/tests/test_040_fetch_native_metadata.py index 879ce4d92..102ad6f35 100644 --- a/tests/test_040_fetch_native_metadata.py +++ b/tests/test_040_fetch_native_metadata.py @@ -485,7 +485,7 @@ def test_result_metadata_transaction_preserved_cursor(metadata_cursor, operation else mssql_python.SQL_CURSOR_COMMIT_BEHAVIOR ) if connection.getinfo(info) != 2: # SQL_CB_PRESERVE - pytest.skip("Driver does not preserve cursors; native helper coverage is required") + pytest.skip("Driver does not preserve cursors; cache/helper coverage is in tests/native") cursor.execute(_query(["id"], 3)) _assert_rows([cursor.fetchone()], [(1,)]) if operation == "autocommit": @@ -734,15 +734,9 @@ def test_result_metadata_all_null_rows(tmp_path, method): scalar_null = [] scalar_status = native.DDBCSQLFetchOne(cursor.hstmt, scalar_null) diagnostics = native.DDBCSQLGetAllDiagRecords(cursor.hstmt) + assert scalar_status == 0, (scalar_status, diagnostics) assert scalar_null == [None] - if scalar_status == 0: - assert diagnostics == [] - recovery_descriptions = 0 - else: - assert scalar_status == -1 - assert len(diagnostics) == 1 and "22002" in diagnostics[0][0], diagnostics - assert "Indicator variable required but not supplied" in diagnostics[0][1] - recovery_descriptions = 4 if {method!r} == "many" else 2 + assert diagnostics == [] values = ",".join(f"({{i}})" for i in range(1, 16)) cursor.execute( "SELECT CASE WHEN id%7=0 THEN NULL ELSE id END AS c0," @@ -778,9 +772,7 @@ def test_result_metadata_all_null_rows(tmp_path, method): ] if profiling: stats = native.profiling.get_stats() - # Only drivers/builds reporting a real scalar NULL error - # require the additional post-error cache repopulations. - expected_describes = (16 if {method!r} == "one" else 17) + recovery_descriptions + expected_describes = 16 if {method!r} == "one" else 17 assert stats["ddbc::SQLDescribeCol::driver_call"]["calls"] == expected_describes, stats assert stats["ddbc::sql_variant::null_probe"]["calls"] == 15 assert stats["ddbc::sql_variant::subtype"]["calls"] == 13 From 416e54d6ad7e39dd759d97088194a801fe8c3bee Mon Sep 17 00:00:00 2001 From: Jahnvi Thakkar Date: Tue, 22 Sep 2026 20:30:19 +0530 Subject: [PATCH 5/7] CHORE: Address native metadata test review findings Use C++ streams for test-runner output, document the line-scoped allocator rule exception, and trigger native invariant tests for production integration header and binding changes. Keep production fetch code and the allocation-failure probes unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/workflows/native-metadata-tests.yml | 6 ++++++ tests/native/allocation_failure.cpp | 3 ++- tests/native/result_metadata_tests.cpp | 8 ++++---- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/.github/workflows/native-metadata-tests.yml b/.github/workflows/native-metadata-tests.yml index a5887fbfa..d0ea213be 100644 --- a/.github/workflows/native-metadata-tests.yml +++ b/.github/workflows/native-metadata-tests.yml @@ -5,6 +5,9 @@ on: types: [opened, reopened, synchronize, ready_for_review] paths: - 'mssql_python/pybind/result_metadata.hpp' + - 'mssql_python/pybind/ddbc_bindings.cpp' + - 'mssql_python/pybind/ddbc_bindings.h' + - 'mssql_python/pybind/connection/connection.h' - 'mssql_python/pybind/connection/connection.cpp' - 'tests/native/**' - '.github/workflows/native-metadata-tests.yml' @@ -12,6 +15,9 @@ on: branches: [main] paths: - 'mssql_python/pybind/result_metadata.hpp' + - 'mssql_python/pybind/ddbc_bindings.cpp' + - 'mssql_python/pybind/ddbc_bindings.h' + - 'mssql_python/pybind/connection/connection.h' - 'mssql_python/pybind/connection/connection.cpp' - 'tests/native/**' - '.github/workflows/native-metadata-tests.yml' diff --git a/tests/native/allocation_failure.cpp b/tests/native/allocation_failure.cpp index bf9daab61..72cf10cca 100644 --- a/tests/native/allocation_failure.cpp +++ b/tests/native/allocation_failure.cpp @@ -17,7 +17,8 @@ void* operator new(std::size_t size) { ownersAtFailure = observedHandle.use_count(); throw std::bad_alloc(); } - if (void* memory = std::malloc(size ? size : 1)) { + // operator new must not recurse; size is a byte count, with no arithmetic. + if (void* memory = std::malloc(size ? size : 1)) { // DevSkim: ignore DS161085 return memory; } throw std::bad_alloc(); diff --git a/tests/native/result_metadata_tests.cpp b/tests/native/result_metadata_tests.cpp index 963608564..699608a9e 100644 --- a/tests/native/result_metadata_tests.cpp +++ b/tests/native/result_metadata_tests.cpp @@ -7,8 +7,8 @@ #include "result_metadata.hpp" #include -#include #include +#include #include #include #include @@ -183,7 +183,7 @@ static void TestLastOwner() { int main(int argc, char** argv) { if (argc != 2) { - std::fputs("Expected one native metadata test case\n", stderr); + std::cerr << "Expected one native metadata test case\n"; return 2; } const char* name = argv[1]; @@ -200,9 +200,9 @@ int main(int argc, char** argv) { } else if (std::strcmp(name, "last_owner") == 0) { TestLastOwner(); } else { - std::fprintf(stderr, "Unknown native metadata test case: %s\n", name); + std::cerr << "Unknown native metadata test case: " << name << '\n'; return 2; } - std::printf("%s passed\n", name); + std::cout << name << " passed\n"; return 0; } From 65081f060ee878bebb1ad0a595b93ddbf2418d72 Mon Sep 17 00:00:00 2001 From: Jahnvi Thakkar Date: Thu, 24 Sep 2026 11:03:11 +0530 Subject: [PATCH 6/7] REFACTOR: Minimize metadata optimization scope Remove PR-only tests, native test build and CI wiring, and restore the base test guide. Inline the test-only invalidation helper and single-caller metadata setup; stop retaining unused precision/nullability fields while preserving the cache lifetime, generation, Unicode, variant and failure contracts. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/prompts/run-tests.prompt.md | 21 - .github/workflows/native-metadata-tests.yml | 55 -- mssql_python/pybind/connection/connection.cpp | 16 +- mssql_python/pybind/ddbc_bindings.cpp | 89 +- mssql_python/pybind/result_metadata.hpp | 22 - tests/native/CMakeLists.txt | 31 - tests/native/allocation_failure.cpp | 33 - tests/native/result_metadata_tests.cpp | 208 ----- tests/test_040_fetch_native_metadata.py | 816 ------------------ 9 files changed, 51 insertions(+), 1240 deletions(-) delete mode 100644 .github/workflows/native-metadata-tests.yml delete mode 100644 tests/native/CMakeLists.txt delete mode 100644 tests/native/allocation_failure.cpp delete mode 100644 tests/native/result_metadata_tests.cpp delete mode 100644 tests/test_040_fetch_native_metadata.py diff --git a/.github/prompts/run-tests.prompt.md b/.github/prompts/run-tests.prompt.md index 2d6eb3a9b..da8bcfa88 100644 --- a/.github/prompts/run-tests.prompt.md +++ b/.github/prompts/run-tests.prompt.md @@ -122,27 +122,6 @@ Help the developer run tests to validate their changes. Follow this process base ## STEP 1: Choose What to Test -### Native metadata invariants (no database) - -The standalone CMake tests in `tests/native` exercise the production metadata -cache and child-handle invalidation helper without importing the Python package -or connecting to SQL Server. They require a C++17 compiler, CMake, and ODBC -headers (Windows SDK, `unixodbc-dev` on Linux, or `unixodbc` on macOS). -The Native Metadata Tests workflow runs them on Windows, Linux, and macOS. - -```bash -cmake -S tests/native -B build/native-metadata -DCMAKE_BUILD_TYPE=Release -cmake --build build/native-metadata --config Release --parallel 2 -ctest --test-dir build/native-metadata -C Release --output-on-failure -``` - -Assertions remain enabled in Release. Cases cover stale-generation rejection, -held snapshots, failure/EOF guards, concurrent invalidation, child isolation, -reserve failure before strong-reference acquisition, and last-owner destruction -outside the child-list lock. These native checks supplement, not replace, the -live transaction tests, which skip when the driver does not preserve cursors. -Native-only tests do not require the Python-test prerequisites above. - ### Test Categories | Category | Description | When to Use | diff --git a/.github/workflows/native-metadata-tests.yml b/.github/workflows/native-metadata-tests.yml deleted file mode 100644 index d0ea213be..000000000 --- a/.github/workflows/native-metadata-tests.yml +++ /dev/null @@ -1,55 +0,0 @@ -name: Native Metadata Tests - -on: - pull_request: - types: [opened, reopened, synchronize, ready_for_review] - paths: - - 'mssql_python/pybind/result_metadata.hpp' - - 'mssql_python/pybind/ddbc_bindings.cpp' - - 'mssql_python/pybind/ddbc_bindings.h' - - 'mssql_python/pybind/connection/connection.h' - - 'mssql_python/pybind/connection/connection.cpp' - - 'tests/native/**' - - '.github/workflows/native-metadata-tests.yml' - push: - branches: [main] - paths: - - 'mssql_python/pybind/result_metadata.hpp' - - 'mssql_python/pybind/ddbc_bindings.cpp' - - 'mssql_python/pybind/ddbc_bindings.h' - - 'mssql_python/pybind/connection/connection.h' - - 'mssql_python/pybind/connection/connection.cpp' - - 'tests/native/**' - - '.github/workflows/native-metadata-tests.yml' - -permissions: - contents: read - -jobs: - native-metadata: - name: Native metadata (${{ matrix.os }}) - runs-on: ${{ matrix.os }} - timeout-minutes: 10 - strategy: - fail-fast: false - matrix: - os: [ubuntu-latest, windows-latest, macos-latest] - steps: - - name: Checkout - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - with: - persist-credentials: false - - name: Install ODBC headers (Linux) - if: runner.os == 'Linux' - run: | - sudo apt-get update - sudo apt-get install -y unixodbc-dev - - name: Install ODBC headers (macOS) - if: runner.os == 'macOS' - run: brew install unixodbc - - name: Configure - run: cmake -S tests/native -B build/native-metadata -DCMAKE_BUILD_TYPE=Release - - name: Build - run: cmake --build build/native-metadata --config Release --parallel 2 - - name: Test - run: ctest --test-dir build/native-metadata -C Release --output-on-failure diff --git a/mssql_python/pybind/connection/connection.cpp b/mssql_python/pybind/connection/connection.cpp index de4eb45c0..9c4304d2f 100644 --- a/mssql_python/pybind/connection/connection.cpp +++ b/mssql_python/pybind/connection/connection.cpp @@ -267,7 +267,21 @@ void Connection::checkError(SQLRETURN ret) const { } void Connection::clearResultMetadata() { - ClearChildResultMetadata(_childHandlesMutex, _childStatementHandles); + std::vector handles; + { + std::lock_guard lock(_childHandlesMutex); + handles.reserve(_childStatementHandles.size()); + for (const auto& weakHandle : _childStatementHandles) { + if (auto handle = weakHandle.lock()) { + handles.push_back(std::move(handle)); + } + } + } + // Releasing the last handle can acquire the connection cleanup gate. + // Keep that destruction outside the child-list lock. + for (const auto& handle : handles) { + handle->resultMetadata.clear(); + } } void Connection::commit() { diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index cf2dc5c0f..5068f3c19 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -3101,42 +3101,6 @@ SQLRETURN DescribeColumns(SqlHandlePtr StatementHandle, AppendColumn&& appendCol return SQL_SUCCESS; } -SQLRETURN GetResultMetadata(const SqlHandlePtr& statement, SQLSMALLINT columnCount, - std::shared_ptr& metadata) { - const auto snapshot = statement->resultMetadata.snapshot(); - const bool matches = snapshot.metadata && columnCount >= 0 && - snapshot.metadata->columns.size() == static_cast(columnCount); - if (matches && snapshot.metadata->namesValidated) { - metadata = snapshot.metadata; - return SQL_SUCCESS; - } - auto pending = matches ? std::make_shared(*snapshot.metadata) - : std::make_shared(); - if (!matches) { - SQLRETURN ret = DescribeColumns( - statement, [&](std::u16string name, SQLSMALLINT type, SQLULEN size, - SQLSMALLINT digits, SQLSMALLINT nullable) { - // Preserve eager name validation before advancing the result set. - py::cast(name); - pending->columns.push_back( - {std::move(name), type, type == SQL_SS_VARIANT ? 0 : size, digits, nullable}); - }); - if (!SQL_SUCCEEDED(ret)) { - return ret; - } - } else { - // Row-wise fetches originally read names without decoding them. A later - // many/all fetch must still validate those names before its first advance. - for (const auto& column : pending->columns) { - py::cast(column.name); - } - } - pending->namesValidated = true; - statement->resultMetadata.publish(snapshot.generation, pending); - metadata = std::move(pending); - return SQL_SUCCESS; -} - } // namespace // Wrap SQLDescribeCol @@ -3446,8 +3410,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p dupeSqlWCharAsUtf16Le( uncachedColumnName, std::min(static_cast(columnNameLen), std::size(uncachedColumnName) - 1)), - dataType, dataType == SQL_SS_VARIANT ? 0 : columnSize, decimalDigits, - nullable}); + dataType, dataType == SQL_SS_VARIANT ? 0 : columnSize}); } } @@ -4827,12 +4790,37 @@ SQLRETURN FetchMany_wrap(SqlHandlePtr StatementHandle, py::list& rows, int fetch SQLSMALLINT numCols = SQLNumResultCols_wrap(StatementHandle); // Retrieve column metadata - std::shared_ptr metadata; - ret = GetResultMetadata(StatementHandle, numCols, metadata); - if (!SQL_SUCCEEDED(ret)) { - LOG("FetchMany_wrap: Failed to get column descriptions - SQLRETURN=%d", ret); - return ret; + auto snapshot = StatementHandle->resultMetadata.snapshot(); + auto metadata = std::move(snapshot.metadata); + const bool matches = metadata && numCols >= 0 && + metadata->columns.size() == static_cast(numCols); + if (!matches || !metadata->namesValidated) { + auto pending = matches ? std::make_shared(*metadata) + : std::make_shared(); + if (!matches) { + ret = DescribeColumns( + StatementHandle, [&](std::u16string name, SQLSMALLINT type, SQLULEN size, + SQLSMALLINT, SQLSMALLINT) { + // Preserve eager name validation before advancing the result set. + py::cast(name); + pending->columns.push_back( + {std::move(name), type, type == SQL_SS_VARIANT ? 0 : size}); + }); + if (!SQL_SUCCEEDED(ret)) { + LOG("FetchMany_wrap: Failed to get column descriptions - SQLRETURN=%d", ret); + return ret; + } + } else { + // Row-wise fetches read names without decoding them. Validate before advancing. + for (const auto& column : pending->columns) { + py::cast(column.name); + } + } + pending->namesValidated = true; + StatementHandle->resultMetadata.publish(snapshot.generation, pending); + metadata = std::move(pending); } + ret = SQL_SUCCESS; const auto& columnNames = metadata->columns; if (numCols < 0 || columnNames.size() != static_cast(numCols)) { LOG("FetchMany_wrap: Column metadata count does not match result column count"); @@ -4841,11 +4829,8 @@ SQLRETURN FetchMany_wrap(SqlHandlePtr StatementHandle, py::list& rows, int fetch std::vector lobColumns; for (SQLSMALLINT i = 0; i < numCols; i++) { - const auto& colMeta = GetFetchColumnMetadata(columnNames, i); - SQLSMALLINT dataType = GetFetchColumnType(colMeta); - SQLULEN columnSize = GetFetchColumnSize(colMeta); - - if (IsLobOrVariantColumn(dataType, columnSize)) { + const auto& column = columnNames.at(i); + if (IsLobOrVariantColumn(column.dataType, column.columnSize)) { lobColumns.push_back(i + 1); // 1-based } } @@ -5991,13 +5976,11 @@ SQLRETURN FetchAll_wrap(SqlHandlePtr StatementHandle, py::list& rows, metadata->namesValidated = true; metadata->columns.reserve(numCols); for (SQLSMALLINT i = 0; i < numCols; ++i) { - const auto column = GetFetchColumnMetadata(columnNames, i); - SQLSMALLINT type = GetFetchColumnType(column); + const auto column = columnNames[i].cast(); + SQLSMALLINT type = column["DataType"].cast(); metadata->columns.push_back({ column["ColumnName"].cast(), type, - type == SQL_SS_VARIANT ? 0 : GetFetchColumnSize(column), - column["DecimalDigits"].cast(), - column["Nullable"].cast()}); + type == SQL_SS_VARIANT ? 0 : column["ColumnSize"].cast()}); } StatementHandle->resultMetadata.publish(metadataSnapshot.generation, std::move(metadata)); while (true) { diff --git a/mssql_python/pybind/result_metadata.hpp b/mssql_python/pybind/result_metadata.hpp index 0e08e28bd..3a9912860 100644 --- a/mssql_python/pybind/result_metadata.hpp +++ b/mssql_python/pybind/result_metadata.hpp @@ -16,8 +16,6 @@ struct FetchColumnMetadata { std::u16string name; SQLSMALLINT dataType; SQLULEN columnSize; - SQLSMALLINT decimalDigits; - SQLSMALLINT nullable; }; struct ResultMetadata { @@ -58,26 +56,6 @@ class ResultMetadataCache { std::shared_ptr metadata_; }; -template -void ClearChildResultMetadata(std::mutex& childHandlesMutex, - const std::vector>& childHandles) { - std::vector> handles; - { - std::lock_guard lock(childHandlesMutex); - handles.reserve(childHandles.size()); - for (const auto& weakHandle : childHandles) { - if (auto handle = weakHandle.lock()) { - handles.push_back(std::move(handle)); - } - } - } - // Releasing the last handle can acquire the connection cleanup gate. - // Keep that destruction outside the child-list lock. - for (const auto& handle : handles) { - handle->resultMetadata.clear(); - } -} - class ResultMetadataFailureGuard { public: ResultMetadataFailureGuard(ResultMetadataCache& cache, const SQLRETURN& result) diff --git a/tests/native/CMakeLists.txt b/tests/native/CMakeLists.txt deleted file mode 100644 index 83c8f3861..000000000 --- a/tests/native/CMakeLists.txt +++ /dev/null @@ -1,31 +0,0 @@ -cmake_minimum_required(VERSION 3.15) -project(mssql_python_native_tests LANGUAGES CXX) - -enable_testing() -find_package(Threads REQUIRED) - -add_executable(result_metadata_tests result_metadata_tests.cpp allocation_failure.cpp) -target_compile_features(result_metadata_tests PRIVATE cxx_std_17) -target_include_directories(result_metadata_tests PRIVATE ../../mssql_python/pybind) -target_link_libraries(result_metadata_tests PRIVATE Threads::Threads) - -if(WIN32) - target_compile_definitions(result_metadata_tests PRIVATE WIN32_LEAN_AND_MEAN NOMINMAX) -else() - find_path(ODBC_INCLUDE_DIR sql.h PATHS /opt/homebrew/include /usr/local/include) - if(NOT ODBC_INCLUDE_DIR) - message(FATAL_ERROR "ODBC headers are required: install unixodbc-dev or unixodbc.") - endif() - target_include_directories(result_metadata_tests PRIVATE "${ODBC_INCLUDE_DIR}") -endif() - -if(MSVC) - target_compile_options(result_metadata_tests PRIVATE /W4 /WX /UNDEBUG) -else() - target_compile_options(result_metadata_tests PRIVATE -Wall -Wextra -Werror -UNDEBUG) -endif() - -foreach(case_name IN ITEMS snapshots failures concurrent children allocation last_owner) - add_test(NAME result_metadata_${case_name} COMMAND result_metadata_tests ${case_name}) - set_tests_properties(result_metadata_${case_name} PROPERTIES TIMEOUT 20) -endforeach() diff --git a/tests/native/allocation_failure.cpp b/tests/native/allocation_failure.cpp deleted file mode 100644 index 72cf10cca..000000000 --- a/tests/native/allocation_failure.cpp +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#include -#include -#include - -struct TestHandle; -extern std::weak_ptr observedHandle; -extern bool failAllocation; -extern long ownersAtFailure; - -// Keep replacement allocation functions opaque to optimized test call sites. -void* operator new(std::size_t size) { - if (failAllocation) { - failAllocation = false; - ownersAtFailure = observedHandle.use_count(); - throw std::bad_alloc(); - } - // operator new must not recurse; size is a byte count, with no arithmetic. - if (void* memory = std::malloc(size ? size : 1)) { // DevSkim: ignore DS161085 - return memory; - } - throw std::bad_alloc(); -} - -void operator delete(void* memory) noexcept { - std::free(memory); -} - -void operator delete(void* memory, std::size_t) noexcept { - std::free(memory); -} diff --git a/tests/native/result_metadata_tests.cpp b/tests/native/result_metadata_tests.cpp deleted file mode 100644 index 699608a9e..000000000 --- a/tests/native/result_metadata_tests.cpp +++ /dev/null @@ -1,208 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#ifdef _WIN32 -#include -#endif -#include "result_metadata.hpp" - -#include -#include -#include -#include -#include -#include - -#ifdef NDEBUG -#error Native metadata tests require assertions, including Release builds. -#endif - -struct TestHandle { - ResultMetadataCache resultMetadata; - std::mutex* childMutex = nullptr; - - ~TestHandle() { - if (childMutex) { - bool acquired = false; - std::thread observer([&] { - acquired = childMutex->try_lock(); - if (acquired) { - childMutex->unlock(); - } - }); - observer.join(); - assert(acquired); - } - } -}; - -std::weak_ptr observedHandle; -bool failAllocation = false; -long ownersAtFailure = -1; - -static std::shared_ptr MakeMetadata() { - auto metadata = std::make_shared(); - metadata->columns.push_back({u"owned", SQL_INTEGER, 10, 0, 1}); - return metadata; -} - -static void Populate(ResultMetadataCache& cache) { - const auto snapshot = cache.snapshot(); - cache.publish(snapshot.generation, MakeMetadata()); -} - -static void TestSnapshots() { - ResultMetadataCache cache; - const auto initial = cache.snapshot(); - assert(!initial.metadata); - auto metadata = MakeMetadata(); - std::weak_ptr weak = metadata; - cache.publish(initial.generation, metadata); - auto held = cache.snapshot(); - assert(held.metadata == metadata); - cache.clear(); - assert(!cache.snapshot().metadata); - assert(cache.snapshot().generation != initial.generation); - - Populate(cache); - const auto replacement = cache.snapshot(); - cache.publish(initial.generation, metadata); - assert(cache.snapshot().metadata == replacement.metadata); - assert(held.metadata->columns.at(0).name == u"owned"); - metadata.reset(); - assert(!weak.expired()); - held.metadata.reset(); - assert(weak.expired()); -} - -static void TestFailures() { - ResultMetadataCache cache; - const SQLRETURN results[] = {SQL_SUCCESS, SQL_SUCCESS_WITH_INFO, SQL_NO_DATA, - SQL_ERROR, SQL_INVALID_HANDLE}; - for (SQLRETURN result : results) { - Populate(cache); - const auto before = cache.snapshot(); - { - ResultMetadataFailureGuard guard(cache, result); - } - const auto after = cache.snapshot(); - if (SQL_SUCCEEDED(result) || result == SQL_NO_DATA) { - assert(after.metadata == before.metadata); - assert(after.generation == before.generation); - } else { - assert(!after.metadata); - assert(after.generation != before.generation); - } - } - Populate(cache); - SQLRETURN result = SQL_SUCCESS; - try { - ResultMetadataFailureGuard guard(cache, result); - throw std::runtime_error("conversion failure"); - } catch (const std::runtime_error&) { - assert(!cache.snapshot().metadata); - } -} - -static void TestConcurrentInvalidation() { - ResultMetadataCache cache; - const auto metadata = MakeMetadata(); - std::thread invalidator([&] { - for (int i = 0; i < 1000; ++i) { - cache.clear(); - } - }); - for (int i = 0; i < 1000; ++i) { - const auto snapshot = cache.snapshot(); - cache.publish(snapshot.generation, metadata); - if (snapshot.metadata) { - assert(snapshot.metadata->columns.at(0).name == u"owned"); - } - } - invalidator.join(); - cache.clear(); - assert(!cache.snapshot().metadata); -} - -static void TestChildren() { - std::mutex childMutex; - auto first = std::make_shared(); - auto second = std::make_shared(); - auto unrelated = std::make_shared(); - std::vector> children{first, {}, second}; - Populate(first->resultMetadata); - Populate(second->resultMetadata); - Populate(unrelated->resultMetadata); - const auto held = first->resultMetadata.snapshot(); - ClearChildResultMetadata(childMutex, children); - assert(!first->resultMetadata.snapshot().metadata); - assert(!second->resultMetadata.snapshot().metadata); - assert(unrelated->resultMetadata.snapshot().metadata); - assert(held.metadata->columns.at(0).name == u"owned"); - ClearChildResultMetadata(childMutex, children); -} - -static void TestAllocationFailure() { - std::mutex childMutex; - auto owner = std::make_shared(); - observedHandle = owner; - std::vector> children{owner}; - Populate(owner->resultMetadata); - const auto before = owner->resultMetadata.snapshot(); - failAllocation = true; - try { - ClearChildResultMetadata(childMutex, children); - assert(false); - } catch (const std::bad_alloc&) { - assert(ownersAtFailure == 1); - assert(owner->resultMetadata.snapshot().metadata == before.metadata); - assert(childMutex.try_lock()); - childMutex.unlock(); - } - assert(!failAllocation); - ClearChildResultMetadata(childMutex, children); - assert(!owner->resultMetadata.snapshot().metadata); -} - -static void TestLastOwner() { - std::mutex childMutex; - auto owner = std::make_shared(); - owner->childMutex = &childMutex; - const std::weak_ptr weak = owner; - std::vector> children{owner}; - // Drop the external owner during invalidation, leaving only the helper's snapshot. - auto metadata = std::shared_ptr(new ResultMetadata, [&](auto* value) { - owner.reset(); - delete value; - }); - const auto generation = owner->resultMetadata.snapshot().generation; - owner->resultMetadata.publish(generation, std::move(metadata)); - ClearChildResultMetadata(childMutex, children); - assert(!owner && weak.expired()); -} - -int main(int argc, char** argv) { - if (argc != 2) { - std::cerr << "Expected one native metadata test case\n"; - return 2; - } - const char* name = argv[1]; - if (std::strcmp(name, "snapshots") == 0) { - TestSnapshots(); - } else if (std::strcmp(name, "failures") == 0) { - TestFailures(); - } else if (std::strcmp(name, "concurrent") == 0) { - TestConcurrentInvalidation(); - } else if (std::strcmp(name, "children") == 0) { - TestChildren(); - } else if (std::strcmp(name, "allocation") == 0) { - TestAllocationFailure(); - } else if (std::strcmp(name, "last_owner") == 0) { - TestLastOwner(); - } else { - std::cerr << "Unknown native metadata test case: " << name << '\n'; - return 2; - } - std::cout << name << " passed\n"; - return 0; -} diff --git a/tests/test_040_fetch_native_metadata.py b/tests/test_040_fetch_native_metadata.py deleted file mode 100644 index 102ad6f35..000000000 --- a/tests/test_040_fetch_native_metadata.py +++ /dev/null @@ -1,816 +0,0 @@ -"""Native fetch metadata must preserve public descriptions and result-set state.""" - -import datetime as dt -import os -from pathlib import Path -import subprocess -import sys -import textwrap -from decimal import Decimal -from uuid import UUID - -import pytest - -import mssql_python -from mssql_python import ddbc_bindings - - -@pytest.fixture -def metadata_cursor(conn_str): - with mssql_python.connect(conn_str) as connection: - with connection.cursor() as cursor: - yield cursor - - -def _query(columns, count=20): - values = ",".join(f"({i})" for i in range(1, 21)) - return ( - f"SELECT {','.join(columns)} FROM (VALUES {values}) AS source(id) " - f"WHERE id<={count} ORDER BY id" - ) - - -def _assert_rows(rows, expected): - assert [tuple(row) for row in rows] == expected - assert [[type(value) for value in row] for row in rows] == [ - [type(value) for value in row] for row in expected - ] - - -def _describe(cursor): - result = [] - assert ddbc_bindings.DDBCSQLDescribeCol(cursor.hstmt, result) == 0 - for column in result: - assert set(column) == {"ColumnName", "DataType", "ColumnSize", "DecimalDigits", "Nullable"} - assert type(column["ColumnName"]) is str - for key in ("DataType", "ColumnSize", "DecimalDigits", "Nullable"): - assert type(column[key]) is int - return result - - -@pytest.mark.parametrize("width", [3, 24]) -@pytest.mark.parametrize("size", [None, 1, 10, 1000, "varied"]) -@pytest.mark.parametrize("count", [0, 20]) -def test_fetchmany_shape_sizes_and_eof(metadata_cursor, width, size, count): - cursor = metadata_cursor - expressions = ["id", "CONVERT(NVARCHAR(30),N'row')", "CONVERT(FLOAT,id)*0.25"] - columns = [f"{expressions[i % 3]} AS c{i}" for i in range(width)] - cursor.execute(_query(columns, count)) - assert cursor.arraysize == 1 - description = cursor.description - assert all(len(column) == 7 for column in description) - assert [column[0] for column in description] == [f"c{i}" for i in range(width)] - metadata = _describe(cursor) - output = [] - iteration = 0 - while True: - fetch_size = (1, 10, 3, 1000)[iteration % 4] if size == "varied" else size - batch = cursor.fetchmany() if fetch_size is None else cursor.fetchmany(fetch_size) - assert cursor.description == description - if not batch: - break - output.extend(batch) - iteration += 1 - _assert_rows(output, [(i, "row", i * 0.25) * (width // 3) for i in range(1, count + 1)]) - assert _describe(cursor) == metadata - assert cursor.fetchmany(1) == [] - assert cursor.fetchone() is None - assert cursor.fetchall() == [] - - -@pytest.mark.parametrize("method", ["fetchmany", "fetchall", "arrow_batch"]) -def test_public_metadata_names_and_fields(metadata_cursor, method): - cursor = metadata_cursor - names = [ - "duplicate", - "duplicate", - "\u03a9\u540d", - "emoji_\U0001f600", - "bracket]name", - "x" * 128, - ] - columns = [f"CONVERT(INT,id) AS [{name.replace(']', ']]')}]" for name in names] - cursor.execute(_query(columns, 1)) - metadata = _describe(cursor) - assert metadata == [ - {"ColumnName": name, "DataType": 4, "ColumnSize": 10, "DecimalDigits": 0, "Nullable": 1} - for name in names - ] - assert [column[0] for column in cursor.description] == names - if method == "arrow_batch": - pytest.importorskip("pyarrow") - batch = cursor.arrow_batch(1) - assert batch.schema.names == names - assert [column.to_pylist() for column in batch.columns] == [[1]] * len(names) - else: - rows = cursor.fetchmany(1) if method == "fetchmany" else cursor.fetchall() - _assert_rows(rows, [(1,) * len(names)]) - assert _describe(cursor) == metadata - - -_TYPES = [ - ("INT", "7", 7), - ("SMALLINT", "-7", -7), - ("BIGINT", "2147483649", 2147483649), - ("TINYINT", "255", 255), - ("BIT", "1", True), - ("REAL", "1.5", 1.5), - ("FLOAT", "2.25", 2.25), - ("DECIMAL(20,4)", "123.4500", Decimal("123.4500")), - ("NUMERIC(28,8)", "-0.125", Decimal("-0.125")), - ("MONEY", "4.25", Decimal("4.25")), - ("DATE", "'2001-02-03'", dt.date(2001, 2, 3)), - ("TIME(7)", "'12:34:56.1234567'", dt.time(12, 34, 56, 123456)), - ("DATETIME2(7)", "'2001-02-03T12:34:56.1234567'", dt.datetime(2001, 2, 3, 12, 34, 56, 123456)), - ("DATETIME", "'2001-02-03T12:34:56'", dt.datetime(2001, 2, 3, 12, 34, 56)), - ( - "DATETIMEOFFSET(7)", - "'2001-02-03T12:34:56.1234567+05:30'", - dt.datetime(2001, 2, 3, 12, 34, 56, 123456, dt.timezone(dt.timedelta(minutes=330))), - ), - ( - "UNIQUEIDENTIFIER", - "'12345678-1234-5678-1234-567812345678'", - UUID("12345678-1234-5678-1234-567812345678"), - ), - ("VARCHAR(20)", "'ascii'", "ascii"), - ("CHAR(8)", "'ascii'", "ascii "), - ("NVARCHAR(30)", "N'\u03a9\U0001f600'", "\u03a9\U0001f600"), - ("NCHAR(5)", "N'\u03a9'", "\u03a9 "), - ("VARBINARY(10)", "0x00010200", b"\x00\x01\x02\x00"), - ("BINARY(4)", "0x00010203", b"\x00\x01\x02\x03"), - ("VARCHAR(1)", "''", ""), - ("NVARCHAR(1)", "N''", ""), -] - - -@pytest.mark.parametrize("size", [1, 10, 1000]) -def test_fetchmany_typed_nulls_and_values(metadata_cursor, size): - columns = [ - f"CASE WHEN id%3=0 THEN CAST(NULL AS {sqltype}) " - f"ELSE CAST({literal} AS {sqltype}) END AS c{i}" - for i, (sqltype, literal, _) in enumerate(_TYPES) - ] - cursor = metadata_cursor - cursor.execute(_query(columns)) - description = cursor.description - metadata = _describe(cursor) - assert len(metadata) == 24 - output = [] - while batch := cursor.fetchmany(size): - output.extend(batch) - values = tuple(value for _, _, value in _TYPES) - _assert_rows(output, [(None,) * 24 if i % 3 == 0 else values for i in range(1, 21)]) - assert cursor.description == description - assert _describe(cursor) == metadata - - -def test_reexecute_and_nextset_change_shape(metadata_cursor): - cursor = metadata_cursor - for _ in range(3): - cursor.execute("SELECT 1 AS first_name; SELECT N'new' AS second_name, 2 AS extra") - _assert_rows(cursor.fetchmany(1), [(1,)]) - assert cursor.nextset() - assert [col[0] for col in cursor.description] == ["second_name", "extra"] - _assert_rows(cursor.fetchmany(10), [("new", 2)]) - assert not cursor.nextset() - cursor.execute("SELECT CAST(3.5 AS DECIMAL(6,2)) AS replacement") - assert _describe(cursor)[0]["ColumnName"] == "replacement" - _assert_rows(cursor.fetchmany(), [(Decimal("3.50"),)]) - - -def test_converter_changes_on_execute_and_live_decoding(metadata_cursor): - cursor = metadata_cursor - connection = cursor.connection - cursor.execute(_query(["id", "CAST('ascii' AS VARCHAR(12)) AS txt"], 4)) - _assert_rows(cursor.fetchmany(1), [(1, "ascii")]) - calls = [] - - def convert(value): - calls.append(value) - return value + 100 - - connection.add_output_converter(mssql_python.SQL_INTEGER, convert) - connection.setdecoding(mssql_python.SQL_CHAR, "utf-8", mssql_python.SQL_CHAR) - cursor.execute(_query(["id", "CAST('ascii' AS VARCHAR(12)) AS txt"], 4)) - _assert_rows(cursor.fetchmany(1), [(101, "ascii")]) - assert calls == [1] - connection.setdecoding(mssql_python.SQL_CHAR, "latin1", mssql_python.SQL_CHAR) - _assert_rows(cursor.fetchmany(1), [(102, "ascii")]) - assert calls == [1, 2] - connection.remove_output_converter(mssql_python.SQL_INTEGER) - cursor.execute(_query(["id", "CAST('ascii' AS VARCHAR(12)) AS txt"], 1)) - _assert_rows(cursor.fetchmany(1), [(1, "ascii")]) - connection.setdecoding(mssql_python.SQL_CHAR) - cursor.execute(_query(["id", "CAST('ascii' AS VARCHAR(12)) AS txt"], 1)) - _assert_rows(cursor.fetchmany(10), [(1, "ascii")]) - - -@pytest.mark.parametrize("size", [1, 10]) -def test_fetchmany_lob_and_xml_typed_nulls(metadata_cursor, size): - cursor = metadata_cursor - columns = [ - "CASE WHEN id%2=0 THEN CAST(NULL AS NVARCHAR(MAX)) ELSE " - "REPLICATE(CAST(N'x' AS NVARCHAR(MAX)),9001) END AS txt", - "CASE WHEN id%2=0 THEN CAST(NULL AS VARBINARY(MAX)) ELSE " - "CAST(REPLICATE(CAST('a' AS VARCHAR(MAX)),10003) AS VARBINARY(MAX)) END AS bin", - "CASE WHEN id%2=0 THEN CAST(NULL AS XML) ELSE CAST('value' AS XML) END AS xml", - ] - cursor.execute(_query(columns, 4)) - metadata = _describe(cursor) - rows = [] - while batch := cursor.fetchmany(size): - rows.extend(batch) - values = ("x" * 9001, b"a" * 10003, "value") - _assert_rows(rows, [values, (None, None, None), values, (None, None, None)]) - assert _describe(cursor) == metadata - - -def _isolated(script, tmp_path): - environment = dict(os.environ) - root = str(Path(mssql_python.__file__).resolve().parent.parent) - environment["PYTHONPATH"] = os.pathsep.join([root, environment.get("PYTHONPATH", "")]) - result = subprocess.run( - [sys.executable, "-c", textwrap.dedent(script)], - cwd=tmp_path, - env=environment, - capture_output=True, - text=True, - timeout=45, - ) - assert result.returncode == 0, result.stdout + result.stderr - - -def test_interleaving_movement_and_variant_freshness(tmp_path): - _isolated( - """ - import os - import gc - import mssql_python as db - with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: - with connection.cursor() as cursor: - query = "SELECT id FROM (VALUES(1),(2),(3),(4),(5),(6),(7)) s(id) ORDER BY id" - for _ in range(4): - cursor.execute(query) - assert cursor.fetchmany(1)[0][0] == 1 - gc.collect() - assert cursor.fetchone()[0] == 2 - cursor.scroll(1) - assert cursor.fetchmany(1)[0][0] == 4 - cursor.skip(1) - assert [tuple(row) for row in cursor.fetchall()] == [(6,), (7,)] - cursor.execute("CREATE TABLE #metadata_variant(id INT, v SQL_VARIANT)") - cursor.execute( - "INSERT INTO #metadata_variant VALUES " - "(1,CAST('abc' AS VARCHAR(3)))," - "(2,CAST('abcdefgh' AS VARCHAR(8)))," - "(3,CAST(REPLICATE('x',30) AS VARCHAR(30)))" - ) - for method in ("fetchmany", "fetchall"): - cursor.execute("SELECT v FROM #metadata_variant ORDER BY id") - assert cursor.fetchone()[0] == "abc" - if method == "fetchmany": - assert cursor.fetchmany(1)[0][0] == "abcdefgh" - assert cursor.fetchmany(1)[0][0] == "x"*30 - else: - assert [r[0] for r in cursor.fetchall()] == ["abcdefgh", "x"*30] - """, - tmp_path, - ) - - -def test_closure_and_error_recovery(tmp_path): - _isolated( - """ - import os - import mssql_python as db - from mssql_python import Cursor, InterfaceError, ProgrammingError, DatabaseError - with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: - with connection.cursor() as cursor: - cursor.execute("SELECT 1 AS c") - assert cursor.fetchmany(0) == [] - assert cursor.fetchmany(-1) == [] - assert cursor.fetchmany(1)[0][0] == 1 - try: - cursor.execute("SELECT invalid_column FROM (VALUES(1)) t(c)") - except DatabaseError: - pass - else: - raise AssertionError("invalid query did not raise") - cursor.execute("SELECT 2 AS changed") - assert cursor.fetchmany(1)[0][0] == 2 - try: - cursor.fetchmany(1) - except ProgrammingError: - pass - else: - raise AssertionError("closed cursor did not raise") - connection = db.connect(os.environ["DB_CONNECTION_STRING"]) - cursor = Cursor(connection) - cursor.execute("SELECT 1") - connection.close() - try: - cursor.fetchmany(1) - except (InterfaceError, ProgrammingError): - pass - else: - raise AssertionError("closed connection did not raise") - cursor.close() - """, - tmp_path, - ) - - -def test_malformed_column_name_fails_before_fetch(tmp_path): - _isolated( - """ - import os - import mssql_python as db - from mssql_python import ddbc_bindings as native - with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: - with connection.cursor() as cursor: - cursor.execute( - "DECLARE @s NVARCHAR(200) = N'SELECT 1 AS [' + " - "CAST(0x00D8 AS NVARCHAR(1)) + N']'; EXEC(@s)" - ) - for operation in ( - lambda: native.DDBCSQLDescribeCol(cursor.hstmt, []), - lambda: cursor.fetchmany(1), - ): - try: - operation() - except UnicodeDecodeError: - pass - else: - raise AssertionError("malformed UTF-16 column name did not raise") - assert native.DDBCSQLFetch(cursor.hstmt) == 0 - assert native.DDBCSQLFetch(cursor.hstmt) == 100 - """, - tmp_path, - ) - - -@pytest.mark.skipif( - not hasattr(ddbc_bindings, "profiling"), reason="requires native profiling instrumentation" -) -def test_fetchmany_avoids_python_description_roundtrip(tmp_path): - _isolated( - """ - import os - import mssql_python as db - from mssql_python import ddbc_bindings as native - with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: - with connection.cursor() as cursor: - cursor.execute("SELECT id FROM (VALUES(1),(2)) s(id) ORDER BY id") - native.profiling.reset() - native.profiling.enable() - try: - metadata = [] - native.DDBCSQLDescribeCol(cursor.hstmt, metadata) - finally: - native.profiling.disable() - assert native.profiling.get_stats()["ddbc::SQLDescribeCol_wrap"]["calls"] == 1 - assert len(metadata) == 1 - native.profiling.reset() - native.profiling.enable() - try: - assert cursor.fetchmany(1)[0][0] == 1 - assert cursor.fetchmany(1)[0][0] == 2 - assert cursor.fetchmany(1) == [] - finally: - native.profiling.disable() - stats = native.profiling.get_stats() - assert stats["ddbc::FetchMany_wrap"]["calls"] == 3 - assert stats.get("ddbc::SQLDescribeCol_wrap", {}).get("calls", 0) == 0 - """, - tmp_path, - ) - - -@pytest.mark.parametrize("method", ["one", "many"]) -def test_result_metadata_prepared_reexecution(metadata_cursor, method): - cursor = metadata_cursor - statement = cursor.hstmt - query = "SELECT CAST(? AS INT) AS n, CAST(? AS NVARCHAR(30)) AS text_value" - for value in range(4): - cursor.execute(query, (value, f"value-{value}")) - assert cursor.hstmt is statement - assert cursor.is_stmt_prepared[0] - profiling = hasattr(ddbc_bindings, "profiling") - if profiling: - ddbc_bindings.profiling.reset() - ddbc_bindings.profiling.enable() - try: - rows = [cursor.fetchone()] if method == "one" else cursor.fetchmany() - finally: - if profiling: - ddbc_bindings.profiling.disable() - _assert_rows(rows, [(value, f"value-{value}")]) - if profiling: - assert ( - ddbc_bindings.profiling.get_stats()["ddbc::SQLDescribeCol::driver_call"]["calls"] - == 2 - ) - assert cursor.fetchone() is None - cursor.execute("SELECT CAST(? AS DECIMAL(8,2)) AS amount", (Decimal("3.25"),)) - _assert_rows(cursor.fetchall(), [(Decimal("3.25"),)]) - - -def test_result_metadata_catalog_replacement(metadata_cursor): - cursor = metadata_cursor - for _ in range(2): - cursor.execute("SELECT 42 AS previous_column") - _assert_rows(cursor.fetchmany(), [(42,)]) - cursor.getTypeInfo(mssql_python.SQL_INTEGER) - description = cursor.description - assert len(description) > 1 - row = cursor.fetchone() - assert row is not None and len(row) == len(description) - assert row[1] == mssql_python.SQL_INTEGER - cursor.fetchall() - cursor.execute("SELECT N'replaced' AS new_column, 5 AS extra") - _assert_rows(cursor.fetchmany(), [("replaced", 5)]) - - -def test_result_metadata_native_reset_and_replacement(tmp_path): - _isolated( - """ - import os - import mssql_python as db - from mssql_python import ddbc_bindings as native - with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: - stmt = connection._conn.alloc_statement_handle() - try: - for _ in range(3): - assert native.DDBCSQLExecDirect(stmt, "SELECT 1 AS a") in (0, 1) - rows = [] - assert native.DDBCSQLFetchMany(stmt, rows, 1) in (0, 1) - assert rows == [[1]] - assert native.DDBCSQLResetStmt(stmt) in (0, 1) - assert native.DDBCSQLExecDirect( - stmt, "SELECT CAST(2 AS BIGINT) AS b, N'new' AS c" - ) in (0, 1) - row = [] - assert native.DDBCSQLFetchOne(stmt, row) in (0, 1) - assert row == [2, "new"] - stmt._close_cursor() - finally: - stmt.free() - """, - tmp_path, - ) - - -@pytest.mark.parametrize("operation", ["commit", "rollback", "autocommit"]) -def test_result_metadata_transaction_recovery(metadata_cursor, operation): - cursor = metadata_cursor - connection = cursor.connection - cursor.execute(_query(["id"], 3)) - _assert_rows(cursor.fetchmany(), [(1,)]) - if operation == "autocommit": - connection.autocommit = True - else: - getattr(connection, operation)() - cursor.execute("SELECT CAST(5.75 AS DECIMAL(8,2)) AS changed, N'text' AS extra") - _assert_rows(cursor.fetchall(), [(Decimal("5.75"), "text")]) - - -@pytest.mark.parametrize("operation", ["commit", "rollback", "autocommit"]) -def test_result_metadata_transaction_preserved_cursor(metadata_cursor, operation): - cursor = metadata_cursor - connection = cursor.connection - info = ( - mssql_python.SQL_CURSOR_ROLLBACK_BEHAVIOR - if operation == "rollback" - else mssql_python.SQL_CURSOR_COMMIT_BEHAVIOR - ) - if connection.getinfo(info) != 2: # SQL_CB_PRESERVE - pytest.skip("Driver does not preserve cursors; cache/helper coverage is in tests/native") - cursor.execute(_query(["id"], 3)) - _assert_rows([cursor.fetchone()], [(1,)]) - if operation == "autocommit": - connection.autocommit = True - else: - getattr(connection, operation)() - profiling = hasattr(ddbc_bindings, "profiling") - if profiling: - ddbc_bindings.profiling.reset() - ddbc_bindings.profiling.enable() - try: - _assert_rows([cursor.fetchone()], [(2,)]) - _assert_rows(cursor.fetchmany(), [(3,)]) - finally: - if profiling: - ddbc_bindings.profiling.disable() - if profiling: - assert ( - ddbc_bindings.profiling.get_stats()["ddbc::SQLDescribeCol::driver_call"]["calls"] == 1 - ) - - -def test_result_metadata_arrow_interleave(tmp_path): - pytest.importorskip("pyarrow") - _isolated( - """ - import gc - import os - import mssql_python as db - with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: - with connection.cursor() as cursor: - query = ("SELECT id, CAST(id AS BIGINT) AS big FROM " - "(VALUES(1),(2),(3),(4),(5),(6)) s(id) ORDER BY id") - for _ in range(3): - cursor.execute(query) - assert tuple(cursor.fetchone()) == (1, 1) - assert [tuple(r) for r in cursor.fetchmany(2)] == [(2, 2), (3, 3)] - batch = cursor.arrow_batch(1) - assert [c.to_pylist() for c in batch.columns] == [[4], [4]] - gc.collect() - assert tuple(cursor.fetchone()) == (5, 5) - assert [tuple(r) for r in cursor.fetchall()] == [(6, 6)] - assert cursor.fetchmany() == [] - """, - tmp_path, - ) - - -@pytest.mark.parametrize("method", ["one", "many", "all"]) -def test_result_metadata_variant_type_and_size_changes(metadata_cursor, method): - cursor = metadata_cursor - cursor.execute( - "CREATE TABLE #metadata_mixed_variant (id INT, v SQL_VARIANT, txt NVARCHAR(MAX))" - ) - cursor.execute( - "INSERT INTO #metadata_mixed_variant VALUES " - "(1,CAST(NULL AS SQL_VARIANT),N'first')," - "(2,CAST(CAST('abc' AS VARCHAR(3)) AS SQL_VARIANT),NULL)," - "(3,CAST(CAST('abcdefgh' AS VARCHAR(8)) AS SQL_VARIANT),N'third')," - "(4,CAST(CAST(17 AS INT) AS SQL_VARIANT),NULL)," - "(5,CAST(CAST(3.25 AS DECIMAL(8,2)) AS SQL_VARIANT),N'fifth')," - "(6,CAST(NULL AS SQL_VARIANT),NULL)," - "(7,CAST(CAST(0x010200 AS VARBINARY(3)) AS SQL_VARIANT),N'last')" - ) - cursor.execute("SELECT v, txt FROM #metadata_mixed_variant ORDER BY id") - if method == "one": - rows = list(cursor) - elif method == "many": - rows = [] - while batch := cursor.fetchmany(): - rows.extend(batch) - else: - rows = cursor.fetchall() - _assert_rows( - rows, - [ - (None, "first"), - ("abc", None), - ("abcdefgh", "third"), - (17, None), - (Decimal("3.25"), "fifth"), - (None, None), - (b"\x01\x02\x00", "last"), - ], - ) - - -def test_result_metadata_one_then_malformed_name_many_does_not_advance(tmp_path): - _isolated( - """ - import os - import mssql_python as db - from mssql_python import ddbc_bindings as native - with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: - with connection.cursor() as cursor: - cursor.execute( - "DECLARE @s NVARCHAR(300) = N'SELECT id AS [' + " - "CAST(0x00D8 AS NVARCHAR(1)) + " - "N'] FROM (VALUES(1),(2),(3)) s(id) ORDER BY id'; EXEC(@s)" - ) - # The low-level row path never decoded a supported column's name. - row = [] - assert native.DDBCSQLFetchOne(cursor.hstmt, row) in (0, 1) - assert row == [1] - for _ in range(2): - try: - native.DDBCSQLFetchMany(cursor.hstmt, [], 1) - except UnicodeDecodeError: - pass - else: - raise AssertionError("many accepted the malformed column name") - row = [] - profiling = hasattr(native, "profiling") - if profiling: - native.profiling.reset() - native.profiling.enable() - try: - assert native.DDBCSQLFetchOne(cursor.hstmt, row) in (0, 1) - finally: - if profiling: - native.profiling.disable() - assert row == [2] - if profiling: - assert native.profiling.get_stats()["ddbc::SQLDescribeCol::driver_call"]["calls"] == 1 - cursor.execute("SELECT 4 AS valid_name, N'recovered' AS text_value") - assert tuple(cursor.fetchone()) == (4, "recovered") - assert cursor.fetchmany() == [] - """, - tmp_path, - ) - - -@pytest.mark.skipif( - not hasattr(ddbc_bindings, "profiling"), reason="requires actual ODBC call instrumentation" -) -@pytest.mark.parametrize("method", ["one", "many"]) -def test_result_metadata_actual_description_counts(tmp_path, method): - _isolated( - f""" - import os - from decimal import Decimal - import mssql_python as db - from mssql_python import ddbc_bindings as native - with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: - with connection.cursor() as cursor: - columns = ",".join(f"id AS c{{i}}" for i in range(24)) - query = ("WITH n AS (SELECT TOP(10000) ROW_NUMBER() OVER " - "(ORDER BY a.object_id,b.object_id) AS id " - "FROM sys.all_objects a CROSS JOIN sys.all_objects b) " - f"SELECT {{columns}} FROM n ORDER BY id") - cursor.execute(query) - native.profiling.reset() - native.profiling.enable() - try: - for value in range(1, 10001): - row = cursor.fetchone() if {method!r} == "one" else cursor.fetchmany()[0] - assert tuple(row) == (value,) * 24 - assert cursor.fetchone() is None - assert cursor.fetchmany() == [] - finally: - native.profiling.disable() - stats = native.profiling.get_stats() - assert stats["ddbc::SQLDescribeCol::driver_call"]["calls"] == 24, stats - assert stats.get("ddbc::SQLDescribeCol_wrap", {{}}).get("calls", 0) == 0 - native.profiling.reset() - native.profiling.enable() - try: - metadata = [] - assert native.DDBCSQLDescribeCol(cursor.hstmt, metadata) in (0, 1) - finally: - native.profiling.disable() - assert len(metadata) == 24 - assert native.profiling.get_stats()["ddbc::SQLDescribeCol::driver_call"]["calls"] == 24 - cursor.execute( - "SELECT CAST(3 AS INT) AS changed, CAST(N'x' AS NVARCHAR(1)) AS text_value; " - "SELECT CAST(7.25 AS DECIMAL(8,2)) AS amount, " - "CAST(N'next long value' AS NVARCHAR(40)) AS name" - ) - assert tuple(cursor.fetchone()) == (3, "x") - assert cursor.nextset() - native.profiling.reset() - native.profiling.enable() - try: - assert tuple(cursor.fetchmany()[0]) == (Decimal("7.25"), "next long value") - assert cursor.fetchmany() == [] - finally: - native.profiling.disable() - assert native.profiling.get_stats()["ddbc::SQLDescribeCol::driver_call"]["calls"] == 2 - """, - tmp_path, - ) - - -@pytest.mark.skipif( - not hasattr(ddbc_bindings, "profiling"), reason="requires actual ODBC call instrumentation" -) -@pytest.mark.parametrize("method", ["one", "many", "all"]) -def test_result_metadata_variant_descriptions_remain_per_row(tmp_path, method): - _isolated( - f""" - import os - import mssql_python as db - from mssql_python import ddbc_bindings as native - with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: - with connection.cursor() as cursor: - cursor.execute( - "SELECT id, v FROM (VALUES " - "(1,CAST(NULL AS SQL_VARIANT))," - "(2,CAST('abc' AS SQL_VARIANT))," - "(3,CAST(17 AS SQL_VARIANT))) s(id,v) ORDER BY id" - ) - native.profiling.reset() - native.profiling.enable() - try: - if {method!r} == "one": - rows = list(cursor) - elif {method!r} == "all": - rows = cursor.fetchall() - else: - rows = [] - while batch := cursor.fetchmany(): - rows.extend(batch) - finally: - native.profiling.disable() - assert [tuple(row) for row in rows] == [(1,None),(2,"abc"),(3,17)] - stats = native.profiling.get_stats() - expected = 4 if {method!r} == "one" else 5 - assert stats["ddbc::SQLDescribeCol::driver_call"]["calls"] == expected, stats - assert stats["ddbc::sql_variant::null_probe"]["calls"] == 3, stats - assert stats["ddbc::sql_variant::subtype"]["calls"] == 2, stats - """, - tmp_path, - ) - - -@pytest.mark.parametrize("method", ["one", "many", "all"]) -def test_result_metadata_all_null_rows(tmp_path, method): - _isolated( - f""" - import os - import mssql_python as db - from mssql_python import ddbc_bindings as native - with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: - with connection.cursor() as cursor: - cursor.execute("SELECT CAST(NULL AS INT) AS scalar_null") - scalar_null = [] - scalar_status = native.DDBCSQLFetchOne(cursor.hstmt, scalar_null) - diagnostics = native.DDBCSQLGetAllDiagRecords(cursor.hstmt) - assert scalar_status == 0, (scalar_status, diagnostics) - assert scalar_null == [None] - assert diagnostics == [] - values = ",".join(f"({{i}})" for i in range(1, 16)) - cursor.execute( - "SELECT CASE WHEN id%7=0 THEN NULL ELSE id END AS c0," - "CASE WHEN id%7=0 THEN CAST(NULL AS SQL_VARIANT) " - "WHEN id%3=0 THEN CAST(id AS SQL_VARIANT) " - "WHEN id%3=1 THEN CAST(N'row-'+CONVERT(NVARCHAR(12),id) AS SQL_VARIANT) " - "ELSE CAST(CONVERT(FLOAT,id)*0.25 AS SQL_VARIANT) END AS c1 " - f"FROM (VALUES{{values}}) s(id) ORDER BY id" - ) - profiling = hasattr(native, "profiling") - if profiling: - native.profiling.reset() - native.profiling.enable() - try: - if {method!r} == "one": - rows = list(cursor) - elif {method!r} == "all": - rows = cursor.fetchall() - else: - rows = [] - while batch := cursor.fetchmany(): - rows.extend(batch) - finally: - if profiling: - native.profiling.disable() - expected = [ - (None,None) if i%7==0 else (i,(i,f"row-{{i}}",i*0.25)[i%3]) - for i in range(1,16) - ] - assert [tuple(row) for row in rows] == expected - assert [[type(value) for value in row] for row in rows] == [ - [type(value) for value in row] for row in expected - ] - if profiling: - stats = native.profiling.get_stats() - expected_describes = 16 if {method!r} == "one" else 17 - assert stats["ddbc::SQLDescribeCol::driver_call"]["calls"] == expected_describes, stats - assert stats["ddbc::sql_variant::null_probe"]["calls"] == 15 - assert stats["ddbc::sql_variant::subtype"]["calls"] == 13 - """, - tmp_path, - ) - - -def test_result_metadata_odbc_error_invalidates(tmp_path): - _isolated( - """ - import os - import mssql_python as db - from mssql_python import ddbc_bindings as native - with db.connect(os.environ["DB_CONNECTION_STRING"]) as connection: - with connection.cursor() as cursor: - cursor.execute("SELECT id FROM (VALUES(1),(2),(3)) s(id) ORDER BY id") - row = [] - assert native.DDBCSQLFetchOne(cursor.hstmt, row) in (0, 1) - assert row == [1] - assert native.DDBCSQLGetData( - cursor.hstmt, 2, [], "utf-16le", "utf-16le", db.SQL_WCHAR - ) == -1 - diagnostics = native.DDBCSQLGetAllDiagRecords(cursor.hstmt) - assert any("07009" in state for state, _ in diagnostics), diagnostics - profiling = hasattr(native, "profiling") - if profiling: - native.profiling.reset() - native.profiling.enable() - try: - row = [] - assert native.DDBCSQLFetchOne(cursor.hstmt, row) in (0, 1) - assert row == [2] - finally: - if profiling: - native.profiling.disable() - if profiling: - assert native.profiling.get_stats()["ddbc::SQLDescribeCol::driver_call"]["calls"] == 1 - """, - tmp_path, - ) From be3efbb67181da006fe820659aa222e3f5ff9ee0 Mon Sep 17 00:00:00 2001 From: Jahnvi Thakkar Date: Thu, 24 Sep 2026 12:08:15 +0530 Subject: [PATCH 7/7] CHORE: Strengthen existing cache lifetime assertions Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/test_004_cursor.py | 21 +++++++++++++++---- ...st_016_connection_invalidation_segfault.py | 7 ++++--- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/tests/test_004_cursor.py b/tests/test_004_cursor.py index 40ea4d09d..f30a24202 100644 --- a/tests/test_004_cursor.py +++ b/tests/test_004_cursor.py @@ -2961,13 +2961,26 @@ def test_executemany_DecimalMix_List(cursor, db_connection): def test_nextset(cursor): - """Test nextset""" - cursor.execute("SELECT * FROM #pytest_all_data_types WHERE id = 1;") - assert cursor.nextset() is False, "Nextset should return False" + """Test metadata invalidation when re-executing and skipping unread results.""" + cursor.execute( + "SELECT id AS first_id FROM #pytest_all_data_types WHERE id IN (1, 2) ORDER BY id;" + ) + first_row = cursor.fetchmany(1)[0] + assert first_row.first_id == 1 + # Equal column counts must not hide changed types or names. cursor.execute( - "SELECT * FROM #pytest_all_data_types WHERE id = 2; SELECT * FROM #pytest_all_data_types WHERE id = 3;" + "SELECT CAST(id AS NVARCHAR(10)) AS second_id FROM #pytest_all_data_types " + "WHERE id IN (2, 3) ORDER BY id; " + "SELECT id AS third_id FROM #pytest_all_data_types WHERE id = 3;" ) + second_row = cursor.fetchmany(1)[0] + assert second_row.second_id == "2" assert cursor.nextset() is True, "Nextset should return True" + assert cursor.fetchmany(1)[0].third_id == 3 + assert cursor.description[0][0] == "third_id" + assert first_row.first_id == 1 + assert second_row.second_id == "2" + assert cursor.nextset() is False, "Nextset should return False" def test_delete_table(cursor, db_connection): diff --git a/tests/test_016_connection_invalidation_segfault.py b/tests/test_016_connection_invalidation_segfault.py index 4ae07306a..e3c7e760d 100644 --- a/tests/test_016_connection_invalidation_segfault.py +++ b/tests/test_016_connection_invalidation_segfault.py @@ -49,10 +49,11 @@ def test_connection_invalidation_with_multiple_cursors(conn_str): # Create multiple cursors with statement handles cursors = [] + rows = [] for i in range(5): cursor = conn.cursor() cursor.execute("SELECT 1 AS id, 'test' AS name") - cursor.fetchall() # Fetch results to complete the query + rows.append(cursor.fetchmany(1)[0]) cursors.append(cursor) # Close connection without explicitly closing cursors first @@ -62,10 +63,10 @@ def test_connection_invalidation_with_multiple_cursors(conn_str): # Force garbage collection to trigger cursor cleanup # This is where the segfault would occur without the fix cursors = None + del cursor gc.collect() - # If we reach here without crashing, the fix is working - assert True + assert [(row.id, row.name) for row in rows] == [(1, "test")] * 5 def test_connection_invalidation_without_cursor_close(conn_str):