Skip to content

Commit

Permalink
Tidy up metadata resolve (#527)
Browse files Browse the repository at this point in the history
<!-- Thank you for submitting this PR! :) -->

## Description

This stacks on top of hasura/v3-engine#526 and
is the tidying up step following it.

There is a lot going on here, none of it particularly interesting.

- We remove the old `resolve_metadata` function, so all of "the action"
for resolving metadata happens inside `resolved::stages::mod`.
- We move the remaining modules into two folders, `helpers` and `types`.
This is a pretty rough cut, and there are definitely better homes for
the things here, I just wanted the separation of stages and "other bits"
to be clearer.
- We split the `types` helpers into `types` and `type_mappings`, as the
`type_mappings` stuff was clearly defined and didn't belong with the
grab-bag of functions that remain in `types`.
- We explicitly export everything used outside metadata resolve, and
stop other modules dipping into it's internal structure. This will make
future changes (particularly refactors that change the stages etc) a lot
easier as we don't need the rest of the app concerning itself with it.
- I _think_ we may now be able to move metadata resolve into a separate
crate. I have not tried yet, this PR is already silly enough.

When given a choice between "fixing" something, and doing the most
mechanical obvious change, I have opted for the latter. That is to
derisk this big change, and to ensure it doesn't linger, collecting
awful merge conflicts.

Functional no-op.

V3_GIT_ORIGIN_REV_ID: 2b52403cbfddb0427b8a3e61ad2edaef9d1b3b46
  • Loading branch information
