From 81725e3ee5ded51828bd6d82b98af5125fd1dc5b Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Fri, 24 May 2024 11:06:53 -0700 Subject: [PATCH 1/8] chore: remove sl_ tables --- superset/columns/models.py | 115 ------------------- superset/daos/datasource.py | 6 +- superset/datasets/models.py | 118 -------------------- superset/datasets/schemas.py | 13 --- superset/tables/models.py | 206 ----------------------------------- superset/utils/core.py | 1 - 6 files changed, 1 insertion(+), 458 deletions(-) delete mode 100644 superset/columns/models.py delete mode 100644 superset/datasets/models.py delete mode 100644 superset/tables/models.py diff --git a/superset/columns/models.py b/superset/columns/models.py deleted file mode 100644 index 142e3d6723f8..000000000000 --- a/superset/columns/models.py +++ /dev/null @@ -1,115 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -""" -Column model. - -This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909), -and represents a "column" in a table or dataset. In addition to a column, new models for -tables, metrics, and datasets were also introduced. - -These models are not fully implemented, and shouldn't be used yet. -""" - -import sqlalchemy as sa -from flask_appbuilder import Model - -from superset.models.helpers import ( - AuditMixinNullable, - ExtraJSONMixin, - ImportExportMixin, -) - -UNKNOWN_TYPE = "UNKNOWN" - - -class Column( - Model, - AuditMixinNullable, - ExtraJSONMixin, - ImportExportMixin, -): - """ - A "column". - - The definition of column here is overloaded: it can represent a physical column in a - database relation (table or view); a computed/derived column on a dataset; or an - aggregation expression representing a metric. - """ - - __tablename__ = "sl_columns" - - id = sa.Column(sa.Integer, primary_key=True) - - # Assuming the column is an aggregation, is it additive? Useful for determining which - # aggregations can be done on the metric. Eg, ``COUNT(DISTINCT user_id)`` is not - # additive, so it shouldn't be used in a ``SUM``. - is_additive = sa.Column(sa.Boolean, default=False) - - # Is this column an aggregation (metric)? - is_aggregation = sa.Column(sa.Boolean, default=False) - - is_filterable = sa.Column(sa.Boolean, nullable=False, default=True) - is_dimensional = sa.Column(sa.Boolean, nullable=False, default=False) - - # Is an increase desired? Useful for displaying the results of A/B tests, or setting - # up alerts. Eg, this is true for "revenue", but false for "latency". - is_increase_desired = sa.Column(sa.Boolean, default=True) - - # Column is managed externally and should be read-only inside Superset - is_managed_externally = sa.Column(sa.Boolean, nullable=False, default=False) - - # Is this column a partition? Useful for scheduling queries and previewing the latest - # data. - is_partition = sa.Column(sa.Boolean, default=False) - - # Does the expression point directly to a physical column? - is_physical = sa.Column(sa.Boolean, default=True) - - # Is this a spatial column? This could be leveraged in the future for spatial - # visualizations. - is_spatial = sa.Column(sa.Boolean, default=False) - - # Is this a time column? Useful for plotting time series. - is_temporal = sa.Column(sa.Boolean, default=False) - - # We use ``sa.Text`` for these attributes because (1) in modern databases the - # performance is the same as ``VARCHAR``[1] and (2) because some table names can be - # **really** long (eg, Google Sheets URLs). - # - # [1] https://www.postgresql.org/docs/9.1/datatype-character.html - name = sa.Column(sa.Text) - # Raw type as returned and used by db engine. - type = sa.Column(sa.Text, default=UNKNOWN_TYPE) - - # Assigns column advanced type to determine custom behavior - # does nothing unless feature flag ENABLE_ADVANCED_DATA_TYPES in true - advanced_data_type = sa.Column(sa.Text) - - # Columns are defined by expressions. For tables, these are the actual columns names, - # and should match the ``name`` attribute. For datasets, these can be any valid SQL - # expression. If the SQL expression is an aggregation the column is a metric, - # otherwise it's a computed column. - expression = sa.Column(sa.Text) - unit = sa.Column(sa.Text) - - # Additional metadata describing the column. - description = sa.Column(sa.Text) - warning_text = sa.Column(sa.Text) - external_url = sa.Column(sa.Text, nullable=True) - - def __repr__(self) -> str: - return f"" diff --git a/superset/daos/datasource.py b/superset/daos/datasource.py index 0e6058d6abc8..9a48bb86b7b6 100644 --- a/superset/daos/datasource.py +++ b/superset/daos/datasource.py @@ -22,14 +22,12 @@ from superset.connectors.sqla.models import SqlaTable from superset.daos.base import BaseDAO from superset.daos.exceptions import DatasourceNotFound, DatasourceTypeNotSupportedError -from superset.datasets.models import Dataset from superset.models.sql_lab import Query, SavedQuery -from superset.tables.models import Table from superset.utils.core import DatasourceType logger = logging.getLogger(__name__) -Datasource = Union[Dataset, SqlaTable, Table, Query, SavedQuery] +Datasource = Union[SqlaTable, Query, SavedQuery] class DatasourceDAO(BaseDAO[Datasource]): @@ -37,8 +35,6 @@ class DatasourceDAO(BaseDAO[Datasource]): DatasourceType.TABLE: SqlaTable, DatasourceType.QUERY: Query, DatasourceType.SAVEDQUERY: SavedQuery, - DatasourceType.DATASET: Dataset, - DatasourceType.SLTABLE: Table, } @classmethod diff --git a/superset/datasets/models.py b/superset/datasets/models.py deleted file mode 100644 index 76e2156e61ff..000000000000 --- a/superset/datasets/models.py +++ /dev/null @@ -1,118 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -""" -Dataset model. - -This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909), -and represents a "dataset" -- either a physical table or a virtual. In addition to a -dataset, new models for columns, metrics, and tables were also introduced. - -These models are not fully implemented, and shouldn't be used yet. -""" - -import sqlalchemy as sa -from flask_appbuilder import Model -from sqlalchemy.orm import backref, relationship - -from superset import security_manager -from superset.columns.models import Column -from superset.models.core import Database -from superset.models.helpers import ( - AuditMixinNullable, - ExtraJSONMixin, - ImportExportMixin, -) -from superset.tables.models import Table - -dataset_column_association_table = sa.Table( - "sl_dataset_columns", - Model.metadata, # pylint: disable=no-member - sa.Column( - "dataset_id", - sa.ForeignKey("sl_datasets.id"), - primary_key=True, - ), - sa.Column( - "column_id", - sa.ForeignKey("sl_columns.id"), - primary_key=True, - ), -) - -dataset_table_association_table = sa.Table( - "sl_dataset_tables", - Model.metadata, # pylint: disable=no-member - sa.Column("dataset_id", sa.ForeignKey("sl_datasets.id"), primary_key=True), - sa.Column("table_id", sa.ForeignKey("sl_tables.id"), primary_key=True), -) - -dataset_user_association_table = sa.Table( - "sl_dataset_users", - Model.metadata, # pylint: disable=no-member - sa.Column("dataset_id", sa.ForeignKey("sl_datasets.id"), primary_key=True), - sa.Column("user_id", sa.ForeignKey("ab_user.id"), primary_key=True), -) - - -class Dataset(AuditMixinNullable, ExtraJSONMixin, ImportExportMixin, Model): - """ - A table/view in a database. - """ - - __tablename__ = "sl_datasets" - - id = sa.Column(sa.Integer, primary_key=True) - database_id = sa.Column(sa.Integer, sa.ForeignKey("dbs.id"), nullable=False) - database: Database = relationship( - "Database", - backref=backref("datasets", cascade="all, delete-orphan"), - foreign_keys=[database_id], - ) - # The relationship between datasets and columns is 1:n, but we use a - # many-to-many association table to avoid adding two mutually exclusive - # columns(dataset_id and table_id) to Column - columns: list[Column] = relationship( - "Column", - secondary=dataset_column_association_table, - cascade="all, delete-orphan", - single_parent=True, - backref="datasets", - ) - owners = relationship( - security_manager.user_model, secondary=dataset_user_association_table - ) - tables: list[Table] = relationship( - "Table", secondary=dataset_table_association_table, backref="datasets" - ) - - # Does the dataset point directly to a ``Table``? - is_physical = sa.Column(sa.Boolean, default=False) - - # Column is managed externally and should be read-only inside Superset - is_managed_externally = sa.Column(sa.Boolean, nullable=False, default=False) - - # We use ``sa.Text`` for these attributes because (1) in modern databases the - # performance is the same as ``VARCHAR``[1] and (2) because some table names can be - # **really** long (eg, Google Sheets URLs). - # - # [1] https://www.postgresql.org/docs/9.1/datatype-character.html - name = sa.Column(sa.Text) - expression = sa.Column(sa.Text) - external_url = sa.Column(sa.Text, nullable=True) - - def __repr__(self) -> str: - return f"" diff --git a/superset/datasets/schemas.py b/superset/datasets/schemas.py index 5ce062167595..426cde01b5e1 100644 --- a/superset/datasets/schemas.py +++ b/superset/datasets/schemas.py @@ -22,9 +22,7 @@ from flask_babel import lazy_gettext as _ from marshmallow import fields, pre_load, Schema, ValidationError from marshmallow.validate import Length -from marshmallow_sqlalchemy import SQLAlchemyAutoSchema -from superset.datasets.models import Dataset from superset.exceptions import SupersetMarshmallowValidationError get_delete_ids_schema = {"type": "array", "items": {"type": "integer"}} @@ -291,17 +289,6 @@ class GetOrCreateDatasetSchema(Schema): always_filter_main_dttm = fields.Boolean(load_default=False) -class DatasetSchema(SQLAlchemyAutoSchema): - """ - Schema for the ``Dataset`` model. - """ - - class Meta: # pylint: disable=too-few-public-methods - model = Dataset - load_instance = True - include_relationships = True - - class DatasetCacheWarmUpRequestSchema(Schema): db_name = fields.String( required=True, diff --git a/superset/tables/models.py b/superset/tables/models.py deleted file mode 100644 index 2616aaf90f6b..000000000000 --- a/superset/tables/models.py +++ /dev/null @@ -1,206 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -""" -Table model. - -This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909), -and represents a "table" in a given database -- either a physical table or a view. In -addition to a table, new models for columns, metrics, and datasets were also introduced. - -These models are not fully implemented, and shouldn't be used yet. -""" - -from collections.abc import Iterable -from typing import Any, Optional, TYPE_CHECKING - -import sqlalchemy as sa -from flask_appbuilder import Model -from sqlalchemy import inspect -from sqlalchemy.orm import backref, relationship, Session -from sqlalchemy.schema import UniqueConstraint -from sqlalchemy.sql import and_, or_ - -from superset.columns.models import Column -from superset.connectors.sqla.utils import get_physical_table_metadata -from superset.models.core import Database -from superset.models.helpers import ( - AuditMixinNullable, - ExtraJSONMixin, - ImportExportMixin, -) -from superset.sql_parse import Table as TableName -from superset.superset_typing import ResultSetColumnType - -if TYPE_CHECKING: - from superset.datasets.models import Dataset - -table_column_association_table = sa.Table( - "sl_table_columns", - Model.metadata, # pylint: disable=no-member - sa.Column( - "table_id", - sa.ForeignKey("sl_tables.id", ondelete="CASCADE"), - primary_key=True, - ), - sa.Column( - "column_id", - sa.ForeignKey("sl_columns.id", ondelete="CASCADE"), - primary_key=True, - ), -) - - -class Table(AuditMixinNullable, ExtraJSONMixin, ImportExportMixin, Model): - """ - A table/view in a database. - """ - - __tablename__ = "sl_tables" - - # Note this uniqueness constraint is not part of the physical schema, i.e., it does - # not exist in the migrations. The reason it does not physically exist is MySQL, - # PostgreSQL, etc. have a different interpretation of uniqueness when it comes to NULL - # which is problematic given the catalog and schema are optional. - __table_args__ = (UniqueConstraint("database_id", "catalog", "schema", "name"),) - - id = sa.Column(sa.Integer, primary_key=True) - database_id = sa.Column(sa.Integer, sa.ForeignKey("dbs.id"), nullable=False) - database: Database = relationship( - "Database", - # TODO (betodealmeida): rename the backref to ``tables`` once we get rid of the - # old models. - backref=backref("new_tables", cascade="all, delete-orphan"), - foreign_keys=[database_id], - ) - # The relationship between datasets and columns is 1:n, but we use a - # many-to-many association table to avoid adding two mutually exclusive - # columns(dataset_id and table_id) to Column - columns: list[Column] = relationship( - "Column", - secondary=table_column_association_table, - cascade="all, delete-orphan", - single_parent=True, - # backref is needed for session to skip detaching `dataset` if only `column` - # is loaded. - backref="tables", - ) - datasets: list["Dataset"] # will be populated by Dataset.tables backref - - # We use ``sa.Text`` for these attributes because (1) in modern databases the - # performance is the same as ``VARCHAR``[1] and (2) because some table names can be - # **really** long (eg, Google Sheets URLs). - # - # [1] https://www.postgresql.org/docs/9.1/datatype-character.html - catalog = sa.Column(sa.Text) - schema = sa.Column(sa.Text) - name = sa.Column(sa.Text) - - # Column is managed externally and should be read-only inside Superset - is_managed_externally = sa.Column(sa.Boolean, nullable=False, default=False) - external_url = sa.Column(sa.Text, nullable=True) - - @property - def fullname(self) -> str: - return str(TableName(table=self.name, schema=self.schema, catalog=self.catalog)) - - def __repr__(self) -> str: - return f"" - - def sync_columns(self) -> None: - """Sync table columns with the database. Keep metadata for existing columns""" - try: - column_metadata = get_physical_table_metadata( - self.database, self.name, self.schema - ) - except Exception: # pylint: disable=broad-except - column_metadata = [] - - existing_columns = {column.name: column for column in self.columns} - quote_identifier = self.database.quote_identifier - - def update_or_create_column(column_meta: ResultSetColumnType) -> Column: - column_name: str = column_meta["column_name"] - if column_name in existing_columns: - column = existing_columns[column_name] - else: - column = Column(name=column_name) - column.type = column_meta["type"] - column.is_temporal = column_meta["is_dttm"] - column.expression = quote_identifier(column_name) - column.is_aggregation = False - column.is_physical = True - column.is_spatial = False - column.is_partition = False # TODO: update with accurate is_partition - return column - - self.columns = [update_or_create_column(col) for col in column_metadata] - - @staticmethod - def bulk_load_or_create( - database: Database, - table_names: Iterable[TableName], - default_schema: Optional[str] = None, - sync_columns: Optional[bool] = False, - default_props: Optional[dict[str, Any]] = None, - ) -> list["Table"]: - """ - Load or create multiple Table instances. - """ - if not table_names: - return [] - - if not database.id: - raise Exception( # pylint: disable=broad-exception-raised - "Database must be already saved to metastore" - ) - - default_props = default_props or {} - session: Session = inspect(database).session # pylint: disable=disallowed-name - # load existing tables - predicate = or_( - *[ - and_( - Table.database_id == database.id, - Table.schema == (table.schema or default_schema), - Table.name == table.table, - ) - for table in table_names - ] - ) - all_tables = session.query(Table).filter(predicate).order_by(Table.id).all() - - # add missing tables and pull its columns - existing = {(table.schema, table.name) for table in all_tables} - for table in table_names: - schema = table.schema or default_schema - name = table.table - if (schema, name) not in existing: - new_table = Table( - database=database, - database_id=database.id, - name=name, - schema=schema, - catalog=None, - **default_props, - ) - if sync_columns: - new_table.sync_columns() - all_tables.append(new_table) - existing.add((schema, name)) - session.add(new_table) - - return all_tables diff --git a/superset/utils/core.py b/superset/utils/core.py index 37a11ea7ff59..18e7e60d7891 100644 --- a/superset/utils/core.py +++ b/superset/utils/core.py @@ -147,7 +147,6 @@ class GenericDataType(IntEnum): class DatasourceType(StrEnum): - SLTABLE = "sl_table" TABLE = "table" DATASET = "dataset" QUERY = "query" From 3a89118f13bd10d5f56f56dcd962c3788748a1e8 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Fri, 24 May 2024 11:36:44 -0700 Subject: [PATCH 2/8] add db migration --- ...24_11-31_02f4f7811799_remove_sl__tables.py | 197 ++++++++++++++++++ .../integration_tests/fixtures/datasource.py | 2 - 2 files changed, 197 insertions(+), 2 deletions(-) create mode 100644 superset/migrations/versions/2024-05-24_11-31_02f4f7811799_remove_sl__tables.py diff --git a/superset/migrations/versions/2024-05-24_11-31_02f4f7811799_remove_sl__tables.py b/superset/migrations/versions/2024-05-24_11-31_02f4f7811799_remove_sl__tables.py new file mode 100644 index 000000000000..c333400ba62d --- /dev/null +++ b/superset/migrations/versions/2024-05-24_11-31_02f4f7811799_remove_sl__tables.py @@ -0,0 +1,197 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""remove sl_ tables + +Revision ID: 02f4f7811799 +Revises: f7b6750b67e8 +Create Date: 2024-05-24 11:31:57.115586 + +""" + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = "02f4f7811799" +down_revision = "f7b6750b67e8" + + +def upgrade(): + op.drop_table("sl_columns") + op.drop_table("sl_table_columns") + op.drop_table("sl_dataset_tables") + op.drop_table("sl_dataset_columns") + op.drop_table("sl_tables") + op.drop_table("sl_datasets") + op.drop_table("sl_dataset_users") + + +def downgrade(): + op.create_table( + "sl_dataset_users", + sa.Column("dataset_id", sa.INTEGER(), nullable=False), + sa.Column("user_id", sa.INTEGER(), nullable=False), + sa.ForeignKeyConstraint( + ["dataset_id"], + ["sl_datasets.id"], + ), + sa.ForeignKeyConstraint( + ["user_id"], + ["ab_user.id"], + ), + sa.PrimaryKeyConstraint("dataset_id", "user_id"), + ) + op.create_table( + "sl_datasets", + sa.Column("uuid", sa.NUMERIC(precision=16), nullable=True), + sa.Column("created_on", sa.DATETIME(), nullable=True), + sa.Column("changed_on", sa.DATETIME(), nullable=True), + sa.Column("id", sa.INTEGER(), nullable=False), + sa.Column("database_id", sa.INTEGER(), nullable=False), + sa.Column("is_physical", sa.BOOLEAN(), nullable=True), + sa.Column("is_managed_externally", sa.BOOLEAN(), nullable=False), + sa.Column("name", sa.TEXT(), nullable=True), + sa.Column("expression", sa.TEXT(), nullable=True), + sa.Column("external_url", sa.TEXT(), nullable=True), + sa.Column("extra_json", sa.TEXT(), nullable=True), + sa.Column("created_by_fk", sa.INTEGER(), nullable=True), + sa.Column("changed_by_fk", sa.INTEGER(), nullable=True), + sa.ForeignKeyConstraint( + ["changed_by_fk"], + ["ab_user.id"], + ), + sa.ForeignKeyConstraint( + ["created_by_fk"], + ["ab_user.id"], + ), + sa.ForeignKeyConstraint( + ["database_id"], + ["dbs.id"], + ), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint("uuid"), + ) + op.create_table( + "sl_tables", + sa.Column("uuid", sa.NUMERIC(precision=16), nullable=True), + sa.Column("created_on", sa.DATETIME(), nullable=True), + sa.Column("changed_on", sa.DATETIME(), nullable=True), + sa.Column("id", sa.INTEGER(), nullable=False), + sa.Column("database_id", sa.INTEGER(), nullable=False), + sa.Column("is_managed_externally", sa.BOOLEAN(), nullable=False), + sa.Column("catalog", sa.TEXT(), nullable=True), + sa.Column("schema", sa.TEXT(), nullable=True), + sa.Column("name", sa.TEXT(), nullable=True), + sa.Column("external_url", sa.TEXT(), nullable=True), + sa.Column("extra_json", sa.TEXT(), nullable=True), + sa.Column("created_by_fk", sa.INTEGER(), nullable=True), + sa.Column("changed_by_fk", sa.INTEGER(), nullable=True), + sa.ForeignKeyConstraint( + ["changed_by_fk"], + ["ab_user.id"], + ), + sa.ForeignKeyConstraint( + ["created_by_fk"], + ["ab_user.id"], + ), + sa.ForeignKeyConstraint( + ["database_id"], + ["dbs.id"], + ), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint("uuid"), + ) + op.create_table( + "sl_dataset_columns", + sa.Column("dataset_id", sa.INTEGER(), nullable=False), + sa.Column("column_id", sa.INTEGER(), nullable=False), + sa.ForeignKeyConstraint( + ["column_id"], + ["sl_columns.id"], + ), + sa.ForeignKeyConstraint( + ["dataset_id"], + ["sl_datasets.id"], + ), + sa.PrimaryKeyConstraint("dataset_id", "column_id"), + ) + op.create_table( + "sl_dataset_tables", + sa.Column("dataset_id", sa.INTEGER(), nullable=False), + sa.Column("table_id", sa.INTEGER(), nullable=False), + sa.ForeignKeyConstraint( + ["dataset_id"], + ["sl_datasets.id"], + ), + sa.ForeignKeyConstraint( + ["table_id"], + ["sl_tables.id"], + ), + sa.PrimaryKeyConstraint("dataset_id", "table_id"), + ) + op.create_table( + "sl_table_columns", + sa.Column("table_id", sa.INTEGER(), nullable=False), + sa.Column("column_id", sa.INTEGER(), nullable=False), + sa.ForeignKeyConstraint( + ["column_id"], + ["sl_columns.id"], + ), + sa.ForeignKeyConstraint( + ["table_id"], + ["sl_tables.id"], + ), + sa.PrimaryKeyConstraint("table_id", "column_id"), + ) + op.create_table( + "sl_columns", + sa.Column("uuid", sa.NUMERIC(precision=16), nullable=True), + sa.Column("created_on", sa.DATETIME(), nullable=True), + sa.Column("changed_on", sa.DATETIME(), nullable=True), + sa.Column("id", sa.INTEGER(), nullable=False), + sa.Column("is_aggregation", sa.BOOLEAN(), nullable=False), + sa.Column("is_additive", sa.BOOLEAN(), nullable=False), + sa.Column("is_dimensional", sa.BOOLEAN(), nullable=False), + sa.Column("is_filterable", sa.BOOLEAN(), nullable=False), + sa.Column("is_increase_desired", sa.BOOLEAN(), nullable=False), + sa.Column("is_managed_externally", sa.BOOLEAN(), nullable=False), + sa.Column("is_partition", sa.BOOLEAN(), nullable=False), + sa.Column("is_physical", sa.BOOLEAN(), nullable=False), + sa.Column("is_temporal", sa.BOOLEAN(), nullable=False), + sa.Column("is_spatial", sa.BOOLEAN(), nullable=False), + sa.Column("name", sa.TEXT(), nullable=True), + sa.Column("type", sa.TEXT(), nullable=True), + sa.Column("unit", sa.TEXT(), nullable=True), + sa.Column("expression", sa.TEXT(), nullable=True), + sa.Column("description", sa.TEXT(), nullable=True), + sa.Column("warning_text", sa.TEXT(), nullable=True), + sa.Column("external_url", sa.TEXT(), nullable=True), + sa.Column("extra_json", sa.TEXT(), nullable=True), + sa.Column("created_by_fk", sa.INTEGER(), nullable=True), + sa.Column("changed_by_fk", sa.INTEGER(), nullable=True), + sa.Column("advanced_data_type", sa.TEXT(), nullable=True), + sa.ForeignKeyConstraint( + ["changed_by_fk"], + ["ab_user.id"], + ), + sa.ForeignKeyConstraint( + ["created_by_fk"], + ["ab_user.id"], + ), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint("uuid"), + ) diff --git a/tests/integration_tests/fixtures/datasource.py b/tests/integration_tests/fixtures/datasource.py index e91ac6727dcb..974f2c408772 100644 --- a/tests/integration_tests/fixtures/datasource.py +++ b/tests/integration_tests/fixtures/datasource.py @@ -23,11 +23,9 @@ from sqlalchemy import Column, create_engine, Date, Integer, MetaData, String, Table from sqlalchemy.ext.declarative import declarative_base -from superset.columns.models import Column as Sl_Column # noqa: F401 from superset.connectors.sqla.models import SqlaTable, TableColumn from superset.extensions import db from superset.models.core import Database -from superset.tables.models import Table as Sl_Table # noqa: F401 from superset.utils.core import get_example_default_schema from superset.utils.database import get_example_database # noqa: F401 from tests.integration_tests.test_app import app From dee1cd0c52f3497f00fa34152f1fc2e490307266 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Fri, 24 May 2024 22:38:17 -0700 Subject: [PATCH 3/8] change the ordering --- .../2024-05-24_11-31_02f4f7811799_remove_sl__tables.py | 6 +++--- tests/integration_tests/datasets/api_tests.py | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/superset/migrations/versions/2024-05-24_11-31_02f4f7811799_remove_sl__tables.py b/superset/migrations/versions/2024-05-24_11-31_02f4f7811799_remove_sl__tables.py index c333400ba62d..7f87040a5da6 100644 --- a/superset/migrations/versions/2024-05-24_11-31_02f4f7811799_remove_sl__tables.py +++ b/superset/migrations/versions/2024-05-24_11-31_02f4f7811799_remove_sl__tables.py @@ -31,13 +31,13 @@ def upgrade(): - op.drop_table("sl_columns") + op.drop_table("sl_dataset_columns") op.drop_table("sl_table_columns") op.drop_table("sl_dataset_tables") - op.drop_table("sl_dataset_columns") + op.drop_table("sl_columns") op.drop_table("sl_tables") - op.drop_table("sl_datasets") op.drop_table("sl_dataset_users") + op.drop_table("sl_datasets") def downgrade(): diff --git a/tests/integration_tests/datasets/api_tests.py b/tests/integration_tests/datasets/api_tests.py index 6cc3cc8828dc..5cabe3c58501 100644 --- a/tests/integration_tests/datasets/api_tests.py +++ b/tests/integration_tests/datasets/api_tests.py @@ -38,7 +38,6 @@ DAODeleteFailedError, DAOUpdateFailedError, ) -from superset.datasets.models import Dataset # noqa: F401 from superset.extensions import db, security_manager from superset.models.core import Database from superset.models.slice import Slice From 76715d825b6a8b378ac6e516ebe9336db31beb16 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Sat, 25 May 2024 12:34:49 -0700 Subject: [PATCH 4/8] rm 2 files --- superset/columns/schemas.py | 40 ------------------------------------- superset/tables/schemas.py | 40 ------------------------------------- 2 files changed, 80 deletions(-) delete mode 100644 superset/columns/schemas.py delete mode 100644 superset/tables/schemas.py diff --git a/superset/columns/schemas.py b/superset/columns/schemas.py deleted file mode 100644 index 5368bfbcca7e..000000000000 --- a/superset/columns/schemas.py +++ /dev/null @@ -1,40 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -""" -Schema for the column model. - -This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909), -and represents a "column" in a table or dataset. In addition to a column, new models for -tables, metrics, and datasets were also introduced. - -These models are not fully implemented, and shouldn't be used yet. -""" - -from marshmallow_sqlalchemy import SQLAlchemyAutoSchema - -from superset.columns.models import Column - - -class ColumnSchema(SQLAlchemyAutoSchema): - """ - Schema for the ``Column`` model. - """ - - class Meta: # pylint: disable=too-few-public-methods - model = Column - load_instance = True - include_relationships = True diff --git a/superset/tables/schemas.py b/superset/tables/schemas.py deleted file mode 100644 index 701a1359ba00..000000000000 --- a/superset/tables/schemas.py +++ /dev/null @@ -1,40 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -""" -Schema for table model. - -This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909), -and represents a "table" in a given database -- either a physical table or a view. In -addition to a table, new models for columns, metrics, and datasets were also introduced. - -These models are not fully implemented, and shouldn't be used yet. -""" - -from marshmallow_sqlalchemy import SQLAlchemyAutoSchema - -from superset.tables.models import Table - - -class TableSchema(SQLAlchemyAutoSchema): - """ - Schema for the ``Table`` model. - """ - - class Meta: # pylint: disable=too-few-public-methods - model = Table - load_instance = True - include_relationships = True From 1c2420f5f0af815402500164aabfeae907c8b499 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Sat, 25 May 2024 12:36:00 -0700 Subject: [PATCH 5/8] fix test --- tests/integration_tests/charts/api_tests.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration_tests/charts/api_tests.py b/tests/integration_tests/charts/api_tests.py index 65ede9221cff..9f52a850e65f 100644 --- a/tests/integration_tests/charts/api_tests.py +++ b/tests/integration_tests/charts/api_tests.py @@ -552,7 +552,7 @@ def test_create_chart_validate_datasource(self): { "message": { "datasource_type": [ - "Must be one of: sl_table, table, dataset, query, saved_query, view." + "Must be one of: table, dataset, query, saved_query, view." ] } }, @@ -913,7 +913,7 @@ def test_update_chart_validate_datasource(self): { "message": { "datasource_type": [ - "Must be one of: sl_table, table, dataset, query, saved_query, view." + "Must be one of: table, dataset, query, saved_query, view." ] } }, From fc7e95db561bbd9b035c4c8189e6ffb418cec867 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Sat, 25 May 2024 14:59:03 -0700 Subject: [PATCH 6/8] remove more sl refs --- .../integration_tests/datasource/api_tests.py | 17 +-------- tests/unit_tests/datasource/dao_tests.py | 35 ------------------- 2 files changed, 1 insertion(+), 51 deletions(-) diff --git a/tests/integration_tests/datasource/api_tests.py b/tests/integration_tests/datasource/api_tests.py index 044aead80acd..087a4da53ede 100644 --- a/tests/integration_tests/datasource/api_tests.py +++ b/tests/integration_tests/datasource/api_tests.py @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. import json -from unittest.mock import ANY, Mock, patch +from unittest.mock import ANY, patch import pytest @@ -132,21 +132,6 @@ def test_get_column_values_no_datasource_access(self): "database or `all_datasource_access` permission", ) - @patch("superset.datasource.api.DatasourceDAO.get_datasource") - def test_get_column_values_not_implemented_error(self, get_datasource_mock): - datasource = Mock() - datasource.values_for_column.side_effect = NotImplementedError - get_datasource_mock.return_value = datasource - - self.login(ADMIN_USERNAME) - rv = self.client.get("api/v1/datasource/sl_table/1/column/col1/values/") - self.assertEqual(rv.status_code, 400) - response = json.loads(rv.data.decode("utf-8")) - self.assertEqual( - response["message"], - "Unable to get column values for datasource type: sl_table", - ) - @pytest.mark.usefixtures("app_context", "virtual_dataset") @patch("superset.models.helpers.ExploreMixin.values_for_column") def test_get_column_values_normalize_columns_enabled(self, values_for_column_mock): diff --git a/tests/unit_tests/datasource/dao_tests.py b/tests/unit_tests/datasource/dao_tests.py index 8fed1b73d159..dbe0c84605ef 100644 --- a/tests/unit_tests/datasource/dao_tests.py +++ b/tests/unit_tests/datasource/dao_tests.py @@ -138,36 +138,9 @@ def test_get_datasource_saved_query(session_with_data: Session) -> None: assert isinstance(result, SavedQuery) -def test_get_datasource_sl_table(session_with_data: Session) -> None: - from superset.daos.datasource import DatasourceDAO - from superset.tables.models import Table - - result = DatasourceDAO.get_datasource( - datasource_type=DatasourceType.SLTABLE, - datasource_id=1, - ) - - assert result.id == 1 - assert isinstance(result, Table) - - -def test_get_datasource_sl_dataset(session_with_data: Session) -> None: - from superset.daos.datasource import DatasourceDAO - from superset.datasets.models import Dataset - - result = DatasourceDAO.get_datasource( - datasource_type=DatasourceType.DATASET, - datasource_id=1, - ) - - assert result.id == 1 - assert isinstance(result, Dataset) - - def test_get_datasource_w_str_param(session_with_data: Session) -> None: from superset.connectors.sqla.models import SqlaTable from superset.daos.datasource import DatasourceDAO - from superset.tables.models import Table assert isinstance( DatasourceDAO.get_datasource( @@ -177,14 +150,6 @@ def test_get_datasource_w_str_param(session_with_data: Session) -> None: SqlaTable, ) - assert isinstance( - DatasourceDAO.get_datasource( - datasource_type="sl_table", - datasource_id=1, - ), - Table, - ) - def test_get_all_datasources(session_with_data: Session) -> None: from superset.connectors.sqla.models import SqlaTable From 7494c8a2e44c0f0e31ddca9427058e6673a551d4 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Sat, 25 May 2024 15:11:59 -0700 Subject: [PATCH 7/8] trimming unit tests --- tests/unit_tests/datasource/dao_tests.py | 29 ------------------------ 1 file changed, 29 deletions(-) diff --git a/tests/unit_tests/datasource/dao_tests.py b/tests/unit_tests/datasource/dao_tests.py index dbe0c84605ef..af456a1186c8 100644 --- a/tests/unit_tests/datasource/dao_tests.py +++ b/tests/unit_tests/datasource/dao_tests.py @@ -25,12 +25,9 @@ @pytest.fixture def session_with_data(session: Session) -> Iterator[Session]: - from superset.columns.models import Column from superset.connectors.sqla.models import SqlaTable, TableColumn - from superset.datasets.models import Dataset from superset.models.core import Database from superset.models.sql_lab import Query, SavedQuery - from superset.tables.models import Table engine = session.get_bind() SqlaTable.metadata.create_all(engine) # pylint: disable=no-member @@ -65,32 +62,6 @@ def session_with_data(session: Session) -> Iterator[Session]: saved_query = SavedQuery(database=database, sql="select * from foo") - table = Table( - name="my_table", - schema="my_schema", - catalog="my_catalog", - database=database, - columns=[], - ) - - dataset = Dataset( - database=table.database, - name="positions", - expression=""" -SELECT array_agg(array[longitude,latitude]) AS position -FROM my_catalog.my_schema.my_table -""", - tables=[table], - columns=[ - Column( - name="position", - expression="array_agg(array[longitude,latitude])", - ), - ], - ) - - session.add(dataset) - session.add(table) session.add(saved_query) session.add(query_obj) session.add(database) From 12b65310f21c5ff2e8840607dae021dd1c07ae65 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Sat, 25 May 2024 15:47:02 -0700 Subject: [PATCH 8/8] trimming more tests --- tests/unit_tests/columns/test_models.py | 58 ------------------------- tests/unit_tests/tables/test_models.py | 56 ------------------------ 2 files changed, 114 deletions(-) delete mode 100644 tests/unit_tests/columns/test_models.py delete mode 100644 tests/unit_tests/tables/test_models.py diff --git a/tests/unit_tests/columns/test_models.py b/tests/unit_tests/columns/test_models.py deleted file mode 100644 index 0ea230da1779..000000000000 --- a/tests/unit_tests/columns/test_models.py +++ /dev/null @@ -1,58 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - -# pylint: disable=import-outside-toplevel, unused-argument - -from sqlalchemy.orm.session import Session - - -def test_column_model(session: Session) -> None: - """ - Test basic attributes of a ``Column``. - """ - from superset import db - from superset.columns.models import Column - - engine = db.session.get_bind() - Column.metadata.create_all(engine) # pylint: disable=no-member - - column = Column( - name="ds", - type="TIMESTAMP", - expression="ds", - ) - - db.session.add(column) - db.session.flush() - - assert column.id == 1 - assert column.uuid is not None - - assert column.name == "ds" - assert column.type == "TIMESTAMP" - assert column.expression == "ds" - - # test that default values are set correctly - assert column.description is None - assert column.warning_text is None - assert column.unit is None - assert column.is_temporal is False - assert column.is_spatial is False - assert column.is_partition is False - assert column.is_aggregation is False - assert column.is_additive is False - assert column.is_increase_desired is True diff --git a/tests/unit_tests/tables/test_models.py b/tests/unit_tests/tables/test_models.py deleted file mode 100644 index 926e059261cd..000000000000 --- a/tests/unit_tests/tables/test_models.py +++ /dev/null @@ -1,56 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -# pylint: disable=import-outside-toplevel, unused-argument -from sqlalchemy.orm.session import Session - -from superset import db - - -def test_table_model(session: Session) -> None: - """ - Test basic attributes of a ``Table``. - """ - from superset.columns.models import Column - from superset.models.core import Database - from superset.tables.models import Table - - engine = db.session.get_bind() - Table.metadata.create_all(engine) # pylint: disable=no-member - - table = Table( - name="my_table", - schema="my_schema", - catalog="my_catalog", - database=Database(database_name="my_database", sqlalchemy_uri="test://"), - columns=[ - Column( - name="ds", - type="TIMESTAMP", - expression="ds", - ) - ], - ) - db.session.add(table) - db.session.flush() - - assert table.id == 1 - assert table.uuid is not None - assert table.database_id == 1 - assert table.catalog == "my_catalog" - assert table.schema == "my_schema" - assert table.name == "my_table" - assert [column.name for column in table.columns] == ["ds"]