Skip to content

Commit

Permalink
OpenDd query planning: Support unique field filtering for select_one …
Browse files Browse the repository at this point in the history
…queries (hasura#1539)

<!-- The PR description should answer 2 important questions: -->

### What

- Support unique field filter expressions for select_one model selection
in OpenDd query planning.
- Enable OpenDd execution tests for select_one queries.

### How
- Delay the requirement for a boolean expression type until custom
comparison operator resolution.

<!-- How is it trying to accomplish it (what are the implementation
steps)? -->

V3_GIT_ORIGIN_REV_ID: 457521363580e0407f73f08c15f73341f963b419
  • Loading branch information
rakeshkky authored and hasura-bot committed Jan 16, 2025
1 parent c298e87 commit 431b0af
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 32 deletions.
12 changes: 6 additions & 6 deletions v3/crates/engine/tests/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ fn test_model_select_one_simple_select() -> anyhow::Result<()> {
common::test_execution_expectation(
test_path_string,
&[common_metadata_path_string],
common::TestOpenDDPipeline::Skip,
common::TestOpenDDPipeline::YesPlease,
)
}

Expand All @@ -22,7 +22,7 @@ fn test_model_select_one_with_type_permission() -> anyhow::Result<()> {
common::test_execution_expectation(
test_path_string,
&[common_metadata_path_string],
common::TestOpenDDPipeline::Skip,
common::TestOpenDDPipeline::YesPlease,
)
}

Expand All @@ -44,7 +44,7 @@ fn test_model_select_one_filter() -> anyhow::Result<()> {
common::test_execution_expectation(
test_path_string,
&[common_metadata_path_string],
common::TestOpenDDPipeline::Skip,
common::TestOpenDDPipeline::YesPlease,
)
}

Expand All @@ -55,7 +55,7 @@ fn test_model_select_one_custom_scalar() -> anyhow::Result<()> {
common::test_execution_expectation(
test_path_string,
&[common_metadata_path_string],
common::TestOpenDDPipeline::Skip,
common::TestOpenDDPipeline::YesPlease,
)
}

Expand Down Expand Up @@ -925,7 +925,7 @@ fn test_model_select_many_object_type_input_arguments() -> anyhow::Result<()> {
common::test_execution_expectation(
test_path_string,
&[common_metadata_path_string],
common::TestOpenDDPipeline::Skip,
common::TestOpenDDPipeline::YesPlease,
)
}

Expand Down Expand Up @@ -1654,7 +1654,7 @@ fn test_model_argument_presets_select_one() -> anyhow::Result<()> {
vec!["execute/common_metadata/custom_connector_v02_schema.json"],
),
]),
common::TestOpenDDPipeline::Skip,
common::TestOpenDDPipeline::YesPlease,
)
}

Expand Down
16 changes: 12 additions & 4 deletions v3/crates/plan/src/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ pub fn to_resolved_filter_expr(
type_mappings: &BTreeMap<Qualified<CustomTypeName>, TypeMapping>,
type_name: &Qualified<CustomTypeName>,
model_object_type: &metadata_resolve::ObjectTypeWithRelationships,
boolean_expression_type: &metadata_resolve::ResolvedObjectBooleanExpressionType,
boolean_expression_type: Option<&metadata_resolve::ResolvedObjectBooleanExpressionType>,
expr: &BooleanExpression,
data_connector: &DataConnectorLink,
) -> Result<ResolvedFilterExpression, PlanError> {
Expand Down Expand Up @@ -100,9 +100,6 @@ pub fn to_resolved_filter_expr(
))),
}?;

let comparison_expression_info =
boolean_expression_for_comparison(metadata, boolean_expression_type, field)?;

// ideally everything would be resolved via boolean expression types, but
// we need this information from the object types as we mark which operators
// mean equality there
Expand Down Expand Up @@ -288,6 +285,17 @@ data_connector.capabilities.supported_ndc_version))
))
}
ComparisonOperator::Custom(custom_operator) => {
// Boolean expression type is required to resolve custom operators
let boolean_expression_type = boolean_expression_type.ok_or_else(|| {
PlanError::Internal(
"Custom operators require a boolean expression type".into(),
)
})?;
let comparison_expression_info = boolean_expression_for_comparison(
metadata,
boolean_expression_type,
field,
)?;
let data_connector_operators = comparison_expression_info
.operator_mapping
.get(&data_connector.name)
Expand Down
31 changes: 9 additions & 22 deletions v3/crates/plan/src/query/model_target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,29 +78,16 @@ pub fn model_target_to_ndc_query(
unique_number,
)?;

// if we are going to filter our results, we must have a `BooleanExpressionType` defined for
// our model
// todo: what to do for SELECT ONE filters, we shouldn't need a bool exp for that?
let model_filter = match &model_target.filter {
Some(expr) => {
let boolean_expression_type =
model.filter_expression_type.as_ref().ok_or_else(|| {
PlanError::Internal(format!(
"Cannot filter model {} as no boolean expression is defined for it",
model.model.name
))
})?;

Ok(Some(to_resolved_filter_expr(
metadata,
&model_source.type_mappings,
&model.model.data_type,
model_object_type,
boolean_expression_type,
expr,
&model_source.data_connector,
)?))
}
Some(expr) => Ok(Some(to_resolved_filter_expr(
metadata,
&model_source.type_mappings,
&model.model.data_type,
model_object_type,
model.filter_expression_type.as_ref(),
expr,
&model_source.data_connector,
)?)),
_ => Ok(None),
}?;

Expand Down

0 comments on commit 431b0af

Please sign in to comment.