Skip to content
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

Jump to bottom

expose procedures as part of the schema endpoint #198

Merged
merged 4 commits into from
Dec 4, 2023
Merged

expose procedures as part of the schema endpoint #198

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
786e0b0
expose procedures as part of the schema endpoint
Dec 4, 2023
6e5c5a5
insta review
Dec 4, 2023
49e8064
change description
Dec 4, 2023
df97239
change description
Dec 4, 2023
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
13 changes: 10 additions & 3 deletions crates/connectors/ndc-postgres/src/configuration/version1.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,7 @@ fn native_query_to_current(nq: &NativeQueryInfo) -> metadata::NativeQueryInfo {
columns: columns_to_current(&nq.columns),
arguments: columns_to_current(&nq.arguments),
description: nq.description.clone(),
is_procedure: nq.is_procedure,
}
}

Expand Down Expand Up @@ -603,13 +604,19 @@ pub struct NativeQueries(pub BTreeMap<String, NativeQueryInfo>);
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
#[serde(rename_all = "camelCase")]
pub struct NativeQueryInfo {
/** SQL expression to use for the Native Query. We can interpolate values using `{{variable_name}}` syntax, such as `SELECT * FROM authors WHERE name = {{author_name}}` */
/// SQL expression to use for the Native Query.
/// We can interpolate values using `{{variable_name}}` syntax,
/// such as `SELECT * FROM authors WHERE name = {{author_name}}`
pub sql: metadata::NativeQuerySql,
/** Columns returned by the Native Query */
/// Columns returned by the Native Query
pub columns: BTreeMap<String, ColumnInfo>,
#[serde(default)]
/** Names and types of arguments that can be passed to this Native Query */
/// Names and types of arguments that can be passed to this Native Query
pub arguments: BTreeMap<String, ColumnInfo>,
#[serde(default)]
pub description: Option<String>,
/// True if this native query mutates the database
#[serde(skip_serializing_if = "std::ops::Not::not")]
#[serde(default)]
pub is_procedure: bool,
}
28 changes: 27 additions & 1 deletion crates/connectors/ndc-postgres/src/schema.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ pub async fn get_schema(
.native_queries
.0
.iter()
.filter(|(_, info)| !info.is_procedure)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't include procedures in the collections list

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps use Iterator::partition to filter the native queries once.

.map(|(name, info)| models::CollectionInfo {
name: name.clone(),
description: info.description.clone(),
Expand Down Expand Up @@ -207,9 +208,34 @@ pub async fn get_schema(
let mut object_types = table_types;
object_types.extend(native_queries_types);

let procedures: Vec<models::ProcedureInfo> = metadata
.native_queries
.0
.iter()
.filter(|(_, info)| info.is_procedure)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

only include procedures in the procedures list.

.map(|(name, info)| models::ProcedureInfo {
name: name.clone(),
description: info.description.clone(),
arguments: info
.arguments
.iter()
.map(|(name, column_info)| {
(
name.clone(),
models::ArgumentInfo {
description: column_info.description.clone(),
argument_type: column_to_type(column_info),
},
)
})
.collect(),
result_type: models::Type::Named { name: name.clone() },
})
.collect();

Ok(models::SchemaResponse {
collections,
procedures: vec![],
procedures,
functions: vec![],
object_types,
scalar_types,
Expand Down
4 changes: 4 additions & 0 deletions crates/documentation/openapi/src/snapshots/openapi_generator__tests__main.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,10 @@ expression: generated_schema_json
"default": null,
"type": "string",
"nullable": true
},
"isProcedure": {
"description": "True if this native query mutates the database",
"type": "boolean"
}
}
},
Expand Down
12 changes: 9 additions & 3 deletions crates/query-engine/metadata/src/metadata/native_queries.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,21 @@ pub struct NativeQueries(pub BTreeMap<String, NativeQueryInfo>);
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
#[serde(rename_all = "camelCase")]
pub struct NativeQueryInfo {
/** SQL expression to use for the Native Query. We can interpolate values using `{{variable_name}}` syntax, such as `SELECT * FROM authors WHERE name = {{author_name}}` */
/// SQL expression to use for the Native Query.
/// We can interpolate values using `{{variable_name}}` syntax,
/// such as `SELECT * FROM authors WHERE name = {{author_name}}`
pub sql: NativeQuerySql,
/** Columns returned by the Native Query */
/// Columns returned by the Native Query
pub columns: BTreeMap<String, ColumnInfo>,
#[serde(default)]
/** Names and types of arguments that can be passed to this Native Query */
/// Names and types of arguments that can be passed to this Native Query
pub arguments: BTreeMap<String, ColumnInfo>,
#[serde(default)]
pub description: Option<String>,
/// True if this native query mutates the database
#[serde(skip_serializing_if = "std::ops::Not::not")]
#[serde(default)]
Comment on lines +32 to +33
Copy link
Contributor

@plcplc plcplc Dec 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Versioning considerations:

  • Backwards compatible config (Old configurations work on new server versions. Esp, is_procedure is not a mandatory field) ✅
  • Forwards compatible config (New configurations work on older server versions) (OK if they don't use any procedures. Procedures present in the config will be fail to deserialize, IIUC) ✔️

I don't think this is an issue just now. I'm mostly writing this comment to practise being dilligent wrt versioning.

pub is_procedure: bool,
}

/// A part of a Native Query text, either raw text or a parameter.
Expand Down
4 changes: 4 additions & 0 deletions ..._tests__postgres__configuration_tests__configuration_tests__get_configuration_schema.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,10 @@ expression: schema
"string",
"null"
]
},
"isProcedure": {
"description": "True if this native query mutates the database",
"type": "boolean"
}
}
},
Expand Down
4 changes: 4 additions & 0 deletions ...sts__postgres__configuration_tests__configuration_tests__get_rawconfiguration_schema.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,10 @@ expression: schema
"string",
"null"
]
},
"isProcedure": {
"description": "True if this native query mutates the database",
"type": "boolean"
}
}
},
Expand Down
162 changes: 83 additions & 79 deletions .../postgres/snapshots/databases_tests__postgres__schema_tests__schema_test__get_schema.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2865,84 +2865,6 @@ expression: result
"uniqueness_constraints": {},
"foreign_keys": {}
},
{
"name": "delete_playlist_track",
"arguments": {
"track_id": {
"type": {
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "int4"
}
}
}
},
"type": "delete_playlist_track",
"uniqueness_constraints": {},
"foreign_keys": {}
},
{
"name": "insert_album",
"arguments": {
"artist_id": {
"type": {
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "int4"
}
}
},
"id": {
"type": {
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "int4"
}
}
},
"title": {
"type": {
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "varchar"
}
}
}
},
"type": "insert_album",
"uniqueness_constraints": {},
"foreign_keys": {}
},
{
"name": "insert_artist",
"arguments": {
"id": {
"type": {
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "int4"
}
}
},
"name": {
"type": {
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "varchar"
}
}
}
},
"type": "insert_artist",
"uniqueness_constraints": {},
"foreign_keys": {}
},
{
"name": "value_types",
"arguments": {
Expand Down Expand Up @@ -3097,5 +3019,87 @@ expression: result
}
],
"functions": [],
"procedures": []
"procedures": [
{
"name": "delete_playlist_track",
"arguments": {
"track_id": {
"type": {
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "int4"
}
}
}
},
"result_type": {
"type": "named",
"name": "delete_playlist_track"
}
},
{
"name": "insert_album",
"arguments": {
"artist_id": {
"type": {
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "int4"
}
}
},
"id": {
"type": {
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "int4"
}
}
},
"title": {
"type": {
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "varchar"
}
}
}
},
"result_type": {
"type": "named",
"name": "insert_album"
}
},
{
"name": "insert_artist",
"arguments": {
"id": {
"type": {
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "int4"
}
}
},
"name": {
"type": {
"type": "nullable",
"underlying_type": {
"type": "named",
"name": "varchar"
}
}
}
},
"result_type": {
"type": "named",
"name": "insert_artist"
}
}
]
}
Loading