betodealmeida commented on code in PR #19421:
URL: https://github.com/apache/superset/pull/19421#discussion_r852565715
##########
superset/connectors/sqla/models.py:
##########
@@ -2273,95 +2286,54 @@ def write_shadow_dataset( # pylint:
disable=too-many-locals
For more context: https://github.com/apache/superset/issues/14909
"""
-
- engine = database.get_sqla_engine(schema=dataset.schema)
- conditional_quote = engine.dialect.identifier_preparer.quote
+ session = inspect(self).session
+ if self.database_id and (
+ not self.database or self.database.id != self.database_id
+ ):
+ self.database =
session.query(Database).filter_by(id=self.database_id).one()
# create columns
columns = []
- for column in dataset.columns:
- # ``is_active`` might be ``None`` at this point, but it defaults
to ``True``.
- if column.is_active is False:
- continue
-
- try:
- extra_json = json.loads(column.extra or "{}")
- except json.decoder.JSONDecodeError:
- extra_json = {}
- for attr in {"groupby", "filterable", "verbose_name",
"python_date_format"}:
- value = getattr(column, attr)
- if value:
- extra_json[attr] = value
-
- 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=dataset.is_managed_externally,
- external_url=dataset.external_url,
- ),
- )
-
- # create metrics
- for metric in dataset.metrics:
- try:
- extra_json = json.loads(metric.extra or "{}")
- except json.decoder.JSONDecodeError:
- extra_json = {}
- 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
- )
-
- 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,
- 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=dataset.is_managed_externally,
- external_url=dataset.external_url,
- ),
- )
+ for item in self.columns + self.metrics:
+ item.created_by = self.created_by
+ item.changed_by = self.changed_by
+ # on `SqlaTable.after_insert`` event, although the table itself
+ # already has a `uuid`, the associated columns will not.
+ # Here we pre-assign a uuid so they can still be matched to the new
+ # Column after creation.
+ if not item.uuid:
+ item.uuid = uuid4()
+ columns.append(item.to_sl_column())
# physical dataset
- if not dataset.sql:
- physical_columns = [column for column in columns if
column.is_physical]
-
- # create table
- table = NewTable(
- name=dataset.table_name,
- schema=dataset.schema,
- catalog=None, # currently not supported
- database_id=dataset.database_id,
- columns=physical_columns,
- is_managed_externally=dataset.is_managed_externally,
- external_url=dataset.external_url,
+ if not self.sql:
+ # always create separate column entities for Dataset and Table
+ # so updating a dataset would not update the columns in the table
+ physical_columns = [
+ clone_model(
+ column,
+ ignore=["uuid"],
+ # `created_by` will always be left empty because it'd
always
+ # be created via some sort of automated system.
+ # But keep `changed_by` in case someone manually changes
+ # column attributes such as `is_dttm`.
+ additional=["changed_by"],
+ )
+ for column in columns
+ if column.is_physical
+ ]
+ tables = NewTable.load_or_create(
Review Comment:
Ah, you're right, and I doubled check while reviewing! I think I got
confused by the name like you said...
##########
superset/connectors/sqla/models.py:
##########
@@ -2237,29 +2245,34 @@ def after_update( # pylint: disable=too-many-branches,
too-many-locals, too-man
column.is_physical = False
# update referenced tables if SQL changed
- if inspector.attrs.sql.history.has_changes():
- parsed = ParsedQuery(target.sql)
- referenced_tables = parsed.tables
-
- predicate = or_(
- *[
- and_(
- NewTable.schema == (table.schema or target.schema),
- NewTable.name == table.table,
- )
- for table in referenced_tables
- ]
+ if sqla_table.sql and inspector.attrs.sql.history.has_changes():
+ referenced_tables = extract_table_references(
+ sqla_table.sql, sqla_table.database.get_dialect().name
+ )
+ dataset.tables = NewTable.load_or_create(
Review Comment:
Ignore
--
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]
