Merge pull request #878 from Thorium/fixed-issue

Fix for #877 IndexOutOfRangeException

Author: Thorium

Directory.Build.props

@@ -1,7 +1,7 @@
 <Project>
     <PropertyGroup>
         <!-- summary is not migrated from project.json, but you can use the <Description> property for that if needed. -->
-        <PackageTags>f#, fsharp</PackageTags>
+        <PackageTags>sql, database, typeprovider, f#, fsharp, dotnet, orm, linq</PackageTags>
         <PackageProjectUrl>https://github.com/fsprojects/SQLProvider</PackageProjectUrl>
         <PackageLicenseUrl>https://github.com/fsprojects/SQLProvider/blob/master/LICENSE.txt</PackageLicenseUrl>
         <PackageRequireLicenseAcceptance>false</PackageRequireLicenseAcceptance>

paket.dependencies

@@ -26,7 +26,8 @@ nuget Microsoft.Extensions.Logging.Abstractions 8.0.3
 group Build
 
     source https://api.nuget.org/v3/index.json
-    storage: none
+    // build.fsx references packages/build/* directly, so this group needs the packages folder
+    storage: packages
     framework: net6.0
 
     nuget Microsoft.SourceLink.GitHub prerelease copy_local: true

src/SQLProvider.Common/SqlSchema.fs

@@ -58,6 +58,14 @@ type QueryParameter =
               Direction = defaultArg direction ParameterDirection.Input
               Length = match length with Some x -> ValueSome x | None -> ValueNone }
 
+/// Helpers for stored procedure parameter lists.
+module SprocParameters =
+    /// Drops repeated named parameters (same name and direction), keeping the first occurrence.
+    /// Unnamed parameters (e.g. a function's return value) are always kept.
+    let distinctNamed (parameters: QueryParameter list) =
+        let seen = System.Collections.Generic.HashSet<struct(string * ParameterDirection)>()
+        parameters |> List.filter (fun p -> String.IsNullOrEmpty p.Name || seen.Add(struct(p.Name, p.Direction)))
+
 /// Represents a database table column with its metadata and characteristics.
 /// Contains all information needed to generate typed properties and handle data conversion.
 [<Struct>]

src/SQLProvider.DesignTime/SqlDesignTime.fs

@@ -124,12 +124,13 @@ module DesignTimeUtils =
             match con with
             | Some con ->
                 let returnParams = sprocDefinition.ReturnColumns con param
-                prov.GetSchemaCache().SprocsParams.AddOrUpdate(sprocname, returnParams, fun _ inputParams -> inputParams @ returnParams) |> ignore
+                // InputOutput parameters are also return columns, so don't store them twice
+                prov.GetSchemaCache().SprocsParams.AddOrUpdate(sprocname, returnParams, fun _ inputParams -> SprocParameters.distinctNamed (inputParams @ returnParams)) |> ignore
                 returnParams
             | None ->
                 let ok, pars = prov.GetSchemaCache().SprocsParams.TryGetValue sprocname
                 if ok then
