Skip to content

Add enum type representations #397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 1, 2024
Merged

Add enum type representations #397

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c393486
enum type representations
Mar 29, 2024
90f40b3
changelog
Mar 29, 2024
76fc33c
Merge branch 'main' into gil/enum-type-reps
Apr 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 26 additions & 2 deletions crates/configuration/src/version3/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))?;
Expand All @@ -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.
Expand Down Expand Up @@ -149,6 +152,7 @@ pub async fn introspect(
aggregate_functions,
metadata::ComparisonOperators(comparison_operators),
composite_types,
type_representations,
))
}
.instrument(info_span!("Decode introspection result"))
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -310,7 +317,7 @@ pub fn occurring_scalar_types(
.collect::<BTreeSet<metadata::ScalarType>>()
}

/// 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.
Expand Down Expand Up @@ -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<metadata::ScalarType>,
type_representations: metadata::TypeRepresentations,
) -> metadata::TypeRepresentations {
metadata::TypeRepresentations(
type_representations
.0
.into_iter()
.filter(|(typ, _)| scalar_types.contains(typ))
.collect(),
)
}
19 changes: 18 additions & 1 deletion crates/configuration/src/version3/version3.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
24 changes: 22 additions & 2 deletions crates/connectors/ndc-postgres/src/schema.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really something of an edge case. I wonder if we actually need to register int4, as in, I wonder if there actually is a functional difference between "scalarTypes":{"int4": {"aggregateFunctions": {}, "comparisonOperators": {}} and "scalarTypes": {}.

At any rate, it is a curious detail that this edge case gives a different result than the common case where int4 is defined, but we don't yet assign type representations to anything but enums.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we definitely need it. I think I added it because a test failed. Try and see :)

The comments about this are not visible in this diff by default, adding it here:

    // If int4 doesn't exist anywhere else in the schema, we need to add it here. However, a user
    // can't filter or aggregate based on the affected rows of a procedure, so we don't need to add
    // any aggregate functions or comparison operators. However, if int4 exists elsewhere in the
    // schema and has already been added, it will also already contain these functions and
    // operators.

aggregate_functions: BTreeMap::new(),
comparison_operators: BTreeMap::new(),
});
Expand Down
50 changes: 50 additions & 0 deletions crates/query-engine/metadata/src/metadata/database.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,3 +203,53 @@ pub struct AggregateFunctions(pub BTreeMap<ScalarType, BTreeMap<String, Aggregat
pub struct AggregateFunction {
pub return_type: ScalarType,
}

/// Type representation of scalar types, grouped by type.
#[derive(Debug, Clone, PartialEq, Eq, Default, Serialize, Deserialize, JsonSchema)]
#[serde(rename_all = "camelCase")]
pub struct TypeRepresentations(pub BTreeMap<ScalarType, TypeRepresentation>);

/// 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<String>),
}

// tests

#[cfg(test)]
mod tests {
use super::{ScalarType, TypeRepresentation, TypeRepresentations};

#[test]
fn parse_type_representations() {
assert_eq!(
serde_json::from_str::<TypeRepresentations>(
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()
)
);
}
}
2 changes: 2 additions & 0 deletions crates/query-engine/metadata/src/metadata/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,6 @@ pub struct Metadata {
pub aggregate_functions: AggregateFunctions,
#[serde(default)]
pub comparison_operators: ComparisonOperators,
#[serde(default)]
pub type_representations: TypeRepresentations,
}
9 changes: 9 additions & 0 deletions ...ts/src/citus/snapshots/databases_tests__citus__schema_tests__schema_test__get_schema.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,15 @@ expression: result
}
},
"card_suit": {
"representation": {
"type": "enum",
"one_of": [
"hearts",
"clubs",
"diamonds",
"spades"
]
},
"aggregate_functions": {
"max": {
"result_type": {
Expand Down
9 changes: 9 additions & 0 deletions ...ockroach/snapshots/databases_tests__cockroach__schema_tests__schema_test__get_schema.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,15 @@ expression: result
}
},
"card_suit": {
"representation": {
"type": "enum",
"one_of": [
"hearts",
"clubs",
"diamonds",
"spades"
]
},
"aggregate_functions": {
"max": {
"result_type": {
Expand Down
67 changes: 66 additions & 1 deletion ...s/snapshots/databases_tests__postgres__configuration_tests__get_configuration_schema.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ expression: schema
"compositeTypes": {},
"nativeQueries": {},
"aggregateFunctions": {},
"comparisonOperators": {}
"comparisonOperators": {},
"typeRepresentations": {}
},
"allOf": [
{
Expand Down Expand Up @@ -503,6 +504,14 @@ expression: schema
"$ref": "#/definitions/ComparisonOperators"
}
]
},
"typeRepresentations": {
"default": {},
"allOf": [
{
"$ref": "#/definitions/TypeRepresentations"
}
]
}
}
},
Expand Down Expand Up @@ -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",
Expand Down
Loading