Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(table-api): unify exception type for all backends to TableNotFound when a table does not exist #9695

Merged
merged 41 commits into from
Sep 16, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
a7b2efe
refactor: unify exception when TableNotFound
ncclementi Jul 25, 2024
efb1381
chore: update test_create_and_drop_table postgres
ncclementi Jul 26, 2024
e9604c8
chore: catch original mysql no table exc and reraise
ncclementi Jul 26, 2024
c8b1bde
chore: check table name is in the message
ncclementi Jul 26, 2024
8a4f3c8
chore: update message to match regex
ncclementi Jul 26, 2024
fe24900
chore: remove redundant attribute
ncclementi Jul 26, 2024
c5366e4
chore: fix exception catching in mysql
ncclementi Jul 31, 2024
5b0d6a6
chore: fix clickhouse exception catch
ncclementi Jul 31, 2024
33f2fbb
chore: fix exception catch in datafusion
ncclementi Jul 31, 2024
2f865da
chore: fix exception catch in pandas and dask
ncclementi Jul 31, 2024
33c7507
chore: fix exception catching in mysql
ncclementi Jul 31, 2024
ea4571b
chore: fix exception catching in polars
ncclementi Jul 31, 2024
f6f3103
chore: fix regex in trino test
ncclementi Jul 31, 2024
14bb13c
chore: add comment about pyspark exception catch
ncclementi Jul 31, 2024
b79ca92
chore: fix druid exception catch
ncclementi Jul 31, 2024
93f6810
chore: exception catch for flink
ncclementi Jul 31, 2024
71368f7
chore: fix druid exception and test case
ncclementi Jul 31, 2024
00be355
chore: fix druid exception in test_window
ncclementi Jul 31, 2024
9dde9d4
chore: add error to noimpl param to druid test window
ncclementi Aug 1, 2024
4f854fe
chore(impala): implement tablenotfound raising
cpcloud Aug 1, 2024
4d9c14b
chore(exasol): implement tablenotfound raising
cpcloud Aug 1, 2024
ca1f6c6
chore: update exception
ncclementi Aug 1, 2024
dd10ede
chore: update match
ncclementi Aug 1, 2024
ba27187
chore: clean up
ncclementi Aug 1, 2024
1652e50
chore: clean up
ncclementi Aug 1, 2024
6b67f4b
chore: update table name for test
ncclementi Aug 1, 2024
6551046
chore: clean up
ncclementi Aug 1, 2024
c13dac9
chore: cleanup pyspark exception handling
ncclementi Aug 1, 2024
20af342
chore: cleanup test
ncclementi Aug 1, 2024
52df21c
chore: fix import error in pyspark 3.3
ncclementi Aug 1, 2024
9fe1ed9
chore(snowflake): please, no more
cpcloud Aug 1, 2024
afccae4
chore(bigquery): raise tablenotfound and consolidate
cpcloud Aug 1, 2024
33bf6a1
Apply suggestions from code review
cpcloud Aug 1, 2024
e501cc2
chore: fix typo
ncclementi Aug 1, 2024
0541e17
chore(pyspark): handle the difference between 3.3 and 3.5
cpcloud Aug 1, 2024
ecc40e7
Apply suggestions from code review
cpcloud Aug 1, 2024
14ccba6
chore(druid): check for table existence instead of pattern matching
cpcloud Aug 1, 2024
a087c14
chore: comment about flink; ignore case and table name
cpcloud Aug 1, 2024
1e2992f
chore: fix conflicts
ncclementi Sep 13, 2024
061860a
chore: swap in common exception for tablenotfound
gforsyth Sep 13, 2024
c22979f
chore: address review comments
ncclementi Sep 15, 2024
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
11 changes: 9 additions & 2 deletions ibis/backends/clickhouse/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import ast
import contextlib
import glob
import re
from contextlib import closing
from functools import partial
from typing import TYPE_CHECKING, Any, Literal
Expand All @@ -14,6 +15,7 @@
import sqlglot as sg
import sqlglot.expressions as sge
import toolz
from clickhouse_connect.driver.exceptions import DatabaseError
from clickhouse_connect.driver.external import ExternalData

import ibis
Expand Down Expand Up @@ -501,8 +503,13 @@ def get_schema(
"`catalog` namespaces are not supported by clickhouse"
)
query = sge.Describe(this=sg.table(table_name, db=database))
with self._safe_raw_sql(query) as results:
names, types, *_ = results.result_columns
try:
with self._safe_raw_sql(query) as results:
names, types, *_ = results.result_columns
except DatabaseError as e:
if re.search(r"\bUNKNOWN_TABLE\b", str(e)):
raise com.TableNotFound(table_name) from e

