[GitHub] [superset] ktmud commented on a diff in pull request #19421: perf: migrate new dataset models with INSERT FROM

[GitBox <gi...@apache.org>]

ktmud commented on code in PR #19421:
URL: https://github.com/apache/superset/pull/19421#discussion_r843395324


##########
superset/columns/models.py:
##########
@@ -52,6 +52,38 @@ class Column(
 
     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)

Review Comment:
   A-Z reordering



##########
superset/connectors/sqla/models.py:
##########
@@ -1901,9 +1903,7 @@ def update_table(  # pylint: disable=unused-argument
         session.execute(update(SqlaTable).where(SqlaTable.id == target.table.id))
 
         dataset = (
-            session.query(NewDataset)
-            .filter_by(sqlatable_id=target.table.id)
-            .one_or_none()
+            session.query(NewDataset).filter_by(uuid=target.table.uuid).one_or_none()

Review Comment:
   We now link old and new datasets by uuid instead of a `sqlatable_id` field. Note since `NewDataset` is designed to be a 1:1 replacement for SqlaTable, it should be okay for them to share the same uuid. However, `NewTable` is a totally new entity type, so they should have separate uuids. Therefore a `sqlatable_id` is added to `NewTable` instead.



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