[#12210] docs: Add Metric View design - #12360
Conversation
|
@jerryshao @markhoerth could you please help review this design doc when you have time? thanks! |
Code Coverage Report
|
| - `Field` names are unique within each `Dataset`. | ||
| - Internal field references resolve within the model. | ||
| - Each `source` is a `NameIdentifier` with namespace `[catalog, schema]`; `source.name` identifies a `Table` or `View`, and the enclosing object supplies the metalake. | ||
| - For `Table` and logical `View` sources, Gravitino validates columns explicitly declared in `primary_key`, `unique_keys`, `from_columns`, and `to_columns` against the source schema. It does not infer source-column references from field or metric expressions. |
There was a problem hiding this comment.
What is the meaning of "the enclosing object supplies the metalake"?
There was a problem hiding this comment.
How do you differentiate a table or a view only by the name identifier?
There was a problem hiding this comment.
What is the meaning of "the enclosing object supplies the metalake"?
It means that Dataset.source stores catalog.schema.name, Gravitino resolves it within the Metric View's metalake. Cross-catalog references within that metalake are allowed, while cross-metalake references are not supported.
How do you differentiate a table or a view only by the name identifier?
We do not need Dataset.source to distinguish between a Table and a View. We only need to verify that the referenced entity exists. During validation, Gravitino can call loadTable first and, if no Table is found, call loadView.
| - Each `source` is a `NameIdentifier` with namespace `[catalog, schema]`; `source.name` identifies a `Table` or `View`, and the enclosing object supplies the metalake. | ||
| - For `Table` and logical `View` sources, Gravitino validates columns explicitly declared in `primary_key`, `unique_keys`, `from_columns`, and `to_columns` against the source schema. It does not infer source-column references from field or metric expressions. | ||
| - Metric View sources validate direct existence only. | ||
| - Inline query sources are not supported; register the query as a logical View and reference that View through a `NameIdentifier`. |
There was a problem hiding this comment.
Can you explain more about the meaning here?
There was a problem hiding this comment.
For example, instead of storing
SELECT * FROM sales.orders WHERE status = 'active'directly inDataset.source, create a logical View namedsales.mart.active_orderswith that SQL and setDataset.sourcetosales.mart.active_orders. Raw SQL is not stored directly in the Metric View.
I will add the above explanation to the doc.
|
|
||
| The required `MetricModel.name` is independent of the enclosing View name. This preserves semantic-model identity across imports and View renames. | ||
|
|
||
| Every supported `custom_extensions` array is retained losslessly. Unknown standardized fields are rejected until the declared OSI profile supports them. |
There was a problem hiding this comment.
"Unknown standardized fields are rejected until the declared OSI profile supports them."
Is this the OSI's behavior?
There was a problem hiding this comment.
Yes. For standardized OSI model objects, the schema defines the supported fields and sets additionalProperties to false. Vendor-specific data remains supported through explicitly defined extension points such as custom_extensions.
See the OSI schema definition.
|
I dont think so. The standard allows customized and vendor specific
fields.
…On Wed, Aug 5, 2026 at 8:42 PM Jerry Shao ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In design-docs/gravitino-metric-view-design.md
<#12360 (comment)>:
> +CustomExtension
+ vendor_name: string
+ data: string
+
+Dialect = "ANSI_SQL" | "SNOWFLAKE" | "MDX" | "TABLEAU"
+ | "DATABRICKS" | "MAQL" | "BIGQUERY"
+```
+
+- Each `Expression` contains at least one `DialectExpression`.
+- Every dialect entry uses a supported `Dialect`.
+- Every dialect entry has a non-empty `expression`.
+- `Dimension`, `AIContext`, and `CustomExtension` values satisfy the structures above.
+
+The required `MetricModel.name` is independent of the enclosing View name. This preserves semantic-model identity across imports and View renames.
+
+Every supported `custom_extensions` array is retained losslessly. Unknown standardized fields are rejected until the declared OSI profile supports them.
"Unknown standardized fields are rejected until the declared OSI profile
supports them."
Is this the OSI's behavior?
—
Reply to this email directly, view it on GitHub
<#12360?email_source=notifications&email_token=ALNHCVXJJKSCL5OG3OIUATT5IP5BBA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIOBXGA3TMOBTHAZ2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#pullrequestreview-4870768383>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALNHCVR7SL4L6WNJVBAE2V35IP5BBAVCNFSNUABFKJSXA33TNF2G64TZHM3DGMJUGMYTANRRHNEXG43VMU5TKMBWGEYTGNZSGE22C5QC>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/ALNHCVWZ5XQ7G3VHM3TBUOD5IP5BBA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIOBXGA3TMOBTHAZ2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y>
and Android
<https://github.com/notifications/mobile/android/ALNHCVUG2E3LYDIRKSQD5ZL5IP5BBA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIOBXGA3TMOBTHAZ2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>.
Download it today!
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
|
||
| Create and alter reject an unsupported `osiVersion` before validating or persisting `semanticModel`. The REST API returns HTTP 400 with the requested and supported versions, and creates no View or View version. | ||
|
|
||
| Once an `osiVersion` has been accepted for persistence by a released Gravitino version, later releases retain read support for it. Write support may be removed only through a major-version compatibility change with a documented migration path. Unknown stored versions fail explicitly and are never silently coerced. |
There was a problem hiding this comment.
It's hard to support multi-version write. We can revisit the restrictions when we want to bump the supported versions.
There was a problem hiding this comment.
The original purpose of osiVersion was to preserve interpretability if the OSI specification changes field semantics across versions. Each Metric View would be bound to the OSI version under which it was defined, allowing clients and validators to select the correct interpretation.
After reconsideration, I think introducing this multi-version mechanism now is premature. If OSI introduces an incompatible semantic change in the future, Gravitino can add explicit version metadata then and treat definitions without it as using the initial semantics. I will simplify the current design by removing osiVersion.
| .build(); | ||
|
|
||
| MetricModel model = | ||
| MetricModel.builder() |
There was a problem hiding this comment.
Shall we let users to specify the version number if we want to support multiple version?
There was a problem hiding this comment.
I will remove the version number in current design. see #12360 (comment)
|
|
||
| #### Java API | ||
|
|
||
| The Java API uses immutable builders for the structured definition and the existing ViewCatalog lifecycle methods: |
There was a problem hiding this comment.
Python API should also be supported, which is even more important.
|
@jerryshao @markhoerth all comments resolved; please help review again, thanks! |
There was a problem hiding this comment.
The core call looks right to me: modeling a semantic model as a specialized View so it inherits identity, namespace, versioning, and governance, rather than adding a new top-level entity. The goals/non-goals are disciplined and the validation boundary is stated honestly.
My main concern is that the storage and read path ("stored only in the Gravitino EntityStore" plus the merged listing) and the namespace conflict behavior are one bullet each, and both need working out against the code that exists today. Details inline.
|
|
||
| ### Storage and Connector Behavior | ||
|
|
||
| - **Source of truth.** Metric Views are stored only in the Gravitino EntityStore. Logical Views remain stored by their underlying catalogs. |
There was a problem hiding this comment.
Two things here.
The store-only read path does not exist today. ViewOperationDispatcher.internalLoadView starts with c.doWithViewOps(v -> v.loadView(ident)) and throws NoSuchViewException if the catalog lacks the object (core/src/main/java/org/apache/gravitino/catalog/ViewOperationDispatcher.java:397); listViews (line 84), dropView, and alterView follow the same pattern. So a Metric View stored only in the EntityStore is unreachable through all of them. Worth stating where the branch lives (dispatcher short circuit vs. a Gravitino-managed view-ops impl selected by Capability.Scope.VIEW), whether a Metric View can be created in an Iceberg/HMS-backed schema, and what happens on schema/catalog drop cascade.
"Logical Views remain stored by their underlying catalogs" also conflicts with the accepted logical view design, which defines a fully Gravitino-managed tier for catalogs with no native view support (design-docs/gravitino-logical-view-management.md:152). Gravitino-stored logical Views already exist, so the merge and conflict rules should be defined against the existing capability tiers.
There was a problem hiding this comment.
Most of this is already covered by the lifecycle, storage, and listing sections.
I’ll clarify that Metric Views are managed independently of the underlying catalog’s View capability, align the Logical View wording with the existing capability tiers, and add the schema/catalog cascade semantics. The concrete dispatcher routing will remain an implementation detail.
| - **Source of truth.** Metric Views are stored only in the Gravitino EntityStore. Logical Views remain stored by their underlying catalogs. | ||
| - **Listing.** The server merges authorized catalog-backed logical Views with authorized Gravitino-managed Metric Views into the existing View listing. | ||
| - **Connector capability.** A connector that does not support Metric Views filters them from `listViews` and returns an explicit unsupported-Metric-View error for a direct `loadView`. Generic REST and Java View APIs continue to expose them. | ||
| - **Namespace conflicts.** Create checks both storage sources. If an external client later creates a same-name logical View directly in the catalog, list and load report a conflict and select neither object. The external operation must rename or remove its object; ownership, versions, tags, and policies remain attached to the Gravitino Metric View and never transfer. |
There was a problem hiding this comment.
This failure mode worries me: an external client creating a same-name view in the catalog silently disables loadView for a governed Gravitino object, including the operations an owner needs to resolve it. Does the conflict fail the whole listViews call or just that entry, and how does an owner rename or drop the Metric View while it is conflicting? I would keep the Gravitino-managed object authoritative and loadable with a conflict indicator instead.
There was a problem hiding this comment.
This is intentional fail-closed behavior, not a silent failure. Gravitino returns an explicit namespace-conflict error and tells the user to rename or drop the externally created catalog View. listViews also fails explicitly rather than hiding either object.
The conflict cannot be created through Gravitino because create and rename validate the shared namespace; it only occurs after an out-of-band catalog change. Making the managed object authoritative would hide that inconsistency, while the design encourages Gravitino to remain the unified metadata entry point.
There was a problem hiding this comment.
Fail-closed makes sense, and the cascade and Gravitino-managed wording addresses the rest. One thing worth stating explicitly in the bullet: that drop and rename of the Gravitino-managed Metric View still work while a conflict exists, so an owner can resolve it from the Gravitino side rather than only out-of-band in the catalog.
|
|
||
| The representation has two fields: | ||
|
|
||
| - `type`: The fixed value "metric" classifies the View as a Metric View. |
There was a problem hiding this comment.
RepresentationDTO declares defaultImpl = SQLRepresentationDTO.class (common/src/main/java/org/apache/gravitino/dto/rel/RepresentationDTO.java:37), so an unrecognized type does not fail, it deserializes as a SQL representation. Any rolling upgrade where an older server or client sees type: "metric" hits this. Worth calling out removing defaultImpl or adding explicit unknown-type rejection. The same class sets @JsonIgnoreProperties(ignoreUnknown = true), which works against strict validation of the metric subtree.
There was a problem hiding this comment.
defaultImpl was reasonable when SQL was the only representation type. With Metric View introducing a second subtype, missing or unknown type values will be rejected explicitly.
Metric View payloads will use strict validation, so unknown standardized fields in the Metric subtree will be rejected rather than ignored. Both behaviors will be covered by tests.
| .build(); | ||
|
|
||
| View created = | ||
| catalog.createMetricView( |
There was a problem hiding this comment.
This contradicts the premise that existing View APIs manage both kinds; load, alter, and drop below and the whole REST section use the generic resource. Suggest createView(ident, comment, new Column[0], new Representation[] {representation}, null, null, Map.of()).
Related: the design never says how a caller detects a Metric View. A Representation.TYPE_METRIC constant next to TYPE_SQL (api/src/main/java/org/apache/gravitino/rel/Representation.java:29, whose javadoc still says SQL is the only supported type) and a View.metricRepresentation() accessor mirroring sqlFor(dialect) (View.java:97) would help. Empty columns cannot be the discriminator: ViewCreateRequest.validate() already permits empty columns for logical views (ViewCreateRequest.java:110).
There was a problem hiding this comment.
createMetricView is only a Java convenience method. It delegates to the generic createView API while enforcing the Metric View constraints: empty columns, exactly one MetricRepresentation, and no default catalog or schema. The REST API and the remaining View lifecycle stay generic.
I agree that detection should be explicit. #12386 adds Representation.TYPE_METRIC and View.metricRepresentation(); empty columns are not used as the discriminator. I’ll update the design example accordingly.
|
|
||
| - `Dataset` names are unique within `MetricModel`. | ||
| - `Field` names are unique within each `Dataset`. | ||
| - Internal field references resolve within the model. |
There was a problem hiding this comment.
This appears to contradict the next bullet, "It does not infer source-column references from field or metric expressions." Expressions are opaque per-dialect strings, so there is nothing to resolve without parsing them. Either drop the claim or define which structured fields count as an internal reference.
There was a problem hiding this comment.
“Internal field references” is ambiguous and could imply expression parsing, so I’ll remove that bullet. The following rules already define the validation boundary: relationship endpoints reference existing Datasets, while primary_key, unique_keys, from_columns, and to_columns are checked against the columns exposed by the referenced Table or logical View. Field and metric expressions remain opaque.
| vendor_name: string | ||
| data: string | ||
|
|
||
| Dialect = "ANSI_SQL" | "SNOWFLAKE" | "MDX" | "TABLEAU" |
There was a problem hiding this comment.
This creates a second dialect vocabulary alongside the existing Dialects type (trino, spark, hive, flink) in the same View object, with different naming conventions and different extensibility. A closed enum also makes every new OSI dialect a Gravitino API change plus a hard write-time rejection. Suggest an open string with well-known constants, and a sentence on what a consumer does when its dialect is absent from an Expression.
There was a problem hiding this comment.
#12386 already models the dialect as an open String with MetricDialects well-known constants rather than an enum, so new values do not require changing the API type. Dialects identifies logical View SQL representations, while MetricDialects follows OSI expression dialects.
Gravitino does not interpret or compile these expressions. Dialect selection and fallback are consumer-specific; a downstream consumer may reject a dialect it cannot interpret.
| Dimension | ||
| is_time?: boolean | ||
|
|
||
| AIContext = string | { instructions?: string, synonyms?: string[], |
There was a problem hiding this comment.
The string-or-object union is awkward for Java and Python bindings and for the OpenAPI schema, and the trailing ... in the object form is undefined. Suggest normalizing on write (accept a bare string, store it as instructions) so reads see one shape, and stating whether unknown keys inside AIContext are retained or rejected.
There was a problem hiding this comment.
The union and extensible object are defined by the pinned OSI schema. A bare string is not specified as equivalent to instructions, so normalizing it would introduce additional semantics and lose the original shape. #12386 models both forms explicitly.
I agree that the trailing ... is unclear. I’ll clarify that unknown keys in the object form are allowed and retained losslessly, matching OSI additionalProperties: true.
| Relationship and metric definitions: | ||
|
|
||
| ```text | ||
| Relationship |
There was a problem hiding this comment.
Relationship has no cardinality or join type, so fan-out and double-counting cannot be planned safely; Metric has no result type, aggregation semantics, or declared grain; Dimension carries only is_time, with no granularity, and there is no default time dimension. If these are upstream OSI limits, saying so and naming custom_extensions as the interim extension point would set expectations.
Separately, field names are unique per Dataset and metric names per model, but consumers reference both in one query namespace: worth a cross-collection uniqueness rule or a precedence definition.
There was a problem hiding this comment.
These are limitations of the pinned OSI profile. Relationship implies from as the many side and to as the one side, while the other semantics mentioned are not currently standardized by OSI. Gravitino follows this metadata model, and query planning is outside the scope of this design.
I don’t think cross-collection uniqueness belongs here. Fields are scoped to a Dataset, metrics to the SemanticModel, and any shared query namespace or precedence should be defined by the downstream query compiler.
There was a problem hiding this comment.
Fine to follow the pinned profile. The from = many, to = one convention is useful and not obvious from the schema; worth stating it in the doc so consumers do not have to infer it.
|
|
||
| - **Source of truth.** Metric Views are stored only in the Gravitino EntityStore. Logical Views remain stored by their underlying catalogs. | ||
| - **Listing.** The server merges authorized catalog-backed logical Views with authorized Gravitino-managed Metric Views into the existing View listing. | ||
| - **Connector capability.** A connector that does not support Metric Views filters them from `listViews` and returns an explicit unsupported-Metric-View error for a direct `loadView`. Generic REST and Java View APIs continue to expose them. |
There was a problem hiding this comment.
The server performs the listing, so this needs a mechanism: a client capability header, a query parameter, or connector-side filtering after fetch. Without it, Trino and Spark see View objects with empty columns.
There was a problem hiding this comment.
I consider this an implementation detail within the Gravitino connectors. The list API can expose a simple view-type filter, and the Gravitino Trino and Spark connectors can request logical Views only. Direct Metric View loads can likewise be rejected by the connector based on the representation type.
The design already defines the required behavior, so I don’t think it needs to prescribe the exact filtering mechanism.
| - **Continuous dependency maintenance.** Catalog-wide lineage, automatic revalidation after catalog changes, and transitive cycle analysis are not included. | ||
| - **Member-level authorization.** Datasets, fields, and metrics are governed as members of the enclosing Metric View rather than as independently authorized entities. | ||
|
|
||
| ## Proposed Design |
There was a problem hiding this comment.
A few sections reviewers will likely ask for: storage impact (does the whole model live in view_version_info.representations JSON, and is there a size limit, given every alter copies it into a new version?); OpenAPI updates under docs/open-api; Python client scope; audit and event listeners; feature flag; entity cache interaction (design-docs/cache-improvement-design.md). Also worth defining OSI/Ossie on first use, and folding in a short rationale for this shape versus the superseded alternative.
There was a problem hiding this comment.
The complete structured model is stored in the existing view_version_info.representations snapshot, and each alter creates a full immutable View version, as already described in the storage section. There is no Metric-View-specific size limit; it follows the existing View storage contract. The entity cache stores only the current View entity and uses the existing View invalidation path.
Python and REST are already in scope; OpenAPI updates, View events/listeners, and cache wiring follow the existing View implementation paths. I don’t think a separate feature flag is needed.
The background already explains why the structured model was chosen over opaque YAML. I agree that the first reference should define Apache Ossie as the project formerly known as Open Semantic Interchange (OSI).
There was a problem hiding this comment.
Thanks for working through all of these. The design now answers what I was asking for: explicit unknown-type rejection instead of defaultImpl fallback, TYPE_METRIC and metricRepresentation() as the discriminator, 503 vs 400 for transient catalog unavailability, the contradictory internal-reference bullet removed, AIContextObject given a concrete shape, dialects as an open string with well-known constants, and schema/catalog cascade plus Gravitino-managed storage stated clearly.
LGTM. Three small doc-only notes left inline (drop/rename during a namespace conflict, stating last-write-wins, and the from = many / to = one convention); none of them are blockers.
|
Thanks everyone for the review and discussion. After further offline discussion, we decided to introduce |
4 participants
What changes were proposed in this pull request?
Add a design document for Metric View support in Gravitino. It defines the structured OSI/Ossie-compatible representation model, lifecycle, persistence, connector behavior, validation, governance, and development plan.
Why are the changes needed?
Gravitino needs a governed and versioned metadata contract for managing metric definitions as shared semantic assets.
Fix: #12210
Does this PR introduce any user-facing change?
No. This is a design-document-only change; the proposed APIs and behavior are not implemented yet.
How was this patch tested?
./gradlew rat