Skip to content

Commit

Permalink
refactor(table): deprecate schema
Browse files Browse the repository at this point in the history
`schema` kwarg is deprecated in all `Backend.table` usage. Wherever it
was present, we now have handling to warn if `schema` is passed, and to
encourage users to switch usage to the tuple of `(catalog, database)`.
  • Loading branch information
gforsyth committed Mar 25, 2024
1 parent 6273e7e commit dfb8734
Show file tree
Hide file tree
Showing 9 changed files with 66 additions and 62 deletions.
67 changes: 27 additions & 40 deletions ibis/backends/bigquery/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,7 @@ def __init__(self, *args, **kwargs) -> None:
self._session_dataset: bq.DatasetReference | None = None
self._query_cache.lookup = lambda name: self.table(
name,
schema=self._session_dataset.dataset_id,
database=self._session_dataset.project,
database=(self._session_dataset.project, self._session_dataset.dataset_id),
).op()

def _register_in_memory_table(self, op: ops.InMemoryTable) -> None:
Expand Down Expand Up @@ -518,47 +517,35 @@ def drop_database(
def table(
self, name: str, database: str | None = None, schema: str | None = None
) -> ir.Table:
if database is not None and schema is None:
raise com.IbisInputError(
f"The {self.name} backend cannot return a table expression using only a "
"`database` specifier. Include a `schema` argument."
)
if schema is not None:
# TODO: remove _warn_schema when the schema kwarg is removed
from ibis.util import _warn_schema

_warn_schema()
if database is not None and schema is not None:
if isinstance(database, str):
table_loc = f"{database}.{schema}"
elif isinstance(database, tuple):
table_loc = database + schema
elif schema is not None:
table_loc = schema
elif database is not None:
table_loc = database
else:
table_loc = None

table = sg.parse_one(name, into=sge.Table, read=self.name)
table_loc = self._to_sqlglot_table(table_loc)

# table.catalog will be the empty string
if table.catalog:
if database is not None:
raise com.IbisInputError(
"Cannot specify database both in the table name and as an argument"
)
else:
database = table.catalog
if table_loc is not None:
if (sg_cat := table_loc.args["catalog"]) is not None:
sg_cat.args["quoted"] = False
if (sg_db := table_loc.args["db"]) is not None:
sg_db.args["quoted"] = False
table_loc = table_loc.sql(dialect=self.name)

# table.db will be the empty string
if table.db:
if schema is not None:
raise com.IbisInputError(
"Cannot specify schema both in the table name and as an argument"
)
else:
schema = table.db

if database is not None and schema is None:
database = sg.parse_one(database, into=sge.Table, read=self.name)
database.args["quoted"] = False
database = database.sql(dialect=self.name)
elif database is None and schema is not None:
database = sg.parse_one(schema, into=sge.Table, read=self.name)
database.args["quoted"] = False
database = database.sql(dialect=self.name)
else:
database = (
sg.table(schema, db=database, quoted=False).sql(dialect=self.name)
or None
)
project, dataset = self._parse_project_and_dataset(table_loc)

project, dataset = self._parse_project_and_dataset(database)
table = sg.parse_one(name, into=sge.Table, read=self.name)

bq_table = self.client.get_table(
bq.TableReference(
Expand Down Expand Up @@ -1059,7 +1046,7 @@ def create_table(
sql = stmt.sql(self.name)

self.raw_sql(sql)
return self.table(table.name, schema=table.db, database=table.catalog)
return self.table(table.name, database=(table.catalog, table.db))

def drop_table(
self,
Expand Down
13 changes: 10 additions & 3 deletions ibis/backends/duckdb/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ def table(
name
Table name
schema
Schema name
[deprecated] Schema name
database
Database name
Expand All @@ -274,15 +274,22 @@ def table(
Table expression
"""
table_schema = self.get_schema(name, catalog=schema, database=database)
table_loc = self._warn_and_create_table_loc(database, schema)

catalog, database = None, None
if table_loc is not None:
catalog = table_loc.catalog or None
database = table_loc.db or None

table_schema = self.get_schema(name, catalog=catalog, database=database)
# load geospatial only if geo columns
if any(typ.is_geospatial() for typ in table_schema.types):
self.load_extension("spatial")
return ops.DatabaseTable(
name,
schema=table_schema,
source=self,
namespace=ops.Namespace(database=database, schema=schema),
namespace=ops.Namespace(database=catalog, schema=database),
).to_expr()

def get_schema(
Expand Down
6 changes: 4 additions & 2 deletions ibis/backends/duckdb/tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,13 @@ def test_cross_db(tmpdir):

con2.attach(path1, name="test1", read_only=True)

t1_from_con2 = con2.table("t1", schema="main", database="test1")
with pytest.warns(FutureWarning):
t1_from_con2 = con2.table("t1", schema="main", database="test1")
assert t1_from_con2.schema() == t2.schema()
assert t1_from_con2.execute().equals(t2.execute())

foo_t1_from_con2 = con2.table("t1", schema="foo", database="test1")
with pytest.warns(FutureWarning):
foo_t1_from_con2 = con2.table("t1", schema="foo", database="test1")
assert foo_t1_from_con2.schema() == t2.schema()
assert foo_t1_from_con2.execute().equals(t2.execute())

Expand Down
2 changes: 1 addition & 1 deletion ibis/backends/mysql/tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def tmp_t(con):


def test_get_schema_from_query_other_schema(con, tmp_t):
t = con.table(tmp_t, schema="test_schema")
t = con.table(tmp_t, database="test_schema")
assert t.schema() == ibis.schema({"x": dt.inet})


Expand Down
2 changes: 1 addition & 1 deletion ibis/backends/oracle/tests/test_datatypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
def test_failed_column_inference(con):
# This is a table in the Docker container that we know fails
# column type inference, so if is loaded, then we're in OK shape.
table = con.table("ALL_DOMAINS", schema="SYS")
table = con.table("ALL_DOMAINS", database="SYS")
assert len(table.columns)


Expand Down
2 changes: 1 addition & 1 deletion ibis/backends/snowflake/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ def create_table(
with self._safe_raw_sql(create_stmt):
pass

return self.table(name, schema=db, database=catalog)
return self.table(name, database=(catalog, db))

def read_csv(
self, path: str | Path, table_name: str | None = None, **kwargs: Any
Expand Down
4 changes: 2 additions & 2 deletions ibis/backends/snowflake/tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def test_cross_db_access(con, temp_catalog, temp_db):
con.raw_sql(
f'CREATE TABLE "{temp_catalog}"."{temp_db}"."{table}" ("x" INT)'
).close()
t = con.table(table, schema=temp_db, database=temp_catalog)
t = con.table(table, database=(temp_catalog, temp_db))
assert t.schema() == ibis.schema(dict(x="int"))
assert t.execute().empty

Expand All @@ -55,7 +55,7 @@ def test_cross_db_create_table(con, temp_catalog, temp_db):
table_name = gen_name("tmp_table")
data = pd.DataFrame({"key": list("abc"), "value": [[1], [2], [3]]})
table = con.create_table(table_name, data, database=f"{temp_catalog}.{temp_db}")
queried_table = con.table(table_name, database=temp_catalog, schema=temp_db)
queried_table = con.table(table_name, database=(temp_catalog, temp_db))

tm.assert_frame_equal(table.execute(), data)
tm.assert_frame_equal(queried_table.execute(), data)
Expand Down
16 changes: 13 additions & 3 deletions ibis/backends/sql/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,10 @@ def _fetch_from_cursor(self, cursor, schema: sch.Schema) -> pd.DataFrame:
return df

def table(
self, name: str, schema: str | None = None, database: str | None = None
self,
name: str,
schema: str | None = None,
database: tuple[str, str] | str | None = None,
) -> ir.Table:
"""Construct a table expression.
Expand All @@ -114,7 +117,7 @@ def table(
name
Table name
schema
Schema name
[deprecated] Schema name
database
Database name
Expand All @@ -124,7 +127,14 @@ def table(
Table expression
"""
table_schema = self.get_schema(name, catalog=schema, database=database)
table_loc = self._warn_and_create_table_loc(database, schema)

catalog, database = None, None
if table_loc is not None:
catalog = table_loc.catalog or None
database = table_loc.db or None

table_schema = self.get_schema(name, catalog=catalog, database=database)
return ops.DatabaseTable(
name,
schema=table_schema,
Expand Down
16 changes: 7 additions & 9 deletions ibis/backends/trino/tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,16 +83,16 @@ def test_con_source(source, expected):


@pytest.mark.parametrize(
("database", "schema", "table"),
("catalog", "database", "table"),
[
# tables known to exist
("system", "metadata", "table_comments"),
("tpcds", "sf1", "store"),
("tpch", "sf1", "nation"),
],
)
def test_cross_schema_table_access(con, database, schema, table):
t = con.table(table, schema=schema, database=database)
def test_cross_schema_table_access(con, catalog, database, table):
t = con.table(table, database=(catalog, database))
assert t.count().execute()


Expand Down Expand Up @@ -149,16 +149,14 @@ def test_create_table_timestamp():


def test_table_access_database_schema(con):
t = con.table("region", schema="sf1", database="tpch")
t = con.table("region", database=("tpch", "sf1"))
assert t.count().execute()

with pytest.raises(exc.IbisError, match='Table not found: tpch."tpch.sf1".region'):
con.table("region", schema="tpch.sf1", database="tpch")
con.table("region", database=("tpch", "tpch.sf1"))

with pytest.raises(
exc.IbisError, match='Table not found: system."tpch.sf1".region'
):
con.table("region", schema="tpch.sf1", database="system")
with pytest.raises(exc.IbisError, match="Overspecified table hierarchy provided"):
con.table("region", database="system.tpch.sf1")


def test_list_tables_schema_warning_refactor(con):
Expand Down

0 comments on commit dfb8734

Please sign in to comment.