From 322a6345492568f0948fc31b8a67ed7702840fe0 Mon Sep 17 00:00:00 2001 From: Zaid Date: Thu, 27 Aug 2020 00:43:01 +0200 Subject: [PATCH] Determining parameter nullability for INSERT queries --- .../InformationSchema.fs | 6 +- .../NpgsqlFSharpAnalyzer.Core.fsproj | 3 + src/NpgsqlFSharpAnalyzer.Core/SqlAnalysis.fs | 71 +++++++++++++++++-- tests/NpgsqlFSharpAnalyzer.Tests/Tests.fs | 48 +++++++++++++ tests/examples/hashing/examples.fsproj | 2 + .../insertQueryWithNonNullableParameter.fs | 9 +++ tests/examples/hashing/limitExpressionTest.fs | 9 +++ 7 files changed, 140 insertions(+), 8 deletions(-) create mode 100644 tests/examples/hashing/insertQueryWithNonNullableParameter.fs create mode 100644 tests/examples/hashing/limitExpressionTest.fs diff --git a/src/NpgsqlFSharpAnalyzer.Core/InformationSchema.fs b/src/NpgsqlFSharpAnalyzer.Core/InformationSchema.fs index c3d1c33..cb51fc0 100644 --- a/src/NpgsqlFSharpAnalyzer.Core/InformationSchema.fs +++ b/src/NpgsqlFSharpAnalyzer.Core/InformationSchema.fs @@ -186,7 +186,8 @@ module InformationSchema = Precision: byte Scale : byte Optional: bool - DataType: DataType } + DataType: DataType + IsNullable : bool } with member this.Size = this.MaxLength @@ -249,7 +250,8 @@ module InformationSchema = Precision = p.Precision Scale = p.Scale Optional = allParametersOptional - DataType = DataType.Create(p.PostgresType) } ] + DataType = DataType.Create(p.PostgresType) + IsNullable = true } ] let enums = outputColumns diff --git a/src/NpgsqlFSharpAnalyzer.Core/NpgsqlFSharpAnalyzer.Core.fsproj b/src/NpgsqlFSharpAnalyzer.Core/NpgsqlFSharpAnalyzer.Core.fsproj index 207ccee..3581f13 100644 --- a/src/NpgsqlFSharpAnalyzer.Core/NpgsqlFSharpAnalyzer.Core.fsproj +++ b/src/NpgsqlFSharpAnalyzer.Core/NpgsqlFSharpAnalyzer.Core.fsproj @@ -16,4 +16,7 @@ + + + diff --git a/src/NpgsqlFSharpAnalyzer.Core/SqlAnalysis.fs b/src/NpgsqlFSharpAnalyzer.Core/SqlAnalysis.fs index 5d577f1..49be10e 100644 --- a/src/NpgsqlFSharpAnalyzer.Core/SqlAnalysis.fs +++ b/src/NpgsqlFSharpAnalyzer.Core/SqlAnalysis.fs @@ -3,12 +3,63 @@ namespace Npgsql.FSharp.Analyzers.Core open System open FSharp.Compiler.Range open F23.StringSimilarity +open NpgsqlFSharpParser +open InformationSchema module SqlAnalysis = + + let determineParameterNullability (parameters: Parameter list) (schema: DbSchemaLookups) query = + match Parser.parse query with + | Result.Error error -> parameters + | Result.Ok (Expr.SelectQuery query) -> + match query.Limit with + | None -> parameters + | Some (Expr.Parameter(name)) -> + let parameter = name.TrimStart('@') + parameters + |> List.map (fun param -> + if param.Name = parameter + then { param with IsNullable = false } + else param + ) + | Some otherExpr -> + parameters + + | Result.Ok (Expr.InsertQuery query) -> + + let findParameter (expr, column) = + match expr with + | Expr.Parameter parameterName -> + Some (parameterName.TrimStart '@', column) + | _ -> + None + + let insertParameters = + query.Columns + |> List.zip query.Values + |> List.choose findParameter + + let columnNonNullable columnName = + schema.Columns + |> Seq.exists (fun column -> column.Value.Name = columnName && not column.Value.Nullable) + + parameters + |> List.map (fun requiredParameter -> + insertParameters + |> List.tryFind (fun (parameter, column) -> parameter = requiredParameter.Name && columnNonNullable column) + |> function + | None -> requiredParameter + | Some _ -> { requiredParameter with IsNullable = false } + ) + + | Result.Ok expr -> + parameters + let extractParametersAndOutputColumns(connectionString, commandText, dbSchemaLookups) = try let parameters, output, enums = InformationSchema.extractParametersAndOutputColumns(connectionString, commandText, false, dbSchemaLookups) - Result.Ok (parameters, output) + let enrichedParameters = determineParameterNullability parameters dbSchemaLookups commandText + Result.Ok (enrichedParameters, output) with | ex -> Result.Error ex.Message @@ -166,8 +217,12 @@ module SqlAnalysis = if providedParam.paramFunc <> "Sql.int16" && providedParam.paramFunc <> "Sql.int16OrNone" && providedParam.paramFunc <> "Sql.dbnull" then yield typeMismatch [ "Sql.int16"; "Sql.int16OrNone"; "Sql.dbnull" ] | ("int64" | "bigint" |"bigserial") -> - if providedParam.paramFunc <> "Sql.int64" && providedParam.paramFunc <> "Sql.int64OrNone" && providedParam.paramFunc <> "Sql.dbnull" - then yield typeMismatch [ "Sql.int64"; "Sql.int64OrNone"; "Sql.dbnull" ] + if requiredParam.IsNullable then + if providedParam.paramFunc <> "Sql.int64" && providedParam.paramFunc <> "Sql.int" && providedParam.paramFunc <> "Sql.int64OrNone" && providedParam.paramFunc <> "Sql.dbnull" + then yield typeMismatch [ "Sql.int64"; "Sql.int"; "Sql.int64OrNone"; "Sql.dbnull" ] + else + if providedParam.paramFunc <> "Sql.int64" && providedParam.paramFunc <> "Sql.int" + then yield typeMismatch [ "Sql.int64"; "Sql.int" ] | ("numeric" | "decimal" | "money") -> if providedParam.paramFunc <> "Sql.decimal" && providedParam.paramFunc <> "Sql.decimalOrNone" && providedParam.paramFunc <> "Sql.dbnull" then yield typeMismatch [ "Sql.decimal"; "Sql.decimalOrNone"; "Sql.dbnull" ] @@ -195,8 +250,12 @@ module SqlAnalysis = then yield typeMismatch [ "Sql.jsonb"; "Sql.jsonbOrNone"; "Sql.dbnull" ] | ("text"|"json"|"xml") -> - if providedParam.paramFunc <> "Sql.text" && providedParam.paramFunc <> "Sql.textOrNone" && providedParam.paramFunc <> "Sql.string" && providedParam.paramFunc <> "Sql.textOrNone" && providedParam.paramFunc <> "Sql.dbnull" - then yield typeMismatch [ "Sql.text"; "Sql.string"; "Sql.textOrNone"; "Sql.stringOrNone"; "Sql.dbnull" ] + if requiredParam.IsNullable then + if providedParam.paramFunc <> "Sql.text" && providedParam.paramFunc <> "Sql.textOrNone" && providedParam.paramFunc <> "Sql.string" && providedParam.paramFunc <> "Sql.textOrNone" && providedParam.paramFunc <> "Sql.dbnull" + then yield typeMismatch [ "Sql.text"; "Sql.string"; "Sql.textOrNone"; "Sql.stringOrNone"; "Sql.dbnull" ] + else + if providedParam.paramFunc <> "Sql.text" && providedParam.paramFunc <> "Sql.string" + then yield typeMismatch [ "Sql.text"; "Sql.string" ] | _ -> () else @@ -508,7 +567,7 @@ module SqlAnalysis = match queryAnalysis with | Result.Error queryError -> [ createWarning queryError queryRange ] - | Result.Ok (parameters, outputColunms) -> + | Result.Ok (parameters, outputColunms) -> let readingAttempts = defaultArg (findColumnReadAttempts operation) [ ] [ yield! analyzeParameters operation parameters diff --git a/tests/NpgsqlFSharpAnalyzer.Tests/Tests.fs b/tests/NpgsqlFSharpAnalyzer.Tests/Tests.fs index b0ce051..9bc5d45 100644 --- a/tests/NpgsqlFSharpAnalyzer.Tests/Tests.fs +++ b/tests/NpgsqlFSharpAnalyzer.Tests/Tests.fs @@ -421,6 +421,54 @@ let tests = failwith "Expected only one error message" } + test "SQL query semantic analysis: delect incorrectly used nullable parameter on LIMIT expression" { + use db = createTestDatabase() + + Sql.connect db.ConnectionString + |> Sql.query "CREATE TABLE users (user_id bigserial primary key, username text not null)" + |> Sql.executeNonQuery + |> ignore + + match context (find "../examples/hashing/limitExpressionTest.fs") with + | None -> failwith "Could not crack project" + | Some context -> + match SqlAnalysis.databaseSchema db.ConnectionString with + | Result.Error connectionError -> + failwith connectionError + | Result.Ok schema -> + let operation = List.exactlyOne (SyntacticAnalysis.findSqlOperations context) + let messages = SqlAnalysis.analyzeOperation operation db.ConnectionString schema + match messages with + | [ message ] -> + Expect.stringContains message.Message "Sql.int64 or Sql.int" "Message contains suggestion to use Sql.stringArray" + | _ -> + failwith "Expected only one error message" + } + + test "SQL query semantic analysis: delect incorrectly used nullable parameter on non-nullable column insert" { + use db = createTestDatabase() + + Sql.connect db.ConnectionString + |> Sql.query "CREATE TABLE users (user_id bigserial primary key, username text not null, active boolean not null)" + |> Sql.executeNonQuery + |> ignore + + match context (find "../examples/hashing/insertQueryWithNonNullableParameter.fs") with + | None -> failwith "Could not crack project" + | Some context -> + match SqlAnalysis.databaseSchema db.ConnectionString with + | Result.Error connectionError -> + failwith connectionError + | Result.Ok schema -> + let operation = List.exactlyOne (SyntacticAnalysis.findSqlOperations context) + let messages = SqlAnalysis.analyzeOperation operation db.ConnectionString schema + match messages with + | [ message ] -> + Expect.stringContains message.Message "Sql.text or Sql.string" "Message contains suggestion to use Sql.stringArray" + | _ -> + failwith "Expected only one error message" + } + test "SQL query semantic analysis: type mismatch with integer/serial" { use db = createTestDatabase() diff --git a/tests/examples/hashing/examples.fsproj b/tests/examples/hashing/examples.fsproj index 41360bb..d209d8c 100644 --- a/tests/examples/hashing/examples.fsproj +++ b/tests/examples/hashing/examples.fsproj @@ -5,6 +5,8 @@ + + diff --git a/tests/examples/hashing/insertQueryWithNonNullableParameter.fs b/tests/examples/hashing/insertQueryWithNonNullableParameter.fs new file mode 100644 index 0000000..747cc1d --- /dev/null +++ b/tests/examples/hashing/insertQueryWithNonNullableParameter.fs @@ -0,0 +1,9 @@ +module InsertQueryWithNonNullableParameter + +open Npgsql.FSharp + +let findUsers connection = + connection + |> Sql.query "INSERT INTO users (username, active) VALUES (@username, @active) RETURNING *" + |> Sql.parameters [ "@username", Sql.dbnull; "@active", Sql.bool true ] + |> Sql.execute (fun read -> read.text "username") diff --git a/tests/examples/hashing/limitExpressionTest.fs b/tests/examples/hashing/limitExpressionTest.fs new file mode 100644 index 0000000..1a770e0 --- /dev/null +++ b/tests/examples/hashing/limitExpressionTest.fs @@ -0,0 +1,9 @@ +module LimitExpressionTest + +open Npgsql.FSharp + +let findUsers connection = + connection + |> Sql.query "SELECT * FROM users LIMIT @numberOfUsers" + |> Sql.parameters [ "@numberOfUsers", Sql.int64OrNone (Some 42L) ] + |> Sql.execute (fun read -> read.text "username")