diff --git a/CHANGELOG.md b/CHANGELOG.md index d9c58ebf3..b9db6a0a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ # Unreleased - Fix inline parameters (`use_inline_params=True`) so a sequence directly after `IN` renders as a list again: `x IN %(ids)s` renders as `x IN (1,2,3)` instead of `x IN ARRAY(1,2,3)`, which Spark rejects. Inside the list, tuples render as rows for a multi-column `IN`, and a `set` or `frozenset` is accepted again. Sequences anywhere else still render as `ARRAY(...)` ([#629](https://github.com/databricks/databricks-sql-python/issues/629)) - Speed up Thrift response handling: read replies in 1 MiB chunks and decode them with Thrift's C-accelerated protocol, and stop stringifying every request/response for the default no-op debug logger. Wide result schemas, metadata calls with many rows, and inline Arrow results benefit most. +- **Behavior change:** `escape_catalog_name` now defaults to `True`, so `_` in `catalog_name` is no longer a single-character wildcard in Thrift `schemas()`/`tables()`/`columns()` or kernel `tables()`. `%` still works as a wildcard, and `escape_catalog_name=False` restores the old behavior. This follows the JDBC/ODBC metadata spec, which treats the catalog as an exact name, and matches the Databricks JDBC driver's default since v3.3.1. These calls no longer return rows from other catalogs whose names differ only at a `_` position (e.g. `comparator-tests` for `comparator_tests`), and on catalogs whose names contain `_` they are about 170 ms faster because the server no longer scans every catalog. # 4.9.0 (2026-11-04) - Upgrade Databricks SQL Kernel to 1.4.0, which keeps Lakehouse Real-Time session affinity and flushes shared telemetry only after the final session closes. diff --git a/CONNECTION_PARAMETERS.md b/CONNECTION_PARAMETERS.md index 063176cc3..c6dde7ee0 100644 --- a/CONNECTION_PARAMETERS.md +++ b/CONNECTION_PARAMETERS.md @@ -167,7 +167,7 @@ to change without notice. | `schema` | `str` | ✅ | ✅ | `None` | Initial schema for the session (DBR 9.0+). | | `query_tags` | `Dict[str, Optional[str]]` | ✅ | ✅ | `None` | Connection-level key/value tags serialized into the reserved `QUERY_TAGS` session configuration on both backends. Cursor-level `query_tags` are applied per statement separately. | | `enable_metric_view_metadata` | `bool` | ✅ | ✅ | `False` | Sets `spark.sql.thriftserver.metadata.metricview.enabled` via session config so metric-view metadata surfaces. | -| `escape_catalog_name` | `bool` | ✅ | ✅ | `False` | Escape unescaped `_` in metadata catalog names, preserving existing `\_` escapes and `%` wildcards. Applies to `schemas()`, `tables()`, and `columns()` on Thrift and `tables()` on Kernel; Kernel schemas/columns already use exact catalog identifiers. | +| `escape_catalog_name` | `bool` | ✅ | ✅ | `True` | Escape unescaped `_` in metadata catalog names so `_` matches only itself (the JDBC/ODBC catalog is an exact name), preserving existing `\_` escapes and `%` wildcards. Applies to `schemas()`, `tables()`, and `columns()` on Thrift and `tables()` on Kernel; Kernel schemas/columns and SEA already use exact catalog identifiers. Set `False` to treat `_` as a single-character wildcard (the previous default). | | `use_inline_params` | `bool` \| `"silent"` | ✅ | ✅ | `False` | Render parameters inline (legacy) vs. native bound params (DBR 14.1+). Shared cursor code performs the rendering before dispatch, so both backends honor it identically. | | `ignore_transactions` | `bool` | ✅ | ✅ | `True` | When `True`: `commit()` is a no-op, `rollback()` raises `NotSupportedError`, and setting `autocommit` is a no-op. | | `fetch_autocommit_from_server`| `bool` | ✅ | ✅ | `False` | Query the server (`SET AUTOCOMMIT`) for autocommit state instead of returning the cached value. | @@ -238,6 +238,11 @@ are forwarded when supported by the installed kernel binding. keeps 20 and the kernel keeps 100. - **Custom headers**: Kernel drops `Authorization` and `x-databricks-org-id`; it forwards other `http_headers`. +- **Metadata catalog names**: by default `_` in `catalog_name` matches only + itself on both backends. With `escape_catalog_name=False`, Thrift + `schemas()`/`tables()`/`columns()` and Kernel `tables()` treat `_` as a + single-character wildcard, while Kernel `schemas()`/`columns()` still match + the exact name. - **Telemetry**: Kernel owns its telemetry lifecycle. It does not honor `force_enable_telemetry`, and an omitted `enable_telemetry` or circuit-breaker setting uses the kernel's default. diff --git a/src/databricks/sql/backend/kernel/client.py b/src/databricks/sql/backend/kernel/client.py index 01889e03a..424d91f90 100644 --- a/src/databricks/sql/backend/kernel/client.py +++ b/src/databricks/sql/backend/kernel/client.py @@ -322,7 +322,11 @@ def __init__( ) self._catalog = catalog self._schema = schema - self._escape_catalog_name = kwargs.get("escape_catalog_name", False) + # None means "not set" and keeps the default rather than disabling escaping. + escape_catalog_name = kwargs.get("escape_catalog_name") + self._escape_catalog_name = ( + True if escape_catalog_name is None else escape_catalog_name + ) # ``_use_arrow_native_complex_types`` is the connector-side # toggle for whether complex columns (ARRAY / MAP / STRUCT) # are surfaced as native Arrow shapes or as compact JSON diff --git a/src/databricks/sql/backend/thrift_backend.py b/src/databricks/sql/backend/thrift_backend.py index ee904c368..8f8c36971 100644 --- a/src/databricks/sql/backend/thrift_backend.py +++ b/src/databricks/sql/backend/thrift_backend.py @@ -179,7 +179,11 @@ def __init__( raise ValueError("No valid connection settings.") self._host = server_hostname - self._escape_catalog_name = kwargs.get("escape_catalog_name", False) + # None means "not set" and keeps the default rather than disabling escaping. + escape_catalog_name = kwargs.get("escape_catalog_name") + self._escape_catalog_name = ( + True if escape_catalog_name is None else escape_catalog_name + ) self._initialize_retry_args(kwargs) self._use_arrow_native_complex_types = kwargs.get( "_use_arrow_native_complex_types", True diff --git a/src/databricks/sql/client.py b/src/databricks/sql/client.py index 613b62386..abe3bed0c 100755 --- a/src/databricks/sql/client.py +++ b/src/databricks/sql/client.py @@ -321,11 +321,14 @@ def read(self) -> Optional[OAuthToken]: This allows 1. cursor.tables() to return METRIC_VIEW table type 2. cursor.columns() to return "measure" column type - :param escape_catalog_name: `bool`, optional (default is False) + :param escape_catalog_name: `bool`, optional (default is True) Escape unescaped underscores in catalog names for schemas(), tables(), - and columns() metadata requests. Existing escapes and % wildcards are - preserved. Kernel schemas() and columns() already use exact catalog - identifiers, so their catalog names are passed through unchanged. + and columns() metadata requests, so `_` matches only itself, as in + JDBC/ODBC where the catalog is an exact name. Existing escapes and % + wildcards are preserved. Kernel schemas() and columns() and all SEA + metadata calls already use exact catalog identifiers, so they ignore + this option. Set to False to treat `_` as a single-character wildcard + on Thrift and in kernel tables(). :param fetch_autocommit_from_server: `bool`, optional (default is False) When True, the connection.autocommit property queries the server for current state using SET AUTOCOMMIT instead of returning cached value. @@ -1789,7 +1792,8 @@ def schemas( """ Get schemas corresponding to the catalog_name and schema_name. - Names can contain % wildcards. + Names can contain % wildcards. `_` in catalog_name matches only itself; + on Thrift, ``escape_catalog_name=False`` makes it a single-character wildcard. :returns self """ self._check_not_closed() @@ -1815,7 +1819,9 @@ def tables( """ Get tables corresponding to the catalog_name, schema_name and table_name. - Names can contain % wildcards. + Names can contain % wildcards. `_` in catalog_name matches only itself; + on Thrift and kernel, ``escape_catalog_name=False`` makes it a + single-character wildcard. :returns self """ self._check_not_closed() @@ -1844,7 +1850,8 @@ def columns( """ Get columns corresponding to the catalog_name, schema_name, table_name and column_name. - Names can contain % wildcards. + Names can contain % wildcards. `_` in catalog_name matches only itself; + on Thrift, ``escape_catalog_name=False`` makes it a single-character wildcard. ``catalog_name=None`` is accepted on all backends and matches columns across every catalog (the kernel issues ``SHOW COLUMNS`` diff --git a/src/databricks/sql/session.py b/src/databricks/sql/session.py index 04d603c21..906e93715 100644 --- a/src/databricks/sql/session.py +++ b/src/databricks/sql/session.py @@ -341,7 +341,7 @@ def _create_backend( "enable_geospatial_support", False ), telemetry_options=kernel_telemetry_options, - escape_catalog_name=kwargs.get("escape_catalog_name", False), + escape_catalog_name=kwargs.get("escape_catalog_name"), ) # These reference the lazily-resolved module attributes defined via diff --git a/tests/e2e/test_driver.py b/tests/e2e/test_driver.py index 872e57f16..b1ff57e93 100644 --- a/tests/e2e/test_driver.py +++ b/tests/e2e/test_driver.py @@ -702,6 +702,74 @@ def test_get_catalogs(self): ("TABLE_CAT", "string", None, None, None, None, None) ] + @pytest.mark.parametrize( + "extra_params, expect_rows", + [({}, False), ({"escape_catalog_name": False}, True)], + ids=["default", "wildcard-opt-out"], + ) + def test_catalog_name_underscore_is_literal_by_default( + self, extra_params, expect_rows + ): + # No catalog is named "syste_"; only the opt-out lets `_` match "system". + with self.cursor(extra_params) as cursor: + cursor.schemas(catalog_name="syste_", schema_name="information_schema") + rows = cursor.fetchall() + assert bool(rows) == expect_rows + assert all(row[1] == "system" for row in rows) + + def test_escaped_catalog_name_matches_exact_catalog(self): + with self.cursor({}) as cursor: + cursor.catalogs() + names = [row[0] for row in cursor.fetchall()] + # hive_metastore is covered separately below (legacy catalog with no + # information_schema); it is not excluded because escaping breaks it. + candidates = [ + n + for n in names + if "_" in n and not n.startswith("__") and n != "hive_metastore" + ] + if not candidates: + pytest.skip("no catalog with `_` in its name is visible") + name = candidates[0] + + with self.cursor({"escape_catalog_name": False}) as cursor: + cursor.schemas(catalog_name=name) + unescaped = sorted(tuple(r) for r in cursor.fetchall() if r[1] == name) + if not unescaped: + pytest.skip(f"no schemas visible in catalog {name}") + with self.cursor({}) as cursor: + cursor.schemas(catalog_name=name) + escaped = sorted(tuple(r) for r in cursor.fetchall()) + assert escaped == unescaped + + def test_hive_metastore_metadata_unchanged_by_escaping(self): + # hive_metastore is the most common catalog name with `_`, so escaping must + # not change its results; spark_catalog (its alias) must not lose rows. + with self.connection({}) as escaped, self.connection( + {"escape_catalog_name": False} + ) as raw: + + def rows(conn, call, **filters): + with conn.cursor() as cursor: + getattr(cursor, call)(**filters) + return {tuple(r) for r in cursor.fetchall()} + + hms = {"catalog_name": "hive_metastore", "schema_name": "default"} + expected = rows(raw, "schemas", **hms) + if not expected: + pytest.skip("hive_metastore.default is not visible") + assert rows(escaped, "schemas", **hms) == expected + + tables = rows(raw, "tables", **hms) + if tables: + one = dict(hms, table_name=min(t[2] for t in tables)) + assert rows(escaped, "tables", **one) == rows(raw, "tables", **one) + + spark = {"catalog_name": "spark_catalog", "schema_name": "default"} + spark_rows = rows(escaped, "schemas", **spark) + assert spark_rows, "escaped spark_catalog should resolve to hive_metastore" + assert rows(raw, "schemas", **spark) <= spark_rows + @skipUnless(pysql_supports_arrow(), "arrow test need arrow support") @pytest.mark.parametrize( "extra_params", diff --git a/tests/e2e/test_kernel_backend.py b/tests/e2e/test_kernel_backend.py index ae0301fb6..2fcdcbaa8 100644 --- a/tests/e2e/test_kernel_backend.py +++ b/tests/e2e/test_kernel_backend.py @@ -359,6 +359,41 @@ def test_metadata_columns(conn): # ── Metadata filter semantics ───────────────────────────────────── +@pytest.mark.parametrize( + "extra_params, expect_rows", + [({}, False), ({"escape_catalog_name": False}, True)], + ids=["default", "wildcard-opt-out"], +) +def test_tables_catalog_name_underscore_is_literal_by_default( + kernel_conn_params, extra_params, expect_rows +): + # No catalog is named "syste_"; only the opt-out lets `_` match "system". + with sql.connect(**kernel_conn_params, **extra_params) as c, c.cursor() as cur: + cur.tables( + catalog_name="syste_", + schema_name="information_schema", + table_name="tables", + ) + rows = cur.fetchall() + assert bool(rows) == expect_rows + assert all(row[0] == "system" for row in rows) + + +def test_tables_hive_metastore_unchanged_by_escaping(kernel_conn_params): + # Kernel tables() honours escape_catalog_name; hive_metastore is the most + # common catalog name with `_`, so escaping must not change its results. + def rows(extra_params, **filters): + with sql.connect(**kernel_conn_params, **extra_params) as c, c.cursor() as cur: + cur.tables(catalog_name="hive_metastore", schema_name="default", **filters) + return {tuple(r) for r in cur.fetchall()} + + listing = rows({"escape_catalog_name": False}) + if not listing: + pytest.skip("hive_metastore.default is not visible") + one = {"table_name": min(t[2] for t in listing)} + assert rows({}, **one) == rows({"escape_catalog_name": False}, **one) + + def test_schemas_with_empty_string_filter_matches_nothing(conn): """An empty string is a real pattern, distinct from absent ``None``.""" with conn.cursor() as cur: diff --git a/tests/unit/test_kernel_client.py b/tests/unit/test_kernel_client.py index e0b8d99e0..0cc68466f 100644 --- a/tests/unit/test_kernel_client.py +++ b/tests/unit/test_kernel_client.py @@ -1946,9 +1946,16 @@ def test_sync_execute_leaves_rowcount_default_when_num_modified_rows_none(): @pytest.mark.parametrize( - "options", - [{}, {"escape_catalog_name": False}, {"escape_catalog_name": True}], - ids=["default", "disabled", "enabled"], + "options, expect_escaped", + [ + # Escaping is on by default; it only changes tables(), because kernel + # schemas()/columns() already take an exact catalog identifier. + ({}, True), + ({"escape_catalog_name": None}, True), + ({"escape_catalog_name": False}, False), + ({"escape_catalog_name": True}, True), + ], + ids=["default", "none-uses-default", "disabled", "enabled"], ) @pytest.mark.parametrize( "method, filters, kernel_filters", @@ -2008,7 +2015,14 @@ def test_sync_execute_leaves_rowcount_default_when_num_modified_rows_none(): ], ) def test_metadata_catalog_escaping( - monkeypatch, options, method, filters, kernel_filters, catalog_name, escaped + monkeypatch, + options, + expect_escaped, + method, + filters, + kernel_filters, + catalog_name, + escaped, ): from databricks import sql @@ -2031,11 +2045,7 @@ def test_metadata_catalog_escaping( cursor = connection.cursor() assert getattr(cursor, method)(catalog_name=catalog_name, **filters) is cursor - expected = ( - escaped - if method == "tables" and options.get("escape_catalog_name", False) - else catalog_name - ) + expected = escaped if method == "tables" and expect_escaped else catalog_name if expected in ("%", "*"): expected = None rpc.assert_called_once_with(catalog=expected, **kernel_filters) diff --git a/tests/unit/test_thrift_backend.py b/tests/unit/test_thrift_backend.py index f127ad8e2..273e0c8e4 100644 --- a/tests/unit/test_thrift_backend.py +++ b/tests/unit/test_thrift_backend.py @@ -42,9 +42,15 @@ def retry_policy_factory(): @pytest.mark.parametrize( - "options", - [{}, {"escape_catalog_name": False}, {"escape_catalog_name": True}], - ids=["default", "disabled", "enabled"], + "options, expect_escaped", + [ + # Escaping is on by default: `_` in a catalog name matches only itself. + ({}, True), + ({"escape_catalog_name": None}, True), + ({"escape_catalog_name": False}, False), + ({"escape_catalog_name": True}, True), + ], + ids=["default", "none-uses-default", "disabled", "enabled"], ) @pytest.mark.parametrize( "method, rpc_name, filters", @@ -96,7 +102,7 @@ def retry_policy_factory(): ], ) def test_metadata_catalog_escaping( - options, method, rpc_name, filters, catalog_name, escaped + options, expect_escaped, method, rpc_name, filters, catalog_name, escaped ): with ( patch( @@ -129,10 +135,7 @@ def test_metadata_catalog_escaping( ) request = rpc.call_args.args[0] - expected = ( - escaped if options.get("escape_catalog_name", False) else catalog_name - ) - assert request.catalogName == expected + assert request.catalogName == (escaped if expect_escaped else catalog_name) for arg, value in filters.items(): thrift_field = { "schema_name": "schemaName", @@ -1952,7 +1955,8 @@ def test_get_schemas_calls_client_and_handle_execute_response( req = tcli_service_instance.GetSchemas.call_args[0][0] get_direct_results = ttypes.TSparkGetDirectResults(maxRows=100, maxBytes=200) self.assertEqual(req.getDirectResults, get_direct_results) - self.assertEqual(req.catalogName, "catalog_pattern") + # The backend escapes catalog-name underscores by default. + self.assertEqual(req.catalogName, r"catalog\_pattern") self.assertEqual(req.schemaName, "schema_pattern") # Check response handling thrift_backend._handle_execute_response.assert_called_with( @@ -1997,7 +2001,8 @@ def test_get_tables_calls_client_and_handle_execute_response( req = tcli_service_instance.GetTables.call_args[0][0] get_direct_results = ttypes.TSparkGetDirectResults(maxRows=100, maxBytes=200) self.assertEqual(req.getDirectResults, get_direct_results) - self.assertEqual(req.catalogName, "catalog_pattern") + # The backend escapes catalog-name underscores by default. + self.assertEqual(req.catalogName, r"catalog\_pattern") self.assertEqual(req.schemaName, "schema_pattern") self.assertEqual(req.tableName, "table_pattern") self.assertEqual(req.tableTypes, ["type1", "type2"]) @@ -2044,7 +2049,8 @@ def test_get_columns_calls_client_and_handle_execute_response( req = tcli_service_instance.GetColumns.call_args[0][0] get_direct_results = ttypes.TSparkGetDirectResults(maxRows=100, maxBytes=200) self.assertEqual(req.getDirectResults, get_direct_results) - self.assertEqual(req.catalogName, "catalog_pattern") + # The backend escapes catalog-name underscores by default. + self.assertEqual(req.catalogName, r"catalog\_pattern") self.assertEqual(req.schemaName, "schema_pattern") self.assertEqual(req.tableName, "table_pattern") self.assertEqual(req.columnName, "column_pattern")