Skip to content

graph,graphql,server,store: Subgraph Sql Service #5382

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.

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

Closed
wants to merge 9 commits into from
6 changes: 6 additions & 0 deletions graph/src/env/graphql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ pub struct EnvVarsGraphQl {
/// Set by the flag `GRAPH_GRAPHQL_DISABLE_BOOL_FILTERS`. Off by default.
/// Disables AND/OR filters
pub disable_bool_filters: bool,
/// Set by the flag `GRAPH_GRAPHQL_ENABLE_SQL_SERVICE`. Off by default.
/// Enables queries on the `sql()` field of the root query
pub enable_sql_service: bool,
/// Set by the flag `GRAPH_GRAPHQL_DISABLE_CHILD_SORTING`. Off by default.
/// Disables child-based sorting
pub disable_child_sorting: bool,
Expand Down Expand Up @@ -146,6 +149,7 @@ impl From<InnerGraphQl> for EnvVarsGraphQl {
error_result_size: x.error_result_size.0 .0,
max_operations_per_connection: x.max_operations_per_connection,
disable_bool_filters: x.disable_bool_filters.0,
enable_sql_service: x.enable_sql_service.0,
disable_child_sorting: x.disable_child_sorting.0,
query_trace_token: x.query_trace_token,
parallel_block_constraints: x.parallel_block_constraints.0,
Expand Down Expand Up @@ -198,6 +202,8 @@ pub struct InnerGraphQl {
pub disable_bool_filters: EnvVarBoolean,
#[envconfig(from = "GRAPH_GRAPHQL_DISABLE_CHILD_SORTING", default = "false")]
pub disable_child_sorting: EnvVarBoolean,
#[envconfig(from = "GRAPH_GRAPHQL_ENABLE_SQL_SERVICE", default = "false")]
pub enable_sql_service: EnvVarBoolean,
#[envconfig(from = "GRAPH_GRAPHQL_TRACE_TOKEN", default = "")]
query_trace_token: String,
#[envconfig(from = "GRAPH_PARALLEL_BLOCK_CONSTRAINTS", default = "false")]
Expand Down
14 changes: 11 additions & 3 deletions graph/src/schema/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,10 @@ pub(in crate::schema) fn api_schema(
// Refactor: Don't clone the schema.
let mut api = init_api_schema(input_schema)?;
add_meta_field_type(&mut api.document);
add_sql_field_type(&mut api.document);

if ENV_VARS.graphql.enable_sql_service {
add_sql_field_type(&mut api.document);
}
add_types_for_object_types(&mut api, input_schema)?;
add_types_for_interface_types(&mut api, input_schema)?;
add_types_for_aggregation_types(&mut api, input_schema)?;
Expand Down Expand Up @@ -1084,7 +1087,9 @@ fn add_query_type(api: &mut s::Document, input_schema: &InputSchema) -> Result<(
fields.append(&mut agg_fields);
fields.append(&mut fulltext_fields);
fields.push(meta_field());
fields.push(sql_field());
if ENV_VARS.graphql.enable_sql_service {
fields.push(sql_field());
}

let typedef = s::TypeDefinition::Object(s::ObjectType {
position: Pos::default(),
Expand Down Expand Up @@ -1183,7 +1188,10 @@ fn add_subscription_type(
.collect::<Vec<s::Field>>();
fields.append(&mut agg_fields);
fields.push(meta_field());
fields.push(sql_field());

if ENV_VARS.graphql.enable_sql_service {
fields.push(sql_field());
}

let typedef = s::TypeDefinition::Object(s::ObjectType {
position: Pos::default(),
Expand Down
3 changes: 1 addition & 2 deletions graphql/src/store/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,6 @@ impl StoreResolver {
}

fn handle_sql(&self, field: &a::Field) -> Result<r::Value, QueryExecutionError> {

let input = field
.argument_value("input")
.ok_or_else(|| QueryExecutionError::EmptyQuery)?;
Expand Down Expand Up @@ -414,7 +413,7 @@ impl Resolver for StoreResolver {
return self.lookup_meta(field).await;
}

if object_type.is_sql() {
if ENV_VARS.graphql.enable_sql_service && object_type.is_sql() {
return self.handle_sql(field);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ENV_VARS check shouldn't be necessary - when SQL is turned off, a query that uses it shouldn't even make it here as it would have been rejected during validation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the reason why we have this check here is in the remote case where the schema contains a type called Sql which a disabled SQL service graph-node would allow it.


Expand Down