WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Commit 909d087

Browse files
danieljharveyhasura-bot
authored andcommitted
Correctly wrap command headers in OpenDD responses (#1679)
<!-- The PR description should answer 2 important questions: --> ### What Make sure that when a command returns headers that we unwrap them correctly. Functional no-op. V3_GIT_ORIGIN_REV_ID: 4e37f6d1f3045b0cd60e45d3e9f586c35f2bc878
1 parent 65539c6 commit 909d087

File tree

2 files changed

+52
-16
lines changed

2 files changed

+52
-16
lines changed

v3/crates/engine/tests/execution.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2949,7 +2949,7 @@ fn test_command_query_forwarded_headers() -> anyhow::Result<()> {
29492949
vec!["execute/common_metadata/custom_connector_v02_schema.json"],
29502950
),
29512951
]),
2952-
common::TestOpenDDPipeline::Skip,
2952+
common::TestOpenDDPipeline::YesPlease,
29532953
)
29542954
}
29552955

v3/crates/plan/src/query/command.rs

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -126,24 +126,17 @@ fn from_command_output_type(
126126
unique_number,
127127
)?;
128128

129-
match &command_source.data_connector.response_config {
129+
let extract_response_from = match &command_source.data_connector.response_config {
130130
// if the data connector has 'responseHeaders' configured, we'll need to wrap the ndc fields
131131
// under the 'result' field if the command's response at opendd layer refers to the 'result'
132132
// field's type. Note that we aren't requesting the 'header's field as we don't forward the
133133
// response headers in the SQL layer yet
134134
Some(response_config) if !command_source.ndc_type_opendd_type_same => {
135-
let result_field_name =
136-
NdcFieldAlias::from(response_config.result_field.as_str());
137-
let result_field = Field::Column {
138-
column: response_config.result_field.clone(),
139-
fields: Some(NestedField::Object(NestedObject { fields: ndc_fields })),
140-
arguments: BTreeMap::new(),
141-
};
142-
let fields = IndexMap::from_iter([(result_field_name, result_field)]);
143-
Ok((fields, Some(response_config.result_field.clone())))
135+
Some(response_config.result_field.clone())
144136
}
145-
_ => Ok((ndc_fields, None)),
146-
}
137+
_ => None,
138+
};
139+
Ok((ndc_fields, extract_response_from))
147140
}
148141
}
149142
}
@@ -226,9 +219,9 @@ pub(crate) fn from_command_selection(
226219
query_execution_plan: QueryExecutionPlan {
227220
query_node: QueryNodeNew {
228221
fields: Some(plan_types::FieldsSelection {
229-
fields: wrap_scalar_select(wrap_function_ndc_field(
230-
&output_shape,
231-
ndc_fields,
222+
fields: wrap_scalar_select(wrap_selection_in_response_config(
223+
command_source,
224+
wrap_function_ndc_field(&output_shape, ndc_fields),
232225
)),
233226
}),
234227
aggregates: None,
@@ -282,6 +275,49 @@ pub(crate) fn from_command_selection(
282275
})
283276
}
284277

278+
/// Wrap a selection set in a `{"headers": ..., "response": ...}` selection
279+
/// shape for command selections, where `CommandsResponseConfig` is configured.
280+
///
281+
/// When the output type of a NDC function/procedure is an object type,
282+
/// containing headers and response fields; and the response field is also an
283+
/// object type -
284+
/// 1. Engine needs to generate fields selection IR such that it contains
285+
/// `{"headers": ..., "response": ...}` shape, and the actual selection from the
286+
/// user-facing query goes inside the `response` field
287+
fn wrap_selection_in_response_config(
288+
command_source: &metadata_resolve::CommandSource,
289+
original_selection: Option<NestedField>,
290+
) -> Option<NestedField> {
291+
match &command_source.data_connector.response_config {
292+
None => original_selection,
293+
Some(response_config) => {
294+
if command_source.ndc_type_opendd_type_same {
295+
original_selection
296+
} else {
297+
let headers_field_name =
298+
NdcFieldAlias::from(response_config.headers_field.as_str());
299+
let headers_field = Field::Column {
300+
column: response_config.headers_field.clone(),
301+
fields: None,
302+
arguments: BTreeMap::new(),
303+
};
304+
let result_field_name = NdcFieldAlias::from(response_config.result_field.as_str());
305+
let result_field = Field::Column {
306+
column: response_config.result_field.clone(),
307+
fields: original_selection,
308+
arguments: BTreeMap::new(),
309+
};
310+
Some(NestedField::Object(NestedObject {
311+
fields: IndexMap::from_iter([
312+
(headers_field_name, headers_field),
313+
(result_field_name, result_field),
314+
]),
315+
}))
316+
}
317+
}
318+
}
319+
}
320+
285321
fn wrap_procedure_ndc_fields(
286322
output_shape: &OutputShape,
287323
ndc_fields: IndexMap<NdcFieldAlias, Field>,

0 commit comments

Comments
 (0)