Add datatype field to Field and Metric; reframe is_time as role marker#11
Closed
vndrewlee wants to merge 2 commits into
Closed
Add datatype field to Field and Metric; reframe is_time as role marker#11vndrewlee wants to merge 2 commits into
vndrewlee wants to merge 2 commits into
Conversation
Introduces a top-level `datatype` on Field and Metric with a closed logical enum: string, integer, number, boolean, date, time, timestamp, timestamp_tz, other. Addresses issue #84. `datatype` and `dimension.is_time` are independent and orthogonal: - `datatype` declares the field's logical data type (casting/serialization). - `is_time` is a temporal-role marker (time-series analysis, temporal filtering). A field with `is_time: true` may carry any `datatype` (e.g. integer for a year grain, string for a month name, date for a calendar date). When `is_time` is unset, it defaults to `true` for temporal datatypes (`date`, `time`, `timestamp`, `timestamp_tz`) and `false` otherwise. Explicit `is_time` always wins, so authors can set `is_time: false` on an audit `created_at` to keep it off the time axis. Taxonomy and type/role split were chosen after benchmarking 14 peer semantic layers and 5 portable type standards. Notable precedent: Snowflake Semantic Views' YAML authoring form has a `time_dimensions:` collection whose entries can carry any `data_type` (the published example annotates `order_year` with `data_type: NUMBER`); LookML's `dimension_group` accepts `date`, `datetime`, `timestamp`, `epoch`, and `yyyymmdd`. Snowflake converter updated: `_classify_field` honors explicit `is_time` first, then falls back to the temporal-datatype default. 9 new tests cover the datatype paths and the mixed-metadata cases (`d_year` with `datatype: integer` and `is_time: true`, audit timestamp opt-out, etc.). tpcds_semantic_model.yaml demonstrates three coexistence patterns: datatype-only, datatype + is_time, and is_time-only. Also added .gitignore for __pycache__ directories. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Vendors enumeration block was introduced during cherry-pick conflict resolution but was not part of apache/ossie#113. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AI Disclosure: I drove this PR and description below iteratively with Claude Code, and have reviewed it fully
Recreates open-semantic-interchange/ossie#113 on the Apache Ossie repository.
Motivation
Addresses open-semantic-interchange/ossie#84.
This PR is the outcome of the discussion I started in open-semantic-interchange/ossie#25 (converted to open-semantic-interchange/ossie#84), also related to open-semantic-interchange/ossie#17.
The issue originally framed this as "use
datatyperather thanis_time". After a bit more thought and investigation into existing semantic layers implementations, I think it makes more sense to add data types as a newfield/metricattribute, and keepis_timeas a role attribute. This fits cleanly with the example in this repo whered_year,d_quarter_name,d_month_namehave different data types, but temporal roles. Snowflake Semantic Views (YAML form) and LookML both split the same two axes.So this PR adds
datatypealongsideis_timeand documents them as independent properties.What changes
New
datatypeenum onFieldandMetric(optional, top-level). Values:string,integer,number,boolean,date,time,timestamp,timestamp_tz,other. Logical types, not SQL-physical. Useotherpluscustom_extensionsfor types outside the enum.dimension.is_timestays valid. It is documented as a temporal-role marker, independent ofdatatype. A field may carrydatatype: integerwithis_time: true(e.g.d_year), ordatatype: timestampwith nois_time(a plain timestamp column), oris_time: truealone (legacy-style).Default rule for
is_time. When unset,is_timedefaults totrueifdatatypeis one ofdate,time,timestamp,timestamp_tz, andfalseotherwise. Explicitis_timealways wins. This lets authors opt a temporal-typed column (e.g. an auditcreated_at) out of time-dimension classification withis_time: false.Snowflake converter updated.
_classify_fieldnow honors explicitis_timefirst, then falls back to the temporal-datatype default. Nine new tests cover the datatype paths, the mixed-metadata case (d_yearwithdatatype: integerandis_time: true), and the opt-out case. The five originalis_timetests are unchanged, proving back-compat.Example updated.
examples/tpcds_semantic_model.yamldemonstrates three coexistence patterns:datatypeonly (majority of fields),datatypeplusis_time: truecoexisting (d_year), andis_time: trueonly (legacy style, retained on two fields as a compatibility demonstration). 25 fields and 5 metrics gaineddatatype.Docs.
converters/index.mdexplains the type-vs-role split and the converter classification rule;docs/index.mdglossary mentionsdatatypeon the Field entry. A newDatatype and is_time: type vs. rolesection incore-spec/spec.mdcovers the combinations, default rule, consumer guidance, and migration recipe, with a Precedent note citing Snowflake Semantic Views and LookML.Also added
*.py[cod]to.gitignorefor Python bytecode files.Impact on existing implementations
The change is additive and backward-compatible:
is_time. Continue to validate and convert unchanged. The Snowflake converter'stime_dimensionclassification is preserved for everyis_time: truefield, including non-temporal grain columns liked_year.datatype. Work as expected. A temporaldatatypeon a field with adimensionblock classifies as atime_dimensionvia the default rule.is_time: falseexplicitly on a temporal-typed column. Classification is nowdimension, nottime_dimension. This is intentional: the author has opted out. The only way to reach this case before this PR was to author an unusual model whereis_time: falsewas on a column that was implicitly temporal in the consumer's view. If such models exist, they will flip classification; we judged this the right behavior because ignoring an explicitis_time: falseis what consumers had implicitly been doing, and that silent override is worse than the opt-out.I didn't update the Snowflake exporter to add the data type itself, as Snowflake has the types of everything internally. If we were to add a Snowflake importer, it would be important to map the Snowflake data types onto these OSI data types.
Test plan
python -m pytest converters/snowflake/tests/test_osi_to_snowflake_yaml_converter.pyexamples/tpcds_semantic_model.yamlagainst updated schema