Skip to content

Support complex & array types in native query creation cli utility #742

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

Open
BenoitRanque wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Support complex & array types in native query creation cli utility #742

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
042e727
Support complex & array types in native query creation cli utility
BenoitRanque Apr 3, 2025
dc7fb6b
add changelog
BenoitRanque Apr 3, 2025
b4aa1d2
Add tests for array args and failing test for nested array args
BenoitRanque May 8, 2025
d9b9b6a
update test and add comment re: multi dimensional array being identic…
BenoitRanque May 8, 2025
96005c7
Add missing handling for composite types for config v4
BenoitRanque May 8, 2025
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
37 changes: 20 additions & 17 deletions crates/configuration/src/version4/native_operations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ pub async fn oids_to_typenames(
configuration: &super::ParsedConfiguration,
connection_string: &str,
oids: &Vec<i64>,
) -> Result<BTreeMap<i64, models::ScalarTypeName>, sqlx::Error> {
) -> Result<BTreeMap<i64, models::TypeName>, sqlx::Error> {
let mut connection = PgConnection::connect(connection_string)
.instrument(info_span!("Connect to database"))
.await?;
Expand All @@ -216,37 +216,40 @@ pub async fn oids_to_typenames(
.instrument(info_span!("Run oid lookup query"))
.await?;

let mut oids_map: BTreeMap<i64, models::ScalarTypeName> = BTreeMap::new();
let mut oids_map: BTreeMap<i64, models::TypeName> = BTreeMap::new();

// Reverse lookup the schema.typename and find the ndc type name,
// if we find all we can just add the nq and call it a day.
for row in rows {
'rows: for row in rows {
let schema_name: String = row.schema_name;
let type_name: String = row.type_name;
let oid: i64 = row.oid;

let mut found = false;
for (scalar_type_name, info) in &configuration.metadata.scalar_types.0 {
if info.schema_name == schema_name && info.type_name == type_name {
oids_map.insert(oid, scalar_type_name.clone());
found = true;
continue;
oids_map.insert(oid, scalar_type_name.inner().clone());
continue 'rows;
}
}

for (composite_type_name, info) in &configuration.metadata.composite_types.0 {
Copy link
Contributor Author

@BenoitRanque BenoitRanque May 8, 2025

Choose a reason for hiding this comment

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

This section was missing, and since v4 does have composite types I think this was a mistake

if info.schema_name == schema_name && info.type_name == type_name {
oids_map.insert(oid, composite_type_name.clone());
continue 'rows;
}
}

// If we don't find it we generate a name which is either schema_typename
// or just typename depending if the schema is in the unqualified list or not,
// then add the nq and run the introspection.
if !found {
if configuration
.introspection_options
.unqualified_schemas_for_types_and_procedures
.contains(&schema_name)
{
oids_map.insert(oid, type_name.into());
} else {
oids_map.insert(oid, format!("{schema_name}_{type_name}").into());
}
if configuration
.introspection_options
.unqualified_schemas_for_types_and_procedures
.contains(&schema_name)
{
oids_map.insert(oid, type_name.into());
} else {
oids_map.insert(oid, format!("{schema_name}_{type_name}").into());
}
}

Expand Down
27 changes: 11 additions & 16 deletions crates/configuration/src/version5/native_operations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,40 +225,35 @@ pub async fn oids_to_typenames(

// Reverse lookup the schema.typename and find the ndc type name,
// if we find all we can just add the nq and call it a day.
for row in rows {
'rows: for row in rows {
let schema_name: String = row.schema_name;
let type_name: String = row.type_name;
let oid: i64 = row.oid;

let mut found = false;
for (scalar_type_name, info) in &configuration.metadata.types.scalar.0 {
if info.schema_name == schema_name && info.type_name == type_name {
oids_map.insert(oid, scalar_type_name.inner().clone());
found = true;
continue;
Copy link
Contributor Author

@BenoitRanque BenoitRanque May 8, 2025

Choose a reason for hiding this comment

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

This continue statement did nothing.

There's a couple options for intent:

  • either this was meant to be a break, to exit the inner loop on first match
  • or, the intent was to proceed to the next iteration of the outer loop

I think the intent was the latter, so

  • added label to loop
  • continue using label so the right loop is continued
  • removed found since that becomes redundant

We could, alternatively, use break, and keep found if we think that's more readable?

Copy link
Contributor Author

@BenoitRanque BenoitRanque May 8, 2025

Choose a reason for hiding this comment

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

Note that using break would mean we'd iterate over composite types always, even if we did find our type as a scalar

Using continue 'rows I think better matches the intent here:

  • iterate over rows
  • for each row, try to find a matching scalar type and insert that to the map
  • if no scalar found, try to find a matching composite
  • if neither found, use type name as is

continue 'rows;
}
}
for (composite_type_name, info) in &configuration.metadata.types.composite.0 {
if info.schema_name == schema_name && info.type_name == type_name {
oids_map.insert(oid, composite_type_name.clone());
found = true;
continue;
continue 'rows;
}
}

// If we don't find it we generate a name which is either schema_typename
// or just typename depending if the schema is in the unqualified list or not,
// then add the nq and run the introspection.
if !found {
if configuration
.introspection_options
.unqualified_schemas_for_types_and_procedures
.contains(&schema_name)
{
oids_map.insert(oid, type_name.into());
} else {
oids_map.insert(oid, format!("{schema_name}_{type_name}").into());
}
if configuration
.introspection_options
.unqualified_schemas_for_types_and_procedures
.contains(&schema_name)
{
oids_map.insert(oid, type_name.into());
} else {
oids_map.insert(oid, format!("{schema_name}_{type_name}").into());
}
}

Expand Down
Loading