Determining parameter nullability for INSERT queries

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

src/NpgsqlFSharpAnalyzer.Core/NpgsqlFSharpAnalyzer.Core.fsproj

@@ -16,4 +16,7 @@
         <PackageReference Include="FSharp.Compiler.Service" Version="36.0.3" />
         <PackageReference Include="Npgsql" Version="4.1.3" />
     </ItemGroup>
+    <ItemGroup>
+      <ProjectReference Include="..\NpgsqlFSharpParser\NpgsqlFSharpParser.fsproj" />
+    </ItemGroup>
 </Project>

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

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()
 

tests/examples/hashing/examples.fsproj

@@ -5,6 +5,8 @@
   </PropertyGroup>
 
   <ItemGroup>
+    <Compile Include="insertQueryWithNonNullableParameter.fs" />
+    <Compile Include="limitExpressionTest.fs" />
     <Compile Include="executingQueryInsteadOfNonQuery.fs" />
     <Compile Include="usingIntArrayParameter.fs" />
     <Compile Include="readingUuidArray.fs" />

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")

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")