Skip to content

Keep SQL Server integer columns exact when they contain NULLs - #11

Open
maharanay22 wants to merge 1 commit into
mindsdb:mainfrom
maharanay22:fix-mssql-nullable-int-types
Open

maharanay22 wants to merge 1 commit into
mindsdb:mainfrom
maharanay22:fix-mssql-nullable-int-types

Conversation

@maharanay22

Copy link
Copy Markdown

Description

The SQL Server handler builds the result DataFrame in _make_table_response() without any dtypes and then guesses the column types from the pandas dtype. When an int, bigint or bit column contains a NULL, pandas falls back to float64 (or object for bit), and that causes three problems:

  1. Large integers lose precision. A bigint ID of 9007199254740993 comes back as 9007199254740992.0, which is a different ID
  2. Small integers turn into floats, so a quantity of 3 comes back as 3.0
  3. The column is reported as FLOAT (or DOUBLE for bit) instead of INT / TINYINT

This happens with both the native pymssql connection and the ODBC (pyodbc) connection. The ODBC path has a second problem: it ignores cursor.description[1], which pyodbc fills with the exact Python type of each column. Because it only looks at the pandas dtype, DECIMAL / NUMERIC columns are reported as TEXT, DATE columns as TEXT (or DATETIME, depending on whether the first row is NULL) and TIME columns as TEXT

This change does two things:

  1. Integer and bit columns that pandas couldn't keep as int or bool because of NULLs are rebuilt from the raw values with the pandas nullable Int64 and boolean dtypes. This only happens when every non-NULL value in the column is an int (or every one is a bool). Columns without NULLs keep their current dtypes, and float, decimal and other columns are left alone. The PostgreSQL handler already returns nullable Int64 / boolean columns, so this matches what MindsDB gets from it
  2. For ODBC connections, the MySQL type comes from the Python type in cursor.description when pyodbc provides one (int, float, Decimal, date, time, datetime, str, bytes, bool). The existing dtype-based guess is still used when the type is missing

Fixes: no linked issue, because issues are turned off for this repository. The bug is described in full above

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Verification Process

To ensure the changes are working as expected:

  • Test Location: tests/unit/handlers/test_mssql.py, new tests TestMSSQLHandler::test_types_casting_with_nulls (pymssql) and TestMSSQLHandlerODBC::test_odbc_types_from_cursor_description (ODBC)
  • Verification Steps: call _make_table_response() with cursor descriptions and rows shaped like what pymssql and pyodbc return for a bigint / int column with a NULL, plus ODBC decimal, date, bit and nvarchar columns

Before (current main):

ODBC types: [('amount', 'TEXT'), ('qty', 'FLOAT'), ('d', 'TEXT'), ('flag', 'TINYINT'), ('note', 'TEXT')]
pymssql: ['FLOAT', 'FLOAT'] [{'id': 9007199254740992.0, 'qty': 3.0}, {'id': nan, 'qty': nan}]   expected id 9007199254740993
odbc:    ['FLOAT'] [{'id': 9007199254740992.0}, {'id': nan}]

After:

ODBC types: [('amount', 'DECIMAL'), ('qty', 'INT'), ('d', 'DATE'), ('flag', 'TINYINT'), ('note', 'TEXT')]
pymssql: ['INT', 'INT'] [{'id': 9007199254740993, 'qty': 3}, {'id': None, 'qty': None}]   expected id 9007199254740993
odbc:    ['INT'] [{'id': 9007199254740993}, {'id': None}]

Unit tests. The two new tests fail on main and pass with the fix, and the existing tests still pass:

# main
FAILED tests/unit/handlers/test_mssql.py::TestMSSQLHandler::test_types_casting_with_nulls
FAILED tests/unit/handlers/test_mssql.py::TestMSSQLHandlerODBC::test_odbc_types_from_cursor_description
E       AssertionError: Lists differ: [<MYSQL_DATA_TYPE.FLOAT: 'FLOAT'>, ...] != [<MYSQL_DATA_TYPE.INT: 'INT'>, ...]
2 failed, 31 passed, 16 subtests passed

# this branch
33 passed, 16 subtests passed

I didn't have a SQL Server instance to test against, so the rows and descriptions in the tests follow the shapes pymssql and pyodbc return. ruff check and ruff format --check (0.11.11) pass on the changed files. No dependencies changed

Additional Media:

  • I have attached a brief loom video or screenshots showcasing the new functionality or change.

I didn't record a video or take screenshots. The before and after terminal output above shows the change instead

Checklist:

  • My code follows the style guidelines(PEP 8) of MindsDB.
  • I have appropriately commented on my code, especially in complex areas.
  • Necessary documentation updates are either made or tracked in issues.
  • Relevant unit and integration tests are updated or added.

No documentation change is needed

I used an AI assistant to help investigate this and draft the change and tests, and I reviewed and tested it myself

The SQL Server handler built the result DataFrame without dtypes, so a
NULL in an int, bigint or bit column made pandas fall back to float64 or
object. Large IDs lost precision (9007199254740993 came back as
9007199254740992.0), small values turned into floats (3 -> 3.0) and the
columns were reported as FLOAT or DOUBLE instead of INT.

Rebuild integer and bit columns that contain NULLs with the pandas
nullable Int64 and boolean dtypes. For ODBC connections, also take the
column type from cursor.description, which pyodbc fills with the Python
type of the column, so DECIMAL, DATE and TIME columns are no longer
reported as TEXT or DATETIME.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant