Use newtypes for operator names (#672)

<!-- Thank you for submitting this PR! :) -->

## Description

We were using `String` for these, and making some assumptions about NDCs
and OpenDD operator names being the same. This uses newtypes and
documents where these assumptions are being made. Functional no-op.

V3_GIT_ORIGIN_REV_ID: 195e55db6fadf48c956684130e23f647eaa17fb1
This commit is contained in:
Daniel Harvey 2024-06-05 15:45:17 +01:00 committed by hasura-bot
parent ca830c05fb
commit b314d89ff0
6 changed files with 33 additions and 20 deletions

View File

@ -10,7 +10,7 @@ use serde::Serialize;
use crate::ir::error;
use crate::model_tracking::{count_model, UsagesCounts};
use open_dds::{
data_connector::DataConnectorColumnName,
data_connector::{DataConnectorColumnName, DataConnectorOperatorName},
types::{CustomTypeName, FieldName},
};
use schema::FilterRelationshipAnnotation;
@ -133,7 +133,9 @@ fn build_filter_expression<'s>(
)) => {
let FieldMapping { column, .. } =
get_field_mapping_of_field_name(type_mappings, object_type, field_name)?;
let mut expressions = Vec::new();
for (_op_name, op_value) in field.value.as_object()? {
match op_value.info.generic {
schema::Annotation::Input(InputAnnotation::Model(
@ -271,7 +273,7 @@ fn resolve_filter_object<'s>(
/// Generate a binary comparison operator
fn build_binary_comparison_expression(
operator: &str,
operator: &DataConnectorOperatorName,
column: DataConnectorColumnName,
value: &normalized_ast::Value<'_, GDS>,
) -> ndc_models::Expression {
@ -281,7 +283,7 @@ fn build_binary_comparison_expression(
path: Vec::new(),
field_path: None,
},
operator: operator.to_string(),
operator: operator.0.clone(),
value: ndc_models::ComparisonValue::Scalar {
value: value.as_json(),
},

View File

@ -4,7 +4,10 @@ use hasura_authn_core::{SessionVariableValue, SessionVariables};
use lang_graphql::normalized_ast;
use ndc_models;
use open_dds::{data_connector::DataConnectorColumnName, types::InbuiltType};
use open_dds::{
data_connector::{DataConnectorColumnName, DataConnectorOperatorName},
types::InbuiltType,
};
use crate::ir::error;
use crate::model_tracking::{count_model, UsagesCounts};
@ -167,7 +170,7 @@ pub(crate) fn process_model_predicate<'s>(
fn make_permission_binary_boolean_expression(
ndc_column: DataConnectorColumnName,
argument_type: &QualifiedTypeReference,
operator: &str,
operator: &DataConnectorOperatorName,
value_expression: &metadata_resolve::ValueExpression,
session_variables: &SessionVariables,
usage_counts: &mut UsagesCounts,
@ -184,7 +187,7 @@ fn make_permission_binary_boolean_expression(
path: Vec::new(),
field_path: None,
},
operator: operator.to_owned(),
operator: operator.0.clone(),
value: ndc_models::ComparisonValue::Scalar {
value: ndc_expression_value,
},

View File

@ -18,8 +18,9 @@ use crate::types::subgraph::{ArgumentInfo, Qualified, QualifiedBaseType, Qualifi
use indexmap::IndexMap;
use ndc_models;
use open_dds::arguments::ArgumentName;
use open_dds::data_connector::DataConnectorName;
use open_dds::data_connector::DataConnectorObjectType;
use open_dds::data_connector::{
DataConnectorName, DataConnectorObjectType, DataConnectorOperatorName,
};
use open_dds::models::ModelName;
use open_dds::permissions;
use open_dds::types::{CustomTypeName, FieldName, OperatorName};
@ -672,7 +673,7 @@ fn resolve_binary_operator_for_type(
scalars: &data_connector_scalar_types::ScalarTypeWithRepresentationInfoMap,
ndc_scalar_type: &ndc_models::ScalarType,
subgraph: &str,
) -> Result<(String, QualifiedTypeReference), Error> {
) -> Result<(DataConnectorOperatorName, QualifiedTypeReference), Error> {
let field_definition = fields
.get(field_name)
.ok_or_else(|| Error::TypePredicateError {
@ -681,6 +682,7 @@ fn resolve_binary_operator_for_type(
type_name: type_name.clone(),
},
})?;
let comparison_operator_definition = &ndc_scalar_type
.comparison_operators
.get(&operator.0)
@ -690,12 +692,14 @@ fn resolve_binary_operator_for_type(
operator_name: operator.clone(),
},
})?;
match comparison_operator_definition {
ndc_models::ComparisonOperatorDefinition::Equal => {
Ok((operator.0.clone(), field_definition.field_type.clone()))
}
ndc_models::ComparisonOperatorDefinition::Equal => Ok((
DataConnectorOperatorName(operator.to_string()),
field_definition.field_type.clone(),
)),
ndc_models::ComparisonOperatorDefinition::In => Ok((
operator.0.clone(),
DataConnectorOperatorName(operator.to_string()),
QualifiedTypeReference {
underlying_type: QualifiedBaseType::List(Box::new(
field_definition.field_type.clone(),
@ -704,7 +708,7 @@ fn resolve_binary_operator_for_type(
},
)),
ndc_models::ComparisonOperatorDefinition::Custom { argument_type } => Ok((
operator.0.clone(),
DataConnectorOperatorName(operator.to_string()),
resolve_ndc_type(data_connector, argument_type, scalars, subgraph)?,
)),
}

View File

@ -3,7 +3,7 @@ use crate::types::error::{Error, RelationshipError};
use crate::types::permission::ValueExpression;
use crate::types::subgraph::{deserialize_qualified_btreemap, serialize_qualified_btreemap};
use open_dds::{
data_connector::DataConnectorColumnName,
data_connector::{DataConnectorColumnName, DataConnectorOperatorName},
models::ModelName,
relationships::{RelationshipName, RelationshipType},
types::CustomTypeName,
@ -47,7 +47,7 @@ pub enum ModelPredicate {
BinaryFieldComparison {
field: FieldName,
ndc_column: DataConnectorColumnName,
operator: String,
operator: DataConnectorOperatorName,
argument_type: QualifiedTypeReference,
value: ValueExpression,
},

View File

@ -1,6 +1,6 @@
use lang_graphql::ast::common as ast;
use lang_graphql::schema::{self as gql_schema, InputField, Namespaced};
use open_dds::types::CustomTypeName;
use open_dds::{data_connector::DataConnectorOperatorName, types::CustomTypeName};
use std::collections::BTreeMap;
use super::types::input_type;
@ -79,6 +79,10 @@ pub fn build_scalar_comparison_input(
nullable: true,
};
// this feels a bit loose, we're depending on the fact the ast::Name and
// DataConnectorOperatorName should be the same
let ndc_operator_name = DataConnectorOperatorName(op_name.to_string());
input_fields.insert(
op_name.clone(),
builder.allow_all_namespaced(
@ -87,7 +91,7 @@ pub fn build_scalar_comparison_input(
None,
types::Annotation::Input(types::InputAnnotation::Model(
types::ModelInputAnnotation::ComparisonOperation {
operator: op_name.to_string(),
operator: ndc_operator_name,
},
)),
nullable_input_type,

View File

@ -12,7 +12,7 @@ use std::{
use open_dds::{
arguments::ArgumentName,
commands,
data_connector::DataConnectorColumnName,
data_connector::{DataConnectorColumnName, DataConnectorOperatorName},
models,
types::{self},
};
@ -197,7 +197,7 @@ pub enum ModelInputAnnotation {
ndc_table_argument: Option<ConnectorArgumentName>,
},
ComparisonOperation {
operator: String,
operator: DataConnectorOperatorName,
},
IsNullOperation,
ModelOrderByExpression,