Repository navigation
docs: use portable sql fields in canonical examples - #640
OKAN-SOFTWARE wants to merge 3 commits into
Conversation
|
Thank you for your first contribution to Apache Ossie (incubating)! A committer will review your PR. In the meantime, please ensure:
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! |
| dialects: | ||
| - dialect: ANSI_SQL | ||
| expression: airport_code | ||
| sql: airport_code |
There was a problem hiding this comment.
Moving every example to sql-only means none of the three files CI validates (flights.yaml, flights.semantic_model.yaml, tpcds_semantic_model.yaml) goes through the dialects path any more.
Only the unit tests in validation/tests/test_validate.py still cover it, so a regression in the per-dialect dialect/expression handling would not fail on the examples.
Could we keep at least one dialects entry in the examples, for instance a vendor-specific variant on a single field or metric in flights.yaml?
There was a problem hiding this comment.
Addressed in commit f46b752. The canonical flights.yaml example now retains a Snowflake-specific dialect expression alongside its portable SQL expression, ensuring the per-dialect validation path remains covered.
| ### Expression Object | ||
|
|
||
| The expression object supports multiple dialects | ||
| Use `sql` for the primary, portable metric expression. Optional vendor-specific |
There was a problem hiding this comment.
This block is now sql-only, but the text below it says vendor variants can be supplied in dialects, and the field section points to a "multi-dialect example below". No metric example shows that shape.
Someone writing a metric has nothing to copy.
Could we add a metric-level example with sql plus a vendor entry under dialects? That would also fit with keeping one dialects example in the examples files.
There was a problem hiding this comment.
Addressed in commit 930aeaf. The metric expression documentation now includes a complete example combining portable SQL with a BigQuery-specific SAFE_DIVIDE dialect variant.
|
Thanks for the detailed review. Both requested changes are included in the latest commits:
The updated diff addresses both review points. The GitHub Actions workflows currently show Happy to address any remaining feedback. Thank you! |
Updates canonical examples and documentation to use expression.sql for portable SQL. Preserves SQL expressions, model data, and vendor-specific variants.
Closes #639.
Validation: 537 tests passed locally. All five canonical examples validated. Spec synchronization and diff checks passed. Hosted CI and maintainer review are pending.