From 20ba690c7175f5592a636d2fad46a01f20a04988 Mon Sep 17 00:00:00 2001 From: Amin Ghadersohi Date: Sat, 26 Sep 2026 07:12:33 +0000 Subject: [PATCH 1/2] Reflection: Report column nullability, and filter views by schema - get_columns reads information_schema.columns.is_nullable instead of reporting every column, primary keys included, as nullable. - get_view_names filters by the requested schema; the schema was passed as a parameter the query never used, so views of all schemas came back. - has_table reports views as well as tables, matching SQLAlchemy 2.0's Inspector.has_table, and get_view_definition is implemented. - Reflection honours the schema URL parameter that get_table_names already uses, instead of falling back to doc. --- CHANGES.md | 11 ++++ src/sqlalchemy_cratedb/dialect.py | 52 ++++++++++++++----- tests/dialect_test.py | 85 +++++++++++++++++++++++++++++-- 3 files changed, 133 insertions(+), 15 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 22b2f830..f7e56b7a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,17 @@ # Changelog ## Unreleased +- Reflection: `get_columns` reports `nullable` from `information_schema.columns`, + so primary key and `NOT NULL` columns reflect as `nullable=False` instead of + always `True` +- Reflection: `get_view_names` lists only the views of the requested schema; + it previously passed the schema as an unused parameter and listed the views + of every schema +- Reflection: `has_table` reports views as well as tables, like SQLAlchemy + 2.0's `Inspector.has_table`, and `get_view_definition` is implemented +- Reflection: `get_columns`, `get_pk_constraint`, `get_view_names` and + `get_view_definition` use the `schema` URL parameter that `get_table_names` + already honours, instead of falling back to `doc` - Types: Fixed `CLOB`, `NCHAR`, `NVARCHAR`, `DATETIME`, and `DATE` compiling to type names CrateDB cannot parse. They now map to `STRING`, `CHAR`, `VARCHAR`, and `TIMESTAMP` respectively, matching their generic lower-case counterparts diff --git a/src/sqlalchemy_cratedb/dialect.py b/src/sqlalchemy_cratedb/dialect.py index 465b51b5..9738553a 100644 --- a/src/sqlalchemy_cratedb/dialect.py +++ b/src/sqlalchemy_cratedb/dialect.py @@ -25,7 +25,7 @@ from sqlalchemy import types as sqltypes from sqlalchemy.engine import default, reflection -from sqlalchemy.exc import SQLAlchemyError +from sqlalchemy.exc import NoSuchTableError, SQLAlchemyError from sqlalchemy.sql import functions from sqlalchemy.util import asbool, to_list @@ -386,7 +386,14 @@ def has_schema(self, connection, schema, **kw): return schema in self.get_schema_names(connection, **kw) def has_table(self, connection, table_name, schema=None, **kw): - return table_name in self.get_table_names(connection, schema=schema, **kw) + # Like SQLAlchemy 2.0's `Inspector.has_table`, report views as well. + cursor = connection.exec_driver_sql( + "SELECT table_name FROM information_schema.tables " + "WHERE {0} = ? AND table_name = ? " + "AND table_type IN ('BASE TABLE', 'VIEW')".format(self.schema_column), + (self._reflection_schema(connection, schema), table_name), + ) + return table_name in [row[0] for row in cursor.fetchall()] @reflection.cache def get_schema_names(self, connection, **kw): @@ -412,24 +419,38 @@ def get_table_names(self, connection, schema=None, **kw): def get_view_names(self, connection, schema=None, **kw): cursor = connection.exec_driver_sql( "SELECT table_name FROM information_schema.views " - "ORDER BY table_name ASC, {0} ASC".format(self.schema_column), - (schema or self.default_schema_name,), + "WHERE {0} = ? " + "ORDER BY table_name ASC".format(self.schema_column), + (self._reflection_schema(connection, schema),), ) return [row[0] for row in cursor.fetchall()] + @reflection.cache + def get_view_definition(self, connection, view_name, schema=None, **kw): + cursor = connection.exec_driver_sql( + "SELECT view_definition FROM information_schema.views " + "WHERE table_name = ? AND {0} = ?".format(self.schema_column), + (view_name, self._reflection_schema(connection, schema)), + ) + row = cursor.fetchone() + if row is None: + raise NoSuchTableError(view_name) + return row[0] + @reflection.cache def get_columns(self, connection, table_name, schema=None, **kw): query = ( - "SELECT column_name, data_type " + "SELECT column_name, data_type, is_nullable " "FROM information_schema.columns " "WHERE table_name = ? AND {0} = ? " - "AND column_name !~ ?".format(self.schema_column) + "AND column_name !~ ? " + "ORDER BY ordinal_position".format(self.schema_column) ) cursor = connection.exec_driver_sql( query, ( table_name, - schema or self.default_schema_name, + self._reflection_schema(connection, schema), r"(.*)\[\'(.*)\'\]", ), # regex to filter subscript ) @@ -466,7 +487,9 @@ def result_fun(result): rows = result.fetchone() return set(rows[0] if rows else []) - pk_result = engine.exec_driver_sql(query, (table_name, schema or self.default_schema_name)) + pk_result = engine.exec_driver_sql( + query, (table_name, self._reflection_schema(engine, schema)) + ) pks = result_fun(pk_result) return {"constrained_columns": sorted(pks), "name": "PRIMARY KEY"} @@ -489,12 +512,17 @@ def _create_column_info(self, row): return { "name": row[0], "type": self._resolve_type(row[1]), - # In Crate every column is nullable except PK - # Primary Key Constraints are not nullable anyway, no matter what - # we return here, so it's fine to return always `True` - "nullable": True, + # Primary key and `NOT NULL` columns report `is_nullable = false`. + "nullable": bool(row[2]), } + def _reflection_schema(self, connection, schema): + """ + The schema to reflect: the explicit argument, else the URL's `schema` + query parameter that `get_table_names` lists from, else the default. + """ + return schema or self._get_effective_schema_name(connection) or self.default_schema_name + def _resolve_type(self, type_): return TYPES_MAP.get(type_, sqltypes.UserDefinedType) diff --git a/tests/dialect_test.py b/tests/dialect_test.py index 58853766..fc7e4794 100644 --- a/tests/dialect_test.py +++ b/tests/dialect_test.py @@ -124,9 +124,35 @@ def test_get_view_names(self): eq_( self.executed_statement, "SELECT table_name FROM information_schema.views " - "ORDER BY table_name ASC, table_schema ASC", + "WHERE table_schema = ? ORDER BY table_name ASC", ) + def test_get_view_definition(self): + self.init_mock() + self.fake_cursor.fetchone = MagicMock(return_value=["SELECT 1"]) + insp = inspect(self.session.bind) + eq_(insp.get_view_definition("v1", schema="doc"), "SELECT 1") + in_("SELECT view_definition FROM information_schema.views", self.executed_statement) + + def test_get_view_definition_missing(self): + self.init_mock() + self.fake_cursor.fetchone = MagicMock(return_value=None) + insp = inspect(self.session.bind) + with self.assertRaises(sa.exc.NoSuchTableError): + insp.get_view_definition("missing", schema="doc") + + def test_get_columns_nullable(self): + self.init_mock( + return_value=[["id", "integer", False], ["code", "integer", False], ["x", "text", True]] + ) + insp = inspect(self.session.bind) + columns = insp.get_columns("t", schema="doc") + eq_( + [(c["name"], c["nullable"]) for c in columns], + [("id", False), ("code", False), ("x", True)], + ) + in_("is_nullable", self.executed_statement) + @skipIf(SA_VERSION < SA_1_4, "Inspector.has_table only available on SQLAlchemy>=1.4") def test_has_table(self): self.init_mock(return_value=[["foo"], ["bar"]]) @@ -135,10 +161,20 @@ def test_has_table(self): eq_( self.executed_statement, "SELECT table_name FROM information_schema.tables " - "WHERE table_schema = ? AND table_type = 'BASE TABLE' " - "ORDER BY table_name ASC, table_schema ASC", + "WHERE table_schema = ? AND table_name = ? " + "AND table_type IN ('BASE TABLE', 'VIEW')", ) + @skipIf(SA_VERSION < SA_1_4, "Inspector.has_table only available on SQLAlchemy>=1.4") + def test_has_table_view(self): + # Not a base table, but a view. + self.fake_cursor.rowcount = 1 + self.fake_cursor.description = (("foo", None, None, None, None, None, None),) + self.fake_cursor.fetchall = MagicMock(return_value=[["v1"]]) + insp = inspect(self.session.bind) + is_true(insp.has_table("v1")) + in_("'VIEW'", self.executed_statement) + @skipIf(SA_VERSION < SA_2_0, "Inspector.has_schema only available on SQLAlchemy>=2.0") def test_has_schema(self): self.init_mock( @@ -163,3 +199,46 @@ def test_default_paramstyle_is_pyformat(self): so that SQLAlchemy generates %(name)s placeholders. """ eq_(self.engine.dialect.default_paramstyle, "pyformat") + + +@patch("crate.client.connection.Cursor", FakeCursor) +class SqlAlchemyDialectUrlSchemaTest(TestCase): + """ + Reflection uses the `schema` URL parameter that `get_table_names` lists from. + """ + + def setUp(self): + self.fake_cursor = MagicMock(name="fake_cursor") + FakeCursor.return_value = self.fake_cursor + self.parameters = [] + + def execute(query, parameters=None, *args, **kwargs): + self.parameters.append(parameters) + return self.fake_cursor + + self.fake_cursor.execute = execute + self.fake_cursor.rowcount = 1 + self.fake_cursor.description = (("foo", None, None, None, None, None, None),) + self.engine = sa.create_engine("crate://?schema=sales") + self.engine.connect().close() + self.engine.dialect.server_version_info = (5, 10, 0) + + def test_get_columns(self): + self.fake_cursor.fetchall = MagicMock(return_value=[["id", "integer", False]]) + inspect(self.engine).get_columns("t") + eq_(self.parameters[-1][:2], ("t", "sales")) + + def test_get_pk_constraint(self): + self.fake_cursor.fetchall = MagicMock(return_value=[["id"]]) + eq_(inspect(self.engine).get_pk_constraint("t")["constrained_columns"], ["id"]) + eq_(self.parameters[-1], ("t", "sales")) + + def test_get_view_names(self): + self.fake_cursor.fetchall = MagicMock(return_value=[["v1"]]) + eq_(inspect(self.engine).get_view_names(), ["v1"]) + eq_(self.parameters[-1], ("sales",)) + + def test_explicit_schema_wins(self): + self.fake_cursor.fetchall = MagicMock(return_value=[["id"]]) + inspect(self.engine).get_pk_constraint("t", schema="other") + eq_(self.parameters[-1], ("t", "other")) From ce37d81cdd8ef68922cc2c8209a5357dc6a7f22d Mon Sep 17 00:00:00 2001 From: Amin Ghadersohi Date: Wed, 30 Sep 2026 04:50:57 +0000 Subject: [PATCH 2/2] Reflection: Read is_nullable as boolean or 'YES'/'NO' text CrateDB up to 6.0 reports information_schema.columns.is_nullable as a BOOLEAN; later versions report 'YES'/'NO', and bool('NO') is True, so primary key and NOT NULL columns reflected as nullable. Accept both. Give the mocked get_columns cursor a three-column description and use crate:///?schema=sales, so the tests also pass on SQLAlchemy 1.3/1.4. Add a live reflection test. --- src/sqlalchemy_cratedb/dialect.py | 15 +++++++++++++-- tests/dialect_test.py | 26 +++++++++++++++++++++++++- tests/reflection_test.py | 25 +++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 tests/reflection_test.py diff --git a/src/sqlalchemy_cratedb/dialect.py b/src/sqlalchemy_cratedb/dialect.py index 9738553a..d3d1345e 100644 --- a/src/sqlalchemy_cratedb/dialect.py +++ b/src/sqlalchemy_cratedb/dialect.py @@ -512,10 +512,21 @@ def _create_column_info(self, row): return { "name": row[0], "type": self._resolve_type(row[1]), - # Primary key and `NOT NULL` columns report `is_nullable = false`. - "nullable": bool(row[2]), + "nullable": self._is_nullable(row[2]), } + @staticmethod + def _is_nullable(value): + """ + Primary key and `NOT NULL` columns are not nullable. CrateDB up to 6.0 + reports `information_schema.columns.is_nullable` as a `BOOLEAN`; later + versions report the SQL standard's `'YES'` / `'NO'` text, and + `bool('NO')` is `True`. + """ + if isinstance(value, str): + return value.strip().upper() != "NO" + return bool(value) + def _reflection_schema(self, connection, schema): """ The schema to reflect: the explicit argument, else the URL's `schema` diff --git a/tests/dialect_test.py b/tests/dialect_test.py index fc7e4794..d160e5c3 100644 --- a/tests/dialect_test.py +++ b/tests/dialect_test.py @@ -39,6 +39,12 @@ FakeCursor = MagicMock(name="FakeCursor", spec=Cursor) +# Cursor description of the `get_columns` query. +COLUMNS_DESCRIPTION = tuple( + (name, None, None, None, None, None, None) + for name in ("column_name", "data_type", "is_nullable") +) + @patch("crate.client.connection.Cursor", FakeCursor) class SqlAlchemyDialectTest(TestCase): @@ -145,6 +151,8 @@ def test_get_columns_nullable(self): self.init_mock( return_value=[["id", "integer", False], ["code", "integer", False], ["x", "text", True]] ) + # One description entry per selected column, or SQLAlchemy 1.x truncates the rows. + self.fake_cursor.description = COLUMNS_DESCRIPTION insp = inspect(self.session.bind) columns = insp.get_columns("t", schema="doc") eq_( @@ -153,6 +161,21 @@ def test_get_columns_nullable(self): ) in_("is_nullable", self.executed_statement) + def test_get_columns_nullable_text(self): + """ + CrateDB after 6.0 reports `is_nullable` as `'YES'` / `'NO'` text. + """ + self.init_mock( + return_value=[["id", "integer", "NO"], ["code", "integer", "NO"], ["x", "text", "YES"]] + ) + self.fake_cursor.description = COLUMNS_DESCRIPTION + insp = inspect(self.session.bind) + columns = insp.get_columns("t", schema="doc") + eq_( + [(c["name"], c["nullable"]) for c in columns], + [("id", False), ("code", False), ("x", True)], + ) + @skipIf(SA_VERSION < SA_1_4, "Inspector.has_table only available on SQLAlchemy>=1.4") def test_has_table(self): self.init_mock(return_value=[["foo"], ["bar"]]) @@ -219,11 +242,12 @@ def execute(query, parameters=None, *args, **kwargs): self.fake_cursor.execute = execute self.fake_cursor.rowcount = 1 self.fake_cursor.description = (("foo", None, None, None, None, None, None),) - self.engine = sa.create_engine("crate://?schema=sales") + self.engine = sa.create_engine("crate:///?schema=sales") self.engine.connect().close() self.engine.dialect.server_version_info = (5, 10, 0) def test_get_columns(self): + self.fake_cursor.description = COLUMNS_DESCRIPTION self.fake_cursor.fetchall = MagicMock(return_value=[["id", "integer", False]]) inspect(self.engine).get_columns("t") eq_(self.parameters[-1][:2], ("t", "sales")) diff --git a/tests/reflection_test.py b/tests/reflection_test.py new file mode 100644 index 00000000..83100910 --- /dev/null +++ b/tests/reflection_test.py @@ -0,0 +1,25 @@ +import sqlalchemy as sa + + +def test_get_columns_nullable_live(cratedb_service): + """ + Primary key and `NOT NULL` columns reflect as not nullable on a real server, + whichever type the server reports `information_schema.columns.is_nullable` as. + """ + engine = cratedb_service.database.engine + with engine.begin() as conn: + conn.exec_driver_sql("DROP TABLE IF EXISTS testdrive.nullable_reflection") + conn.exec_driver_sql( + "CREATE TABLE testdrive.nullable_reflection " + "(id INT PRIMARY KEY, code INT NOT NULL, x TEXT)" + ) + try: + columns = sa.inspect(engine).get_columns("nullable_reflection", schema="testdrive") + assert [(c["name"], c["nullable"]) for c in columns] == [ + ("id", False), + ("code", False), + ("x", True), + ] + finally: + with engine.begin() as conn: + conn.exec_driver_sql("DROP TABLE IF EXISTS testdrive.nullable_reflection")