From 7aa47be22c2d99d6046df04a5b9bb29ab9cb97f1 Mon Sep 17 00:00:00 2001 From: Flavian Desverne Date: Fri, 23 Aug 2024 15:32:44 +0200 Subject: [PATCH 1/3] feat(typedsql): support scalar list in `@param` docs --- .../src/sql_doc_parser.rs | 133 +++++++++++++++--- .../tests/query_introspection/docs.rs | 42 +++++- 2 files changed, 146 insertions(+), 29 deletions(-) diff --git a/schema-engine/connectors/sql-schema-connector/src/sql_doc_parser.rs b/schema-engine/connectors/sql-schema-connector/src/sql_doc_parser.rs index a7f9c4912d24..c52a3f09bbd3 100644 --- a/schema-engine/connectors/sql-schema-connector/src/sql_doc_parser.rs +++ b/schema-engine/connectors/sql-schema-connector/src/sql_doc_parser.rs @@ -128,6 +128,10 @@ impl<'a> Input<'a> { self.0.strip_suffix(pat).map(Self) } + fn strip_suffix_str(&self, pat: &str) -> Option { + self.0.strip_suffix(pat).map(Self) + } + fn starts_with(&self, pat: &str) -> bool { self.0.starts_with(pat) } @@ -199,35 +203,37 @@ fn parse_typ_opt<'a>( input: Input<'a>, enum_names: &'a [String], ) -> ConnectorResult<(Input<'a>, Option>)> { + fn list_accepted_types(enum_names: &[String]) -> String { + format!( + "'Int', 'BigInt', 'Float', 'Boolean', 'String', 'DateTime', 'Json', 'Bytes', 'Decimal'{}", + render_enum_names(enum_names) + ) + } + if let Some(start) = input.find(&['{']) { if let Some(end) = input.find(&['}']) { let typ = input.move_between(start + 1, end); if typ.is_empty() { - return Err(build_error(input, "missing type (accepted types are: 'Int', 'BigInt', 'Float', 'Boolean', 'String', 'DateTime', 'Json', 'Bytes', 'Decimal')")); + return Err(build_error( + input, + &format!("missing type (accepted types are: {})", list_accepted_types(enum_names)), + )); } - let parsed_typ = ScalarType::try_from_str(typ.inner(), false) - .map(|st| match st { - ScalarType::Int => ColumnType::Int32, - ScalarType::BigInt => ColumnType::Int64, - ScalarType::Float => ColumnType::Float, - ScalarType::Boolean => ColumnType::Boolean, - ScalarType::String => ColumnType::Text, - ScalarType::DateTime => ColumnType::DateTime, - ScalarType::Json => ColumnType::Json, - ScalarType::Bytes => ColumnType::Bytes, - ScalarType::Decimal => ColumnType::Numeric, - }) - .map(ParsedParamType::ColumnType) - .or_else(|| { - enum_names.iter().any(|enum_name| *enum_name == typ.inner()) - .then(|| ParsedParamType::Enum(typ.inner())) - }) - .ok_or_else(|| build_error( - input, - &format!("invalid type: '{typ}' (accepted types are: 'Int', 'BigInt', 'Float', 'Boolean', 'String', 'DateTime', 'Json', 'Bytes', 'Decimal'{})", render_enum_names(enum_names)), - ))?; + let parsed_typ = typ + .strip_suffix_str("[]") + .and_then(|typ| parse_typ_name(typ, true, enum_names)) + .or_else(|| parse_typ_name(typ, false, enum_names)) + .ok_or_else(|| { + build_error( + input, + &format!( + "invalid type: '{typ}' (accepted types are: {})", + list_accepted_types(enum_names) + ), + ) + })?; Ok((input.move_from(end + 1), Some(parsed_typ))) } else { @@ -238,6 +244,38 @@ fn parse_typ_opt<'a>( } } +fn parse_typ_name<'a>(typ: Input<'a>, list: bool, enum_names: &[String]) -> Option> { + ScalarType::try_from_str(typ.inner(), false) + .map(|st| match (st, list) { + (ScalarType::Int, false) => ColumnType::Int32, + (ScalarType::BigInt, false) => ColumnType::Int64, + (ScalarType::Float, false) => ColumnType::Float, + (ScalarType::Boolean, false) => ColumnType::Boolean, + (ScalarType::String, false) => ColumnType::Text, + (ScalarType::DateTime, false) => ColumnType::DateTime, + (ScalarType::Json, false) => ColumnType::Json, + (ScalarType::Bytes, false) => ColumnType::Bytes, + (ScalarType::Decimal, false) => ColumnType::Numeric, + + (ScalarType::Int, true) => ColumnType::Int32Array, + (ScalarType::BigInt, true) => ColumnType::Int64Array, + (ScalarType::Float, true) => ColumnType::FloatArray, + (ScalarType::Boolean, true) => ColumnType::BooleanArray, + (ScalarType::String, true) => ColumnType::TextArray, + (ScalarType::DateTime, true) => ColumnType::DateTimeArray, + (ScalarType::Json, true) => ColumnType::JsonArray, + (ScalarType::Bytes, true) => ColumnType::BytesArray, + (ScalarType::Decimal, true) => ColumnType::NumericArray, + }) + .map(ParsedParamType::ColumnType) + .or_else(|| { + enum_names + .iter() + .any(|enum_name| *enum_name == typ.inner()) + .then(|| ParsedParamType::Enum(typ.inner())) + }) +} + fn parse_position_opt(input: Input<'_>) -> ConnectorResult<(Input<'_>, Option)> { if let Some((param_input, param_pos)) = input .trim_start() @@ -926,6 +964,57 @@ mod tests { expected.assert_debug_eq(&res); } + #[test] + fn parse_param_22() { + use expect_test::expect; + + let res = parse_param(Input("@param {Int[]} $12"), &[]); + + let expected = expect![[r#" + Ok( + ParsedParameterDoc { + alias: None, + typ: Some( + ColumnType( + Int32Array, + ), + ), + nullable: None, + position: Some( + 12, + ), + documentation: None, + }, + ) + "#]]; + + expected.assert_debug_eq(&res); + } + + #[test] + fn parse_param_23() { + use expect_test::expect; + + let res = parse_param(Input("@param {Unknown[]} $12"), &[]); + + let expected = expect![[r#" + Err( + ConnectorErrorImpl { + user_facing_error: None, + message: Some( + "SQL documentation parsing: invalid type: 'Unknown[]' (accepted types are: 'Int', 'BigInt', 'Float', 'Boolean', 'String', 'DateTime', 'Json', 'Bytes', 'Decimal') at '{Unknown[]} $12'.", + ), + source: None, + context: SpanTrace [], + } + SQL documentation parsing: invalid type: 'Unknown[]' (accepted types are: 'Int', 'BigInt', 'Float', 'Boolean', 'String', 'DateTime', 'Json', 'Bytes', 'Decimal') at '{Unknown[]} $12'. + , + ) + "#]]; + + expected.assert_debug_eq(&res); + } + #[test] fn parse_sql_1() { use expect_test::expect; diff --git a/schema-engine/sql-migration-tests/tests/query_introspection/docs.rs b/schema-engine/sql-migration-tests/tests/query_introspection/docs.rs index e7fe7bc630c9..e8bc09b5d96a 100644 --- a/schema-engine/sql-migration-tests/tests/query_introspection/docs.rs +++ b/schema-engine/sql-migration-tests/tests/query_introspection/docs.rs @@ -143,7 +143,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: "\n -- @param {MyFancyEnum} $1\n SELECT 1 as \"col\" WHERE 1 = $1;\n ", documentation: None, parameters: [ IntrospectSqlQueryParameterOutput { @@ -155,22 +155,50 @@ fn parses_doc_enum_name(api: TestApi) { ], result_columns: [ IntrospectSqlQueryColumnOutput { - name: "id", + name: "col", typ: "int", + nullable: true, + }, + ], + } + "#]]; + + let sql = r#" + -- @param {MyFancyEnum} $1 + SELECT 1 as "col" WHERE 1 = ?; + "#; + + api.introspect_sql("test_1", sql).send_sync().expect_result(expected) +} + +#[test_connector(tags(Postgres))] +fn parses_doc_array(api: TestApi) { + let expected = expect![[r#" + IntrospectSqlQueryOutput { + name: "test_1", + source: "\n -- @param {Int[]} $1:myIntArray\n SELECT 1 as \"col\" WHERE 1 = $1;\n ", + documentation: None, + parameters: [ + IntrospectSqlQueryParameterOutput { + documentation: None, + name: "myIntArray", + typ: "int-array", nullable: false, }, + ], + result_columns: [ IntrospectSqlQueryColumnOutput { - name: "enum", - typ: "MyFancyEnum", - nullable: false, + name: "col", + typ: "int", + nullable: true, }, ], } "#]]; let sql = r#" - -- @param {MyFancyEnum} $1 - SELECT * FROM model WHERE id = ?; + -- @param {Int[]} $1:myIntArray + SELECT 1 as "col" WHERE 1 = ?; "#; api.introspect_sql("test_1", sql).send_sync().expect_result(expected) From fe1680e262e29cc73d33c2665661ce4185b36018 Mon Sep 17 00:00:00 2001 From: Flavian Desverne Date: Fri, 23 Aug 2024 16:52:52 +0200 Subject: [PATCH 2/3] fix test --- .../sql-migration-tests/tests/query_introspection/docs.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/schema-engine/sql-migration-tests/tests/query_introspection/docs.rs b/schema-engine/sql-migration-tests/tests/query_introspection/docs.rs index e8bc09b5d96a..42cc969045a2 100644 --- a/schema-engine/sql-migration-tests/tests/query_introspection/docs.rs +++ b/schema-engine/sql-migration-tests/tests/query_introspection/docs.rs @@ -143,7 +143,7 @@ fn parses_doc_enum_name(api: TestApi) { let expected = expect![[r#" IntrospectSqlQueryOutput { name: "test_1", - source: "\n -- @param {MyFancyEnum} $1\n SELECT 1 as \"col\" WHERE 1 = $1;\n ", + source: "\n -- @param {MyFancyEnum} $1\n SELECT 'col' as \"col\" WHERE 1 = $1;\n ", documentation: None, parameters: [ IntrospectSqlQueryParameterOutput { @@ -156,7 +156,7 @@ fn parses_doc_enum_name(api: TestApi) { result_columns: [ IntrospectSqlQueryColumnOutput { name: "col", - typ: "int", + typ: "string", nullable: true, }, ], @@ -165,7 +165,7 @@ fn parses_doc_enum_name(api: TestApi) { let sql = r#" -- @param {MyFancyEnum} $1 - SELECT 1 as "col" WHERE 1 = ?; + SELECT 'col' as "col" WHERE 1 = ?; "#; api.introspect_sql("test_1", sql).send_sync().expect_result(expected) From 808e90b126466645db59adbd34acd4fd5e905427 Mon Sep 17 00:00:00 2001 From: Flavian Desverne Date: Fri, 23 Aug 2024 17:06:37 +0200 Subject: [PATCH 3/3] fix crdb test --- .../sql-migration-tests/tests/query_introspection/docs.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/schema-engine/sql-migration-tests/tests/query_introspection/docs.rs b/schema-engine/sql-migration-tests/tests/query_introspection/docs.rs index 42cc969045a2..8eac2fa9cc16 100644 --- a/schema-engine/sql-migration-tests/tests/query_introspection/docs.rs +++ b/schema-engine/sql-migration-tests/tests/query_introspection/docs.rs @@ -136,19 +136,19 @@ fn parses_doc_no_alias(api: TestApi) { api.introspect_sql("test_1", sql).send_sync().expect_result(expected) } -#[test_connector(tags(Postgres))] +#[test_connector(tags(Postgres), exclude(CockroachDb))] fn parses_doc_enum_name(api: TestApi) { api.schema_push(ENUM_SCHEMA).send().assert_green(); let expected = expect![[r#" IntrospectSqlQueryOutput { name: "test_1", - source: "\n -- @param {MyFancyEnum} $1\n SELECT 'col' as \"col\" WHERE 1 = $1;\n ", + source: "\n -- @param {MyFancyEnum} $1:alias\n SELECT 'col' as \"col\" WHERE 1 = $1;\n ", documentation: None, parameters: [ IntrospectSqlQueryParameterOutput { documentation: None, - name: "int4", + name: "alias", typ: "MyFancyEnum", nullable: false, }, @@ -164,7 +164,7 @@ fn parses_doc_enum_name(api: TestApi) { "#]]; let sql = r#" - -- @param {MyFancyEnum} $1 + -- @param {MyFancyEnum} $1:alias SELECT 'col' as "col" WHERE 1 = ?; "#;