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

fix(dataset): create ES-View dataset raise exception #16623 #16624

Merged
merged 6 commits into from
Sep 14, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 1 addition & 1 deletion superset/connectors/sqla/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def get_physical_table_metadata(
# ensure empty schema
_schema_name = schema_name if schema_name else None
# Table does not exist or is not visible to a connection.
if not database.has_table_by_name(table_name, schema=_schema_name):
if not database.has_table_or_view_by_name(table_name, schema=_schema_name):
Copy link
Member

Choose a reason for hiding this comment

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

Could we decouple has_table_by_name and has_view_by_name?

    if any([
        database.has_table_by_name(table_name, schema=_schema_name), 
        database.has_view_by_name(table_name, schema=_schema_name), 
    ]):
        ....

raise NoSuchTableError

cols = database.get_columns(table_name, schema=_schema_name)
Expand Down
17 changes: 16 additions & 1 deletion superset/db_engine_specs/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
from marshmallow import fields, Schema
from marshmallow.validate import Range
from sqlalchemy import column, select, types
from sqlalchemy.engine.base import Engine
from sqlalchemy.engine.base import Connection, Engine
from sqlalchemy.engine.interfaces import Compiled, Dialect
from sqlalchemy.engine.reflection import Inspector
from sqlalchemy.engine.url import make_url, URL
Expand Down Expand Up @@ -1343,6 +1343,21 @@ def cancel_query(cls, cursor: Any, query: Query, cancel_query_id: str) -> bool:

return False

@classmethod
def _has_view(
cls,
conn: Connection,
dialect: Dialect,
view_name: str,
schema: Optional[str] = None,
):
view_names = dialect.get_view_names(connection=conn, schema=schema)
Copy link
Member

Choose a reason for hiding this comment

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

should catch NotImplementedError here

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 fixed, please review

return view_name in view_names

@classmethod
def has_view(cls, engine: Engine, view_name: str, schema: Optional[str] = None):
Copy link
Member

Choose a reason for hiding this comment

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

Could we move has_view to superset.models.core in order to follow has_table pattern.

return engine.run_callable(cls._has_view, engine.dialect, view_name, schema)


# schema for adding a database by providing parameters instead of the
# full SQLAlchemy URI
Expand Down
14 changes: 14 additions & 0 deletions superset/models/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,20 @@ def has_table_by_name(self, table_name: str, schema: Optional[str] = None) -> bo
engine = self.get_sqla_engine()
return engine.has_table(table_name, schema)

def has_view_by_name(self, view_name: str, schema: Optional[str] = None) -> bool:
engine = self.get_sqla_engine()
return db_engine_specs.BaseEngineSpec.has_view(
engine=engine, view_name=view_name, schema=schema
)

def has_table_or_view_by_name(
Copy link
Member

Choose a reason for hiding this comment

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

same before, decouple has_table_by_name and has_view_by_name

self, name: str, schema: Optional[str] = None
) -> bool:
result = self.has_table_by_name(
table_name=name, schema=schema
) or self.has_view_by_name(view_name=name, schema=schema)
return result

@memoized
def get_dialect(self) -> Dialect:
sqla_url = url.make_url(self.sqlalchemy_uri_decrypted)
Expand Down