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/03/30 00:18:39 UTC

[GitHub] [superset] john-bodley commented on a change in pull request #19421: perf: migrate new dataset models with INSERT FROM

john-bodley commented on a change in pull request #19421:
URL: https://github.com/apache/superset/pull/19421#discussion_r838006821



##########
File path: superset/migrations/versions/b8d3a24d9131_new_dataset_models.py
##########
@@ -292,78 +305,70 @@ def after_insert(target: SqlaTable) -> None:  # pylint: disable=too-many-locals
         columns.append(
             NewColumn(
                 name=metric.metric_name,
-                type="Unknown",  # figuring this out would require a type inferrer
-                expression=metric.expression,
-                warning_text=metric.warning_text,
                 description=metric.description,
+                expression=metric.expression,

Review comment:
       Oh my. I do love me some ABC.

##########
File path: superset/migrations/versions/b8d3a24d9131_new_dataset_models.py
##########
@@ -292,78 +305,70 @@ def after_insert(target: SqlaTable) -> None:  # pylint: disable=too-many-locals
         columns.append(
             NewColumn(
                 name=metric.metric_name,
-                type="Unknown",  # figuring this out would require a type inferrer
-                expression=metric.expression,
-                warning_text=metric.warning_text,
                 description=metric.description,
+                expression=metric.expression,
+                external_url=target.external_url,
+                extra_json=json.dumps(extra_json) if extra_json else None,
                 is_aggregation=True,
                 is_additive=is_additive,
-                is_physical=False,
-                is_spatial=False,
-                is_partition=False,
                 is_increase_desired=True,
-                extra_json=json.dumps(extra_json) if extra_json else None,
                 is_managed_externally=target.is_managed_externally,
-                external_url=target.external_url,
+                is_partition=False,
+                is_physical=False,
+                is_spatial=False,
+                is_temporal=False,
+                type="Unknown",  # figuring this out would require a type inferrer
+                warning_text=metric.warning_text,
             ),
         )
 
-    # physical dataset
-    tables = []
-    if target.sql is None:
-        physical_columns = [column for column in columns if column.is_physical]
-
-        # create table
+    if is_physical_table:
+        # create physical sl_table
         table = NewTable(
             name=target.table_name,
             schema=target.schema,
             catalog=None,  # currently not supported
-            database_id=target.database_id,
-            columns=physical_columns,
+            database_id=database_id,
+            # only save physical columns
+            columns=[column for column in columns if column.is_physical],
             is_managed_externally=target.is_managed_externally,
             external_url=target.external_url,
         )
-        tables.append(table)
-
-    # virtual dataset
+        tables = [table]
+        expression = conditional_quote(target.table_name)
     else:
-        # mark all columns as virtual (not physical)
-        for column in columns:
-            column.is_physical = False
-
-        # find referenced tables
+        # find referenced tables and link to dataset
         parsed = ParsedQuery(target.sql)
         referenced_tables = parsed.tables
-
-        # predicate for finding the referenced tables
         predicate = or_(
             *[
                 and_(
+                    NewTable.database_id == database_id,

Review comment:
       Nice!




-- 
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