Skip to content

Commit

Permalink
fix(typedsql): mysql should work with @param notations (#4985)
Browse files Browse the repository at this point in the history
  • Loading branch information
Weakky authored Aug 23, 2024
1 parent 8a3982b commit 0bc306e
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 8 deletions.
7 changes: 4 additions & 3 deletions schema-engine/connectors/sql-schema-connector/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use migration_pair::MigrationPair;
use psl::{datamodel_connector::NativeTypeInstance, parser_database::ScalarType, ValidatedSchema};
use quaint::connector::DescribedQuery;
use schema_connector::{migrations_directory::MigrationDirectory, *};
use sql_doc_parser::parse_sql_doc;
use sql_doc_parser::{parse_sql_doc, sanitize_sql};
use sql_migration::{DropUserDefinedType, DropView, SqlMigration, SqlMigrationStep};
use sql_schema_describer as sql;
use std::{future, sync::Arc};
Expand Down Expand Up @@ -362,11 +362,12 @@ impl SchemaConnector for SqlSchemaConnector {
input: IntrospectSqlQueryInput,
) -> BoxFuture<'_, ConnectorResult<IntrospectSqlQueryOutput>> {
Box::pin(async move {
let sanitized_sql = sanitize_sql(&input.source);
let DescribedQuery {
parameters,
columns,
enum_names,
} = self.flavour.describe_query(&input.source).await?;
} = self.flavour.describe_query(&sanitized_sql).await?;
let enum_names = enum_names.unwrap_or_default();
let sql_source = input.source.clone();
let parsed_doc = parse_sql_doc(&sql_source, enum_names.as_slice())?;
Expand Down Expand Up @@ -397,7 +398,7 @@ impl SchemaConnector for SqlSchemaConnector {

Ok(IntrospectSqlQueryOutput {
name: input.name,
source: input.source,
source: sanitized_sql,
documentation: parsed_doc.description().map(ToOwned::to_owned),
parameters,
result_columns: columns,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,15 @@ pub(crate) fn parse_sql_doc<'a>(sql: &'a str, enum_names: &'a [String]) -> Conne
Ok(parsed_sql)
}

/// Mysql-async poorly parses the sql input to support named parameters, which conflicts with our own syntax for overriding query parameters type and nullability.
/// This function removes all single-line comments from the sql input to avoid conflicts.
pub(crate) fn sanitize_sql(sql: &str) -> String {
sql.lines()
.map(|line| line.trim())
.filter(|line| !line.starts_with("--"))
.join("\n")
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -1107,4 +1116,29 @@ SELECT enum FROM "test_introspect_sql"."model" WHERE id = $1;"#,

expected.assert_debug_eq(&res);
}

#[test]
fn sanitize_sql_test_1() {
use expect_test::expect;

let sql = r#"
-- @description This query returns a user by it's id
-- @param {Int} $1:userId valid user identifier
-- @param {String} $2:parentId valid parent identifier
SELECT enum
FROM
"test_introspect_sql"."model" WHERE id =
$1;
"#;

let expected = expect![[r#"
SELECT enum
FROM
"test_introspect_sql"."model" WHERE id =
$1;
"#]];

expected.assert_eq(&sanitize_sql(sql));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ use super::utils::*;
use sql_migration_tests::test_api::*;

#[test_connector(tags(Postgres))]
fn parses_doc_complex(api: TestApi) {
fn parses_doc_complex_pg(api: TestApi) {
api.schema_push(SIMPLE_SCHEMA).send().assert_green();

let expected = expect![[r#"
IntrospectSqlQueryOutput {
name: "test_1",
source: "\n -- @description some fancy query\n -- @param {Int} $1:myInt some integer\n -- @param {String}$2:myString? some string\n SELECT int FROM model WHERE int = $1 and string = $2;\n ",
source: "\nSELECT int FROM model WHERE int = $1 and string = $2;\n",
documentation: Some(
"some fancy query",
),
Expand Down Expand Up @@ -50,14 +50,70 @@ fn parses_doc_complex(api: TestApi) {
api.introspect_sql("test_1", sql).send_sync().expect_result(expected)
}

#[test_connector(tags(Mysql))]
fn parses_doc_complex_mysql(api: TestApi) {
api.schema_push(SIMPLE_SCHEMA).send().assert_green();

let expected = expect![[r#"
IntrospectSqlQueryOutput {
name: "test_1",
source: "\nSELECT `int` FROM `model` WHERE `int` = ? and `string` = ?;\n",
documentation: Some(
"some fancy query",
),
parameters: [
IntrospectSqlQueryParameterOutput {
documentation: Some(
"some integer",
),
name: "myInt",
typ: "int",
nullable: false,
},
IntrospectSqlQueryParameterOutput {
documentation: Some(
"some string",
),
name: "myString",
typ: "string",
nullable: true,
},
],
result_columns: [
IntrospectSqlQueryColumnOutput {
name: "int",
typ: "int",
nullable: false,
},
],
}
"#]];

let sql = r#"
-- @description some fancy query
-- @param {Int} $1:myInt some integer
-- @param {String}$2:myString? some string
SELECT `int` FROM `model` WHERE `int` = ? and `string` = ?;
"#;

let res = api.introspect_sql("test_1", sql).send_sync();

res.expect_result(expected);

api.query_raw(
&res.output.source,
&[quaint::Value::int32(1), quaint::Value::text("hello")],
);
}

#[test_connector(tags(Sqlite))]
fn parses_doc_no_position(api: TestApi) {
api.schema_push(SIMPLE_SCHEMA).send().assert_green();

let expected = expect![[r#"
IntrospectSqlQueryOutput {
name: "test_1",
source: "\n -- @param {String} :myInt some integer\n SELECT int FROM model WHERE int = :myInt and string = ?;\n ",
source: "\nSELECT int FROM model WHERE int = :myInt and string = ?;\n",
documentation: None,
parameters: [
IntrospectSqlQueryParameterOutput {
Expand Down Expand Up @@ -100,7 +156,7 @@ fn parses_doc_no_alias(api: TestApi) {
let expected = expect![[r#"
IntrospectSqlQueryOutput {
name: "test_1",
source: "\n -- @param {String} $2 some string\n SELECT int FROM model WHERE int = $1 and string = $2;\n ",
source: "\nSELECT int FROM model WHERE int = $1 and string = $2;\n",
documentation: None,
parameters: [
IntrospectSqlQueryParameterOutput {
Expand Down Expand Up @@ -143,7 +199,7 @@ fn parses_doc_enum_name(api: TestApi) {
let expected = expect![[r#"
IntrospectSqlQueryOutput {
name: "test_1",
source: "\n -- @param {MyFancyEnum} $1\n SELECT * FROM model WHERE id = $1;\n ",
source: "\nSELECT * FROM model WHERE id = $1;\n",
documentation: None,
parameters: [
IntrospectSqlQueryParameterOutput {
Expand Down

0 comments on commit 0bc306e

Please sign in to comment.