You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/08/15 21:43:54 UTC

[GitHub] [superset] eschutho commented on a diff in pull request #20793: feat(api): Fetch datasets with a given advanced data type

eschutho commented on code in PR #20793:
URL: https://github.com/apache/superset/pull/20793#discussion_r946153674


##########
superset/advanced_data_type/api.py:
##########
@@ -146,3 +150,72 @@ def get_types(self) -> Response:
         """
 
         return self.response(200, result=list(ADVANCED_DATA_TYPES.keys()))
+
+    @protect()
+    @safe
+    @expose("/datasets", methods=["GET"])
+    @permission_name("read")
+    @event_logger.log_this_with_context(
+        action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.get",
+        log_to_statsd=False,  # pylint: disable-arguments-renamed
+    )
+    @rison(advanced_data_type_datasets_schema)
+    def get_datasets(self, **kwargs: Any) -> Response:
+        """Get all datasets with a column of the specified advanced type
+        ---
+        get:
+          description:
+            Get all datasets with a column of the specified advanced type
+          parameters:
+          - in: query
+            name: q
+            content:
+              application/json:
+                schema:
+                  $ref: '#/components/schemas/advanced_data_type_datasets_schema'
+          responses:
+            200:
+              description: Query result
+              content:
+                application/json:
+                  schema:
+                    type: object
+                    properties:
+                      result:
+                        type: object
+                        properties:
+                          table_id:
+                            type: array
+                            items:
+                              type: integer
+            401:
+              $ref: '#/components/responses/401'
+            404:
+              $ref: '#/components/responses/404'
+            500:
+              $ref: '#/components/responses/500'
+        """
+        item = kwargs["rison"]
+        advanced_data_type = item["type"]
+        columns = (
+            db.session.query(TableColumn.table_id, TableColumn.id)
+            .filter(TableColumn.advanced_data_type == advanced_data_type)
+            .all()
+        )
+        forbidden_datasets = []
+        result = {}
+        for column in columns:
+            if column[0] in forbidden_datasets:
+                continue
+            if column[0] not in result:
+                if DatasetDAO.find_by_id(column[0]) is None:
+                    forbidden_datasets.append(column[0])

Review Comment:
   I know that we've used this pattern in the past to check permissions by running `find_by_id` but it seems that we could improve performance by running the query one time rather than multiple times. 
   
   Here's an example where we create a query and then apply the base filters to it and just hit the db once: https://github.com/apache/superset/blob/1.4/superset/datasets/api.py#L482
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org