Skip to content

Commit 74d883f

Browse files
committed
fix(kernel): handle empty exact catalog filters
Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
1 parent 46e620a commit 74d883f

2 files changed

Lines changed: 117 additions & 17 deletions

File tree

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

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
import logging
2626
import threading
2727
import uuid
28-
from typing import Any, Dict, List, Optional, Set, TYPE_CHECKING, Union
28+
from typing import Any, Dict, List, Optional, Set, Tuple, TYPE_CHECKING, Union
2929

3030
from databricks.sql.backend.databricks_client import DatabricksClient
3131
from databricks.sql.backend.kernel._errors import (
@@ -125,13 +125,30 @@ def _catalog_or_none(value: Optional[str]) -> Optional[str]:
125125
treats the catalog as an exact identifier for SHOW COLUMNS. ``None``
126126
remains the only absent filter. Other strings must be preserved as real
127127
filters; in particular, an empty string matches nothing just as it does
128-
on the Thrift backend.
128+
on the Thrift backend. Non-empty whitespace-only strings remain invalid
129+
and are left for kernel validation.
129130
"""
130131
if value is None or value in ("%", "*"):
131132
return None
132133
return value
133134

134135

136+
def _exact_catalog_and_pattern(
137+
catalog: Optional[str], pattern: Optional[str]
138+
) -> Tuple[Optional[str], Optional[str]]:
139+
"""Adapt an empty exact catalog to an empty subordinate pattern.
140+
141+
At ``KERNEL_REV``, ``Identifier("")`` is invalid while
142+
``LikePattern("")`` means match-nothing. Schema and column metadata
143+
take an exact catalog, so represent an empty catalog as all catalogs
144+
constrained by an empty schema pattern. This preserves empty-catalog
145+
semantics without passing an invalid identifier to the kernel.
146+
"""
147+
if catalog == "":
148+
return None, ""
149+
return _catalog_or_none(catalog), pattern
150+
151+
135152
def _is_staging_statement(operation: str) -> bool:
136153
"""True iff ``operation`` is a volume/staging statement (PUT / GET /
137154
REMOVE).
@@ -918,9 +935,12 @@ def get_schemas(
918935
if self._kernel_session is None:
919936
raise InterfaceError("get_schemas requires an open session.")
920937
try:
938+
catalog, schema_pattern = _exact_catalog_and_pattern(
939+
catalog_name, schema_name
940+
)
921941
stream = self._kernel_session.metadata().list_schemas(
922-
catalog=_catalog_or_none(catalog_name),
923-
schema_pattern=schema_name,
942+
catalog=catalog,
943+
schema_pattern=schema_pattern,
924944
)
925945
return self._make_result_set(stream, cursor, self._synthetic_command_id())
926946
except Exception as exc:
@@ -975,9 +995,12 @@ def get_columns(
975995
# row's `TABLE_CAT` is correctly attributed. Matches the
976996
# Thrift backend's `getColumns(null, …)` behaviour from
977997
# the user's perspective.
998+
catalog, schema_pattern = _exact_catalog_and_pattern(
999+
catalog_name, schema_name
1000+
)
9781001
stream = self._kernel_session.metadata().list_columns(
979-
catalog=_catalog_or_none(catalog_name),
980-
schema_pattern=schema_name,
1002+
catalog=catalog,
1003+
schema_pattern=schema_pattern,
9811004
table_pattern=table_name,
9821005
column_pattern=column_name,
9831006
)

tests/unit/test_kernel_client.py

Lines changed: 88 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1616,6 +1616,28 @@ def test_get_schemas_preserves_empty_pattern():
16161616
list_schemas.assert_called_once_with(catalog="main", schema_pattern="")
16171617

16181618

1619+
def test_get_schemas_empty_catalog_uses_empty_pattern():
1620+
"""Avoid passing an empty exact ``Identifier`` to the kernel."""
1621+
c = _make_client()
1622+
c._kernel_session = MagicMock()
1623+
list_schemas = c._kernel_session.metadata.return_value.list_schemas
1624+
list_schemas.return_value = _stream_with_schema()
1625+
cursor = MagicMock()
1626+
cursor.arraysize = 100
1627+
cursor.buffer_size_bytes = 1024
1628+
1629+
c.get_schemas(
1630+
session_id=MagicMock(),
1631+
max_rows=1,
1632+
max_bytes=1,
1633+
cursor=cursor,
1634+
catalog_name="",
1635+
schema_name="ignored",
1636+
)
1637+
1638+
list_schemas.assert_called_once_with(catalog=None, schema_pattern="")
1639+
1640+
16191641
def test_get_tables_preserves_empty_patterns():
16201642
c = _make_client()
16211643
c._kernel_session = MagicMock()
@@ -1643,9 +1665,64 @@ def test_get_tables_preserves_empty_patterns():
16431665
)
16441666

16451667

1646-
@pytest.mark.parametrize("filter_value", ["", " "])
1647-
def test_get_columns_preserves_blank_filters(filter_value):
1648-
"""Blank strings remain filters instead of becoming match-all ``None``."""
1668+
def test_get_columns_preserves_empty_patterns():
1669+
c = _make_client()
1670+
c._kernel_session = MagicMock()
1671+
list_columns = c._kernel_session.metadata.return_value.list_columns
1672+
list_columns.return_value = _stream_with_schema()
1673+
cursor = MagicMock()
1674+
cursor.arraysize = 100
1675+
cursor.buffer_size_bytes = 1024
1676+
1677+
c.get_columns(
1678+
session_id=MagicMock(),
1679+
max_rows=1,
1680+
max_bytes=1,
1681+
cursor=cursor,
1682+
catalog_name="main",
1683+
schema_name="",
1684+
table_name="",
1685+
column_name="",
1686+
)
1687+
1688+
list_columns.assert_called_once_with(
1689+
catalog="main",
1690+
schema_pattern="",
1691+
table_pattern="",
1692+
column_pattern="",
1693+
)
1694+
1695+
1696+
def test_get_columns_empty_catalog_uses_empty_pattern():
1697+
"""An empty catalog matches nothing without constructing ``Identifier("")``."""
1698+
c = _make_client()
1699+
c._kernel_session = MagicMock()
1700+
list_columns = c._kernel_session.metadata.return_value.list_columns
1701+
list_columns.return_value = _stream_with_schema()
1702+
cursor = MagicMock()
1703+
cursor.arraysize = 100
1704+
cursor.buffer_size_bytes = 1024
1705+
1706+
c.get_columns(
1707+
session_id=MagicMock(),
1708+
max_rows=1,
1709+
max_bytes=1,
1710+
cursor=cursor,
1711+
catalog_name="",
1712+
schema_name="ignored",
1713+
table_name="table",
1714+
column_name="column",
1715+
)
1716+
1717+
list_columns.assert_called_once_with(
1718+
catalog=None,
1719+
schema_pattern="",
1720+
table_pattern="table",
1721+
column_pattern="column",
1722+
)
1723+
1724+
1725+
def test_get_columns_preserves_whitespace_for_kernel_validation():
16491726
c = _make_client()
16501727
c._kernel_session = MagicMock()
16511728
list_columns = c._kernel_session.metadata.return_value.list_columns
@@ -1659,17 +1736,17 @@ def test_get_columns_preserves_blank_filters(filter_value):
16591736
max_rows=1,
16601737
max_bytes=1,
16611738
cursor=cursor,
1662-
catalog_name=filter_value,
1663-
schema_name=filter_value,
1664-
table_name=filter_value,
1665-
column_name=filter_value,
1739+
catalog_name=" ",
1740+
schema_name=" ",
1741+
table_name=" ",
1742+
column_name=" ",
16661743
)
16671744

16681745
list_columns.assert_called_once_with(
1669-
catalog=filter_value,
1670-
schema_pattern=filter_value,
1671-
table_pattern=filter_value,
1672-
column_pattern=filter_value,
1746+
catalog=" ",
1747+
schema_pattern=" ",
1748+
table_pattern=" ",
1749+
column_pattern=" ",
16731750
)
16741751

16751752

0 commit comments

Comments
 (0)