feat(sip-95): new endpoint for extra table metadata

superset-frontend/src/SqlLab/actions/sqlLab.test.js

@@ -511,7 +511,7 @@ describe('async actions', () => {
     const getTableMetadataEndpoint = 'glob:**/api/v1/database/*/table/*/*/';
     fetchMock.get(getTableMetadataEndpoint, {});
     const getExtraTableMetadataEndpoint =
-      'glob:**/api/v1/database/*/table_extra/*/*/';
+      'glob:**/api/v1/database/*/table_metadata/extra/';
     fetchMock.get(getExtraTableMetadataEndpoint, {});
 
     let isFeatureEnabledMock;

superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.tsx

@@ -67,7 +67,7 @@ beforeEach(() => {
       columns: table.columns,
     },
   });
-  fetchMock.get('glob:*/api/v1/database/*/table_extra/*/*', {
+  fetchMock.get('glob:*/api/v1/database/*/table_metadata/extra/', {
     status: 200,
     body: {},
   });

superset-frontend/src/SqlLab/components/TableElement/TableElement.test.tsx

@@ -49,7 +49,7 @@ jest.mock(
 );
 const getTableMetadataEndpoint = 'glob:**/api/v1/database/*/table/*/*/';
 const getExtraTableMetadataEndpoint =
-  'glob:**/api/v1/database/*/table_extra/*/*/';
+  'glob:**/api/v1/database/*/table_metadata/extra/';
 const updateTableSchemaEndpoint = 'glob:*/tableschemaview/*/expanded';
 
 beforeEach(() => {

superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx

@@ -181,7 +181,7 @@ const DndFilterSelect = (props: DndFilterSelectProps) => {
 
       if (!isSqllabView && dbId && name && schema) {
         SupersetClient.get({
-          endpoint: `/api/v1/database/${dbId}/table_extra/${name}/${schema}/`,
+          endpoint: `/api/v1/database/${dbId}/table_metadata/extra/?table=${name}&schema=${schema}`,
         })
           .then(({ json }: { json: Record<string, any> }) => {
             if (json?.partitions) {

superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterControl/index.jsx

@@ -143,7 +143,7 @@ class AdhocFilterControl extends React.Component {
 
       if (!isSqllabView && dbId && name && schema) {
         SupersetClient.get({
-          endpoint: `/api/v1/database/${dbId}/table_extra/${name}/${schema}/`,
+          endpoint: `/api/v1/database/${dbId}/table_metadata/extra/?table=${name}&schema=${schema}`,
         })
           .then(({ json }) => {
             if (json && json.partitions) {

superset-frontend/src/hooks/apiResources/tables.ts

@@ -125,9 +125,9 @@ const tableApi = api.injectEndpoints({
       FetchTableMetadataQueryParams
     >({
       query: ({ dbId, schema, table }) => ({
-        endpoint: `/api/v1/database/${dbId}/table_extra/${encodeURIComponent(
-          table,
-        )}/${encodeURIComponent(schema)}/`,
+        endpoint: schema
+          ? `/api/v1/database/${dbId}/table_metadata/extra/?table=${table}&schema=${schema}`
+          : `/api/v1/database/${dbId}/table_metadata/extra/?table=${table}`,
         transformResponse: ({ json }: JsonResponse) => json,
       }),
     }),

superset/constants.py

@@ -135,6 +135,7 @@ class RouteMethod:  # pylint: disable=too-few-public-methods
     "select_star": "read",
     "table_metadata": "read",
     "table_extra_metadata": "read",
+    "table_extra_metadata_deprecated": "read",
     "test_connection": "write",
     "validate_parameters": "write",
     "favorite_status": "read",

superset/databases/api.py

@@ -15,11 +15,14 @@
 # specific language governing permissions and limitations
 # under the License.
 # pylint: disable=too-many-lines
+
+from __future__ import annotations
+
 import json
 import logging
 from datetime import datetime, timedelta
 from io import BytesIO
-from typing import Any, cast, Optional
+from typing import Any, cast
 from zipfile import is_zipfile, ZipFile
 
 from deprecation import deprecated
@@ -82,6 +85,7 @@
     get_export_ids_schema,
     OAuth2ProviderResponseSchema,
     openapi_spec_methods_override,
+    QualifiedTableSchema,
     SchemasResponseSchema,
     SelectStarResponseSchema,
     TableExtraMetadataResponseSchema,
@@ -92,9 +96,18 @@
 from superset.databases.utils import get_table_metadata
 from superset.db_engine_specs import get_available_engine_specs
 from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
-from superset.exceptions import OAuth2Error, SupersetErrorsException, SupersetException
+from superset.exceptions import (
+    DatabaseNotFoundException,
+    InvalidPayloadSchemaError,
+    OAuth2Error,
+    SupersetErrorsException,
+    SupersetException,
+    SupersetSecurityException,
+    TableNotFoundException,
+)
 from superset.extensions import security_manager
 from superset.models.core import Database
+from superset.sql_parse import Table
 from superset.superset_typing import FlaskResponse
 from superset.utils.core import error_msg_from_exception, parse_js_uri_path_item
 from superset.utils.oauth2 import decode_oauth2_state
@@ -121,6 +134,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
         "tables",
         "table_metadata",
         "table_extra_metadata",
+        "table_extra_metadata_deprecated",
         "select_star",
         "schemas",
         "test_connection",
@@ -764,15 +778,20 @@ def table_metadata(
     @check_table_access
     @safe
     @statsd_metrics
+    @deprecated(deprecated_in="4.0", removed_in="5.0")
     @event_logger.log_this_with_context(
         action=lambda self, *args, **kwargs: f"{self.__class__.__name__}"
-        f".table_extra_metadata",
+        f".table_extra_metadata_deprecated",
         log_to_statsd=False,
     )
-    def table_extra_metadata(
+    def table_extra_metadata_deprecated(
         self, database: Database, table_name: str, schema_name: str
     ) -> FlaskResponse:
         """Get table extra metadata.
+
+        A newer API was introduced between 4.0 and 5.0, with support for catalogs for
+        SIP-95. This method was kept to prevent breaking API integrations, but will be
+        removed in 5.0.
         ---
         get:
           summary: Get table extra metadata
@@ -816,9 +835,87 @@ def table_extra_metadata(
 
         parsed_schema = parse_js_uri_path_item(schema_name, eval_undefined=True)
         table_name = cast(str, parse_js_uri_path_item(table_name))
-        payload = database.db_engine_spec.extra_table_metadata(
-            database, table_name, parsed_schema
-        )
+        table = Table(table_name, parsed_schema)
+        payload = database.db_engine_spec.get_extra_table_metadata(database, table)
+        return self.response(200, **payload)
+
+    @expose("/<int:pk>/table_metadata/extra/", methods=("GET",))
+    @protect()
+    @statsd_metrics
+    @event_logger.log_this_with_context(
+        action=lambda self, *args, **kwargs: f"{self.__class__.__name__}"
+        f".table_extra_metadata",
+        log_to_statsd=False,
+    )
+    def table_extra_metadata(self, pk: int) -> FlaskResponse:
+        """
+        Get extra metadata for a given table.
+
+        Optionally, a schema and a catalog can be passed, if different from the default
+        ones.
+        ---
+        get:
+          summary: Get table extra metadata
+          description: >-
+            Extra metadata associated with the table (partitions, description, etc.)
+          parameters:
+          - in: path
+            schema:
+              type: integer
+            name: pk
+            description: The database id
+          - in: query
+            schema:
+              type: string
+            name: table
+            required: true
+            description: Table name
+          - in: query
+            schema:
+              type: string
+            name: schema
+            description: >-
+              Optional table schema, if not passed default schema will be used
+          - in: query
+            schema:
+              type: string
+            name: catalog
+            description: >-
+              Optional table catalog, if not passed default catalog will be used
+          responses:
+            200:
+              description: Table extra metadata information
+              content:
+                application/json:
+                  schema:
+                    $ref: "#/components/schemas/TableExtraMetadataResponseSchema"
+            401:
+              $ref: '#/components/responses/401'
+            404:
+              $ref: '#/components/responses/404'
+            500:
+              $ref: '#/components/responses/500'
+        """
+        self.incr_stats("init", self.table_metadata.__name__)
+
+        database = DatabaseDAO.find_by_id(pk)
+        if database is None:
+            raise DatabaseNotFoundException("No such database")
+
+        try:
+            parameters = QualifiedTableSchema().load(request.args)
+        except ValidationError as ex:
+            raise InvalidPayloadSchemaError(ex) from ex
+
+        table = Table(**parameters)
+        try:
+            security_manager.raise_for_access(database=database, table=table)
+        except SupersetSecurityException as ex:
+            # instead of raising 403, raise 404 to hide table existence
+            raise TableNotFoundException("No such table") from ex
+
+        payload = database.db_engine_spec.get_extra_table_metadata(database, table)
+
         return self.response(200, **payload)
 
     @expose("/<int:pk>/select_star/<path:table_name>/", methods=("GET",))
@@ -832,7 +929,7 @@ def table_extra_metadata(
         log_to_statsd=False,
     )
     def select_star(
-        self, database: Database, table_name: str, schema_name: Optional[str] = None
+        self, database: Database, table_name: str, schema_name: str | None = None
     ) -> FlaskResponse:
         """Get database select star for table.
         ---

superset/databases/schemas.py

@@ -1195,3 +1195,27 @@ class OAuth2ProviderResponseSchema(Schema):
     class Meta:  # pylint: disable=too-few-public-methods
         # Ignore unknown fields that might be sent by the OAuth2 provider
         unknown = EXCLUDE
+
+
+class QualifiedTableSchema(Schema):
+    """
+    Schema for a qualified table reference.
+
+    Catalog and schema can be ommited, to fallback to default values. Table name must be
+    present.
+    """
+
+    table = fields.String(
+        required=True,
+        metadata={"description": "The table name"},
+    )
+    schema = fields.String(
+        required=False,
+        load_default=None,
+        metadata={"description": "The table schema"},
+    )
+    catalog = fields.String(
+        required=False,
+        load_default=None,
+        metadata={"description": "The table catalog"},
+    )

superset/db_engine_specs/README.md

@@ -603,7 +603,7 @@ For some databases the `df_to_sql` classmethod needs to be implemented. For exam
 
 ### Extra table metadata
 
-DB engine specs can return additional metadata associated with a table. This is done via the `extra_table_metadata` class method. Trino uses this to return information about the latest partition, for example, and Bigquery returns clustering information. This information is then surfaced in the SQL Lab UI, when browsing tables in the metadata explorer (on the left panel).
+DB engine specs can return additional metadata associated with a table. This is done via the `get_extra_table_metadata` class method. Trino uses this to return information about the latest partition, for example, and Bigquery returns clustering information. This information is then surfaced in the SQL Lab UI, when browsing tables in the metadata explorer (on the left panel).
 
 ### DB API exception mapping
 

superset/db_engine_specs/base.py

@@ -1034,7 +1034,23 @@ def normalize_indexes(cls, indexes: list[dict[str, Any]]) -> list[dict[str, Any]
         return indexes
 
     @classmethod
-    def extra_table_metadata(  # pylint: disable=unused-argument
+    def get_extra_table_metadata(  # pylint: disable=unused-argument
+        cls,
+        database: Database,
+        table: Table,
+    ) -> dict[str, Any]:
+        """
+        Returns engine-specific table metadata
+
+        :param database: Database instance
+        :param table: A Table instance
+        :return: Engine-specific table metadata
+        """
+        return cls.extra_table_metadata(database, table.table, table.schema)
+
+    @deprecated(deprecated_in="4.0", removed_in="5.0")
+    @classmethod
+    def extra_table_metadata(
         cls,
         database: Database,
         table_name: str,
@@ -1043,12 +1059,13 @@ def extra_table_metadata(  # pylint: disable=unused-argument
         """
         Returns engine-specific table metadata
 
+        Deprecated, since it doesn't support catalogs.
+
         :param database: Database instance
         :param table_name: Table name
         :param schema_name: Schema name
         :return: Engine-specific table metadata
         """
-        # TODO: Fix circular import caused by importing Database
         return {}
 
     @classmethod

superset/db_engine_specs/bigquery.py

@@ -320,10 +320,12 @@ def get_indexes(
         return cls.normalize_indexes(inspector.get_indexes(table_name, schema))
 
     @classmethod
-    def extra_table_metadata(
-        cls, database: "Database", table_name: str, schema_name: Optional[str]
+    def get_extra_table_metadata(
+        cls,
+        database: "Database",
+        table: Table,
     ) -> dict[str, Any]:
-        indexes = database.get_indexes(table_name, schema_name)
+        indexes = database.get_indexes(table.table, table.schema)
         if not indexes:
             return {}
         partitions_columns = [

superset/db_engine_specs/gsheets.py

@@ -137,15 +137,14 @@ def get_url_for_impersonation(
         return url
 
     @classmethod
-    def extra_table_metadata(
+    def get_extra_table_metadata(
         cls,
         database: Database,
-        table_name: str,
-        schema_name: str | None,
+        table: Table,
     ) -> dict[str, Any]:
-        with database.get_raw_connection(schema=schema_name) as conn:
+        with database.get_raw_connection(schema=table.schema) as conn:
             cursor = conn.cursor()
-            cursor.execute(f'SELECT GET_METADATA("{table_name}")')
+            cursor.execute(f'SELECT GET_METADATA("{table.table}")')
             results = cursor.fetchone()[0]
         try:
             metadata = json.loads(results)

superset/db_engine_specs/lib.py

@@ -63,7 +63,7 @@
 NICE_TO_HAVE_FEATURES = {
     "user_impersonation": "Supports user impersonation",
     "file_upload": "Support file upload",
-    "extra_table_metadata": "Returns extra table metadata",
+    "get_extra_table_metadata": "Returns extra table metadata",
     "dbapi_exception_mapping": "Maps driver exceptions to Superset exceptions",
     "custom_errors": "Parses error messages and returns Superset errors",
     "dynamic_schema": "Supports changing the schema per-query",
@@ -142,7 +142,9 @@ def diagnose(spec: type[BaseEngineSpec]) -> dict[str, Any]:
                 or has_custom_method(spec, "get_url_for_impersonation")
             ),
             "file_upload": spec.supports_file_upload,
-            "extra_table_metadata": has_custom_method(spec, "extra_table_metadata"),
+            "get_extra_table_metadata": has_custom_method(
+                spec, "get_extra_table_metadata"
+            ),
             "dbapi_exception_mapping": has_custom_method(
                 spec, "get_dbapi_exception_mapping"
             ),
@@ -177,7 +179,7 @@ def diagnose(spec: type[BaseEngineSpec]) -> dict[str, Any]:
     nice_to_have = [
         "user_impersonation",
         "file_upload",
-        "extra_table_metadata",
+        "get_extra_table_metadata",
         "dbapi_exception_mapping",
         "custom_errors",
         "dynamic_schema",
@@ -264,7 +266,7 @@ def generate_table() -> list[list[Any]]:
     keys = [
         "user_impersonation",
         "file_upload",
-        "extra_table_metadata",
+        "get_extra_table_metadata",
         "dbapi_exception_mapping",
         "custom_errors",
         "dynamic_schema",