jerryshao commented on code in PR #14210: URL: https://github.com/apache/gravitino/pull/14210#discussion_r4412706249
########## docs/manage-semantic-model-metadata-using-gravitino.md: ########## @@ -326,6 +327,144 @@ loaded = models.load_semantic_model(ident) </TabItem> </Tabs> +### Discover Metrics in a Schema + +Use schema-level metric discovery to find metrics across the current definitions of Semantic +Models in one schema. You can list identifiers or retrieve complete metric definitions without +first listing and loading each model. These examples use the `total_revenue` metric in +`mart.sales_model` created above, before importing any other models. + +#### List Metric Identifiers + +<Tabs groupId='language' queryString> +<TabItem value="shell" label="REST"> + +```shell +curl -X GET -H "Accept: application/vnd.gravitino.v1+json" \ + 'http://localhost:8090/api/metalakes/example/catalogs/sales/schemas/mart/semantic-model-metrics' +``` + +The default is `details=false`. If `sales_model` is the only model with metrics in this schema, +the response is: Review Comment: [Question] The page states the default but not what counts as enabling the flag. The resource declares a primitive `@QueryParam("details") @DefaultValue("false") boolean details` (`server/src/main/java/org/apache/gravitino/server/web/rest/SemanticModelMetricOperations.java:94`) with no explicit validation in the handler, and JAX-RS converts a primitive boolean with `Boolean.valueOf`. If that is right, `?details=1` or `?details=yes` silently returns the identifier list instead of definitions rather than failing - an easy mistake for someone translating the curl example into a client. Would it be worth one clause here, e.g. that only the literal value `true` (case-insensitive) selects complete definitions? I am phrasing this as a question because nothing pins the behavior: `TestSemanticModelMetricOperations` only ever passes real booleans through its helper (`server/src/test/java/org/apache/gravitino/server/web/rest/TestSemanticModelMetricOperations.java:315-317`). Verified by: read the resource method signature and the full handler body for value validation, then the test helper that builds the request. ########## docs/manage-semantic-model-metadata-using-gravitino.md: ########## @@ -326,6 +327,144 @@ loaded = models.load_semantic_model(ident) </TabItem> </Tabs> +### Discover Metrics in a Schema + +Use schema-level metric discovery to find metrics across the current definitions of Semantic +Models in one schema. You can list identifiers or retrieve complete metric definitions without +first listing and loading each model. These examples use the `total_revenue` metric in +`mart.sales_model` created above, before importing any other models. + +#### List Metric Identifiers + +<Tabs groupId='language' queryString> +<TabItem value="shell" label="REST"> + +```shell +curl -X GET -H "Accept: application/vnd.gravitino.v1+json" \ + 'http://localhost:8090/api/metalakes/example/catalogs/sales/schemas/mart/semantic-model-metrics' +``` + +The default is `details=false`. If `sales_model` is the only model with metrics in this schema, +the response is: + +```json +{ + "code": 0, + "identifiers": [ + { + "namespace": ["example", "sales", "mart", "sales_model"], + "name": "total_revenue" + } + ] +} +``` + +</TabItem> +<TabItem value="java" label="Java"> + +```java +NameIdentifier[] metricIdentifiers = models.listMetrics(Namespace.of("mart")); +``` + +</TabItem> +<TabItem value="python" label="Python"> + +```python +metric_identifiers = models.list_metrics(Namespace.of("mart")) +``` + +</TabItem> +</Tabs> + +Metric names are unique within a model. Two models can both define `total_revenue`; use the +namespace together with the metric name to distinguish them. + +| Interface | Namespace in each result | Metric name | +|-----------|--------------------------|-------------| +| REST | `["example", "sales", "mart", "sales_model"]` | `total_revenue` | +| Java / Python, using the loaded `sales` catalog | `["mart", "sales_model"]` | `total_revenue` | + +The SDK methods accept a schema namespace, such as `Namespace.of("mart")`. Do not include the +metalake, catalog, or model in that argument. + +#### Retrieve Metric Definitions + +<Tabs groupId='language' queryString> +<TabItem value="shell" label="REST"> + +```shell +curl -X GET -H "Accept: application/vnd.gravitino.v1+json" \ + 'http://localhost:8090/api/metalakes/example/catalogs/sales/schemas/mart/semantic-model-metrics?details=true' +``` + +For the same model, the response is: + +```json +{ + "code": 0, + "metrics": [ + { + "namespace": ["example", "sales", "mart", "sales_model"], + "metric": { + "name": "total_revenue", + "expression": { + "dialects": [ + {"dialect": "ANSI_SQL", "expression": "SUM(orders.order_amount)"} + ] + }, + "datatype": "Decimal" + } + } + ] +} +``` + +</TabItem> +<TabItem value="java" label="Java"> + +```java +SemanticModelMetric[] metricInfos = models.listMetricInfos(Namespace.of("mart")); +``` + +</TabItem> +<TabItem value="python" label="Python"> + +```python +metric_infos = models.list_metric_infos(Namespace.of("mart")) +``` + +</TabItem> +</Tabs> + +Each SDK result exposes `namespace()` and `metric()`. The metric contains its name, expression, +and any stored data type, description, AI context, or custom extensions. To construct its identifier, +use `NameIdentifier.of(info.namespace(), info.metric().name())` in Java or +`NameIdentifier.builder(info.namespace(), info.metric().name())` in Python. + +#### Ordering, Visibility, and Updates + +Both listing modes sort results by the containing model's identifier and then by case-sensitive +metric name. Models without metrics contribute no entries. If there are no visible metrics, REST +returns an empty `identifiers` or `metrics` array and the SDKs return an empty array or list. + +With authorization enabled, discovery requires access to the parent catalog and schema. It +includes metrics only from models the caller can discover through `USE_SEMANTIC_MODEL`, +`MODIFY_SEMANTIC_MODEL`, or ownership, as described in +[Ownership and Privileges](#ownership-and-privileges). Discovery does not revalidate access to +source tables or views and does not grant access to source data. + +Discovery reflects each model's current committed definition: renaming a model changes the +namespace returned for its metrics, replacing its definition changes the discovered metrics, +and dropping it removes its metrics from discovery. Manage metric definitions through their +containing Semantic Models; discovery does not provide a separate metric lifecycle or execute +metric expressions. + +An empty result is a successful discovery response. Missing schemas, denied parent access, and +unsupported operations are reported as errors. Storage or decoding failures fail the request Review Comment: [Question] "unsupported operations are reported as errors" is the only hint that discovery is relational-only. The endpoint's own OpenAPI description is explicit - "Supported only for relational catalogs" (`docs/open-api/semantic-models.yaml:33`) with a dedicated `501` response (`docs/open-api/semantic-models.yaml:59`) - and the SDK base interfaces raise `UnsupportedOperationException` with "Metric listing is not supported" / "Metric info listing is not supported" for catalogs that do not implement it (`clients/client-python/tests/unittests/api/semantic/test_semantic_model_catalog.py:97-103`). "Before You Start" already assumes a relational catalog named `sales` (line 23), so this may well be deliberate. If it is not, naming the restriction and the `501` here would save a reader on a fileset or model catalog a round trip, and would match how `listSemanticModels` is documented in the same spec file. Verified by: compared this paragraph against the endpoint's OpenAPI description and responses, and against the catalog-capability unit test. ########## docs/manage-semantic-model-metadata-using-gravitino.md: ########## @@ -326,6 +327,144 @@ loaded = models.load_semantic_model(ident) </TabItem> </Tabs> +### Discover Metrics in a Schema + +Use schema-level metric discovery to find metrics across the current definitions of Semantic +Models in one schema. You can list identifiers or retrieve complete metric definitions without +first listing and loading each model. These examples use the `total_revenue` metric in +`mart.sales_model` created above, before importing any other models. + +#### List Metric Identifiers + +<Tabs groupId='language' queryString> +<TabItem value="shell" label="REST"> + +```shell +curl -X GET -H "Accept: application/vnd.gravitino.v1+json" \ + 'http://localhost:8090/api/metalakes/example/catalogs/sales/schemas/mart/semantic-model-metrics' +``` + +The default is `details=false`. If `sales_model` is the only model with metrics in this schema, +the response is: + +```json +{ + "code": 0, + "identifiers": [ + { + "namespace": ["example", "sales", "mart", "sales_model"], + "name": "total_revenue" + } + ] +} +``` + +</TabItem> +<TabItem value="java" label="Java"> + +```java +NameIdentifier[] metricIdentifiers = models.listMetrics(Namespace.of("mart")); +``` + +</TabItem> +<TabItem value="python" label="Python"> + +```python +metric_identifiers = models.list_metrics(Namespace.of("mart")) +``` + +</TabItem> +</Tabs> + +Metric names are unique within a model. Two models can both define `total_revenue`; use the +namespace together with the metric name to distinguish them. + +| Interface | Namespace in each result | Metric name | +|-----------|--------------------------|-------------| +| REST | `["example", "sales", "mart", "sales_model"]` | `total_revenue` | +| Java / Python, using the loaded `sales` catalog | `["mart", "sales_model"]` | `total_revenue` | + +The SDK methods accept a schema namespace, such as `Namespace.of("mart")`. Do not include the +metalake, catalog, or model in that argument. + +#### Retrieve Metric Definitions + +<Tabs groupId='language' queryString> +<TabItem value="shell" label="REST"> + +```shell +curl -X GET -H "Accept: application/vnd.gravitino.v1+json" \ + 'http://localhost:8090/api/metalakes/example/catalogs/sales/schemas/mart/semantic-model-metrics?details=true' +``` + +For the same model, the response is: + +```json +{ + "code": 0, + "metrics": [ + { + "namespace": ["example", "sales", "mart", "sales_model"], + "metric": { + "name": "total_revenue", + "expression": { + "dialects": [ + {"dialect": "ANSI_SQL", "expression": "SUM(orders.order_amount)"} + ] + }, + "datatype": "Decimal" + } + } + ] +} +``` + +</TabItem> +<TabItem value="java" label="Java"> + +```java +SemanticModelMetric[] metricInfos = models.listMetricInfos(Namespace.of("mart")); +``` + +</TabItem> +<TabItem value="python" label="Python"> + +```python +metric_infos = models.list_metric_infos(Namespace.of("mart")) +``` + +</TabItem> +</Tabs> + +Each SDK result exposes `namespace()` and `metric()`. The metric contains its name, expression, +and any stored data type, description, AI context, or custom extensions. To construct its identifier, +use `NameIdentifier.of(info.namespace(), info.metric().name())` in Java or +`NameIdentifier.builder(info.namespace(), info.metric().name())` in Python. + +#### Ordering, Visibility, and Updates + +Both listing modes sort results by the containing model's identifier and then by case-sensitive +metric name. Models without metrics contribute no entries. If there are no visible metrics, REST +returns an empty `identifiers` or `metrics` array and the SDKs return an empty array or list. Review Comment: [Nit] This paragraph covers ordering, empty results and visibility, but not response size. The endpoint has no pagination: `listMetrics` declares `details` as its only query parameter (`server/src/main/java/org/apache/gravitino/server/web/rest/SemanticModelMetricOperations.java:82-94`), and the listing SQL joins every live model in the schema with no limit or offset (`core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SemanticModelMetricBaseSQLProvider.java:140-151`). Since `docs/semantic-models.md:61-62` (added by this PR) points analytical applications and AI tools at this API, one sentence saying a single response carries all visible metrics of the whole schema - and that `details=true` carries their complete definitions - would set the right expectation for a schema with many models. Verified by: read the resource method's parameter list and the mapper SQL; there is no page, limit or offset parameter on either path. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
