From 5ec39c09b81754bc55f3d5480082c2065902efa7 Mon Sep 17 00:00:00 2001 From: Gil Mizrahi Date: Mon, 1 Apr 2024 13:01:04 +0300 Subject: [PATCH] Add enum type representations (#397) ### What ndc-spec 0.1.1 introduce an [optional type representation hint](https://hasura.github.io/ndc-spec/specification/schema/scalar-types.html#type-representations). In this PR we provide type representation for enum values. ### How 1. We add a new `TypeRepresentations` field to our metadata, similar to aggregate functions and comparison operators 2. We already query for the enum types and variants, now we also return those from sql to rust and into the metadata. 3. For each scalar type, we try to look for their name in type reps, and return it if we find it. --- changelog.md | 3 + crates/configuration/src/version3/mod.rs | 28 +++++++- .../configuration/src/version3/version3.sql | 19 +++++- crates/connectors/ndc-postgres/src/schema.rs | 24 ++++++- .../metadata/src/metadata/database.rs | 50 ++++++++++++++ .../query-engine/metadata/src/metadata/mod.rs | 2 + ...schema_tests__schema_test__get_schema.snap | 9 +++ ...schema_tests__schema_test__get_schema.snap | 9 +++ ...ation_tests__get_configuration_schema.snap | 67 ++++++++++++++++++- ...tests__get_rawconfiguration_v3_schema.snap | 67 ++++++++++++++++++- ...v3_initial_configuration_is_unchanged.snap | 10 +++ ..._openapi__up_to_date_generated_schema.snap | 67 ++++++++++++++++++- ...schema_tests__schema_test__get_schema.snap | 9 +++ .../configuration.json | 7 +- .../configuration.json | 5 ++ .../configuration.json | 5 ++ .../configuration.json | 5 ++ static/schema.json | 57 +++++++++++++++- .../configuration.json | 5 ++ 19 files changed, 436 insertions(+), 12 deletions(-) diff --git a/changelog.md b/changelog.md index d779f59a6..628058015 100644 --- a/changelog.md +++ b/changelog.md @@ -4,6 +4,9 @@ ### Added +- Expose the type representation of enums via the ndc schema. + ([#397](https://github.com/hasura/ndc-postgres/pull/397)) + ### Changed ### Fixed diff --git a/crates/configuration/src/version3/mod.rs b/crates/configuration/src/version3/mod.rs index 18efd8245..cc8d04a44 100644 --- a/crates/configuration/src/version3/mod.rs +++ b/crates/configuration/src/version3/mod.rs @@ -111,7 +111,7 @@ pub async fn introspect( .instrument(info_span!("Run introspection query")) .await?; - let (tables, aggregate_functions, comparison_operators, composite_types) = async { + let (tables, aggregate_functions, comparison_operators, composite_types, type_representations) = async { let tables: metadata::TablesInfo = serde_json::from_value(row.get(0))?; let aggregate_functions: metadata::AggregateFunctions = serde_json::from_value(row.get(1))?; @@ -121,6 +121,9 @@ pub async fn introspect( let composite_types: metadata::CompositeTypes = serde_json::from_value(row.get(3))?; + let type_representations: metadata::TypeRepresentations = + serde_json::from_value(row.get(4))?; + // We need to include `in` as a comparison operator in the schema, and since it is syntax, it is not introspectable. // Instead, we will check if the scalar type defines an equals operator and if yes, we will insert the `_in` operator // as well. @@ -149,6 +152,7 @@ pub async fn introspect( aggregate_functions, metadata::ComparisonOperators(comparison_operators), composite_types, + type_representations, )) } .instrument(info_span!("Decode introspection result")) @@ -170,6 +174,8 @@ pub async fn introspect( filter_comparison_operators(&scalar_types, comparison_operators); let relevant_aggregate_functions = filter_aggregate_functions(&scalar_types, aggregate_functions); + let relevant_type_representations = + filter_type_representations(&scalar_types, type_representations); Ok(RawConfiguration { schema: args.schema, @@ -180,6 +186,7 @@ pub async fn introspect( aggregate_functions: relevant_aggregate_functions, comparison_operators: relevant_comparison_operators, composite_types, + type_representations: relevant_type_representations, }, introspection_options: args.introspection_options, mutations_version: args.mutations_version, @@ -310,7 +317,7 @@ pub fn occurring_scalar_types( .collect::>() } -/// Filter predicate for comarison operators. Preserves only comparison operators that are +/// Filter predicate for comparison operators. Preserves only comparison operators that are /// relevant to any of the given scalar types. /// /// This function is public to enable use in later versions that retain the same metadata types. @@ -351,3 +358,20 @@ fn filter_aggregate_functions( .collect(), ) } + +/// Filter predicate for type representations. Preserves only type representations that are +/// relevant to any of the given scalar types. +/// +/// This function is public to enable use in later versions that retain the same metadata types. +fn filter_type_representations( + scalar_types: &BTreeSet, + type_representations: metadata::TypeRepresentations, +) -> metadata::TypeRepresentations { + metadata::TypeRepresentations( + type_representations + .0 + .into_iter() + .filter(|(typ, _)| scalar_types.contains(typ)) + .collect(), + ) +} diff --git a/crates/configuration/src/version3/version3.sql b/crates/configuration/src/version3/version3.sql index b4f1a83aa..27cc97c2c 100644 --- a/crates/configuration/src/version3/version3.sql +++ b/crates/configuration/src/version3/version3.sql @@ -1235,7 +1235,8 @@ SELECT coalesce(tables.result, '{}'::jsonb) AS "Tables", coalesce(aggregate_functions.result, '{}'::jsonb) AS "AggregateFunctions", coalesce(comparison_functions.result, '{}'::jsonb) AS "ComparisonFunctions", - coalesce(composite_types_json.result, '{}'::jsonb) AS "CompositeTypes" + coalesce(composite_types_json.result, '{}'::jsonb) AS "CompositeTypes", + coalesce(type_representations.result, '{}'::jsonb) AS "TypeRepresentations" FROM ( SELECT @@ -1569,6 +1570,22 @@ FROM comparison_operators_by_first_arg AS op ) AS comparison_functions + + CROSS JOIN + ( + -- Type representations. + -- At the moment, we only hint at the type representation of enums. + SELECT + jsonb_object_agg( + enum_type.type_name, + jsonb_build_object( + 'enum', enum_type.enum_labels + ) + ) as result + FROM + enum_types + AS enum_type + ) AS type_representations ; -- Uncomment the following lines to just run the configuration query with reasonable default arguments diff --git a/crates/connectors/ndc-postgres/src/schema.rs b/crates/connectors/ndc-postgres/src/schema.rs index efa5797f8..1bc0258d6 100644 --- a/crates/connectors/ndc-postgres/src/schema.rs +++ b/crates/connectors/ndc-postgres/src/schema.rs @@ -31,7 +31,27 @@ pub async fn get_schema( ( scalar_type.0.clone(), models::ScalarType { - representation: None, + representation: metadata.type_representations.0.get(scalar_type).map( + |type_representation| match type_representation { + metadata::TypeRepresentation::Boolean => { + models::TypeRepresentation::Boolean + } + metadata::TypeRepresentation::Integer => { + models::TypeRepresentation::Integer + } + metadata::TypeRepresentation::Number => { + models::TypeRepresentation::Number + } + metadata::TypeRepresentation::String => { + models::TypeRepresentation::String + } + metadata::TypeRepresentation::Enum(variants) => { + models::TypeRepresentation::Enum { + one_of: variants.to_vec(), + } + } + }, + ), aggregate_functions: metadata .aggregate_functions .0 @@ -479,7 +499,7 @@ fn make_procedure_type( scalar_types .entry("int4".to_string()) .or_insert(models::ScalarType { - representation: None, + representation: Some(models::TypeRepresentation::Integer), aggregate_functions: BTreeMap::new(), comparison_operators: BTreeMap::new(), }); diff --git a/crates/query-engine/metadata/src/metadata/database.rs b/crates/query-engine/metadata/src/metadata/database.rs index 9f084835b..0d72d8502 100644 --- a/crates/query-engine/metadata/src/metadata/database.rs +++ b/crates/query-engine/metadata/src/metadata/database.rs @@ -203,3 +203,53 @@ pub struct AggregateFunctions(pub BTreeMap); + +/// Type representation of a scalar type. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub enum TypeRepresentation { + /// JSON booleans + Boolean, + /// Any JSON string + String, + /// Any JSON number + Number, + /// Any JSON number, with no decimal part + Integer, + /// One of the specified string values + Enum(Vec), +} + +// tests + +#[cfg(test)] +mod tests { + use super::{ScalarType, TypeRepresentation, TypeRepresentations}; + + #[test] + fn parse_type_representations() { + assert_eq!( + serde_json::from_str::( + r#"{"card_suit": {"enum": ["hearts", "clubs", "diamonds", "spades"]}}"# + ) + .unwrap(), + TypeRepresentations( + [( + ScalarType("card_suit".to_string()), + TypeRepresentation::Enum(vec![ + "hearts".into(), + "clubs".into(), + "diamonds".into(), + "spades".into() + ]) + )] + .into() + ) + ); + } +} diff --git a/crates/query-engine/metadata/src/metadata/mod.rs b/crates/query-engine/metadata/src/metadata/mod.rs index 5d96146dc..6d3a48f87 100644 --- a/crates/query-engine/metadata/src/metadata/mod.rs +++ b/crates/query-engine/metadata/src/metadata/mod.rs @@ -25,4 +25,6 @@ pub struct Metadata { pub aggregate_functions: AggregateFunctions, #[serde(default)] pub comparison_operators: ComparisonOperators, + #[serde(default)] + pub type_representations: TypeRepresentations, } diff --git a/crates/tests/databases-tests/src/citus/snapshots/databases_tests__citus__schema_tests__schema_test__get_schema.snap b/crates/tests/databases-tests/src/citus/snapshots/databases_tests__citus__schema_tests__schema_test__get_schema.snap index e1e463781..87f73425c 100644 --- a/crates/tests/databases-tests/src/citus/snapshots/databases_tests__citus__schema_tests__schema_test__get_schema.snap +++ b/crates/tests/databases-tests/src/citus/snapshots/databases_tests__citus__schema_tests__schema_test__get_schema.snap @@ -393,6 +393,15 @@ expression: result } }, "card_suit": { + "representation": { + "type": "enum", + "one_of": [ + "hearts", + "clubs", + "diamonds", + "spades" + ] + }, "aggregate_functions": { "max": { "result_type": { diff --git a/crates/tests/databases-tests/src/cockroach/snapshots/databases_tests__cockroach__schema_tests__schema_test__get_schema.snap b/crates/tests/databases-tests/src/cockroach/snapshots/databases_tests__cockroach__schema_tests__schema_test__get_schema.snap index d2fd3bafb..dbea7ca9b 100644 --- a/crates/tests/databases-tests/src/cockroach/snapshots/databases_tests__cockroach__schema_tests__schema_test__get_schema.snap +++ b/crates/tests/databases-tests/src/cockroach/snapshots/databases_tests__cockroach__schema_tests__schema_test__get_schema.snap @@ -70,6 +70,15 @@ expression: result } }, "card_suit": { + "representation": { + "type": "enum", + "one_of": [ + "hearts", + "clubs", + "diamonds", + "spades" + ] + }, "aggregate_functions": { "max": { "result_type": { diff --git a/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__configuration_tests__get_configuration_schema.snap b/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__configuration_tests__get_configuration_schema.snap index fe3eb72fe..e74df988e 100644 --- a/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__configuration_tests__get_configuration_schema.snap +++ b/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__configuration_tests__get_configuration_schema.snap @@ -44,7 +44,8 @@ expression: schema "compositeTypes": {}, "nativeQueries": {}, "aggregateFunctions": {}, - "comparisonOperators": {} + "comparisonOperators": {}, + "typeRepresentations": {} }, "allOf": [ { @@ -503,6 +504,14 @@ expression: schema "$ref": "#/definitions/ComparisonOperators" } ] + }, + "typeRepresentations": { + "default": {}, + "allOf": [ + { + "$ref": "#/definitions/TypeRepresentations" + } + ] } } }, @@ -967,6 +976,62 @@ expression: schema "custom" ] }, + "TypeRepresentations": { + "description": "Type representation of scalar types, grouped by type.", + "type": "object", + "additionalProperties": { + "$ref": "#/definitions/TypeRepresentation" + } + }, + "TypeRepresentation": { + "description": "Type representation of a scalar type.", + "oneOf": [ + { + "description": "JSON booleans", + "type": "string", + "enum": [ + "boolean" + ] + }, + { + "description": "Any JSON string", + "type": "string", + "enum": [ + "string" + ] + }, + { + "description": "Any JSON number", + "type": "string", + "enum": [ + "number" + ] + }, + { + "description": "Any JSON number, with no decimal part", + "type": "string", + "enum": [ + "integer" + ] + }, + { + "description": "One of the specified string values", + "type": "object", + "required": [ + "enum" + ], + "properties": { + "enum": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "additionalProperties": false + } + ] + }, "IntrospectionOptions": { "description": "Options which only influence how the configuration is updated.", "type": "object", diff --git a/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__configuration_tests__get_rawconfiguration_v3_schema.snap b/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__configuration_tests__get_rawconfiguration_v3_schema.snap index 3b0c77908..ac72f7da3 100644 --- a/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__configuration_tests__get_rawconfiguration_v3_schema.snap +++ b/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__configuration_tests__get_rawconfiguration_v3_schema.snap @@ -34,7 +34,8 @@ expression: schema "compositeTypes": {}, "nativeQueries": {}, "aggregateFunctions": {}, - "comparisonOperators": {} + "comparisonOperators": {}, + "typeRepresentations": {} }, "allOf": [ { @@ -491,6 +492,14 @@ expression: schema "$ref": "#/definitions/ComparisonOperators" } ] + }, + "typeRepresentations": { + "default": {}, + "allOf": [ + { + "$ref": "#/definitions/TypeRepresentations" + } + ] } } }, @@ -955,6 +964,62 @@ expression: schema "custom" ] }, + "TypeRepresentations": { + "description": "Type representation of scalar types, grouped by type.", + "type": "object", + "additionalProperties": { + "$ref": "#/definitions/TypeRepresentation" + } + }, + "TypeRepresentation": { + "description": "Type representation of a scalar type.", + "oneOf": [ + { + "description": "JSON booleans", + "type": "string", + "enum": [ + "boolean" + ] + }, + { + "description": "Any JSON string", + "type": "string", + "enum": [ + "string" + ] + }, + { + "description": "Any JSON number", + "type": "string", + "enum": [ + "number" + ] + }, + { + "description": "Any JSON number, with no decimal part", + "type": "string", + "enum": [ + "integer" + ] + }, + { + "description": "One of the specified string values", + "type": "object", + "required": [ + "enum" + ], + "properties": { + "enum": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "additionalProperties": false + } + ] + }, "IntrospectionOptions": { "description": "Options which only influence how the configuration is updated.", "type": "object", diff --git a/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__configuration_tests__postgres_current_only_configure_v3_initial_configuration_is_unchanged.snap b/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__configuration_tests__postgres_current_only_configure_v3_initial_configuration_is_unchanged.snap index b5df5d3f6..2a67b205d 100644 --- a/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__configuration_tests__postgres_current_only_configure_v3_initial_configuration_is_unchanged.snap +++ b/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__configuration_tests__postgres_current_only_configure_v3_initial_configuration_is_unchanged.snap @@ -2288,6 +2288,16 @@ expression: default_configuration "isInfix": false } } + }, + "typeRepresentations": { + "card_suit": { + "enum": [ + "hearts", + "clubs", + "diamonds", + "spades" + ] + } } }, "introspectionOptions": { diff --git a/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__openapi_tests__openapi__up_to_date_generated_schema.snap b/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__openapi_tests__openapi__up_to_date_generated_schema.snap index e75341467..d3da01524 100644 --- a/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__openapi_tests__openapi__up_to_date_generated_schema.snap +++ b/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__openapi_tests__openapi__up_to_date_generated_schema.snap @@ -42,7 +42,8 @@ expression: generated_schema_json "compositeTypes": {}, "nativeQueries": {}, "aggregateFunctions": {}, - "comparisonOperators": {} + "comparisonOperators": {}, + "typeRepresentations": {} }, "allOf": [ { @@ -495,6 +496,14 @@ expression: generated_schema_json "$ref": "#/components/schemas/ComparisonOperators" } ] + }, + "typeRepresentations": { + "default": {}, + "allOf": [ + { + "$ref": "#/components/schemas/TypeRepresentations" + } + ] } } }, @@ -945,6 +954,62 @@ expression: generated_schema_json "custom" ] }, + "TypeRepresentations": { + "description": "Type representation of scalar types, grouped by type.", + "type": "object", + "additionalProperties": { + "$ref": "#/components/schemas/TypeRepresentation" + } + }, + "TypeRepresentation": { + "description": "Type representation of a scalar type.", + "oneOf": [ + { + "description": "JSON booleans", + "type": "string", + "enum": [ + "boolean" + ] + }, + { + "description": "Any JSON string", + "type": "string", + "enum": [ + "string" + ] + }, + { + "description": "Any JSON number", + "type": "string", + "enum": [ + "number" + ] + }, + { + "description": "Any JSON number, with no decimal part", + "type": "string", + "enum": [ + "integer" + ] + }, + { + "description": "One of the specified string values", + "type": "object", + "required": [ + "enum" + ], + "properties": { + "enum": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "additionalProperties": false + } + ] + }, "IntrospectionOptions": { "description": "Options which only influence how the configuration is updated.", "type": "object", diff --git a/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__schema_tests__schema_test__get_schema.snap b/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__schema_tests__schema_test__get_schema.snap index 7ca0de9da..338e50bfe 100644 --- a/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__schema_tests__schema_test__get_schema.snap +++ b/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__schema_tests__schema_test__get_schema.snap @@ -449,6 +449,15 @@ expression: result } }, "card_suit": { + "representation": { + "type": "enum", + "one_of": [ + "hearts", + "clubs", + "diamonds", + "spades" + ] + }, "aggregate_functions": { "max": { "result_type": { diff --git a/static/aurora/v3-chinook-ndc-metadata/configuration.json b/static/aurora/v3-chinook-ndc-metadata/configuration.json index 8c8bfb650..3983a0d3a 100644 --- a/static/aurora/v3-chinook-ndc-metadata/configuration.json +++ b/static/aurora/v3-chinook-ndc-metadata/configuration.json @@ -1822,10 +1822,10 @@ }, "varchar": { "max": { - "returnType": "bpchar" + "returnType": "varchar" }, "min": { - "returnType": "bpchar" + "returnType": "varchar" } } }, @@ -2714,7 +2714,8 @@ "isInfix": false } } - } + }, + "typeRepresentations": {} }, "introspectionOptions": { "excludedSchemas": [ diff --git a/static/citus/v3-chinook-ndc-metadata/configuration.json b/static/citus/v3-chinook-ndc-metadata/configuration.json index 3f17e0281..4bf79a297 100644 --- a/static/citus/v3-chinook-ndc-metadata/configuration.json +++ b/static/citus/v3-chinook-ndc-metadata/configuration.json @@ -3293,6 +3293,11 @@ "isInfix": false } } + }, + "typeRepresentations": { + "card_suit": { + "enum": ["hearts", "clubs", "diamonds", "spades"] + } } }, "introspectionOptions": { diff --git a/static/cockroach/v3-chinook-ndc-metadata/configuration.json b/static/cockroach/v3-chinook-ndc-metadata/configuration.json index edb8e36ba..29b2c656a 100644 --- a/static/cockroach/v3-chinook-ndc-metadata/configuration.json +++ b/static/cockroach/v3-chinook-ndc-metadata/configuration.json @@ -2796,6 +2796,11 @@ "isInfix": false } } + }, + "typeRepresentations": { + "card_suit": { + "enum": ["hearts", "clubs", "diamonds", "spades"] + } } }, "introspectionOptions": { diff --git a/static/postgres/v3-chinook-ndc-metadata/configuration.json b/static/postgres/v3-chinook-ndc-metadata/configuration.json index 8fdcf94c5..cfa452ca9 100644 --- a/static/postgres/v3-chinook-ndc-metadata/configuration.json +++ b/static/postgres/v3-chinook-ndc-metadata/configuration.json @@ -3670,6 +3670,11 @@ "isInfix": false } } + }, + "typeRepresentations": { + "card_suit": { + "enum": ["hearts", "clubs", "diamonds", "spades"] + } } }, "introspectionOptions": { diff --git a/static/schema.json b/static/schema.json index 4e6b2f1af..7b0326282 100644 --- a/static/schema.json +++ b/static/schema.json @@ -32,7 +32,8 @@ "compositeTypes": {}, "nativeQueries": {}, "aggregateFunctions": {}, - "comparisonOperators": {} + "comparisonOperators": {}, + "typeRepresentations": {} }, "allOf": [ { @@ -473,6 +474,14 @@ "$ref": "#/definitions/ComparisonOperators" } ] + }, + "typeRepresentations": { + "default": {}, + "allOf": [ + { + "$ref": "#/definitions/TypeRepresentations" + } + ] } } }, @@ -861,6 +870,52 @@ "type": "string", "enum": ["equal", "in", "custom"] }, + "TypeRepresentations": { + "description": "Type representation of scalar types, grouped by type.", + "type": "object", + "additionalProperties": { + "$ref": "#/definitions/TypeRepresentation" + } + }, + "TypeRepresentation": { + "description": "Type representation of a scalar type.", + "oneOf": [ + { + "description": "JSON booleans", + "type": "string", + "enum": ["boolean"] + }, + { + "description": "Any JSON string", + "type": "string", + "enum": ["string"] + }, + { + "description": "Any JSON number", + "type": "string", + "enum": ["number"] + }, + { + "description": "Any JSON number, with no decimal part", + "type": "string", + "enum": ["integer"] + }, + { + "description": "One of the specified string values", + "type": "object", + "required": ["enum"], + "properties": { + "enum": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "additionalProperties": false + } + ] + }, "IntrospectionOptions": { "description": "Options which only influence how the configuration is updated.", "type": "object", diff --git a/static/yugabyte/v3-chinook-ndc-metadata/configuration.json b/static/yugabyte/v3-chinook-ndc-metadata/configuration.json index dca66d383..1c8eed60b 100644 --- a/static/yugabyte/v3-chinook-ndc-metadata/configuration.json +++ b/static/yugabyte/v3-chinook-ndc-metadata/configuration.json @@ -3252,6 +3252,11 @@ "isInfix": false } } + }, + "typeRepresentations": { + "card_suit": { + "enum": ["hearts", "clubs", "diamonds", "spades"] + } } }, "introspectionOptions": {