Skip to content

feat(holistics): add a Holistics AML converter - #538

Merged
jbonofre merged 12 commits into
apache:mainfrom
tantmd:holistics-converter
Oct 9, 2026
Merged

jbonofre merged 12 commits into
apache:mainfrom
tantmd:holistics-converter

Conversation

@tantmd

@tantmd tantmd commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Add a Holistics AML converter

Hi, I'm Tan from Holistics. We have been following Ossie from the outside, and we would like to contribute a
converter between Holistics AML and the Ossie semantic model.

For context. AML is the language a Holistics semantic layer is written in: datasets, models, dimensions, measures and relationships. AQL is its expression language, similar to MAQL and DAX.

What it does

  • to-ossie reads the JSON holistics aml compile produces, and writes one Ossie semantic model.
  • to-aml reads one Ossie semantic model, and writes AML source files.

One AML Dataset becomes one Ossie semantic model. Its models become Ossie datasets, its relationships become
Ossie relationships, and its measures and metrics become Ossie metrics.

This converter also rewrites some Ossie metrics into AQL. An Ossie metric can calculate across datasets, and
Holistics only allows that in AQL.

This is similar to DAX, and I see that converters/microsoft already does the same thing. It turns the
TPC-DS customer_lifetime_value into DIVIDE(SUM('store_sales'[ss_ext_sales_price]), DISTINCTCOUNTNOBLANK('customer'[c_customer_sk])). We turn the same metric into sum(store_sales.ss_ext_sales_price) / count_distinct(customer.c_customer_sk).

Some interop issues

Similar to MAQL and DAX, I do not attempt to convert AQL to Ossie SQL here. An AQL body is stashed under HOLISTICS_AQL in the Dialect enumeration, which the first commit registers, following #351.