-                    pars |> List.filter (fun p ->
+                    pars |> SprocParameters.distinctNamed |> List.filter (fun p ->
                         not (p.Name |> String.IsNullOrWhiteSpace)
                         && (p.Direction = ParameterDirection.Output
                         || p.Direction = ParameterDirection.InputOutput
@@ -323,7 +324,8 @@ module DesignTimeUtils =
                         match con with
                         | None ->
                             match prov.GetSchemaCache().SprocsParams.TryGetValue sprocname with
-                            | true, x -> x
+                            // Schema files saved before de-duplication may list InputOutput parameters twice
+                            | true, x -> SprocParameters.distinctNamed x
                             | false, _ -> []
                         | Some con ->
                             (lazy

src/SQLProvider.Runtime/Providers.MsSqlServer.Dynamic.fs

@@ -365,7 +365,9 @@ module MSSqlServerDynamic =
 
         let inps =
              inputParameters
-             |> Array.filter (fun p -> p.Direction <> ParameterDirection.ReturnValue)
+             // Values are supplied only for Input/InputOutput parameters (the generated Invoke signature);
+             // an offline ContextSchema can also list result-set and return-value columns here.
+             |> Array.filter (fun p -> p.Direction = ParameterDirection.Input || p.Direction = ParameterDirection.InputOutput)
              |> Array.mapi(fun i ip ->
                  let p = createCommandParameter ip values.[i]
                  (ip.Ordinal, ip.Name, p))

src/SQLProvider.Runtime/Providers.MsSqlServer.fs

@@ -288,7 +288,9 @@ module MSSqlServer =
 
         let inps =
              inputParameters
-             |> Array.filter (fun p -> p.Direction <> ParameterDirection.ReturnValue)
+             // Values are supplied only for Input/InputOutput parameters (the generated Invoke signature);
+             // an offline ContextSchema can also list result-set and return-value columns here.
+             |> Array.filter (fun p -> p.Direction = ParameterDirection.Input || p.Direction = ParameterDirection.InputOutput)
              |> Array.mapi(fun i ip ->
                  let p = createCommandParameter ip values.[i]
                  (ip.Ordinal, ip.Name, p))

tests/SqlProvider.Tests/SprocParameterTests.fs

@@ -0,0 +1,66 @@
+module SprocParameterTests
+
+// Issue #877: stored procedures compiled against an offline ContextSchema
+// have result-set / return-value columns in their parameter list.
+
+open System.Data
+open FSharp.Data.Sql.Schema
+open FSharp.Data.Sql.Providers
+open NUnit.Framework
+
+let private intType = TypeMapping.Create(typeof<int>.ToString(), DbType.Int32)
+let private cursorType = TypeMapping.Create(typeof<obj>.ToString(), DbType.Object, "cursor")
+
+let private param name ordinal typeMapping direction =
+    QueryParameter.Create(name, ordinal, typeMapping, direction)
+
+[<Test>]
+let ``MSSQL sproc call ignores result set columns from an offline schema``() =
+    let resultSet = param "ResultSet" 0 cursorType ParameterDirection.Output
+    // Offline the cached parameter list has the result set appended to the real parameters
+    let inputParameters =
+        [| param "@a" 1 intType ParameterDirection.Input
+           param "@b" 2 intType ParameterDirection.InputOutput
+           resultSet |]
+    let values = [| box 1; box 2 |]
+
+    let allParams, outps = MSSqlServer.executeSprocCommandCommon inputParameters [| resultSet |] values
+
+    let names = allParams |> Array.map (fun (_,name,_) -> name)
+    Assert.AreEqual([| "@a"; "@b" |], names)
+    let values = allParams |> Array.map (fun (_,_,p) -> p.Value)
+    Assert.AreEqual([| box 1; box 2 |], values)
+    Assert.AreEqual(1, outps.Length)
+
+[<Test>]
+let ``MSSQL function call passes the return value parameter``() =
+    let returnValue = param "ReturnValue" 0 intType ParameterDirection.ReturnValue
+    let inputParameters =
+        [| param "" 0 intType ParameterDirection.ReturnValue
+           param "@a" 1 intType ParameterDirection.Input
+           returnValue |]
+
+    let allParams, _ = MSSqlServer.executeSprocCommandCommon inputParameters [| returnValue |] [| box 5 |]
+
+    let names = allParams |> Array.map (fun (_,_,p) -> (p :?> IDataParameter).ParameterName)
+    Assert.AreEqual([| "@RETURN_VALUE"; "@a" |], names)
+
+[<Test>]
+let ``distinctNamed drops repeated named parameters only``() =
+    let parameters =
+        [ param "" 0 intType ParameterDirection.ReturnValue
+          param "@a" 1 intType ParameterDirection.Input
+          param "@b" 2 intType ParameterDirection.InputOutput
+          param "ResultSet" 0 cursorType ParameterDirection.Output
+          param "ReturnValue" 0 intType ParameterDirection.ReturnValue
+          param "@b" 2 intType ParameterDirection.InputOutput ]
+
+    let result = SprocParameters.distinctNamed parameters
+
+    Assert.AreEqual(
+        [ "", ParameterDirection.ReturnValue
+          "@a", ParameterDirection.Input
+          "@b", ParameterDirection.InputOutput
+          "ResultSet", ParameterDirection.Output
+          "ReturnValue", ParameterDirection.ReturnValue ],
+        result |> List.map (fun p -> p.Name, p.Direction))

tests/SqlProvider.Tests/SqlProvider.Tests.fsproj

@@ -32,6 +32,7 @@
     <Compile Include="more\ValueOptionTests.fs" />
     <Compile Include="QueryTests.fs" />
     <Compile Include="CrudTests.fs" />
+    <Compile Include="SprocParameterTests.fs" />
 	<!--<Compile Include="PostgreSQLTests.fs" />-->
     <!-- MSSQL tests need a live SQL Server at design time; only built for net48. -->
     <Compile Include="SqlServerTests.fs" Condition="'$(TargetFramework)' == 'net48'" />