danieljharvey authored and hasura-bot committed Apr 30, 2024
1 parent 9046cc4 commit 774bf78
Show file tree
Hide file tree
Showing 74 changed files with 444 additions and 468 deletions.
2 changes: 1 addition & 1 deletion v3/crates/engine/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use thiserror::Error;
#[derive(Error, Debug)]
pub enum BuildError {
#[error("invalid metadata: {0}")]
InvalidMetadata(#[from] metadata::resolved::error::Error),
InvalidMetadata(#[from] metadata::resolved::Error),
#[error("unable to build schema: {0}")]
UnableToBuildSchema(#[from] schema::Error),
#[error("unable to encode schema: {0}")]
Expand Down
2 changes: 1 addition & 1 deletion v3/crates/engine/src/execute/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use tracing_util::{ErrorVisibility, TraceableError};
use transitive::Transitive;

use crate::execute::ndc::client as ndc_client;
use crate::metadata::resolved::{ndc_validation::NDCValidationError, subgraph::Qualified};
use crate::metadata::resolved::{NDCValidationError, Qualified};

use super::types::{Annotation, NamespaceAnnotation};

Expand Down
4 changes: 2 additions & 2 deletions v3/crates/engine/src/execute/explain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ async fn get_execution_steps<'s>(
process_response_as: &ProcessResponseAs<'s>,
join_locations: JoinLocations<(RemoteJoin<'s, '_>, JoinId)>,
ndc_request: types::NDCRequest,
data_connector: &resolved::stages::data_connectors::DataConnectorLink,
data_connector: &resolved::DataConnectorLink,
) -> NonEmpty<Box<types::Step>> {
let mut sequence_steps = match process_response_as {
ProcessResponseAs::CommandResponse { .. } => {
Expand Down Expand Up @@ -304,7 +304,7 @@ fn simplify_step(step: Box<types::Step>) -> Box<types::Step> {
async fn fetch_explain_from_data_connector(
http_context: &HttpContext,
ndc_request: &types::NDCRequest,
data_connector: &resolved::stages::data_connectors::DataConnectorLink,
data_connector: &resolved::DataConnectorLink,
) -> types::NDCExplainResponse {
let tracer = tracing_util::global_tracer();
let response = tracer
Expand Down
4 changes: 2 additions & 2 deletions v3/crates/engine/src/execute/ir/arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ use open_dds::types::{CustomTypeName, InbuiltType};

use crate::execute::error;
use crate::execute::ir::arguments;
use crate::metadata::resolved::subgraph::{
use crate::metadata::resolved::TypeMapping;
use crate::metadata::resolved::{
Qualified, QualifiedBaseType, QualifiedTypeName, QualifiedTypeReference,
};
use crate::metadata::resolved::TypeMapping;
use crate::schema::types::{
Annotation, ArgumentNameAndPath, ArgumentPresets, InputAnnotation, ModelInputAnnotation,
};
Expand Down
13 changes: 6 additions & 7 deletions v3/crates/engine/src/execute/ir/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ use crate::execute::error;
use crate::execute::ir::permissions;
use crate::execute::model_tracking::{count_command, UsagesCounts};
use crate::metadata::resolved;
use crate::metadata::resolved::subgraph;
use crate::metadata::resolved::subgraph::QualifiedTypeReference;
use crate::metadata::resolved::{Qualified, QualifiedTypeReference};
use crate::schema::types::ArgumentNameAndPath;
use crate::schema::types::ArgumentPresets;
use crate::schema::types::CommandSourceDetail;
Expand All @@ -33,13 +32,13 @@ use crate::schema::GDS;
#[derive(Serialize, Debug)]
pub struct CommandInfo<'s> {
/// The name of the command
pub command_name: subgraph::Qualified<commands::CommandName>,
pub command_name: Qualified<commands::CommandName>,

/// The name of the field as published in the schema
pub field_name: ast::Name,

/// The data connector backing this model.
pub data_connector: &'s resolved::stages::data_connectors::DataConnectorLink,
pub data_connector: &'s resolved::DataConnectorLink,

/// Arguments for the NDC table
pub(crate) arguments: BTreeMap<String, json::Value>,
Expand Down Expand Up @@ -81,7 +80,7 @@ pub struct ProcedureBasedCommand<'s> {
/// Generates the IR for a 'command' operation
#[allow(irrefutable_let_patterns)]
pub(crate) fn generate_command_info<'n, 's>(
command_name: &subgraph::Qualified<commands::CommandName>,
command_name: &Qualified<commands::CommandName>,
field: &'n normalized_ast::Field<'s, GDS>,
field_call: &'n normalized_ast::FieldCall<'s, GDS>,
result_type: &QualifiedTypeReference,
Expand Down Expand Up @@ -174,7 +173,7 @@ pub(crate) fn generate_command_info<'n, 's>(

/// Generates the IR for a 'function based command' operation
pub(crate) fn generate_function_based_command<'n, 's>(
command_name: &subgraph::Qualified<commands::CommandName>,
command_name: &Qualified<commands::CommandName>,
function_name: &'s open_dds::commands::FunctionName,
field: &'n normalized_ast::Field<'s, GDS>,
field_call: &'n normalized_ast::FieldCall<'s, GDS>,
Expand Down Expand Up @@ -202,7 +201,7 @@ pub(crate) fn generate_function_based_command<'n, 's>(

/// Generates the IR for a 'procedure based command' operation
pub(crate) fn generate_procedure_based_command<'n, 's>(
command_name: &subgraph::Qualified<commands::CommandName>,
command_name: &Qualified<commands::CommandName>,
procedure_name: &'s open_dds::commands::ProcedureName,
field: &'n normalized_ast::Field<'s, GDS>,
field_call: &'n normalized_ast::FieldCall<'s, GDS>,
Expand Down
4 changes: 2 additions & 2 deletions v3/crates/engine/src/execute/ir/model_selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ use super::selection_set;
use crate::execute::error;
use crate::execute::model_tracking::UsagesCounts;
use crate::metadata::resolved;
use crate::metadata::resolved::subgraph::Qualified;
use crate::metadata::resolved::Qualified;
use crate::schema::GDS;

/// IR fragment for any 'select' operation on a model
#[derive(Debug, Serialize)]
pub struct ModelSelection<'s> {
// The data connector backing this model.
pub data_connector: &'s resolved::stages::data_connectors::DataConnectorLink,
pub data_connector: &'s resolved::DataConnectorLink,

// Source collection in the data connector for this model
pub(crate) collection: &'s String,
Expand Down
14 changes: 6 additions & 8 deletions v3/crates/engine/src/execute/ir/permissions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ use crate::execute::error::{Error, InternalDeveloperError, InternalEngineError,
use crate::execute::model_tracking::{count_model, UsagesCounts};
use crate::metadata::resolved;

use crate::metadata::resolved::subgraph::{
QualifiedBaseType, QualifiedTypeName, QualifiedTypeReference,
};
use crate::metadata::resolved::{QualifiedBaseType, QualifiedTypeName, QualifiedTypeReference};
use crate::schema::types;
use crate::schema::GDS;

Expand Down Expand Up @@ -189,7 +187,7 @@ fn make_permission_binary_boolean_expression(
ndc_column: String,
argument_type: &QualifiedTypeReference,
operator: &str,
value_expression: &resolved::permission::ValueExpression,
value_expression: &resolved::ValueExpression,
session_variables: &SessionVariables,
relationship_paths: &Vec<NDCRelationshipName>,
) -> Result<ndc_models::Expression, Error> {
Expand Down Expand Up @@ -224,13 +222,13 @@ fn make_permission_unary_boolean_expression(
}

pub(crate) fn make_value_from_value_expression(
val_expr: &resolved::permission::ValueExpression,
val_expr: &resolved::ValueExpression,
value_type: &QualifiedTypeReference,
session_variables: &SessionVariables,
) -> Result<serde_json::Value, Error> {
match val_expr {
resolved::permission::ValueExpression::Literal(val) => Ok(val.clone()),
resolved::permission::ValueExpression::SessionVariable(session_var) => {
resolved::ValueExpression::Literal(val) => Ok(val.clone()),
resolved::ValueExpression::SessionVariable(session_var) => {
let value = session_variables.get(session_var).ok_or_else(|| {
InternalDeveloperError::MissingSessionVariable {
session_variable: session_var.clone(),
Expand All @@ -239,7 +237,7 @@ pub(crate) fn make_value_from_value_expression(

typecast_session_variable(value, value_type)
}
resolved::permission::ValueExpression::BooleanExpression(_model_predicate) => {
resolved::ValueExpression::BooleanExpression(_model_predicate) => {
Err(InternalDeveloperError::BooleanExpressionNotImplemented.into())
}
}
Expand Down
11 changes: 5 additions & 6 deletions v3/crates/engine/src/execute/ir/query_root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ use std::collections::HashMap;
use super::commands;
use super::root_field;
use crate::execute::error;
use crate::metadata::resolved::subgraph::QualifiedTypeReference;
use crate::metadata::resolved::{self, subgraph};
use crate::metadata::resolved;
use crate::schema::types::ApolloFederationRootFields;
use crate::schema::types::CommandSourceDetail;
use crate::schema::types::EntityFieldTypeNameMapping;
Expand Down Expand Up @@ -170,12 +169,12 @@ fn generate_type_field_ir<'n, 's>(
fn generate_model_rootfield_ir<'n, 's>(
type_name: &ast::TypeName,
source: &'s Option<resolved::ModelSource>,
data_type: &subgraph::Qualified<CustomTypeName>,
data_type: &resolved::Qualified<CustomTypeName>,
kind: &RootFieldKind,
field: &'n gql::normalized_ast::Field<'s, GDS>,
field_call: &'s gql::normalized_ast::FieldCall<'s, GDS>,
session: &Session,
model_name: &'s subgraph::Qualified<models::ModelName>,
model_name: &'s resolved::Qualified<models::ModelName>,
) -> Result<root_field::QueryRootField<'n, 's>, error::Error> {
let source =
source
Expand Down Expand Up @@ -212,11 +211,11 @@ fn generate_model_rootfield_ir<'n, 's>(
}

fn generate_command_rootfield_ir<'n, 's>(
name: &'s subgraph::Qualified<CommandName>,
name: &'s resolved::Qualified<CommandName>,
type_name: &ast::TypeName,
function_name: &'s Option<open_dds::commands::FunctionName>,
source: &'s Option<CommandSourceDetail>,
result_type: &'s QualifiedTypeReference,
result_type: &'s resolved::QualifiedTypeReference,
result_base_type_kind: &'s TypeKind,
field: &'n gql::normalized_ast::Field<'s, GDS>,
field_call: &'s gql::normalized_ast::FieldCall<'s, GDS>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use crate::execute::ir::filter::ResolvedFilterExpression;
use crate::execute::ir::model_selection;
use crate::execute::model_tracking::UsagesCounts;
use crate::metadata::resolved;
use crate::metadata::resolved::subgraph::Qualified;
use crate::metadata::resolved::types::mk_name;
use crate::metadata::resolved::mk_name;
use crate::metadata::resolved::Qualified;
use crate::schema::types::{EntityFieldTypeNameMapping, NamespaceAnnotation};
use crate::schema::GDS;
use crate::utils::HashMapWithJsonKey;
Expand Down
2 changes: 1 addition & 1 deletion v3/crates/engine/src/execute/ir/query_root/node_field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::execute::ir::filter::ResolvedFilterExpression;
use crate::execute::ir::model_selection;
use crate::execute::model_tracking::UsagesCounts;
use crate::metadata::resolved;
use crate::metadata::resolved::subgraph::Qualified;
use crate::metadata::resolved::Qualified;
use crate::schema::types::{GlobalID, NamespaceAnnotation, NodeFieldTypeNameMapping};
use crate::schema::GDS;
use crate::utils::HashMapWithJsonKey;
Expand Down
2 changes: 1 addition & 1 deletion v3/crates/engine/src/execute/ir/query_root/select_many.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::execute::ir::order_by::build_ndc_order_by;
use crate::execute::ir::permissions;
use crate::execute::model_tracking::{count_model, UsagesCounts};
use crate::metadata::resolved;
use crate::metadata::resolved::subgraph::Qualified;
use crate::metadata::resolved::Qualified;
use crate::schema::types::{self, Annotation, BooleanExpressionAnnotation, ModelInputAnnotation};
use crate::schema::GDS;

Expand Down
2 changes: 1 addition & 1 deletion v3/crates/engine/src/execute/ir/query_root/select_one.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::execute::ir::model_selection;
use crate::execute::ir::permissions;
use crate::execute::model_tracking::{count_model, UsagesCounts};
use crate::metadata::resolved;
use crate::metadata::resolved::subgraph::Qualified;
use crate::metadata::resolved::Qualified;

use crate::schema::types::{self, Annotation, ModelInputAnnotation};
use crate::schema::GDS;
Expand Down
18 changes: 9 additions & 9 deletions v3/crates/engine/src/execute/ir/relationship.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use super::{
};

use crate::execute::model_tracking::{count_model, UsagesCounts};
use crate::metadata::resolved::subgraph::serialize_qualified_btreemap;
use crate::metadata::resolved::{serialize_qualified_btreemap, Qualified};
use crate::schema::types::output_type::relationship::{
ModelRelationshipAnnotation, ModelTargetSource,
};
Expand All @@ -34,7 +34,7 @@ use crate::{
},
};
use crate::{
metadata::resolved::{self, subgraph::Qualified},
metadata::resolved,
schema::{
types::{Annotation, BooleanExpressionAnnotation, InputAnnotation, ModelInputAnnotation},
GDS,
Expand All @@ -46,7 +46,7 @@ pub(crate) struct LocalModelRelationshipInfo<'s> {
pub relationship_name: &'s RelationshipName,
pub relationship_type: &'s RelationshipType,
pub source_type: &'s Qualified<CustomTypeName>,
pub source_data_connector: &'s resolved::stages::data_connectors::DataConnectorLink,
pub source_data_connector: &'s resolved::DataConnectorLink,
#[serde(serialize_with = "serialize_qualified_btreemap")]
pub source_type_mappings: &'s BTreeMap<Qualified<CustomTypeName>, resolved::TypeMapping>,
pub target_source: &'s ModelTargetSource,
Expand All @@ -57,7 +57,7 @@ pub(crate) struct LocalModelRelationshipInfo<'s> {
#[derive(Debug, Serialize)]
pub(crate) struct LocalCommandRelationshipInfo<'s> {
pub annotation: &'s CommandRelationshipAnnotation,
pub source_data_connector: &'s resolved::stages::data_connectors::DataConnectorLink,
pub source_data_connector: &'s resolved::DataConnectorLink,
#[serde(serialize_with = "serialize_qualified_btreemap")]
pub source_type_mappings: &'s BTreeMap<Qualified<CustomTypeName>, resolved::TypeMapping>,
pub target_source: &'s CommandTargetSource,
Expand All @@ -80,7 +80,7 @@ pub(crate) struct RemoteCommandRelationshipInfo<'s> {
}

pub type SourceField = (FieldName, resolved::FieldMapping);
pub type TargetField = (FieldName, resolved::types::NdcColumnForComparison);
pub type TargetField = (FieldName, resolved::NdcColumnForComparison);

pub(crate) fn process_model_relationship_definition(
relationship_info: &LocalModelRelationshipInfo,
Expand Down Expand Up @@ -212,7 +212,7 @@ pub(crate) fn process_command_relationship_definition(
pub(crate) fn generate_model_relationship_ir<'s>(
field: &Field<'s, GDS>,
annotation: &'s ModelRelationshipAnnotation,
source_data_connector: &'s resolved::stages::data_connectors::DataConnectorLink,
source_data_connector: &'s resolved::DataConnectorLink,
type_mappings: &'s BTreeMap<Qualified<CustomTypeName>, resolved::TypeMapping>,
session_variables: &SessionVariables,
usage_counts: &mut UsagesCounts,
Expand Down Expand Up @@ -328,7 +328,7 @@ pub(crate) fn generate_model_relationship_ir<'s>(
pub(crate) fn generate_command_relationship_ir<'s>(
field: &Field<'s, GDS>,
annotation: &'s CommandRelationshipAnnotation,
source_data_connector: &'s resolved::stages::data_connectors::DataConnectorLink,
source_data_connector: &'s resolved::DataConnectorLink,
type_mappings: &'s BTreeMap<Qualified<CustomTypeName>, resolved::TypeMapping>,
session_variables: &SessionVariables,
usage_counts: &mut UsagesCounts,
Expand Down Expand Up @@ -382,7 +382,7 @@ pub(crate) fn build_local_model_relationship<'s>(
field: &normalized_ast::Field<'s, GDS>,
field_call: &normalized_ast::FieldCall<'s, GDS>,
annotation: &'s ModelRelationshipAnnotation,
data_connector: &'s resolved::stages::data_connectors::DataConnectorLink,
data_connector: &'s resolved::DataConnectorLink,
type_mappings: &'s BTreeMap<Qualified<CustomTypeName>, resolved::TypeMapping>,
target_source: &'s ModelTargetSource,
filter_clause: ResolvedFilterExpression<'s>,
Expand Down Expand Up @@ -428,7 +428,7 @@ pub(crate) fn build_local_command_relationship<'s>(
field: &normalized_ast::Field<'s, GDS>,
field_call: &normalized_ast::FieldCall<'s, GDS>,
annotation: &'s CommandRelationshipAnnotation,
data_connector: &'s resolved::stages::data_connectors::DataConnectorLink,
data_connector: &'s resolved::DataConnectorLink,
type_mappings: &'s BTreeMap<Qualified<CustomTypeName>, resolved::TypeMapping>,
target_source: &'s CommandTargetSource,
session_variables: &SessionVariables,
Expand Down
23 changes: 10 additions & 13 deletions v3/crates/engine/src/execute/ir/selection_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ use crate::execute::error::{self};
use crate::execute::global_id;
use crate::execute::model_tracking::UsagesCounts;
use crate::metadata::resolved;
use crate::metadata::resolved::subgraph::{
Qualified, QualifiedBaseType, QualifiedTypeName, QualifiedTypeReference,
};
use crate::schema::types::TypeKind;
use crate::schema::{
types::{Annotation, OutputAnnotation, RootFieldAnnotation},
Expand Down Expand Up @@ -87,7 +84,7 @@ pub struct NDCRelationshipName(pub(crate) String);

impl NDCRelationshipName {
pub fn new(
source_type: &Qualified<CustomTypeName>,
source_type: &resolved::Qualified<CustomTypeName>,
relationship_name: &RelationshipName,
) -> Result<Self, error::Error> {
let name = serde_json::to_string(&(source_type, relationship_name))?;
Expand Down Expand Up @@ -151,16 +148,16 @@ fn build_global_id_fields(
}

pub(crate) fn generate_nested_selection<'s>(
qualified_type_reference: &QualifiedTypeReference,
qualified_type_reference: &resolved::QualifiedTypeReference,
field_base_type_kind: TypeKind,
field: &normalized_ast::Field<'s, GDS>,
data_connector: &'s resolved::stages::data_connectors::DataConnectorLink,
type_mappings: &'s BTreeMap<Qualified<CustomTypeName>, resolved::TypeMapping>,
data_connector: &'s resolved::DataConnectorLink,
type_mappings: &'s BTreeMap<resolved::Qualified<CustomTypeName>, resolved::TypeMapping>,
session_variables: &SessionVariables,
usage_counts: &mut UsagesCounts,
) -> Result<Option<NestedSelection<'s>>, error::Error> {
match &qualified_type_reference.underlying_type {
QualifiedBaseType::List(element_type) => {
resolved::QualifiedBaseType::List(element_type) => {
let array_selection = generate_nested_selection(
element_type,
field_base_type_kind,
Expand All @@ -172,10 +169,10 @@ pub(crate) fn generate_nested_selection<'s>(
)?;
Ok(array_selection.map(|a| NestedSelection::Array(Box::new(a))))
}
QualifiedBaseType::Named(qualified_type_name) => {
resolved::QualifiedBaseType::Named(qualified_type_name) => {
match qualified_type_name {
QualifiedTypeName::Inbuilt(_) => Ok(None), // Inbuilt types are all scalars so there should be no subselections.
QualifiedTypeName::Custom(data_type) => match field_base_type_kind {
resolved::QualifiedTypeName::Inbuilt(_) => Ok(None), // Inbuilt types are all scalars so there should be no subselections.
resolved::QualifiedTypeName::Custom(data_type) => match field_base_type_kind {
TypeKind::Scalar => Ok(None),
TypeKind::Object => {
let resolved::TypeMapping::Object { field_mappings, .. } = type_mappings
Expand Down Expand Up @@ -205,8 +202,8 @@ pub(crate) fn generate_nested_selection<'s>(
/// sources depending on the model being queried.
pub(crate) fn generate_selection_set_ir<'s>(
selection_set: &normalized_ast::SelectionSet<'s, GDS>,
data_connector: &'s resolved::stages::data_connectors::DataConnectorLink,
type_mappings: &'s BTreeMap<Qualified<CustomTypeName>, resolved::TypeMapping>,
data_connector: &'s resolved::DataConnectorLink,
type_mappings: &'s BTreeMap<resolved::Qualified<CustomTypeName>, resolved::TypeMapping>,
field_mappings: &BTreeMap<FieldName, resolved::FieldMapping>,
session_variables: &SessionVariables,
usage_counts: &mut UsagesCounts,
Expand Down
Loading

0 comments on commit 774bf78

Please sign in to comment.