fix(thoughtspot): guard wrongly-typed TML values from bare tracebacks

converters/thoughtspot/src/ossie_thoughtspot/tml_to_ossie.py

@@ -378,6 +378,21 @@ def _physical_datatype(
     data_type = (physical.get("db_column_properties") or {}).get("data_type")
     if data_type is None:
         return None
+    if not isinstance(data_type, str):
+        # A non-string data_type (e.g. a list in a hand-edited document) has no
+        # Ossie equivalent and would crash datatypes.to_ossie on an unhashable
+        # value; report it like an unmapped type and emit no datatype.
+        log.add(
+            code=f"{code_prefix}-DATATYPE-UNMAPPED",
+            severity=Severity.WARNING,
+            message=(
+                f"physical column {column_name!r} on table {table_name!r} has "
+                f"non-string data_type {data_type!r}, which has no Ossie "
+                f"equivalent; no datatype is emitted for this {kind}"
+            ),
+            object_ref=object_ref,
+        )
+        return None
     ossie_type = datatypes.to_ossie(data_type)
     if ossie_type is None:
         log.add(
@@ -1015,7 +1030,11 @@ def convert_metric(
     object_ref = f"metric:{display_name}"
 
     aggregation_raw = properties.get("aggregation", "NONE")
-    if aggregation_raw not in _AGGREGATION:
+    # A non-string aggregation (a list or dict in a hand-edited document) would
+    # make the `not in _AGGREGATION` membership test raise on an unhashable
+    # value; treat it as unrecognised and fall back to NONE like any other
+    # unknown aggregation rather than letting a bare TypeError escape.
+    if not isinstance(aggregation_raw, str) or aggregation_raw not in _AGGREGATION:
         log.add(
             code="TS-METRIC-AGGREGATION-UNKNOWN",
             severity=Severity.WARNING,
@@ -1316,6 +1335,39 @@ def _normalized_physical_columns(body: dict, kind: str) -> list[dict]:
     return [_normalize_physical_column(entry, kind) for entry in _raw_physical_columns(body, kind)]
 
 
+def _physical_columns_with_valid_db_names(
+    columns: list[dict], prefix: str, log: IssueLog
+) -> list[dict]:
+    """`columns` with any non-string `db_column_name` dropped to `None`.
+
+    Every consumer of a physical column -- the ANSI_SQL resolver, the field
+    stash, and `_physical_db_column_name` -- reads `db_column_name` from
+    `physical_columns_by_prefix`. A wrongly-typed value (an int, a list, or a
+    YAML-parsed date in a hand-edited document) left in place emits a nonsense
+    warehouse reference like `ORDERS.42`, or raises a bare error in to-tml's
+    json.dumps. Dropping it to `None` here, once, sends every consumer down its
+    existing "no warehouse name" path, and the loss is reported rather than
+    silently emitted.
+    """
+    sanitized = []
+    for column in columns:
+        db_column_name = column.get("db_column_name")
+        if db_column_name is not None and not isinstance(db_column_name, str):
+            log.add(
+                code="TS-COLUMN-DB-NAME-INVALID",
+                severity=Severity.WARNING,
+                message=(
+                    f"physical column {column.get('name')!r} on dataset {prefix!r} "
+                    f"has a non-string db_column_name {db_column_name!r}; it is "
+                    f"ignored and no warehouse column name is used for it"
+                ),
+                object_ref=f"dataset:{prefix}",
+            )
+            column = {**column, "db_column_name": None}
+        sanitized.append(column)
+    return sanitized
+
+
 #: The TML `db_column_properties.data_type` spelling `datatypes.to_tml` would
 #: emit by default for each Ossie datatype whose TML source has more than one
 #: valid spelling (the datatype map's Boolean and Float rows). Stashing the
@@ -1814,7 +1866,8 @@ def _relationship_from_join(
     """One join -> `(relationship, unrepresentable_entry, has_residual_predicates)`.
 
     Exactly one of `relationship`/`unrepresentable_entry` is non-`None` (or
-    both `None` when there is no condition at all to report). Implements the
+    both `None` when there is no condition to report, or the condition is not a
+    representable string and the join is dropped). Implements the
     *Non-equality joins* table: at least one equality pair emits a
     `Relationship`, with any residual predicates riding along in its own
     `custom_extensions` rather than withholding the relationship; zero
@@ -1830,6 +1883,26 @@ def _relationship_from_join(
     apply to, so `from`/`to` there stay exactly TML's own, unswapped.
     """
     object_ref = f"relationship:{name}"
+    if on_expression and not isinstance(on_expression, str):
+        # A truthy non-string `on` (a list or int in a hand-edited document)
+        # would raise a bare AttributeError on the .strip() below. It is not a
+        # representable condition, so the join is dropped -- not preserved in
+        # `unrepresentable_joins`, whose stash stores `on` verbatim and would
+        # then carry the non-string value on into to-tml. A distinct code, not
+        # TS-JOIN-MALFORMED, because that one preserves the join. A falsy
+        # non-string (`[]`, `{}`, `0`, `False`) keeps its prior "no condition"
+        # meaning and falls through to the check below.
+        log.add(
+            code="TS-JOIN-CONDITION-INVALID",
+            severity=Severity.WARNING,
+            message=(
+                f"join {name!r} from {from_prefix!r} to {to_prefix!r} has a "
+                f"non-string condition {on_expression!r}; it cannot be represented "
+                f"as a relationship and is dropped"
+            ),
+            object_ref=object_ref,
+        )
+        return None, None, False
     if not on_expression or not on_expression.strip():
         log.add(
             code="TS-JOIN-NO-CONDITION",
@@ -2250,8 +2323,8 @@ def convert(document_set: DocumentSet) -> OssieConversion:
         dataset_bodies[prefix] = dataset_dict
         dataset_stashes[prefix] = ds_stash
         table_docs[prefix] = table_doc.body
-        physical_columns_by_prefix[prefix] = _normalized_physical_columns(
-            table_doc.body, table_doc.kind
+        physical_columns_by_prefix[prefix] = _physical_columns_with_valid_db_names(
+            _normalized_physical_columns(table_doc.body, table_doc.kind), prefix, log
         )
         fields_by_dataset[prefix] = []
 
@@ -2293,9 +2366,29 @@ def resolve(table: str, column: str) -> str | None:
         return f"{table}.{warehouse_reference}"
 
     # -- Phase 3: fields and metrics ------------------------------------------
-    formulas: dict[str, dict] = {
-        f["id"]: f for f in (model_body.get("formulas") or []) if f.get("id")
-    }
+    # A non-string formula expr (an int, a list, or a YAML-parsed date in a
+    # hand-edited document) reaches every reader below -- convert_field,
+    # convert_metric, and both the unattributed and unsurfaced stash paths --
+    # and would raise a bare TypeError in to-ossie or in to-tml's json.dumps.
+    # Report it once here and drop the key, so each reader takes its existing
+    # "entry has no expr" path instead.
+    formulas: dict[str, dict] = {}
+    for f in model_body.get("formulas") or []:
+        formula_id = f.get("id")
+        if not formula_id:
+            continue
+        if "expr" in f and not isinstance(f["expr"], str):
+            log.add(
+                code="TS-FORMULA-EXPR-INVALID",
+                severity=Severity.WARNING,
+                message=(
+                    f"formula {formula_id!r} has a non-string expr {f['expr']!r}; "
+                    f"it is ignored and no expression is emitted or preserved for it"
+                ),
+                object_ref=f"formula:{formula_id}",
+            )
+            f = {k: v for k, v in f.items() if k != "expr"}
+        formulas[formula_id] = f
     metrics: list[dict] = []
     # Field identifiers are scoped per dataset in Ossie (Field.name is unique
     # "within the dataset"); metrics are scoped to the whole model (Metric.name

converters/thoughtspot/tests/test_tml_to_ossie_typed_values.py

@@ -0,0 +1,185 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Wrongly-*typed* TML values degrade cleanly instead of raising bare tracebacks.
+
+Issue #469: a hand-edited TML document can carry a value of the wrong Python
+type (a list where a string is expected, an int for an expression, and so on).
+PR #482 covered the column and model `name` cases; these cover the remaining
+TML->Ossie scalar reads it enumerated as untouched -- `aggregation`, a formula
+`expr`, a physical `data_type`, a `db_column_name`, and a join `on`.
+
+The guards for the values that also flow into the model-scope stash (`expr` and
+`db_column_name`) live at the point the data is built, not at one reader, so
+the whole `to-ossie` -> `to-tml` round trip stays free of bare tracebacks
+rather than just the field/metric readers. These tests therefore drive full
+documents through `convert()` (and `ossie_to_thoughtspot.convert()` for the
+stashed values) and assert on the issue log and the emitted document, not only
+on the single-helper return value.
+"""
+
+from ossie_thoughtspot import ossie_to_thoughtspot
+from ossie_thoughtspot.issues import IssueLog
+from ossie_thoughtspot.tml import DocumentSet, TmlDocument
+from ossie_thoughtspot.tml_to_ossie import _relationship_from_join, convert
+
+
+# -- builders (mirroring tests/test_tml_to_ossie.py) --------------------------
+
+def _table(name, columns):
+    return TmlDocument(
+        kind="table",
+        body={"name": name, "db": "SALES", "schema": "PUBLIC", "db_table": name,
+              "connection": {"name": "My Snowflake"}, "columns": columns},
+        guid=None,
+    )
+
+
+def _column(name, db_column_name, data_type="VARCHAR"):
+    return {"name": name, "db_column_name": db_column_name,
+            "db_column_properties": {"data_type": data_type}}
+
+
+def _model(columns, formulas=None):
+    body = {"name": "Sales Analytics", "model_tables": [{"name": "ORDERS"}],
+            "columns": columns}
+    if formulas is not None:
+        body["formulas"] = formulas
+    return TmlDocument(kind="model", body=body, guid=None)
+
+
+def _document_set(model_doc, table_doc):
+    return DocumentSet(model=model_doc, tables=(table_doc,))
+
+
+def _codes(issues):
+    return {issue["code"] for issue in issues}
+
+
+def _no_traceback_to_tml(ossie_document):
+    """to-tml must not raise on the converted document (the stash is the part a
+    user is least likely to hand-edit correctly)."""
+    result = ossie_to_thoughtspot.convert(ossie_document)
+    assert result is not None
+    return result
+
+
+# -- aggregation --------------------------------------------------------------
+
+def test_non_string_aggregation_falls_back_to_none():
+    # A list aggregation would crash the `not in _AGGREGATION` membership test on
+    # an unhashable value; it is reported and the metric emits as NONE (a bare
+    # column reference, no aggregate wrapper), not dropped.
+    orders = _table("ORDERS", [_column("Amount", "O_TOTALPRICE", "DOUBLE")])
+    model = _model(columns=[
+        {"name": "Total", "column_id": "ORDERS::Amount",
+         "properties": {"column_type": "MEASURE", "aggregation": ["SUM"]}},
+    ])
+
+    result = convert(_document_set(model, orders))
+
+    assert "TS-METRIC-AGGREGATION-UNKNOWN" in _codes(result.issues.as_dicts())
+    metric = result.model["metrics"][0]
+    thoughtspot = next(d["expression"] for d in metric["expression"]["dialects"]
+                       if d["dialect"] == "THOUGHTSPOT")
+    assert thoughtspot == "[ORDERS::Amount]"  # NONE: no sum(...)/avg(...) wrapper
+
+
+# -- formula expr (reaches convert_field/metric AND the stash) ----------------
+
+def test_non_string_formula_expr_is_reported_and_not_stashed():
+    orders = _table("ORDERS", [_column("Amount", "O_TOTALPRICE", "DOUBLE")])
+    model = _model(
+        columns=[{"name": "Bad", "formula_id": "f1",
+                  "properties": {"column_type": "MEASURE", "aggregation": "SUM"}}],
+        formulas=[{"id": "f1", "name": "Bad", "expr": 42}],
+    )
+
+    result = convert(_document_set(model, orders))
+
+    assert "TS-FORMULA-EXPR-INVALID" in _codes(result.issues.as_dicts())
+    assert result.model.get("metrics", []) == []  # the bad formula built no metric
+    # The invalid expr must not have reached the model-scope stash, or to-tml
+    # would raise a bare TypeError on it.
+    _no_traceback_to_tml(result.model)
+
+
+def test_unsurfaced_non_string_formula_expr_is_reported_and_not_stashed():
+    # A formula no column references is normally preserved verbatim in the
+    # unsurfaced-formulas stash; a non-string expr must be dropped there too.
+    orders = _table("ORDERS", [_column("Amount", "O_TOTALPRICE", "DOUBLE")])
+    model = _model(
+        columns=[{"name": "Amount", "column_id": "ORDERS::Amount",
+                  "properties": {"column_type": "ATTRIBUTE"}}],
+        formulas=[{"id": "orphan", "name": "Orphan", "expr": ["not", "a", "string"]}],
+    )
+
+    result = convert(_document_set(model, orders))
+
+    assert "TS-FORMULA-EXPR-INVALID" in _codes(result.issues.as_dicts())
+    _no_traceback_to_tml(result.model)
+
+
+# -- db_column_name (reaches the resolver, the field stash, and the helper) ---
+
+def test_non_string_db_column_name_emits_no_warehouse_reference():
+    orders = _table("ORDERS", [_column("Amount", 42, "DOUBLE")])
+    model = _model(columns=[
+        {"name": "Amount", "column_id": "ORDERS::Amount",
+         "properties": {"column_type": "ATTRIBUTE"}},
+    ])
+
+    result = convert(_document_set(model, orders))
+
+    assert "TS-COLUMN-DB-NAME-INVALID" in _codes(result.issues.as_dicts())
+    field = result.model["datasets"][0]["fields"][0]
+    # No `ORDERS.42` leaked into an ANSI_SQL sibling; the invalid name is absent.
+    for dialect in field.get("expression", {}).get("dialects", []):
+        assert "42" not in dialect["expression"]
+    _no_traceback_to_tml(result.model)
+
+
+# -- join `on` ----------------------------------------------------------------
+
+def _run_join(on_expression):
+    log = IssueLog()
+    result = _relationship_from_join(
+        name="orders_to_customers", from_prefix="ORDERS", to_prefix="CUSTOMERS",
+        on_expression=on_expression, join_type=None, cardinality=None,
+        join_shape="inline", referencing_join=None,
+        table_lookup=lambda name: None, log=log,
+    )
+    return result, log.as_dicts()
+
+
+def test_truthy_non_string_join_condition_is_dropped_and_reported():
+    (relationship, unrepresentable, has_residual), issues = _run_join(42)
+
+    assert relationship is None
+    assert unrepresentable is None  # dropped, not preserved
+    assert has_residual is False
+    assert "TS-JOIN-CONDITION-INVALID" in _codes(issues)
+
+
+def test_falsy_non_string_join_condition_keeps_its_no_condition_meaning():
+    # `on: []`, `on: 0`, etc. meant "no condition" before this change and must
+    # keep reporting TS-JOIN-NO-CONDITION, not the new invalid-condition code.
+    for falsy in ([], {}, 0, False):
+        (relationship, unrepresentable, _), issues = _run_join(falsy)
+        assert relationship is None and unrepresentable is None
+        assert "TS-JOIN-NO-CONDITION" in _codes(issues)
+        assert "TS-JOIN-CONDITION-INVALID" not in _codes(issues)