ktmud commented on code in PR #19421:
URL: https://github.com/apache/superset/pull/19421#discussion_r846582107
##########
superset/connectors/sqla/models.py:
##########
@@ -2063,172 +2169,74 @@ def after_update( # pylint:
disable=too-many-branches, too-many-locals, too-man
For more context: https://github.com/apache/superset/issues/14909
"""
- inspector = inspect(target)
+ # set permissions
+ security_manager.set_perm(mapper, connection, sqla_table)
+
+ inspector = inspect(sqla_table)
session = inspector.session
# double-check that ``UPDATE``s are actually pending (this method is
called even
# for instances that have no net changes to their column-based
attributes)
- if not session.is_modified(target, include_collections=True):
+ if not session.is_modified(sqla_table, include_collections=True):
return
- # set permissions
- security_manager.set_perm(mapper, connection, target)
-
- dataset = (
-
session.query(NewDataset).filter_by(sqlatable_id=target.id).one_or_none()
+ # find the dataset from the known instance list first
+ # (it could be either from a previous query or newly created)
+ dataset = next(
+ find_cached_objects_in_session(
+ session, NewDataset, uuids=[sqla_table.uuid]
+ ),
+ None,
)
+ # if not found, pull from database
if not dataset:
+ dataset = (
+
session.query(NewDataset).filter_by(uuid=sqla_table.uuid).one_or_none()
+ )
+ if not dataset:
+ sqla_table.write_shadow_dataset()
return
- # get DB-specific conditional quoter for expressions that point to
columns or
- # table names
- database = (
- target.database
- or session.query(Database).filter_by(id=target.database_id).one()
- )
- engine = database.get_sqla_engine(schema=target.schema)
- conditional_quote = engine.dialect.identifier_preparer.quote
-
- # update columns
- if inspector.attrs.columns.history.has_changes():
- # handle deleted columns
- if inspector.attrs.columns.history.deleted:
- column_names = {
- column.column_name
- for column in inspector.attrs.columns.history.deleted
- }
- dataset.columns = [
- column
- for column in dataset.columns
- if column.name not in column_names
- ]
-
- # handle inserted columns
- for column in inspector.attrs.columns.history.added:
- # ``is_active`` might be ``None``, but it defaults to ``True``.
- if column.is_active is False:
- continue
-
- extra_json = json.loads(column.extra or "{}")
- for attr in {
- "groupby",
- "filterable",
- "verbose_name",
- "python_date_format",
- }:
- value = getattr(column, attr)
- if value:
- extra_json[attr] = value
-
- dataset.columns.append(
- NewColumn(
- name=column.column_name,
- type=column.type or "Unknown",
- expression=column.expression
- or conditional_quote(column.column_name),
- description=column.description,
- is_temporal=column.is_dttm,
- is_aggregation=False,
- is_physical=column.expression is None,
- 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,
- )
- )
-
- # update metrics
- if inspector.attrs.metrics.history.has_changes():
- # handle deleted metrics
- if inspector.attrs.metrics.history.deleted:
- column_names = {
- metric.metric_name
- for metric in inspector.attrs.metrics.history.deleted
- }
- dataset.columns = [
- column
- for column in dataset.columns
- if column.name not in column_names
- ]
-
- # handle inserted metrics
- for metric in inspector.attrs.metrics.history.added:
- extra_json = json.loads(metric.extra or "{}")
- for attr in {"verbose_name", "metric_type", "d3format"}:
- value = getattr(metric, attr)
- if value:
- extra_json[attr] = value
-
- is_additive = (
- metric.metric_type
- and metric.metric_type.lower() in ADDITIVE_METRIC_TYPES
- )
-
- dataset.columns.append(
- NewColumn(
- name=metric.metric_name,
- type="Unknown",
- expression=metric.expression,
- warning_text=metric.warning_text,
- description=metric.description,
- 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,
- )
- )
+ # sync column list and delete removed columns
+ if (
+ inspector.attrs.columns.history.has_changes()
+ or inspector.attrs.metrics.history.has_changes()
+ ):
+ # add pending new columns to known columns list, too, so if calling
+ # `after_update` twice before changes are persisted will not create
+ # two duplicate columns with the same uuids.
+ dataset.columns = sqla_table.get_sl_columns()
Review Comment:
Now we just always re-create or update all NewColumn's as long as there is
any update to sqla_table.columns or sqal_table.metrics.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
