Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Release History

# Unreleased
- Kernel metadata filters are now forwarded unchanged instead of collapsing empty strings to `None`. Only `None` leaves a filter unset; empty pattern filters match nothing (PECOBLR-4221).
- Kernel backend (`use_kernel=True`): OAuth **M2M with a JWT private-key client assertion** (RFC 7523) is now supported. Pass `oauth_client_id` + `oauth_jwt_key_file` + `oauth_jwt_kid` (with optional `oauth_jwt_passphrase` for an encrypted PKCS#8 key, `oauth_jwt_algorithm` defaulting to `RS256`, `oauth_scopes`, and `token_url` for the IdP token endpoint) and the connector routes them to the kernel's `auth_type="oauth-m2m-jwt"`, which signs a short-lived assertion with the private key instead of sending a client secret. The kernel owns the token lifecycle. A private-key file is treated as unambiguous JWT M2M intent and is mutually exclusive with `oauth_client_secret` / `credentials_provider` (both raise `NotSupportedError`). Verified end-to-end against an Azure Databricks workspace with the service principal's public certificate registered on its Entra ID app registration. Requires `databricks-sql-kernel >= 0.2.0` with JWT support.
- Kernel backend (`use_kernel=True`): OAuth U2M with `auth_type="databricks-oauth"` now forwards the connector's `databricks-sql-python` OAuth-app bundle (`client_id` + `sql offline_access` scopes + redirect port) into the kernel, so a bare U2M connection authenticates as `databricks-sql-python` — parity with the Thrift path — instead of inheriting the kernel's own `databricks-sql-connector` default. A caller-supplied `oauth_client_id` (with its coupled `oauth_redirect_port`) is honored, as is a caller-supplied `oauth_scopes`; absent one, the connector default (`sql offline_access`) is forwarded. Note: the kernel binds a single U2M redirect port, so unlike the Thrift path (which tries the full `8020..8024` range) the kernel path uses only one port and does not fall back to the next port if it is already bound — pass `oauth_redirect_port` (with `oauth_client_id`) to pick a free one on a port collision (PECOBLR-4040)
- Kernel backend (`use_kernel=True`): **Azure Entra (Azure AD) service-principal M2M is now supported.** `auth_type="azure-sp-m2m"` forwards `azure_client_id` / `azure_client_secret`; the kernel is the Azure-aware auth core — it builds the Entra v2.0 token endpoint and the `{app_id}/.default` scope, and **auto-discovers the tenant** from the workspace's `/aad/auth` redirect when `azure_tenant_id` is omitted (matching Thrift). The `Authorization` bearer is the Databricks-audience data token, which alone authenticates a workspace-member SP. Set `azure_workspace_resource_id` and the kernel also sends the Azure SP management token (`X-Databricks-Azure-SP-Management-Token`) + `X-Databricks-Azure-Workspace-Resource-Id` header (matching the JDBC driver), so a service principal with an Azure RBAC role but no workspace membership can authenticate; omit it and no ARM management-scope token is fetched. Azure AD **U2M** (`auth_type="azure-oauth"`) now routes to the kernel's OAuth U2M flow, identically to `auth_type="databricks-oauth"`: the kernel runs the in-house workspace-federated browser flow, which Azure workspaces support (the workspace federates login to Entra). It forwards the connector's `databricks-sql-python` OAuth app, not the Thrift Azure app (`96eecda7` / port 8030), which is registered for Thrift's direct-Entra flow the kernel does not perform (PECOBLR-4141; PECOBLR-4120)
Expand Down
27 changes: 18 additions & 9 deletions src/databricks/sql/backend/databricks_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,10 @@ def get_schemas(
max_rows: Maximum number of rows to fetch in a single batch
max_bytes: Maximum number of bytes to fetch in a single batch
cursor: The cursor object that will handle the results
catalog_name: Optional catalog name pattern to filter by
schema_name: Optional schema name pattern to filter by
catalog_name: Optional exact catalog name to filter by, forwarded
unchanged. ``None`` leaves the filter unset.
schema_name: Optional schema name pattern to filter by. ``None``
leaves the filter unset; an empty string matches nothing.

Returns:
ResultSet: An object containing the schema metadata
Expand Down Expand Up @@ -284,10 +286,13 @@ def get_tables(
max_bytes: Maximum number of bytes to fetch in a single batch
cursor: The cursor object that will handle the results
catalog_name: Optional catalog name pattern to filter by
if catalog_name is None, we fetch across all catalogs
if catalog_name is None, we fetch across all catalogs; an empty

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Low — The three metadata docstrings now describe catalog_name inconsistently after this change: get_schemas (L251) and get_columns (L330) were updated to "Optional exact catalog name to filter by, forwarded unchanged", but get_tables (L288) was left as "Optional catalog name pattern to filter by". Since all three now forward the catalog verbatim to the kernel, the public contract reads as if tables() accepts a catalog pattern while schemas()/columns() accept an exact name — a distinction a caller would reasonably act on. Either the get_tables wording should be aligned with the sibling methods, or (if the divergence is intentional because the kernel treats the catalog differently for SHOW TABLES/SHOW SCHEMAS vs SHOW COLUMNS, as the now-removed _catalog_or_none docstring described) that per-method difference should be stated explicitly rather than left as an accidental wording mismatch.

(Anchored to the nearest changed line — see the description for the exact location.)

string matches nothing
schema_name: Optional schema name pattern to filter by
if schema_name is None, we fetch across all schemas
table_name: Optional table name pattern to filter by
if schema_name is None, we fetch across all schemas; an empty
string matches nothing
table_name: Optional table name pattern to filter by. ``None``
leaves the filter unset; an empty string matches nothing.
table_types: Optional list of table types to filter by (e.g., ['TABLE', 'VIEW'])

Returns:
Expand Down Expand Up @@ -322,11 +327,15 @@ def get_columns(
max_rows: Maximum number of rows to fetch in a single batch
max_bytes: Maximum number of bytes to fetch in a single batch
cursor: The cursor object that will handle the results
catalog_name: Optional catalog name pattern to filter by
schema_name: Optional schema name pattern to filter by
catalog_name: Optional exact catalog name to filter by, forwarded
unchanged. ``None`` leaves the filter unset.
schema_name: Optional schema name pattern to filter by. ``None``
leaves the filter unset; an empty string matches nothing.
table_name: Optional table name pattern to filter by
if table_name is None, we fetch across all tables
column_name: Optional column name pattern to filter by
if table_name is None, we fetch across all tables; an empty
string matches nothing
column_name: Optional column name pattern to filter by. ``None``
leaves the filter unset; an empty string matches nothing.

Returns:
ResultSet: An object containing the column metadata
Expand Down
51 changes: 9 additions & 42 deletions src/databricks/sql/backend/kernel/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,39 +117,6 @@ def _is_not_found(exc: BaseException) -> bool:
)


def _none_if_blank(value: Optional[str]) -> Optional[str]:
"""Map an empty/whitespace-only metadata filter to ``None``
("match all"), matching the Thrift backend's effective behaviour.

The kernel's ``Identifier`` / ``LikePattern`` reject ``""`` with
``InvalidArgument`` (-> ``ProgrammingError``); ``None`` is the
kernel's canonical "match all". Applied to schema / table / column
*pattern* args (which otherwise keep ``%`` / ``_`` as real LIKE
wildcards)."""
if value is None:
return None
return value if value.strip() else None


def _catalog_or_none(value: Optional[str]) -> Optional[str]:
"""Normalise a catalog filter: ``None`` / blank / ``'%'`` / ``'*'``
all mean "all catalogs" -> ``None``.

This makes ``columns(catalog='%')`` behave like
``tables(catalog='%')`` / ``schemas(catalog='%')`` — the kernel
already treats blank/``%``/``*`` as "all catalogs" for SHOW SCHEMAS
/ SHOW TABLES (``is_null_or_wildcard``) but treats the catalog as an
exact identifier for SHOW COLUMNS, so the three diverged. Normalising
connector-side makes them symmetric. This intentionally diverges from
raw-Thrift literalness (Thrift treats ``%`` as a literal catalog
name) in favour of JDBC "catalog is exact-or-all, not a pattern" +
internal consistency. Catalog is the only arg normalised this way;
schema/table/column patterns keep ``%`` / ``*`` as LIKE wildcards."""
if value is None or not value.strip() or value in ("%", "*"):
return None
return value


def _is_staging_statement(operation: str) -> bool:
"""True iff ``operation`` is a volume/staging statement (PUT / GET /
REMOVE).
Expand Down Expand Up @@ -937,8 +904,8 @@ def get_schemas(
raise InterfaceError("get_schemas requires an open session.")
try:
stream = self._kernel_session.metadata().list_schemas(
catalog=_catalog_or_none(catalog_name),
schema_pattern=_none_if_blank(schema_name),
catalog=catalog_name,
schema_pattern=schema_name,
)
return self._make_result_set(stream, cursor, self._synthetic_command_id())
except Exception as exc:
Expand All @@ -964,9 +931,9 @@ def get_tables(
# do the work — no connector-side drain + refilter. Passing it
# through preserves streaming for large schemas.
stream = self._kernel_session.metadata().list_tables(
catalog=_catalog_or_none(catalog_name),
schema_pattern=_none_if_blank(schema_name),
table_pattern=_none_if_blank(table_name),
catalog=catalog_name,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High — get_tables passes catalog=catalog_name directly to list_tables, bypassing the new _exact_catalog_and_pattern bridge that get_schemas and get_columns use for the empty-catalog case.

The bridge exists specifically because the kernel does not match-nothing on an empty catalog. The docstring of the removed _catalog_or_none (this PR's own diff) states the kernel "treats blank/%/* as 'all catalogs' for SHOW SCHEMAS / SHOW TABLES (is_null_or_wildcard)". By that same logic, list_tables(catalog="") will be interpreted as all catalogs — matching everything — rather than nothing. (If instead the kernel routes the tables catalog through an exact Identifier, "" raises InvalidArgumentProgrammingError, which is exactly what the bridge was introduced to avoid.)

Either way, cursor.tables(catalog_name="") diverges from the stated contract and from the sibling methods: the docstring this PR adds to databricks_client.py (get_tables) reads "if catalog_name is None, we fetch across all catalogs; an empty string matches nothing", and the PR summary claims empty catalog/schema/table/column filters all "match nothing." get_tables with an empty catalog does not honor that.

Suggest routing get_tables through the same bridge:

catalog, schema_pattern = _exact_catalog_and_pattern(catalog_name, schema_name)
stream = self._kernel_session.metadata().list_tables(
    catalog=catalog,
    schema_pattern=schema_pattern,
    table_pattern=table_name,
    table_types=table_types if table_types else None,
)

schema_pattern=schema_name,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium — get_tables handles an empty catalog differently from get_schemas/get_columns, and no test locks its actual match-nothing behavior.

get_schemas and get_columns route the catalog through _exact_catalog_and_pattern, which special-cases catalog == ""catalog=None, schema_pattern="" precisely because (per the new helper docstring) those kernel APIs take an exact Identifier that rejects "". get_tables instead passes catalog=_catalog_or_none(catalog_name), which returns "" verbatim for an empty string.

This leaves the empty-catalog semantics of tables(catalog_name="") resting entirely on how the kernel's list_tables interprets a blank catalog. The comment removed in this PR documented that the kernel treats blank/%/* as "all catalogs" for SHOW TABLES via is_null_or_wildcard (exact-identifier only for SHOW COLUMNS). If that is still true at KERNEL_REV, then tables(catalog_name="") would match every catalog — the opposite of the "empty string matches nothing" contract this PR adds to the docstrings/CHANGELOG, and inconsistent with schemas()/columns().

The e2e test_columns_with_empty_string_filter_matches_nothing and test_schemas_with_empty_string_filter_matches_nothing cover columns and schemas, and the unit test_get_tables_preserves_empty_patterns only asserts the pass-through call args (catalog="") — none verify that tables(catalog_name="") actually returns nothing. Please confirm the kernel's list_tables treats an empty catalog as match-nothing (not match-all); if it doesn't, get_tables needs the same _exact_catalog_and_pattern adaptation. Either way, an e2e case for tables(catalog_name="") would lock the intended semantics.

(Anchored to the nearest changed line — see the description for the exact location.)

table_pattern=table_name,
table_types=table_types if table_types else None,
)
return self._make_result_set(stream, cursor, self._synthetic_command_id())
Expand Down Expand Up @@ -994,10 +961,10 @@ def get_columns(
# Thrift backend's `getColumns(null, …)` behaviour from
# the user's perspective.
stream = self._kernel_session.metadata().list_columns(
catalog=_catalog_or_none(catalog_name),
schema_pattern=_none_if_blank(schema_name),
table_pattern=_none_if_blank(table_name),
column_pattern=_none_if_blank(column_name),
catalog=catalog_name,
schema_pattern=schema_name,
table_pattern=table_name,
column_pattern=column_name,
)
return self._make_result_set(stream, cursor, self._synthetic_command_id())
except Exception as exc:
Expand Down
12 changes: 9 additions & 3 deletions src/databricks/sql/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -1577,7 +1577,9 @@ def schemas(
"""
Get schemas corresponding to the catalog_name and schema_name.

Names can contain % wildcards.
Filters are forwarded unchanged; only ``None`` leaves one unset.
``catalog_name`` is exact. ``schema_name`` is a pattern, can contain
% wildcards, and an empty pattern matches nothing.
:returns self
"""
self._check_not_closed()
Expand All @@ -1603,7 +1605,9 @@ def tables(
"""
Get tables corresponding to the catalog_name, schema_name and table_name.

Names can contain % wildcards.
Filters are forwarded unchanged; only ``None`` leaves one unset.
Names are patterns, can contain % wildcards, and empty patterns match
nothing.
:returns self
"""
self._check_not_closed()
Expand Down Expand Up @@ -1632,7 +1636,9 @@ def columns(
"""
Get columns corresponding to the catalog_name, schema_name, table_name and column_name.

Names can contain % wildcards.
Filters are forwarded unchanged; only ``None`` leaves one unset.
``catalog_name`` is exact. Other names are patterns, can contain
% wildcards, and empty patterns match nothing.

``catalog_name=None`` is accepted on all backends and matches
columns across every catalog (the kernel issues ``SHOW COLUMNS``
Expand Down
28 changes: 21 additions & 7 deletions tests/e2e/test_kernel_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -356,17 +356,31 @@ def test_metadata_columns(conn):
assert len(rows) > 0


# ── Metadata filter normalization (batch 3) ───────────────────────
# ── Metadata filter semantics ─────────────────────────────────────


def test_schemas_with_empty_string_filter_matches_all(conn):
"""An empty-string schema pattern normalizes to match-all rather
than raising ``ProgrammingError`` (kernel rejects ``""``) — locks
``_none_if_blank`` on the pattern args."""
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:
cur.schemas(catalog_name="main", schema_name="")
rows = cur.fetchall()
assert len(rows) > 0
assert cur.fetchall() == []


@pytest.mark.parametrize(
"empty_filter", ["schema_name", "table_name", "column_name"]
)
def test_columns_with_empty_string_filter_matches_nothing(conn, empty_filter):
filters = {
"catalog_name": "system",
"schema_name": "information_schema",
"table_name": "tables",
"column_name": "table_catalog",
}
filters[empty_filter] = ""

with conn.cursor() as cur:
cur.columns(**filters)
assert cur.fetchall() == []


def test_tables_table_types_filter_is_case_insensitive(conn):
Expand Down
Loading
Loading