AML supports query templating in a QueryModel, think a Looker derived table, including params supplied at
query time. This does not translate cleanly to Ossie, so this converter raises an ERROR when it detects one
and carries the body unresolved. The forms are {{ #model }}, {{ #model.* }} and {% filter(param) %}. A
filter block cannot be resolved at conversion time at all, because the param value only exists at query time.

I also noticed that examples/tpcds_semantic_model.yaml uses window functions over dataset fields, for example RANK() OVER (PARTITION BY store.s_store_sk ORDER BY SUM(store_sales.ss_ext_sales_price) DESC). I don't think this works in most BI vendors. Omni does the substitution and keeps the window: converters/omni/tests/fixtures/tpcds_omni/views/store_sales.view.yaml carries
RANK() OVER (PARTITION BY ${store.s_store_sk} ORDER BY SUM(${store_sales.ss_ext_sales_price}) DESC). I
believe that fails with a group-by error once a query selects it without grouping by those columns.

AQL does support arbitrary window functions, including the case where the query grain does not include the
field, and composition over them. I could translate these into the equivalent AQL. But core-spec/expression_language.md says "When, adding the query interface, window functions will be subject to
where they are allowed", so where one is allowed and what it means at a given grain is not settled yet. So this converter raises an ERROR for those instead.

The Power BI converter does much the same. It emits BLANK() for all three window metrics in that file. Cube takes the mirror case on import with the same reasoning: a measure that "computes over a grain other than the query's" has no Ossie expression form.

For reviewers

converters/holistics/README.md has the command lines and the full option list. The documents under converters/holistics/docs/:

  • The fixture.
    The AML source, how to recompile it, and the shape of the compiled JSON.
  • Mapping.
    Which AML construct becomes which Ossie construct, including relationships and data types.
  • Expression translation.
    What the converter emits for each definition body, and what it leaves untranslated.
  • Ossie to AML.
    The generated file layout, and where a metric lands.
  • Ossie SQL to AQL.
    The function table, the raw-SQL call for a function AQL has no name for, and what is dropped instead.
  • Measures.
    How aggregation_type and a definition body compose into one Ossie metric expression.
  • The HOLISTICS payload.
    Everything carried in custom_extensions.
  • Limitations.
    The coverage matrix, and the 24 issue codes with their severities.

Related Issues

None. No issue precedes this, and the spec change has not been raised on the mailing list. I will send one shortly after opening this PR. Code is cheap nowadays, so I'd rather have something concrete to discuss on.

Checklist

Specification

  • Spec changes are included in core-spec/ and follow the existing structure
  • Spec changes have been discussed on the mailing list or in a linked issue (not yet, see above)
  • Breaking changes to the spec are clearly called out in the summary (there are none; the change adds one
    enumeration member)

Ontology

  • Ontology changes in ontology/ are consistent with spec changes (not applicable; no ontology change)
  • New or modified terms are defined and documented (not applicable)

Converters

  • Converter logic in converters/ is updated to reflect spec or ontology changes
  • New converters include tests under the converter's test directory
  • If adding a new converter, .github/labeler.yml is updated with the new path

Validation

  • Validation rules in validation/ are updated if the spec changed
  • New validation cases are covered by tests

Documentation

  • docs/ is updated to reflect any user-facing changes
  • New features or behaviors are documented with examples where appropriate
  • CONTRIBUTING.md is updated if the contribution process changed (not applicable)

Examples

  • examples/ are added or updated for any new spec constructs or converter support (not applicable; the
    dialect member needs no example, and examples/tpcds_semantic_model.yaml is used as a test input
    unchanged)

Tests

  • All existing tests pass (121 converter tests, 164 validation tests)
  • New functionality is covered by tests

Compliance

  • ASF license headers are present on all new source files (every .py, .md, .yml, .yaml, .aml,
    .toml and .gitignore; the two generated files, uv.lock and a compiled-JSON fixture, carry none, the
    same as their counterparts in the sibling converters)
  • No third-party dependencies are added without PMC/IPMC approval (PyYAML, and sqlglot>=30.12.0, which
    converters/microsoft, converters/snowflake and converters/nvidia already pin at the same version)
  • If third-party source code is vendored/copied (not just declared as a dependency), NOTICE and/or
    LICENSE have been updated per ASF policy (nothing is
    vendored)

@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown

Thank you for your first contribution to Apache Ossie (incubating)!

A committer will review your PR. In the meantime, please ensure:

  • All CI checks pass
  • ASF license headers are present on new source files
  • Tests cover new functionality

No ICLA is required to contribute. If you are elected as a committer in the future, you will need an Individual Contributor License Agreement (ICLA) on file with the ASF.

Questions? Ask in the PR comments or join us on Slack.

We appreciate your contribution!

Comment on lines +910 to +926
from_columns = payload.get("from_columns") or []
to_columns = payload.get("to_columns") or []

if len(from_columns) == 1 and kind in ("many_to_one", "one_to_one"):
operator = ">" if kind == "many_to_one" else "-"
return (
f"relationship({from_model.reference}.{from_columns[0]} {operator} "
f"{to_model.reference}.{to_columns[0]}, {active})"
)

config = Block("RelationshipConfig")
rel = config.block("EqualityRelationship")
rel.header = "rel: EqualityRelationship"
rel.property("type", quote(kind))
rel.property("from", _field_ref(from_model.aml_name, from_columns[0]))
rel.property("to", _field_ref(to_model.aml_name, to_columns[0]))
for from_column, to_column in zip(from_columns[1:], to_columns[1:]):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When from_columns is empty or missing, len(from_columns) == 1 is false and execution falls through to from_columns[0], which raises IndexError. to_columns[0] does the same. The CLI catches only ConversionError and OSError, so to-aml exits with a traceback.

The zip(from_columns[1:], to_columns[1:]) also silently truncates when the lists differ in length, so a three-column key with two target columns emits a two-column join.

Could we require equal, non-zero lengths up front and raise ConversionError naming the relationship?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 62d626d


# -- dataset ----------------------------------------------------------

def relationship(self, payload: dict[str, Any]) -> str:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aml_to_ossie.py stashes direction, nullable, rlp_propagation and where, but this function reads only type and active from the stash. A Holistics -> Ossie -> Holistics round trip therefore drops all four. where matters most: the join filter disappears and the join silently widens.

Could these be read back from the stash? If the loss is intentional, it should emit a warning on this path and be documented in limitations.md.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 17c2dbf

Comment on lines +789 to +792
declared = {
field["name"] for field in target.payload.get("fields") or []
} | {self.metric_aml_name[m["name"]] for m in target.measures}
if name not in declared and name not in target.borrowed:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

declared is built from field["name"] (Ossie names) plus self.metric_aml_name[...] (AML names), but it is compared against name = self.name_in_aml(column, scope) (sanitized). A declared field whose Ossie name is not a legal AML identifier then looks undeclared. That adds a spurious hidden dimension and a BORROWED_COLUMN warning.

Should declared hold sanitized names throughout?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in e35d98e

Comment on lines +173 to +177
def dialects_of(expression: Any) -> dict[str, str]:
dialects = (expression or {}).get("dialects") or []
if not dialects:
raise ConversionError("expression has no dialects")
return {entry.get("dialect", "ANSI_SQL"): entry.get("expression", "") for entry in dialects}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(expression or {}).get(...) and entry.get(...) assume mappings. A string expression or a non-mapping dialect entry raises AttributeError. _named_objects already guards the other collections, so a ConversionError here would be consistent.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 11d898e

child = block.block(f"persistence: {kind}")
for key, value in persistence.items():
if key.startswith("__"):
continue

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

str(value).lower() turns a list or dict into Python repr text (['a']), which isn't valid AML, and None into none.

Could we handle bool, number, string, list and mapping explicitly, and raise on anything else?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 6937306

@jbonofre
jbonofre self-requested a review October 9, 2026 03:43
@tantmd
tantmd force-pushed the holistics-converter branch from 92e09d8 to 54ba551 Compare October 9, 2026 03:45
tantmd and others added 9 commits October 9, 2026 05:55
AQL is the Holistics analytical query language. It is not SQL: it has
relationship-aware semantics, so `count(orders.id) | where(...) | of_all()`
has no SQL rendering that is correct without the whole dataset's relationship
graph.

The specification defines `dialects[]` as a list of `{dialect, expression}`
pairs so that a value a converter cannot translate still travels, tagged with
the language it is valid in. Without a member for AQL, a Holistics field
defined in AQL has nowhere to put its body, and `expression` is required on
every field. Registering it dissolves that.

Same pattern as THOUGHTSPOT in
[apache#351](apache#351): the schema enumeration, the
specification table, the generated types and the Python enumeration. AQL goes
into validate.py's SKIP_SQL_VALIDATION alongside MDX, TABLEAU, MAQL, SIGMA,
THOUGHTSPOT and DAX, because sqlglot cannot parse it.

Two tests cover it: that the dialect maps to None and sits in
SKIP_SQL_VALIDATION, and that an AQL body with a pipe operator passes
validate_sql rather than being reported as invalid SQL.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Offline conversion between a Holistics AML dataset and an Apache Ossie
semantic model, in both directions.

  to-ossie reads the JSON `holistics aml compile` produces, and writes one
  Ossie semantic model.
  to-aml reads one Ossie semantic model, and writes AML source files.

Reading goes through the compiled JSON because that is what the CLI emits.
Writing produces AML source text directly, since nothing turns that JSON
back into AML and AML source is what a person edits and what the CLI accepts.

One AML `Dataset` becomes one Ossie semantic model. Its models become Ossie
datasets, its relationships become Ossie relationships, and its measures and
metrics become Ossie metrics.

The two SQLs
------------

Holistics SQL is an AML `@sql` body: it interpolates through `{{ ... }}` and
resolves inside the one model it sits in. Ossie SQL is the `expression` of a
`dialects[]` entry: it writes a reference as `dataset.field`, and its dialect
tag names the warehouse. Warehouse SQL is what Holistics generates and sends
to the database. Every message and document here names which one it means.

What the converter will not do
------------------------------

It translates no AQL into SQL. An AQL body travels verbatim under the
HOLISTICS_AQL dialect.

It rewrites no warehouse SQL between dialects. A computed `@sql` body is
emitted under the dialect the operator names with --sql-dialect. AML records
only `data_source_name`, an instance-local connection name, so reading a
dialect out of it would be a guess.

It resolves no query template. A `QueryModel` whose body uses `{{ #model }}`,
`{{ #model.* }}` or `{% filter(param) %}` is carried unresolved with an ERROR
naming the dataset. A filter block cannot be resolved at conversion time at
all, because the param value exists only at query time, so resolving the
other two would turn a body that is visibly not SQL into one that looks like
SQL and still is not.

Restoring AML interpolation
---------------------------

The reverse path has to turn `CAST(created_at AS DATE)` back into
`CAST({{ #SOURCE.created_at }} AS DATE)`. A bare column name does compile,
and it is not equivalent: AQL reads the interpolation to learn which field a
body depends on, and a body without it loses the query rewriting and the
optimisation that link drives.

That means telling a column apart from a keyword, a function name and a date
part, which is why sqlglot is a runtime dependency here rather than dev-only.
The converter parses at the dialect the expression declares, takes the
character offsets sqlglot reports for each column, and splices the
replacement into the original string. In BigQuery's
`DATE_TRUNC(created_at, month)` sqlglot reads `created_at` as a column and
`month` as a date part.

The Ossie SQL is never written back out through sqlglot. That same string
rendered through the default dialect returns `DATE_TRUNC(created_at, MONTH)`,
which the default dialect then re-reads with the unit and the expression
exchanged.

A model-qualified reference is a column
---------------------------------------

`{{ field }}` substitutes the sibling field's definition. `{{ #model.field }}`
reads the column of that model's relation. Holistics renders
`FLOOR({{ #ecommerce_users.age }} / 10)` as
`FLOOR(`ecommerce_users`.`age` / 10)`, where `{{ age }}` would have rendered
the whole `extract(year from age(...))` body. So a model-qualified reference
naming the model it sits in becomes a bare column, and only one naming a
different model becomes a cross-dataset reference. A measure is the
exception, because a measure is never a relation column.

Ossie SQL to AQL
----------------

An Ossie metric may name two datasets. A `@sql` body resolves inside one
model, and a dataset-level AML `metric` accepts only `@aql`, so such a metric
has an AML home only as AQL. The converter renders it one construct at a
time: 51 function names, the operators, and `sql_number('NAME', ...)` for a
function with no AQL name, recovered from the source offsets so `LPAD` does
not come back as sqlglot's `PAD`. Rendering can also add a hidden dimension
for a column no dataset declares, and a `number` type for a field the
document never typed, because AQL resolves against declared fields where SQL
resolves against the table.

Window functions are refused with an ERROR. AQL has them, and the frame comes
from the query grain rather than from an `OVER` clause, so translating one
means choosing a grain. The Ossie specification defines no semantics for how
a window function behaves when queried, so nothing says which grain that is.

A metric that names another metric
----------------------------------

Which AML home the named metric landed in decides where the naming one can
go, and placement follows the reference to get there. A measure on the same
model is `{{ items_quantity }}`. A measure on another model, or a
dataset-level metric, moves the naming metric onto the dataset where AQL
resolves it. A `@sql` body pinned to a model by a stash or a qualifier, and
naming either, is an ERROR: Holistics accepts the file and fails at SQL
generation with "Cannot refer to external field in TableModel/QueryModel".
A reference cycle is an ERROR as well, which `holistics aml validate` does
not catch.

Verification
------------

Snapshot tests over four fixtures, plus three checks against the real
artifact. The converted document is validated against
core-spec/ossie-schema.json. The generated AML is run through
`holistics aml validate`. And the round trip compiles that AML back to JSON,
converts it to Ossie, and compares it with the document the reverse path
started from, which for the ecommerce fixture is byte-identical.

The checks that drive the Holistics CLI skip when it is absent. CI installs
it, so they do not skip there. See converters/holistics/README.md.

tests/fixtures/tpcds/ is examples/tpcds_semantic_model.yaml converted to AML.
That document carries no HOLISTICS stash, so it exercises every path that has
to derive an AML property rather than read one. 5 of its 8 metrics span two
datasets: 2 become AQL, and the 3 that are window functions are dropped with
an ERROR each.

tests/fixtures/aql/ covers each path through the translation, and
tests/fixtures/metric_refs/ each way a metric-to-metric reference lands.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`.github/labeler.yml` gives each converter directory its own label, and the
new one had no entry, so a pull request touching it got only the catch-all
`converter` label.

Kept out of the converter's own commit, which touches nothing outside
`converters/holistics/` so that it can move to apache/ossie-converters by
dropping one path prefix. This entry stays with the repository that runs the
labeler.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dialects_of validates the shape it is handed.

`(expression or {}).get(...)` assumed a mapping, so an `expression` written as
a bare string, or a `dialects` entry that is not a mapping, raised
`AttributeError` instead of a message. The CLI catches `ConversionError` alone,
so that reached the user as a traceback.

`_named_objects` already guarded the other collections. Three more cases join
the malformed-input table.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A stashed value is spelled explicitly rather than through str().lower().

`str(value).lower()` is right for a bool and a number and wrong for the rest:
a list came out as Python repr text (`['a']`) and `None` as `none`, neither of
which AML parses.

bool, number, string and list each have a spelling now. A nested object
carrying a `__type__` becomes a nested block under its own key, and a type with
no spelling stops the run rather than writing a body that fails to compile
later and further from its cause.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
aql_reference compares declared names on the AML spelling.

`declared` held Ossie names and `self.metric_aml_name[...]`, and was tested
against `self.name_in_aml(column, scope)`, which is sanitized. A field whose
Ossie name is not a legal AML identifier therefore read as undeclared.

That costs more than the spurious BORROWED_COLUMN warning in the report: the
metric borrows a hidden dimension under the sanitized name, which the model
already declares under that same name, and Holistics rejects the file as a
duplicate identifier.

`sanitize` rather than `name_in_aml`, which logs a rename warning per call.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Relationship column lists are checked before either one is indexed.

An absent or empty `from_columns` fails `len(from_columns) == 1`, falls through
to `from_columns[0]` and raises `IndexError`. The CLI catches `ConversionError`,
`OSError` and `UnicodeDecodeError`, so `to-aml` ends in a traceback.

Unequal lengths are worse, because they produce output rather than an error:
`zip(from_columns[1:], to_columns[1:])` stops at the shorter list, so a
three-column key against two target columns emits a two-column join that
compiles and joins on a condition nobody wrote.

Both now raise `ConversionError` naming the relationship and both counts.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The relationship stash round trips.

`aml_to_ossie` stashes `direction`, `nullable`, `rlp_propagation` and `where`,
and the reverse path read only `type` and `active`, so a Holistics -> Ossie ->
Holistics trip dropped all four. `where` mattered most: the join filter
disappears and the join silently widens.

All four are settable. `direction`, `nullable` and `rlp_propagation` are
properties of `RelationshipConfig`, the last taking an enumeration whose
members include `'inherit'` and `'one_way'`. Each is written only when it
differs from the compiler's default, so a relationship sitting at its defaults
keeps the short `relationship(...)` form rather than expanding into a block
that means the same thing.

`where` belongs to the relationship itself and takes a block of `from` and `to`
AQL predicates. A relationship written inline in `relationships:` has nowhere
to put one, so a relationship carrying a filter is declared above the `Dataset`
block under a name and the list holds `RelationshipConfig { rel: <name>, ... }`.
It is written as the `Relationship` supertype and the compiler narrows it from
`type`, which the compiled payload confirms by tagging it
`EqualityRelationship`.

Two more that the long form needed: `FieldRef { model: ..., field: ... }` is
now `ref(...)`, aml-std's constructor for the same value and how AML is
normally written, and a module model is named dotted the way the short form
names it rather than by its last segment, which did not resolve.

Verified against `holistics aml compile`: the filter returns under
`rel.where.from` with its content unchanged, and `direction: 'one_way'`
survives.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cover the relationship round trip in the fixture rather than only in
assertions.

The ecommerce fixture carried the short form, a composite key, a one-to-one, an
inactive edge and a module-path target, and none of the properties the previous
commit restores. Two relationships now cover the rest. `delivered_to_users` is
declared by name and carries a `where` filter on both ends, and
`products -> merchants` sets `direction`, `nullable` and `rlp_propagation` away
from their defaults.

That puts them through the whole pipeline: `holistics aml compile`, the forward
path, the reverse path, `holistics aml validate`, and the round trip back to
Ossie, which stays byte-identical.

Doing so caught one defect the inline assertions could not. `_write_where`
stripped the heredoc content, and the compiler preserves it, so a predicate
written `@aql x > 0 ;;` came back a character shorter and the round trip no
longer matched. The content is now written verbatim.

A range and a many-to-many relationship carry a `match` predicate rather than
column pairs, so neither has an Ossie form and both are dropped with an ERROR.
That was declared in limitations.md and never asserted; a test now injects each
kind into the compiled payload and checks the issue and the drop.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jbonofre
jbonofre force-pushed the holistics-converter branch from ffa32ea to adfe5b9 Compare October 9, 2026 03:57
tantmd and others added 2 commits October 9, 2026 11:05
Carry a match relationship in the stash rather than dropping it.

A range or many-to-many join states its whole condition as one AQL predicate
over both models. Ossie encodes a relationship as `from_columns` and
`to_columns`, which cannot hold that, so the edge had no `relationships[]`
entry and nothing else kept it either.

The compiler's payload now travels on the document stash under
`match_relationships`, and the reverse path rebuilds the declaration from it:
the predicate, the endpoints and the kind all come back as written. Same
treatment `param` already gets, and the same reason.

That makes the issue a WARNING rather than an ERROR. A consumer reading Ossie
still sees the two datasets as unrelated, which is what the message says, but
the join itself survives a Holistics -> Ossie -> Holistics trip.

`aml.Relationship` keeps the payload it was parsed from, so the stash is the
compiler's own object rather than one rebuilt from the parsed fields.

The ecommerce fixture gains a `ManyToManyRelationship`, which puts it through
`holistics aml compile`, both directions, `holistics aml validate` and the
round trip. `range` has no natural pair in that schema, so a test reaches it by
injecting one into the compiled payload.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Drop the cross-vendor sweep.

It converted every Ossie document under `converters/` and `examples/` and
compiled each result, which tied this suite to fixtures nobody here owns.
Upstream folded `converters/hex` into `converters/databricks/python` and the
sweep's floor failed, with nothing in this converter having changed.

Its value was in what it found rather than in running forever, and both
findings now have their own test: a dataset sharing the semantic model's name,
which AML rejects as a duplicate, and an `aggregation_type` whose wrapper does
not enclose the whole expression, where stripping it would leave a fragment.
Neither was reachable from the fixtures, so both would have gone untested when
the file went.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tantmd
tantmd force-pushed the holistics-converter branch from 43634de to b4e8dd8 Compare October 9, 2026 04:16
Comment on lines +103 to +105
def test_the_sweep_finds_documents():
"""A heuristic that silently matches nothing would pass every test below."""
assert len(FOREIGN) >= 20, _ids(FOREIGN)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI failed here with assert 18 >= 20.

Recent commit f4fa4e6 on main removed the Databricks Python converter (#531), which deleted 3 fixture files (fixtureA_ossie.yaml, fixtureB_ossie.yaml, tpcds_ossie.yaml), reducinig the total count of foreign flat Ossie documents from 21 down to 18.

Could we adjust the threshold to >= 18 (or >= 15)?

Also, in _foreign_ossie_documents(), we should filter out "target" in path.parts alongside .venv. Otherwise, Maven build artifacts (target/test-classes/) present in local checkouts are counted as fixtures and can mask this threshold difference locally.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deleted in b4e8dd8

That test was for catching bugs during development. It is a bit overzealous, and it breaks on changes to other converters, so I have deleted it.

SIGMA = "SIGMA"
THOUGHTSPOT = "THOUGHTSPOT"
DAX = "DAX"
HOLISTICS_AQL = "HOLISTICS_AQL"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Registering HOLISTICS_AQL in OssieDialect triggered CI failures in the Hex converter (converters/hex), which was recently merged in #381

  1. ty check fails at converters/hex/src/ossie_hex/ossie_to_hex/load_ossie_dialect_expression.py:118 because Hex does exhaustive pattern matching over OssieDialect and calls assert_never(ossie_dialect) on unhandled members.
  2. tests/cli/test_cli_export.py:226 in Hex fails its snapshot comparison between holistics_aql is dynamically added to the --dialect CLI choices.

When introducing a new dialect to OssieDialect, _to_sqlglot_dialect in converters/hex and its CLI test snapshot will need to be updated as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 848471a

Teach the Hex converter the new dialect member.

`converters/hex` matches exhaustively over `OssieDialect` and closes with
`assert_never`, so a new member is a type error rather than a silent fallthrough.
`ty check` reports it at `load_ossie_dialect_expression.py:118`.

AQL resolves through a dataset's relationship graph and sqlglot cannot parse it,
so it joins `MDX`, `MAQL`, `THOUGHTSPOT`, `SIGMA` and `DAX` as a language with no
engine to read it with. `_DIALECT_NAME_MAP` in `convert_ossie_dialect.py` is
typed over the whole enum and gets the same treatment.

The `--dialect` CLI choices come from the enum, so the export CLI's snapshot
gains `holistics_aql` between `dax` and `ossie_sql_2026`, where the member sits.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jbonofre
jbonofre self-requested a review October 9, 2026 04:29
@tantmd

tantmd commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the quick review, @jbonofre 🤟 . I've addressed all the issues as fixup commits, one per comment, unsquashed so each reads against its thread. Will squash it later before merging.

@jbonofre jbonofre left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @tantmd for addressing the previous review comments!

LGTM!

@jbonofre
jbonofre merged commit 28a91f5 into apache:main Oct 9, 2026
77 checks passed
tantmd pushed a commit to tantmd/ossie-website that referenced this pull request Oct 9, 2026
apache/ossie#538 merged, so converters/holistics exists on main and the entry
links it the way every other vendor's does.
tantmd pushed a commit to tantmd/ossie-website that referenced this pull request Oct 9, 2026
Holistics is a BI platform whose semantic layer is written as code in AML,
covering datasets, models, dimensions, measures and relationships.

The entry links the bidirectional converter between a Holistics AML dataset
and an Ossie semantic model, merged as apache/ossie#538 and living at
converters/holistics.

The logo is the official mark from docs.holistics.io, an SVG carrying both
the icon and the wordmark.
@tantmd

tantmd commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the quick turnaround and the thorough review, much appreciated. I've also opened apache/ossie-website#37 to add Holistics to the ecosystem grid. Would you mind reviewing that one too?

jbonofre pushed a commit to apache/ossie-website that referenced this pull request Oct 9, 2026
Holistics is a BI platform whose semantic layer is written as code in AML,
covering datasets, models, dimensions, measures and relationships.

The entry links the bidirectional converter between a Holistics AML dataset
and an Ossie semantic model, merged as apache/ossie#538 and living at
converters/holistics.

The logo is the official mark from docs.holistics.io, an SVG carrying both
the icon and the wordmark.

Co-authored-by: Huynh Tan <tan.huynh@holistics.io>
@jbonofre jbonofre mentioned this pull request Oct 11, 2026
5 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants