Skip to content

Commit 268b6bc

Browse files
vuanhphungIsaac
andcommitted
fix(metadata): treat escape_catalog_name=None as the default
An explicit None (e.g. from a wrapper forwarding unset options) used to match the old False default; keep it meaning "not set" now that the default is True. Also assert that escaped spark_catalog resolves. Signed-off-by: Vu Anh Phung <vu.phung@databricks.com> Co-authored-by: Isaac <no-reply@databricks.com>
1 parent ed49b2e commit 268b6bc

6 files changed

Lines changed: 20 additions & 6 deletions

File tree

‎src/databricks/sql/backend/kernel/client.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,11 @@ def __init__(
322322
)
323323
self._catalog = catalog
324324
self._schema = schema
325-
self._escape_catalog_name = kwargs.get("escape_catalog_name", True)
325+
# None means "not set" and keeps the default rather than disabling escaping.
326+
escape_catalog_name = kwargs.get("escape_catalog_name")
327+
self._escape_catalog_name = (
328+
True if escape_catalog_name is None else escape_catalog_name
329+
)
326330
# ``_use_arrow_native_complex_types`` is the connector-side
327331
# toggle for whether complex columns (ARRAY / MAP / STRUCT)
328332
# are surfaced as native Arrow shapes or as compact JSON

‎src/databricks/sql/backend/thrift_backend.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,11 @@ def __init__(
179179
raise ValueError("No valid connection settings.")
180180

181181
self._host = server_hostname
182-
self._escape_catalog_name = kwargs.get("escape_catalog_name", True)
182+
# None means "not set" and keeps the default rather than disabling escaping.
183+
escape_catalog_name = kwargs.get("escape_catalog_name")
184+
self._escape_catalog_name = (
185+
True if escape_catalog_name is None else escape_catalog_name
186+
)
183187
self._initialize_retry_args(kwargs)
184188
self._use_arrow_native_complex_types = kwargs.get(
185189
"_use_arrow_native_complex_types", True

‎src/databricks/sql/session.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ def _create_backend(
341341
"enable_geospatial_support", False
342342
),
343343
telemetry_options=kernel_telemetry_options,
344-
escape_catalog_name=kwargs.get("escape_catalog_name", True),
344+
escape_catalog_name=kwargs.get("escape_catalog_name"),
345345
)
346346

347347
# These reference the lazily-resolved module attributes defined via

‎tests/e2e/test_driver.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -721,6 +721,8 @@ def test_escaped_catalog_name_matches_exact_catalog(self):
721721
with self.cursor({}) as cursor:
722722
cursor.catalogs()
723723
names = [row[0] for row in cursor.fetchall()]
724+
# hive_metastore is covered separately below (legacy catalog with no
725+
# information_schema); it is not excluded because escaping breaks it.
724726
candidates = [
725727
n
726728
for n in names
@@ -764,7 +766,9 @@ def rows(conn, call, **filters):
764766
assert rows(escaped, "tables", **one) == rows(raw, "tables", **one)
765767

766768
spark = {"catalog_name": "spark_catalog", "schema_name": "default"}
767-
assert rows(raw, "schemas", **spark) <= rows(escaped, "schemas", **spark)
769+
spark_rows = rows(escaped, "schemas", **spark)
770+
assert spark_rows, "escaped spark_catalog should resolve to hive_metastore"
771+
assert rows(raw, "schemas", **spark) <= spark_rows
768772

769773
@skipUnless(pysql_supports_arrow(), "arrow test need arrow support")
770774
@pytest.mark.parametrize(

‎tests/unit/test_kernel_client.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1951,10 +1951,11 @@ def test_sync_execute_leaves_rowcount_default_when_num_modified_rows_none():
19511951
# Escaping is on by default; it only changes tables(), because kernel
19521952
# schemas()/columns() already take an exact catalog identifier.
19531953
({}, True),
1954+
({"escape_catalog_name": None}, True),
19541955
({"escape_catalog_name": False}, False),
19551956
({"escape_catalog_name": True}, True),
19561957
],
1957-
ids=["default", "disabled", "enabled"],
1958+
ids=["default", "none-uses-default", "disabled", "enabled"],
19581959
)
19591960
@pytest.mark.parametrize(
19601961
"method, filters, kernel_filters",

‎tests/unit/test_thrift_backend.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,11 @@ def retry_policy_factory():
4646
[
4747
# Escaping is on by default: `_` in a catalog name matches only itself.
4848
({}, True),
49+
({"escape_catalog_name": None}, True),
4950
({"escape_catalog_name": False}, False),
5051
({"escape_catalog_name": True}, True),
5152
],
52-
ids=["default", "disabled", "enabled"],
53+
ids=["default", "none-uses-default", "disabled", "enabled"],
5354
)
5455
@pytest.mark.parametrize(
5556
"method, rpc_name, filters",

0 commit comments

Comments
 (0)