Opendd Query Planning: Reuse resolve_field_selection in commands (#1410)

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

### What

Reuse the `resolve_field_selection` function in command planning to
resolve the selection set. This automatically takes care of relationship
fields in command requests.

### How

<!-- How is it trying to accomplish it (what are the implementation
steps)? -->
- Refactor the `resolve_field_selection` fn code path by removing the
model_source reference and replacing with `type_permissions` and
`data_connector`.
- Reuse the function in command query planning.
- Update the error messages in test cases.

V3_GIT_ORIGIN_REV_ID: 6042b15c20347bf4147202dbd34e5397b73f8a8d
This commit is contained in:
Rakesh Emmadi 2024-12-03 22:10:25 +05:30 committed by hasura-bot
parent 9b35b62b80
commit 334ae181b8
11 changed files with 80 additions and 114 deletions

View File

@ -1,3 +1,3 @@
{
"error": "error in data fusion: Error during planning: role no-access does not have permission to select any fields of command get_actor_by_id (in subgraph default)"
"error": "error in data fusion: Error during planning: role no-access does not have permission to select any fields of type actor (in subgraph default)"
}

View File

@ -1,3 +1,3 @@
{
"error": "error in data fusion: Error during planning: role user does not have permission to select the field movie_id from type actor (in subgraph default) of command get_actor_by_id (in subgraph default)"
"error": "error in data fusion: Error during planning: role user does not have permission to select the field movie_id from type actor (in subgraph default)"
}

View File

@ -1,3 +1,3 @@
{
"error": "error in data fusion: Error during planning: role no-access does not have permission to select any fields of command uppercase_all_actor_names (in subgraph default)"
"error": "error in data fusion: Error during planning: role no-access does not have permission to select any fields of type actor (in subgraph default)"
}

View File

@ -1,3 +1,3 @@
{
"error": "error in data fusion: Error during planning: role user does not have permission to select the field movie_id from type actor (in subgraph default) of command uppercase_actor_name_by_id (in subgraph default)"
"error": "error in data fusion: Error during planning: role user does not have permission to select the field movie_id from type actor (in subgraph default)"
}

View File

@ -184,7 +184,14 @@ where
command_plan,
output_object_type_name,
extract_response_from: _,
} = command::from_command(command_selection, metadata, session, request_headers)?;
} = command::from_command(
command_selection,
metadata,
session,
http_context,
request_headers,
unique_number,
)?;
match command_plan {
command::CommandPlan::Function(ndc_function) => {
let query_execution_plan = command::execute_plan_from_function(&ndc_function);

View File

@ -1,3 +1,4 @@
use engine_types::HttpContext;
use indexmap::IndexMap;
use std::collections::BTreeMap;
use std::sync::Arc;
@ -10,17 +11,17 @@ use metadata_resolve::{
unwrap_custom_type_name, Metadata, Qualified, QualifiedBaseType, QualifiedTypeName,
QualifiedTypeReference,
};
use open_dds::query::{CommandSelection, ObjectSubSelection};
use open_dds::query::CommandSelection;
use open_dds::{
commands::DataConnectorCommand,
data_connector::{CollectionName, DataConnectorColumnName},
types::CustomTypeName,
};
use plan_types::FUNCTION_IR_VALUE_COLUMN_NAME;
use plan_types::{
Argument, Field, MutationArgument, MutationExecutionPlan, NdcFieldAlias, NestedArray,
NestedField, NestedObject, QueryExecutionPlan, QueryNodeNew,
};
use plan_types::{UniqueNumber, FUNCTION_IR_VALUE_COLUMN_NAME};
#[derive(Debug)]
pub enum CommandPlan {
@ -38,7 +39,9 @@ pub fn from_command(
command_selection: &CommandSelection,
metadata: &Metadata,
session: &Arc<Session>,
http_context: &Arc<HttpContext>,
request_headers: &reqwest::header::HeaderMap,
unique_number: &mut UniqueNumber,
) -> Result<FromCommand, PlanError> {
let command_target = &command_selection.target;
let qualified_command_name = metadata_resolve::Qualified::new(
@ -61,15 +64,6 @@ pub fn from_command(
let output_object_type_name = unwrap_custom_type_name(&command.command.output_type).unwrap();
let metadata_resolve::TypeMapping::Object { field_mappings, .. } = command_source
.type_mappings
.get(output_object_type_name)
.ok_or_else(|| {
PlanError::Internal(format!(
"couldn't fetch type_mapping of type {output_object_type_name} for command {qualified_command_name}",
))
})?;
let output_object_type = metadata
.object_types
.get(output_object_type_name)
@ -79,78 +73,25 @@ pub fn from_command(
))
})?;
let type_permissions = output_object_type
.type_output_permissions
.get(&session.role)
.ok_or_else(|| {
PlanError::Permission(format!(
"role {} does not have permission to select any fields of command {}",
session.role, qualified_command_name
))
})?;
let mut relationships = BTreeMap::new();
let command_selection_set = match &command_selection.selection {
Some(selection_set) => selection_set,
None => &IndexMap::new(),
};
let mut ndc_fields = IndexMap::new();
for (field_alias, object_sub_selection) in command_selection.selection.iter().flatten() {
let ObjectSubSelection::Field(field_selection) = object_sub_selection else {
return Err(PlanError::Internal(
"only normal field selections are supported in NDCPushDownPlanner.".to_string(),
));
};
if !type_permissions
.allowed_fields
.contains(&field_selection.target.field_name)
{
return Err(PlanError::Permission(format!(
"role {} does not have permission to select the field {} from type {} of command {}",
session.role,
field_selection.target.field_name,
&output_object_type_name,
qualified_command_name
)));
}
let field_mapping = field_mappings
.get(&field_selection.target.field_name)
// .map(|field_mapping| field_mapping.column.clone())
.ok_or_else(|| {
PlanError::Internal(format!(
"couldn't fetch field mapping of field {} in type {} for command {}",
field_selection.target.field_name,
output_object_type_name,
qualified_command_name
))
})?;
let field_name = &field_selection.target.field_name;
let field_type = &output_object_type
.object_type
.fields
.get(field_name)
.ok_or_else(|| {
PlanError::Internal(format!(
"could not look up type of field {}",
field_selection.target.field_name
))
})?
.field_type;
let fields = field_selection::ndc_nested_field_selection_for(
metadata,
session,
field_name,
field_type,
&command_source.type_mappings,
)?;
let ndc_field = Field::Column {
column: field_mapping.column.clone(),
fields,
arguments: BTreeMap::new(),
};
ndc_fields.insert(NdcFieldAlias::from(field_alias.as_str()), ndc_field);
}
let ndc_fields = field_selection::resolve_field_selection(
metadata,
session,
http_context,
request_headers,
output_object_type_name,
output_object_type,
&command_source.type_mappings,
&command_source.data_connector,
command_selection_set,
&mut relationships,
unique_number,
)?;
let (ndc_fields, extract_response_from) = match &command_source.data_connector.response_config {
// if the data connector has 'responseHeaders' configured, we'll need to wrap the ndc fields
@ -216,14 +157,14 @@ pub fn from_command(
arguments: ndc_arguments,
data_connector: command_source.data_connector.clone(),
fields: wrap_function_ndc_fields(&output_shape, ndc_fields),
collection_relationships: BTreeMap::new(),
collection_relationships: relationships,
}),
DataConnectorCommand::Procedure(procedure_name) => CommandPlan::Procedure(NDCProcedure {
procedure_name: procedure_name.clone(),
arguments: ndc_arguments,
data_connector: command_source.data_connector.clone(),
fields: Some(wrap_procedure_ndc_fields(&output_shape, ndc_fields)),
collection_relationships: BTreeMap::new(),
collection_relationships: relationships,
}),
};

View File

@ -23,15 +23,14 @@ pub fn resolve_field_selection(
request_headers: &reqwest::header::HeaderMap,
object_type_name: &Qualified<CustomTypeName>,
object_type: &metadata_resolve::ObjectTypeWithRelationships,
model_source: &Arc<metadata_resolve::ModelSource>,
type_mappings: &BTreeMap<Qualified<CustomTypeName>, TypeMapping>,
data_connector: &metadata_resolve::DataConnectorLink,
selection: &IndexMap<Alias, ObjectSubSelection>,
relationships: &mut BTreeMap<plan_types::NdcRelationshipName, plan_types::Relationship>,
unique_number: &mut UniqueNumber,
) -> Result<IndexMap<NdcFieldAlias, Field>, PlanError> {
let metadata_resolve::TypeMapping::Object { field_mappings, .. } = model_source
.type_mappings
.get(object_type_name)
.ok_or_else(|| {
let metadata_resolve::TypeMapping::Object { field_mappings, .. } =
type_mappings.get(object_type_name).ok_or_else(|| {
PlanError::Internal(format!(
"couldn't fetch type_mapping of type {object_type_name}",
))
@ -45,7 +44,8 @@ pub fn resolve_field_selection(
session,
http_context,
request_headers,
model_source,
type_mappings,
data_connector,
field_selection,
field_mappings,
object_type_name,
@ -62,7 +62,8 @@ pub fn resolve_field_selection(
request_headers,
object_type_name,
object_type,
model_source,
type_mappings,
data_connector,
relationships,
unique_number,
)?
@ -83,7 +84,8 @@ fn from_field_selection(
session: &Arc<Session>,
http_context: &Arc<HttpContext>,
request_headers: &reqwest::header::HeaderMap,
model_source: &Arc<metadata_resolve::ModelSource>,
type_mappings: &BTreeMap<Qualified<CustomTypeName>, TypeMapping>,
data_connector: &metadata_resolve::DataConnectorLink,
field_selection: &ObjectFieldSelection,
field_mappings: &BTreeMap<FieldName, metadata_resolve::FieldMapping>,
object_type_name: &Qualified<CustomTypeName>,
@ -137,7 +139,8 @@ fn from_field_selection(
session,
http_context,
request_headers,
model_source,
type_mappings,
data_connector,
field_selection,
field_type,
relationships,
@ -157,7 +160,8 @@ fn resolve_nested_field_selection(
session: &Arc<Session>,
http_context: &Arc<HttpContext>,
request_headers: &reqwest::header::HeaderMap,
model_source: &Arc<metadata_resolve::ModelSource>,
type_mappings: &BTreeMap<Qualified<CustomTypeName>, TypeMapping>,
data_connector: &metadata_resolve::DataConnectorLink,
field_selection: &ObjectFieldSelection,
field_type: &QualifiedTypeReference,
relationships: &mut BTreeMap<plan_types::NdcRelationshipName, plan_types::Relationship>,
@ -171,7 +175,7 @@ fn resolve_nested_field_selection(
session,
&field_selection.target.field_name,
field_type,
&model_source.type_mappings,
type_mappings,
)
}
Some(nested_selection) => {
@ -201,7 +205,8 @@ fn resolve_nested_field_selection(
request_headers,
field_type_name,
nested_object_type,
model_source,
type_mappings,
data_connector,
nested_selection,
relationships,
unique_number,
@ -240,7 +245,8 @@ fn from_relationship_selection(
request_headers: &reqwest::header::HeaderMap,
object_type_name: &Qualified<CustomTypeName>,
object_type: &metadata_resolve::ObjectTypeWithRelationships,
model_source: &Arc<metadata_resolve::ModelSource>,
type_mappings: &BTreeMap<Qualified<CustomTypeName>, TypeMapping>,
data_connector: &metadata_resolve::DataConnectorLink,
relationships: &mut BTreeMap<plan_types::NdcRelationshipName, plan_types::Relationship>,
unique_number: &mut UniqueNumber,
) -> Result<Field, PlanError> {
@ -277,7 +283,7 @@ fn from_relationship_selection(
})?;
// Reject remote relationships
if target_model_source.data_connector.name != model_source.data_connector.name {
if target_model_source.data_connector.name != data_connector.name {
return Err(PlanError::Relationship(format!(
"Remote relationships are not supported: {}",
&target.relationship_name
@ -301,8 +307,8 @@ fn from_relationship_selection(
relationship_name: &target.relationship_name,
relationship_type: &model_relationship_target.relationship_type,
source_type: object_type_name,
source_data_connector: &model_source.data_connector,
source_type_mappings: &model_source.type_mappings,
source_data_connector: data_connector,
source_type_mappings: type_mappings,
target_source: &target_source,
target_type: &model_relationship_target.target_typename,
mappings: &model_relationship_target.mappings,
@ -367,7 +373,7 @@ fn from_relationship_selection(
Ok(ndc_field)
}
pub fn ndc_nested_field_selection_for(
fn ndc_nested_field_selection_for(
metadata: &Metadata,
session: &Arc<Session>,
column_name: &FieldName,

View File

@ -148,7 +148,8 @@ pub fn from_model_selection(
request_headers,
&model.model.data_type,
model_object_type,
model_source,
&model_source.type_mappings,
&model_source.data_connector,
&model_selection.selection,
&mut relationships,
unique_number,

View File

@ -70,6 +70,9 @@ impl ExtensionPlanner for NDCPushDownPlanner {
physical_inputs: &[Arc<dyn ExecutionPlan>],
_session_state: &SessionState,
) -> Result<Option<Arc<dyn ExecutionPlan>>> {
// this will need to be threaded throughout entire query
// if we want to support remote predicates in `sql`
let mut unique_number = plan_types::UniqueNumber::new();
if let Some(model_query) = node.as_any().downcast_ref::<model::ModelQuery>() {
assert_eq!(logical_inputs.len(), 0, "Inconsistent number of inputs");
assert_eq!(physical_inputs.len(), 0, "Inconsistent number of inputs");
@ -78,6 +81,7 @@ impl ExtensionPlanner for NDCPushDownPlanner {
&self.http_context,
&self.metadata,
&self.request_headers,
&mut unique_number,
)?;
Ok(Some(Arc::new(ndc_pushdown)))
} else if let Some(command_query) = node.as_any().downcast_ref::<command::CommandQuery>() {
@ -91,6 +95,7 @@ impl ExtensionPlanner for NDCPushDownPlanner {
&command_query.command_selection,
&command_query.schema,
&command_query.output,
&mut unique_number,
)
.map(Some)
} else if let Some(model_aggregate) = node.as_any().downcast_ref::<model::ModelAggregate>()

View File

@ -1,4 +1,5 @@
use datafusion::{common::DFSchemaRef, error::Result, physical_plan::ExecutionPlan};
use plan_types::UniqueNumber;
use std::sync::Arc;
use hasura_authn_core::Session;
@ -22,13 +23,21 @@ pub fn build_execution_plan(
// schema of the output of the command selection
schema: &DFSchemaRef,
output: &CommandOutput,
unique_number: &mut UniqueNumber,
) -> Result<Arc<dyn ExecutionPlan>> {
let FromCommand {
command_plan,
extract_response_from,
..
} = from_command(command_selection, metadata, session, request_headers)
.map_err(from_plan_error)?;
} = from_command(
command_selection,
metadata,
session,
http_context,
request_headers,
unique_number,
)
.map_err(from_plan_error)?;
match command_plan {
CommandPlan::Function(function) => {

View File

@ -415,18 +415,15 @@ impl ModelQuery {
http_context: &Arc<HttpContext>,
metadata: &metadata_resolve::Metadata,
request_headers: &reqwest::header::HeaderMap,
unique_number: &mut UniqueNumber,
) -> Result<NDCQueryPushDown> {
// this will need to be threaded throughout entire query
// if we want to support remote predicates in `sql`
let mut unique_number = UniqueNumber::new();
let (_, query, ndc_fields) = from_model_selection(
&self.model_selection,
metadata,
session,
http_context,
request_headers,
&mut unique_number,
unique_number,
)
.map_err(from_plan_error)?;