Repository navigation
Conversation
f5e74b8 to
b5effe0
Compare
b5effe0 to
615532b
Compare
The Snowflake converter only exported. This adds the import direction, so a Cortex Analyst semantic model can be read into an Ossie model and handed to any other spoke in the hub. `ossie-snowflake snowflake-to-ossie -i cortex_analyst.yaml -o semantic_model.yaml` is the new subcommand; `ossie-to-snowflake` names the existing one, and the pre-subcommand `-i`/`-o` spelling keeps working. The mapping is documented in the README. Three points worth calling out: - Expressions that are provably standard SQL are labelled `ANSI_SQL`, and the narrower `SNOWFLAKE` label is kept only for SQL this converter cannot vouch for. Every exporter reads its own dialect, then `ANSI_SQL`, then `OSSIE_SQL_2026`, and drops a field labelled with none of them, so a `SNOWFLAKE`-only label would remove the field from every other spoke. - Snowflake content with no Ossie home is carried in `custom_extensions` from a short allowlist, and anything else is dropped with a warning naming it. - Ossie has model-level metrics only, so a table's own metrics are lifted to the model. Lifting qualifies their column references with the table they came from, and warns when it collides on a name. The shared data type tables and dialect names moved to `_common.py` so the two directions cannot drift apart. Closes apache#162
615532b to
56791c8
Compare
|
@jbonofre Rebased onto main and addressed the three review points. The workflow runs on |
| for column in tree.find_all(exp.Column): | ||
| if column.table: | ||
| continue | ||
| start = column.this.meta.get("start") |
There was a problem hiding this comment.
This splices dataset. in at column.this.meta["start"], which assumes that offset is where the identifier begins. For a quoted identifier like SUM("Amount"), does sqlglot report the offset before or after the opening quote? If after, this yields SUM("dataset.Amount"). A dataset name that needs quoting would also go in unquoted. Could you add a test with quoted column names and a quoted dataset name?
| """ | ||
| result = _convert_named_expr(entry, "field") | ||
|
|
||
| if is_time is not None: |
There was a problem hiding this comment.
facts pass is_time=None, so no dimension block is written. On re-export, the datatype default classifies a date or time-typed fact as a time_dimension, so its role changes across a Snowflake -> Ossie -> Snowflake round trip. Non-blocking: either stash the original role or add a round-trip test that documents this.
| "synonyms", "dimensions", "time_dimensions", "facts", "metrics", | ||
| ) | ||
| _FIELD_MAPPED_KEYS = ("name", "expr", "description", "synonyms", "data_type", "unique") | ||
| _RELATIONSHIP_MAPPED_KEYS = ( |
There was a problem hiding this comment.
join_type and relationship_type aren't in _RELATIONSHIP_MAPPED_KEYS, so they're dropped with an "unrecognized key" warning. That wording suggests a newer Snowflake feature, when these are common keys and a known gap. Non-blocking: could they get an explicit "not mapped" message?
| }) | ||
|
|
||
|
|
||
| def is_portable_sql(expr): |
There was a problem hiding this comment.
IGNORE NULLS tokenizes as plain names, so FIRST_VALUE(x) IGNORE NULLS passes this check and gets labelled ANSI_SQL. It isn't standard SQL, so other warehouses would reject it. Non-blocking: could the check look for the IGNORE/RESPECT NULLS sequence explicitly?
| raise ValueError( | ||
| f"SNOWFLAKE custom_extensions data is not valid JSON: {e}" | ||
| ) from e | ||
| data.pop("_v", None) |
There was a problem hiding this comment.
If the stash is valid JSON but not an object (for example a list), data.pop("_v", None) raises AttributeError instead of the ValueError used for bad JSON above. An isinstance(data, dict) check that raises ValueError would keep the error handling consistent.
| for column in tree.find_all(exp.Column): | ||
| if column.table: | ||
| continue | ||
| start = column.this.meta.get("start") |
There was a problem hiding this comment.
This splices dataset. in at column.this.meta["start"], which assumes that offset is where the identifier begins. For a quoted identifier like SUM("Amount"), does sqlglot report the offset before or after the opening quote? If after, this yields SUM("dataset.Amount").
A dataset name that needs quoting would also go in unquoted.
Could you add a test with quoted column names and a quoted dataset name?
| """ | ||
| result = _convert_named_expr(entry, "field") | ||
|
|
||
| if is_time is not None: |
There was a problem hiding this comment.
facts pass is_time=None, so no dimension block is written. On re-export, the datatype default classifies a date or time-typed fact as a time_dimension, so its role changes across a Snowflake -> Ossie -> Snowflake round trip.
It's non-blocking, just a minor comment.
2 participants
Summary
The Snowflake converter only exported. This adds the import direction, so a
Cortex Analyst semantic model can be read into an Ossie model and handed to any
other spoke in the hub.
ossie-snowflake snowflake-to-ossie -i cortex.yaml -o model.yamlis the newsubcommand,
ossie-to-snowflakenames the existing one, and the pre-subcommand-i/-ospelling keeps working. The full mapping is in the README; threepoints worth calling out here:
ANSI_SQL, and thenarrower
SNOWFLAKElabel is kept only for SQL this converter cannot vouchfor. Every exporter reads its own dialect, then
ANSI_SQL, thenOSSIE_SQL_2026, and drops a field labelled with none of them, so aSNOWFLAKE-only label would remove the field from every other spoke. Thecheck runs over sqlglot's Snowflake tokenizer, which this package already
depends on.
custom_extensionsfrom ashort allowlist; anything else is dropped with a warning naming it.
model, with a warning when lifting collides on a name.
The shared data type tables and dialect names moved to
_common.pyso the twodirections cannot drift apart.
Verification
uv run pytest: 310 passed, of which 119 are new. Green on Python 3.11, 3.12,3.13 and 3.14;
uv lock --checkclean;dev/check-licensepasses.on
mainand on this branch gives byte-identical Snowflake YAML.core-spec/ossie-schema.json, the way the Microsoft, Cube, NVIDIA, OrionBeltand ThoughtSpot converters validate theirs.
kinds, dimensions, a time dimension, facts, a table-level metric, a
model-level metric and a relationship imports to Ossie (0 schema errors),
exports through the Salesforce converter, and is accepted
201. The org readsback both data objects, all nine fields, both calculated measurements and the
relationship joining
ORDERS.CUSTOMER_IDtoCUSTOMERS.CUSTOMER_ID.every table, field role, key, synonym, relationship and metric.
Related Issues
Closes #162. Supersedes #163, whose approach this builds on.
Builds on #566; the diff here carries that commit until it merges.
Checklist
Sections this change does not touch (Specification, Ontology, Validation,
Examples) are omitted rather than left unchecked.
Converters
.github/labeler.ymlalready coversconverters/snowflake/**Documentation
metric lifting
ROADMAP.mdandconverters/README.mddescribe the spoke asbidirectional, the way they already describe GoodData and OrionBelt
Tests
Compliance
jsonschemais added to the dev group only, for the schema check fiveother converters in this repo already run; nothing is added at runtime