Skip to content

Commit b53da23

Browse files
kesmit13claude
andcommitted
Fix PR review comments for ibis_extras package
- Add error handling for optional ibis imports with informative message - Remove sqlglot dependency, use backtick quoting for identifiers - Fix database context assumption in table methods by extracting database from table's namespace metadata - Table methods now correctly pass database to backend methods Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 5fde25a commit b53da23

File tree

2 files changed

+25
-17
lines changed

2 files changed

+25
-17
lines changed

singlestoredb/ibis_extras/__init__.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,14 @@ def register() -> None:
7878
if _registered:
7979
return
8080

81-
import ibis.expr.types as ir
82-
from ibis.backends.singlestoredb import Backend
81+
try:
82+
import ibis.expr.types as ir
83+
from ibis.backends.singlestoredb import Backend
84+
except ImportError as e:
85+
raise ImportError(
86+
'ibis_extras requires ibis with singlestoredb backend. '
87+
'Install with: pip install "singlestoredb[ibis]"',
88+
) from e
8389

8490
# Check for collisions before adding mixins
8591
_check_collisions(Backend, BackendExtensionsMixin)

singlestoredb/ibis_extras/mixins.py

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55
from typing import Protocol
66
from typing import TYPE_CHECKING
77

8-
import sqlglot as sg
9-
108
if TYPE_CHECKING:
119
from contextlib import AbstractContextManager
1210

@@ -34,12 +32,12 @@ def op(self) -> Any: ...
3432
_BackendBase = object
3533
_TableBase = object
3634

37-
_DIALECT = 'singlestore'
38-
3935

4036
def _quote_identifier(name: str) -> str:
4137
"""Quote an identifier (table, database, column name) for safe SQL usage."""
42-
return sg.to_identifier(name, quoted=True).sql(_DIALECT)
38+
# Escape backticks by doubling them (MySQL/SingleStore convention)
39+
escaped = name.replace('`', '``')
40+
return f'`{escaped}`'
4341

4442

4543
def _escape_string_literal(value: str) -> str:
@@ -48,11 +46,14 @@ def _escape_string_literal(value: str) -> str:
4846
return value.replace('\\', '\\\\').replace("'", "''")
4947

5048

51-
def _get_singlestore_backend(table: ir.Table) -> BackendExtensionsMixin:
52-
"""Get SingleStoreDB backend from table, or raise error."""
49+
def _get_table_backend_and_db(
50+
table: ir.Table,
51+
) -> tuple[BackendExtensionsMixin, str | None]:
52+
"""Get SingleStoreDB backend and database from table."""
5353
op = table.op()
5454
if hasattr(op, 'source') and op.source.name == 'singlestoredb':
55-
return op.source # type: ignore[return-value]
55+
db = getattr(getattr(op, 'namespace', None), 'database', None)
56+
return op.source, db # type: ignore[return-value]
5657
raise TypeError(
5758
f'This method only works with SingleStoreDB tables, '
5859
f"got {getattr(op.source, 'name', 'unknown')} backend",
@@ -184,13 +185,13 @@ class TableExtensionsMixin(_TableBase):
184185

185186
def optimize(self) -> None:
186187
"""Optimize this table (SingleStoreDB only)."""
187-
backend = _get_singlestore_backend(self)
188-
backend.optimize_table(self.get_name())
188+
backend, db = _get_table_backend_and_db(self)
189+
backend.optimize_table(self.get_name(), database=db)
189190

190191
def get_stats(self) -> dict[str, Any]:
191192
"""Get statistics for this table (SingleStoreDB only)."""
192-
backend = _get_singlestore_backend(self)
193-
return backend.get_table_stats(self.get_name())
193+
backend, db = _get_table_backend_and_db(self)
194+
return backend.get_table_stats(self.get_name(), database=db)
194195

195196
def get_column_statistics(self, column: str | None = None) -> ir.Table:
196197
"""Get column statistics (SingleStoreDB only).
@@ -200,10 +201,11 @@ def get_column_statistics(self, column: str | None = None) -> ir.Table:
200201
column
201202
Specific column name, or None for all columns.
202203
"""
203-
backend = _get_singlestore_backend(self)
204-
db = _quote_identifier(backend.current_database)
204+
backend, db = _get_table_backend_and_db(self)
205+
db_name = db or backend.current_database
206+
db_quoted = _quote_identifier(db_name)
205207
table = _quote_identifier(self.get_name())
206-
query = f'SHOW COLUMNAR_SEGMENT_INDEX ON {db}.{table}'
208+
query = f'SHOW COLUMNAR_SEGMENT_INDEX ON {db_quoted}.{table}'
207209
if column:
208210
query += f' COLUMNS ({_quote_identifier(column)})'
209211
return backend.sql(query)

0 commit comments

Comments
 (0)