return sch.Schema(
dict(zip(names, map(self.compiler.type_mapper.from_string, types)))
)
Expand Down
12 changes: 7 additions & 5 deletions ibis/backends/dask/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,13 @@ def read_parquet(
return self.table(table_name)

def get_schema(self, table_name, *, database=None):
try:
schema = self.schemas[table_name]
except KeyError:
df = self.dictionary[table_name]
self.schemas[table_name] = schema = PandasData.infer_table(df.head(1))
df = self.dictionary.get(table_name)
if df is None:
raise com.TableNotFound(table_name)
else:
schema = self.schemas.get(table_name)
if schema is None:
self.schemas[table_name] = schema = PandasData.infer_table(df.head(1))

return schema

Expand Down
3 changes: 3 additions & 0 deletions ibis/backends/datafusion/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,9 @@ def get_schema(
else:
database = catalog.database()

if table_name not in database.names():
raise com.TableNotFound(table_name)

table = database.table(table_name)
return sch.schema(table.schema)

Expand Down
19 changes: 14 additions & 5 deletions ibis/backends/druid/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,22 @@

import contextlib
import json
import re
from typing import TYPE_CHECKING, Any
from urllib.parse import unquote_plus

import pydruid.db
import sqlglot as sg

import ibis.common.exceptions as com
import ibis.expr.datatypes as dt
import ibis.expr.schema as sch
from ibis import util
from ibis.backends.sql import SQLBackend
from ibis.backends.sql.compilers import DruidCompiler
from ibis.backends.sql.compilers.base import STAR
from ibis.backends.sql.datatypes import DruidType
from ibis.backends.tests.errors import PyDruidProgrammingError

if TYPE_CHECKING:
from collections.abc import Iterable, Mapping
Expand Down Expand Up @@ -130,11 +133,17 @@ def get_schema(
catalog: str | None = None,
database: str | None = None,
) -> sch.Schema:
return self._get_schema_using_query(
sg.select(STAR)
.from_(sg.table(table_name, db=database, catalog=catalog))
.sql(self.dialect)
)
try:
schema = self._get_schema_using_query(
sg.select(STAR)
.from_(sg.table(table_name, db=database, catalog=catalog))
.sql(self.dialect)
)
ncclementi marked this conversation as resolved.
Show resolved Hide resolved
except PyDruidProgrammingError as e:
if re.search(r"\bINVALID_INPUT\b", str(e)):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't able to get anything better than this matching, if anyone has any ideas, suggestions welcome

raise com.TableNotFound(table_name) from e
cpcloud marked this conversation as resolved.
Show resolved Hide resolved

return schema

def _fetch_from_cursor(self, cursor, schema: sch.Schema) -> pd.DataFrame:
import pandas as pd
Expand Down
2 changes: 1 addition & 1 deletion ibis/backends/duckdb/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ def get_schema(
meta = cur.fetch_arrow_table()

if not meta:
raise exc.IbisError(f"Table not found: {table_name!r}")
raise exc.TableNotFound(table_name)

names = meta["column_name"].to_pylist()
types = meta["data_type"].to_pylist()
Expand Down
12 changes: 11 additions & 1 deletion ibis/backends/flink/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import itertools
import re
from typing import TYPE_CHECKING, Any

import sqlglot as sg
Expand Down Expand Up @@ -303,7 +304,16 @@
qualified_name = sg.table(table_name, db=catalog, catalog=database).sql(
self.name
)
table = self._table_env.from_path(qualified_name)
try:
table = self._table_env.from_path(qualified_name)
except Py4JJavaError as e:

Check warning on line 309 in ibis/backends/flink/__init__.py

View check run for this annotation

Codecov / codecov/patch

ibis/backends/flink/__init__.py#L307-L309

Added lines #L307 - L309 were not covered by tests
# This seems to msg specific but not sure what a good work around is
if re.search(
rf"Table `{re.escape(table_name)}` was not found",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't like this, but I'm not sure if there is a better way to get the error matching here. Suggestions welcomed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cpcloud do we think this is alright for flink? It's a bit specific but I didn't find an alternative, If so I'll remove the comment here

Copy link
Member

Choose a reason for hiding this comment

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

It seems like there's some poking around the java object that could be done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried multiple thinks, I wasn't able to land in anything better than this, I tried inspecting e.java_exception to get the getSQLState but I wasn't able. Maybe I'm missing something.

Copy link
Member

Choose a reason for hiding this comment

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

I went ahead and changed this to catch the error, and then check for table existence instead of matching.

INVALID_INPUT is way too general of a string to match on in any case.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I was commenting on the druid one...this is the flink one.

str(e.java_exception.toString()),
):
raise exc.TableNotFound(table_name) from e

Check warning on line 315 in ibis/backends/flink/__init__.py

View check run for this annotation

Codecov / codecov/patch

ibis/backends/flink/__init__.py#L315

Added line #L315 was not covered by tests
cpcloud marked this conversation as resolved.
Show resolved Hide resolved

pyflink_schema = table.get_schema()

return sch.Schema.from_pyarrow(
Expand Down
2 changes: 1 addition & 1 deletion ibis/backends/mssql/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ def get_schema(

if not meta:
fqn = sg.table(name, db=database, catalog=catalog).sql(self.dialect)
raise com.IbisError(f"Table not found: {fqn}")
raise com.TableNotFound(fqn)

mapping = {}
for (
Expand Down
11 changes: 8 additions & 3 deletions ibis/backends/mysql/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
import pymysql
import sqlglot as sg
import sqlglot.expressions as sge
from pymysql.constants.ER import NO_SUCH_TABLE
from pymysql.err import ProgrammingError

import ibis
import ibis.common.exceptions as com
Expand Down Expand Up @@ -211,7 +213,6 @@ def _get_schema_using_query(self, query: str) -> sch.Schema:
.limit(0)
.sql(self.dialect)
)

return sch.Schema(
{
field.name: _type_from_cursor_info(descr, field)
Expand All @@ -227,8 +228,12 @@ def get_schema(
).sql(self.dialect)

with self.begin() as cur:
cur.execute(sge.Describe(this=table).sql(self.dialect))
result = cur.fetchall()
try:
cur.execute(sge.Describe(this=table).sql(self.dialect))
result = cur.fetchall()
except ProgrammingError as e:
if e.args[0] == NO_SUCH_TABLE:
raise com.TableNotFound(name) from e
ncclementi marked this conversation as resolved.
Show resolved Hide resolved

type_mapper = self.compiler.type_mapper
fields = {
Expand Down
2 changes: 1 addition & 1 deletion ibis/backends/oracle/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ def get_schema(
results = cur.fetchall()

if not results:
raise exc.IbisError(f"Table not found: {name!r}")
raise exc.TableNotFound(name)

type_mapper = self.compiler.type_mapper
fields = {
Expand Down
12 changes: 7 additions & 5 deletions ibis/backends/pandas/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,13 @@ def table(self, name: str, schema: sch.Schema | None = None):
return ops.DatabaseTable(name, overridden_schema, self).to_expr()

def get_schema(self, table_name, *, database=None):
try:
schema = self.schemas[table_name]
except KeyError:
df = self.dictionary[table_name]
self.schemas[table_name] = schema = PandasData.infer_table(df)
df = self.dictionary.get(table_name)
if df is None:
raise com.TableNotFound(table_name)
else:
schema = self.schemas.get(table_name)
if schema is None:
self.schemas[table_name] = schema = PandasData.infer_table(df)

return schema

Expand Down
6 changes: 5 additions & 1 deletion ibis/backends/polars/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,11 @@ def list_tables(self, like=None, database=None):
return self._filter_with_like(list(self._tables.keys()), like)

def table(self, name: str) -> ir.Table:
schema = sch.infer(self._tables[name])
table = self._tables.get(name)
if table is None:
raise com.TableNotFound(name)

schema = sch.infer(table)
return ops.DatabaseTable(name, schema, self).to_expr()

@deprecated(
Expand Down
2 changes: 1 addition & 1 deletion ibis/backends/postgres/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ def get_schema(
rows = cur.fetchall()

if not rows:
raise com.IbisError(f"Table not found: {name!r}")
raise com.TableNotFound(name)

return sch.Schema(
{
Expand Down
2 changes: 1 addition & 1 deletion ibis/backends/postgres/tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ def test_create_and_drop_table(con, temp_table, params):

con.drop_table(temp_table, **params)

with pytest.raises(com.IbisError):
with pytest.raises(com.TableNotFound, match=temp_table):
con.table(temp_table, **params)


Expand Down
10 changes: 9 additions & 1 deletion ibis/backends/pyspark/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import sqlglot.expressions as sge
from packaging.version import parse as vparse
from pyspark import SparkConf
from pyspark.errors.exceptions.base import AnalysisException
from pyspark.sql import SparkSession
from pyspark.sql.types import BooleanType, DoubleType, LongType, StringType

Expand Down Expand Up @@ -514,7 +515,14 @@ def get_schema(
table_loc = self._to_sqlglot_table((catalog, database))
catalog, db = self._to_catalog_db_tuple(table_loc)
with self._active_catalog_database(catalog, db):
df = self._session.table(table_name)
try:
df = self._session.table(table_name)
except AnalysisException as e:
# we can alternative check for e.getErrorClass == 'TABLE_OR_VIEW_NOT_FOUND'
if e.getSqlState() == "42P01":
# spark sqlerror state for undefined_table see https://spark.apache.org/docs/3.4.1/sql-error-conditions-sqlstates.html
ncclementi marked this conversation as resolved.
Show resolved Hide resolved
raise com.TableNotFound(table_name)
cpcloud marked this conversation as resolved.
Show resolved Hide resolved

struct = PySparkType.to_ibis(df.schema)

return sch.Schema(struct)
Expand Down
2 changes: 1 addition & 1 deletion ibis/backends/sqlite/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ def _inspect_schema(
cur.execute(sql)
rows = cur.fetchall()
if not rows:
raise com.IbisError(f"Table not found: {table_name!r}")
raise com.TableNotFound(table_name)

table_info = {name: (typ, not notnull) for name, typ, notnull in rows}

Expand Down
5 changes: 5 additions & 0 deletions ibis/backends/tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -1629,3 +1629,8 @@ def test_from_connection(con, top_level):
new_con = backend.from_connection(getattr(con, CON_ATTR.get(con.name, "con")))
result = int(new_con.execute(ibis.literal(1, type="int")))
assert result == 1


def test_table_not_found(con):
with pytest.raises(com.TableNotFound, match="Table not found"):
con.table("foo")
ncclementi marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 1 addition & 2 deletions ibis/backends/tests/test_join.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import ibis
import ibis.common.exceptions as com
import ibis.expr.schema as sch
from ibis.backends.tests.errors import PyDruidProgrammingError

sqlite_right_or_full_mark = pytest.mark.notyet(
["sqlite"],
Expand Down Expand Up @@ -292,7 +291,7 @@ def test_join_with_trivial_predicate(awards_players, predicate, how, pandas_valu
assert len(result) == len(expected)


@pytest.mark.notimpl(["druid"], raises=PyDruidProgrammingError)
@pytest.mark.notimpl(["druid"], raises=com.TableNotFound)
@pytest.mark.parametrize(
("how", "nrows", "gen_right", "keys"),
[
Expand Down
7 changes: 6 additions & 1 deletion ibis/backends/tests/test_window.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@

pytestmark = [
pytest.mark.notimpl(
["druid"], raises=(com.OperationNotDefinedError, PyDruidProgrammingError)
["druid"],
raises=(
com.OperationNotDefinedError,
com.TableNotFound,
PyDruidProgrammingError,
),
)
]

Expand Down
2 changes: 1 addition & 1 deletion ibis/backends/trino/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def get_schema(

if not meta:
fqn = sg.table(table_name, db=database, catalog=catalog).sql(self.name)
raise com.IbisError(f"Table not found: {fqn}")
raise com.TableNotFound(fqn)

return sch.Schema(
{
Expand Down
2 changes: 1 addition & 1 deletion ibis/backends/trino/tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ def test_table_access_database_schema(con):
t = con.table("region", database=("tpch", "sf1"))
assert t.count().execute()

with pytest.raises(exc.IbisError, match='Table not found: tpch."tpch.sf1".region'):
with pytest.raises(exc.TableNotFound, match=r"Table not found: .*region"):
ncclementi marked this conversation as resolved.
Show resolved Hide resolved
con.table("region", database=("tpch", "tpch.sf1"))

with pytest.raises(exc.IbisError, match="Overspecified table hierarchy provided"):
Expand Down
6 changes: 6 additions & 0 deletions ibis/common/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@
from collections.abc import Callable


class TableNotFound(Exception):
def __init__(self, table_name, message="Table not found"):
self.message = f"{message}: {table_name!r}"
super().__init__(self.message)
ncclementi marked this conversation as resolved.
Show resolved Hide resolved


class IbisError(Exception):
"""IbisError."""

Expand Down
Loading