这是indexloc提供的服务，不要输入任何密码
Skip to content

Use field type in comparison target instead of column type #82

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 2 commits into from
Jun 19, 2024
Merged

Use field type in comparison target instead of column type #82

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f269bef
Use field type in comparison target instead of column type
dmoverton Jun 19, 2024
b56e6e7
Add changelog
dmoverton Jun 19, 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
2 changes: 2 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,8 @@ This changelog documents the changes between release versions.

## [Unreleased]

- Fix bug with operator lookup when filtering on nested fields ([#82](https://github.com/hasura/ndc-mongodb/pull/82))

## [0.1.0] - 2024-06-13

- Support filtering and sorting by fields of related collections ([#72](https://github.com/hasura/ndc-mongodb/pull/72))
Expand Down
20 changes: 10 additions & 10 deletions crates/mongodb-agent-common/src/query/column_ref.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ mod tests {
let target = ComparisonTarget::Column {
name: "imdb".into(),
field_path: Some(vec!["rating".into()]),
column_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::Double)),
field_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::Double)),
path: Default::default(),
};
let actual = ColumnRef::from_comparison_target(&target);
Expand All @@ -177,7 +177,7 @@ mod tests {
let target = ComparisonTarget::Column {
name: "subtitles".into(),
field_path: Some(vec!["english.us".into()]),
column_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
field_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
path: Default::default(),
};
let actual = ColumnRef::from_comparison_target(&target);
Expand All @@ -199,7 +199,7 @@ mod tests {
let target = ComparisonTarget::Column {
name: "meta.subtitles".into(),
field_path: Some(vec!["english_us".into()]),
column_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
field_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
path: Default::default(),
};
let actual = ColumnRef::from_comparison_target(&target);
Expand All @@ -221,7 +221,7 @@ mod tests {
let target = ComparisonTarget::Column {
name: "meta".into(),
field_path: Some(vec!["$unsafe".into(), "$also_unsafe".into()]),
column_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
field_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
path: Default::default(),
};
let actual = ColumnRef::from_comparison_target(&target);
Expand All @@ -248,7 +248,7 @@ mod tests {
let target = ComparisonTarget::Column {
name: "valid_key".into(),
field_path: Some(vec!["also_valid".into(), "$not_valid".into()]),
column_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
field_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
path: Default::default(),
};
let actual = ColumnRef::from_comparison_target(&target);
Expand All @@ -270,7 +270,7 @@ mod tests {
let target = ComparisonTarget::ColumnInScope {
name: "field".into(),
field_path: Some(vec!["prop1".into(), "prop2".into()]),
column_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
field_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
scope: Scope::Root,
};
let actual = ColumnRef::from_comparison_target(&target);
Expand All @@ -284,7 +284,7 @@ mod tests {
let target = ComparisonTarget::ColumnInScope {
name: "$field".into(),
field_path: Default::default(),
column_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
field_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
scope: Scope::Named("scope_0".into()),
};
let actual = ColumnRef::from_comparison_target(&target);
Expand All @@ -306,7 +306,7 @@ mod tests {
let target = ComparisonTarget::ColumnInScope {
name: "field".into(),
field_path: Some(vec!["$unsafe_name".into()]),
column_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
field_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
scope: Scope::Root,
};
let actual = ColumnRef::from_comparison_target(&target);
Expand All @@ -329,7 +329,7 @@ mod tests {
let target = ComparisonTarget::ColumnInScope {
name: "$field".into(),
field_path: Some(vec!["$unsafe_name1".into(), "$unsafe_name2".into()]),
column_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
field_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
scope: Scope::Root,
};
let actual = ColumnRef::from_comparison_target(&target);
Expand Down Expand Up @@ -361,7 +361,7 @@ mod tests {
let target = ComparisonTarget::ColumnInScope {
name: "field".into(),
field_path: Some(vec!["prop1".into(), "$unsafe_name".into()]),
column_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
field_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
scope: Scope::Root,
};
let actual = ColumnRef::from_comparison_target(&target);
Expand Down
8 changes: 4 additions & 4 deletions crates/mongodb-agent-common/src/query/make_selector.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ mod tests {
column: ComparisonTarget::Column {
name: "Name".to_owned(),
field_path: None,
column_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
field_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
path: vec!["Albums".into(), "Tracks".into()],
},
operator: ComparisonFunction::Equal,
Expand Down Expand Up @@ -236,7 +236,7 @@ mod tests {
column: ComparisonTarget::Column {
name: "Name".to_owned(),
field_path: None,
column_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
field_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
path: vec!["Albums".into(), "Tracks".into()],
},
operator: UnaryComparisonOperator::IsNull,
Expand Down Expand Up @@ -267,15 +267,15 @@ mod tests {
column: ComparisonTarget::Column {
name: "Name".to_owned(),
field_path: None,
column_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
field_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
path: Default::default(),
},
operator: ComparisonFunction::Equal,
value: ComparisonValue::Column {
column: ComparisonTarget::Column {
name: "Title".to_owned(),
field_path: None,
column_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
field_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
path: Default::default(),
},
},
Expand Down
47 changes: 47 additions & 0 deletions crates/ndc-query-plan/src/plan_for_query_request/helpers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,53 @@ pub fn find_object_field<'a, S>(
})
}

pub fn find_object_field_path<'a, S>(
object_type: &'a plan::ObjectType<S>,
field_name: &str,
field_path: &Option<Vec<String>>,
) -> Result<&'a plan::Type<S>> {
match field_path {
None => find_object_field(object_type, field_name),
Some(field_path) => find_object_field_path_helper(object_type, field_name, field_path),
}
}

fn find_object_field_path_helper<'a, S>(
object_type: &'a plan::ObjectType<S>,
field_name: &str,
field_path: &[String],
) -> Result<&'a plan::Type<S>> {
let field_type = find_object_field(object_type, field_name)?;
match field_path {
[] => Ok(field_type),
[nested_field_name, rest @ ..] => {
let o = find_object_type(field_type, &object_type.name, field_name)?;
find_object_field_path_helper(o, nested_field_name, rest)
}
}
}

fn find_object_type<'a, S>(
t: &'a plan::Type<S>,
parent_type: &Option<String>,
field_name: &str,
) -> Result<&'a plan::ObjectType<S>> {
match t {
crate::Type::Scalar(_) => Err(QueryPlanError::ExpectedObjectTypeAtField {
parent_type: parent_type.to_owned(),
field_name: field_name.to_owned(),
got: "scalar".to_owned(),
}),
crate::Type::ArrayOf(_) => Err(QueryPlanError::ExpectedObjectTypeAtField {
parent_type: parent_type.to_owned(),
field_name: field_name.to_owned(),
got: "array".to_owned(),
}),
crate::Type::Nullable(t) => find_object_type(t, parent_type, field_name),
crate::Type::Object(object_type) => Ok(object_type),
}
}

pub fn lookup_relationship<'a>(
relationships: &'a BTreeMap<String, ndc::Relationship>,
relationship: &str,
Expand Down
18 changes: 9 additions & 9 deletions crates/ndc-query-plan/src/plan_for_query_request/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use ndc::{ExistsInCollection, QueryRequest};
use ndc_models as ndc;

use self::{
helpers::{find_object_field, lookup_relationship},
helpers::{find_object_field, find_object_field_path, lookup_relationship},
query_context::QueryContext,
query_plan_error::QueryPlanError,
query_plan_state::QueryPlanState,
Expand Down Expand Up @@ -478,11 +478,11 @@ fn plan_for_binary_comparison<T: QueryContext>(
plan_for_comparison_target(plan_state, root_collection_object_type, object_type, column)?;
let (operator, operator_definition) = plan_state
.context
.find_comparison_operator(comparison_target.get_column_type(), &operator)?;
.find_comparison_operator(comparison_target.get_field_type(), &operator)?;
let value_type = match operator_definition {
plan::ComparisonOperatorDefinition::Equal => comparison_target.get_column_type().clone(),
plan::ComparisonOperatorDefinition::Equal => comparison_target.get_field_type().clone(),
plan::ComparisonOperatorDefinition::In => {
plan::Type::ArrayOf(Box::new(comparison_target.get_column_type().clone()))
plan::Type::ArrayOf(Box::new(comparison_target.get_field_type().clone()))
}
plan::ComparisonOperatorDefinition::Custom { argument_type } => argument_type.clone(),
};
Expand Down Expand Up @@ -519,20 +519,20 @@ fn plan_for_comparison_target<T: QueryContext>(
path,
requested_columns,
)?;
let column_type = find_object_field(&target_object_type, &name)?.clone();
let field_type = find_object_field_path(&target_object_type, &name, &field_path)?.clone();
Ok(plan::ComparisonTarget::Column {
name,
field_path,
path,
column_type,
field_type,
})
}
ndc::ComparisonTarget::RootCollectionColumn { name, field_path } => {
let column_type = find_object_field(root_collection_object_type, &name)?.clone();
let field_type = find_object_field_path(root_collection_object_type, &name, &field_path)?.clone();
Ok(plan::ComparisonTarget::ColumnInScope {
name,
field_path,
column_type,
field_type,
scope: plan_state.scope.clone(),
})
}
Expand Down Expand Up @@ -603,7 +603,7 @@ fn plan_for_exists<T: QueryContext>(
comparison_target.column_name().to_owned(),
plan::Field::Column {
column: comparison_target.column_name().to_string(),
column_type: comparison_target.get_column_type().clone(),
column_type: comparison_target.get_field_type().clone(),
fields: None,
},
)
Expand Down
7 changes: 7 additions & 0 deletions crates/ndc-query-plan/src/plan_for_query_request/query_plan_error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ pub enum QueryPlanError {

#[error("Query referenced a relationship, \"{0}\", but did not include relation metadata in `collection_relationships`")]
UnspecifiedRelation(String),

#[error("Expected field {field_name} of object {} to be an object type. Got {got}.", parent_type.to_owned().unwrap_or("".to_owned()))]
ExpectedObjectTypeAtField {
parent_type: Option<String>,
field_name: String,
got: String,
},
}

fn at_path(path: &[String]) -> String {
Expand Down
20 changes: 10 additions & 10 deletions crates/ndc-query-plan/src/plan_for_query_request/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ fn translates_query_request_relationships() -> Result<(), anyhow::Error> {
column: plan::ComparisonTarget::Column {
name: "_id".into(),
field_path: None,
column_type: plan::Type::Scalar(
field_type: plan::Type::Scalar(
plan_test_helpers::ScalarType::Int,
),
path: vec!["class_department".into()],
Expand All @@ -139,7 +139,7 @@ fn translates_query_request_relationships() -> Result<(), anyhow::Error> {
column: plan::ComparisonTarget::Column {
name: "math_department_id".into(),
field_path: None,
column_type: plan::Type::Scalar(
field_type: plan::Type::Scalar(
plan_test_helpers::ScalarType::Int,
),
path: vec!["school_directory".into()],
Expand Down Expand Up @@ -394,15 +394,15 @@ fn translates_root_column_references() -> Result<(), anyhow::Error> {
column: plan::ComparisonTarget::Column {
name: "author_id".into(),
field_path: Default::default(),
column_type: plan::Type::Scalar(plan_test_helpers::ScalarType::Int),
field_type: plan::Type::Scalar(plan_test_helpers::ScalarType::Int),
path: Default::default(),
},
operator: plan_test_helpers::ComparisonOperator::Equal,
value: plan::ComparisonValue::Column {
column: plan::ComparisonTarget::ColumnInScope {
name: "id".into(),
field_path: Default::default(),
column_type: plan::Type::Scalar(
field_type: plan::Type::Scalar(
plan_test_helpers::ScalarType::Int,
),
scope: plan::Scope::Root,
Expand All @@ -413,7 +413,7 @@ fn translates_root_column_references() -> Result<(), anyhow::Error> {
column: plan::ComparisonTarget::Column {
name: "title".into(),
field_path: Default::default(),
column_type: plan::Type::Scalar(
field_type: plan::Type::Scalar(
plan_test_helpers::ScalarType::String,
),
path: Default::default(),
Expand Down Expand Up @@ -454,7 +454,7 @@ fn translates_root_column_references() -> Result<(), anyhow::Error> {
plan::Expression::BinaryComparisonOperator {
column: plan::ComparisonTarget::Column {
name: "author_id".into(),
column_type: plan::Type::Scalar(
field_type: plan::Type::Scalar(
plan_test_helpers::ScalarType::Int,
),
field_path: None,
Expand All @@ -465,7 +465,7 @@ fn translates_root_column_references() -> Result<(), anyhow::Error> {
column: plan::ComparisonTarget::ColumnInScope {
name: "id".into(),
scope: plan::Scope::Root,
column_type: plan::Type::Scalar(
field_type: plan::Type::Scalar(
plan_test_helpers::ScalarType::Int,
),
field_path: None,
Expand All @@ -475,7 +475,7 @@ fn translates_root_column_references() -> Result<(), anyhow::Error> {
plan::Expression::BinaryComparisonOperator {
column: plan::ComparisonTarget::Column {
name: "title".into(),
column_type: plan::Type::Scalar(
field_type: plan::Type::Scalar(
plan_test_helpers::ScalarType::String,
),
field_path: None,
Expand Down Expand Up @@ -609,7 +609,7 @@ fn translates_relationships_in_fields_predicates_and_orderings() -> Result<(), a
column: plan::ComparisonTarget::Column {
name: "title".into(),
field_path: Default::default(),
column_type: plan::Type::Scalar(plan_test_helpers::ScalarType::String),
field_type: plan::Type::Scalar(plan_test_helpers::ScalarType::String),
path: Default::default(),
},
operator: plan_test_helpers::ComparisonOperator::Regex,
Expand Down Expand Up @@ -873,7 +873,7 @@ fn translates_predicate_referencing_field_of_related_collection() -> anyhow::Res
column: plan::ComparisonTarget::Column {
name: "name".into(),
field_path: None,
column_type: plan::Type::Scalar(plan_test_helpers::ScalarType::String),
field_type: plan::Type::Scalar(plan_test_helpers::ScalarType::String),
path: vec!["author".into()],
},
operator: ndc_models::UnaryComparisonOperator::IsNull,
Expand Down
10 changes: 5 additions & 5 deletions crates/ndc-query-plan/src/query_plan.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ pub enum ComparisonTarget<T: ConnectorTypes> {
/// Path to a nested field within an object column
field_path: Option<Vec<String>>,

column_type: Type<T::ScalarType>,
field_type: Type<T::ScalarType>,

/// Any relationships to traverse to reach this column. These are translated from
/// [ndc_models::PathElement] values in the [ndc_models::QueryRequest] to names of relation
Expand All @@ -321,7 +321,7 @@ pub enum ComparisonTarget<T: ConnectorTypes> {
/// Path to a nested field within an object column
field_path: Option<Vec<String>>,

column_type: Type<T::ScalarType>,
field_type: Type<T::ScalarType>,
},
}

Expand All @@ -342,10 +342,10 @@ impl<T: ConnectorTypes> ComparisonTarget<T> {
}

impl<T: ConnectorTypes> ComparisonTarget<T> {
pub fn get_column_type(&self) -> &Type<T::ScalarType> {
pub fn get_field_type(&self) -> &Type<T::ScalarType> {
match self {
ComparisonTarget::Column { column_type, .. } => column_type,
ComparisonTarget::ColumnInScope { column_type, .. } => column_type,
ComparisonTarget::Column { field_type, .. } => field_type,
ComparisonTarget::ColumnInScope { field_type, .. } => field_type,
}
}
}
Expand Down
Loading