Introducing parameters' DbType before passing on to the database - MsSql (#1442)

## Why make this change?
Fixes #475
Most column types will accept a string null and the db will
automatically infer our intention, but for varbinary in MsSql the db
complains that it cannot convert varchar to varbinary. For more details
about the issue refer:
https://stackoverflow.com/questions/29254690/why-does-dbnull-value-require-a-proper-sqldbtype

## What is this change?
Changing how we form the `parameters` as we pass them to the database.
Previously we used to only pass the parameter's value. Now, we would
also pass the parameter's `DbType` along with the value as suggested in
the stack overflow article.

**Notes:**
1. We would only be passing the DbType for MsSql.
2. Usually, we haven't seen any need to pass the `DbType` for Sql Server
data types other than `varbinary`. However, we will still be supplying
the DbType for all other parameter's for which we are confirmed about
the 1:1 mapping from SystemType to DbType. Sql server datatypes like
`datetime`, `datetime2`, `datetimeoffset` are all treated as `datetime`
by graphql, so there is no 1:1 mapping for datetime datatypes. So we are
skipping supplying the DbType for these.
3. As explained in this stack overflow article
[here](https://stackoverflow.com/questions/6868300/net-guid-uppercase-string-format),
the GUID when converted to string, is output in lowercase. That's why
when the query to verify the MsSql test `InsertOneInSupportedTypes` is
written, we have explicitly cast `guid_types` to lower case.

## Additional change
1. The `real` sql server datatype was incorrectly mapped to `long`
dotnet type. It has been correctly mapped to `decimal` now.

## Issue with DateTime/DateTimeOffset Sql Server data types
For a mutation request like this, we get a response:
<img width="873" alt="image"
src="https://user-images.githubusercontent.com/34566234/235473783-87b810b1-bf10-44a6-8671-c54b74e44dac.png">

and the parameters are generated as:
<img width="510" alt="image"
src="https://user-images.githubusercontent.com/34566234/235470342-5331c755-abdc-4e05-9f9c-467e406d0038.png">

the parameter generated is of type `DateTimeOffset` (as evident from the
value highlighted) but the underlying Sql Server datatype for the
`datetime_types` field is `datetime`. Even though we supplied a value
`1999-01-08 09:20:00` in the mutation, it became `08-01-1999 09:20:00
+05:30`, because that's how graphql treats datetime/datetimoffset/date
types. Then this implicit conversion from a value of type DateTimeOffset
to DateTime cannot happen throwing an exception with message : `Failed
to convert parameter value from a DateTimeOffset to a DateTime.` Hence
we cannot add a mapping from `typeof(DateTime) = DbType.DateTime`.

Another option that came to my mind and would probably come to your mind
as well would be to have a mapping from`typeof(DateTime) =
DbType.DateTimeOffset`, since then there can be implicit conversion from
value of `DateTimeOffset` type to `DbType.DateTimeOffset`. But that has
its own flaws. Lets look at the below screenshots:

Consider this mutation:
<img width="885" alt="image"
src="https://user-images.githubusercontent.com/34566234/235477733-19c18d73-7eaa-42ba-9484-52984c62baad.png">

and the parameters generated for the query (mutation ran successfully
because the default value for the `instant` PK column was inserted into
the table):

<img width="425" alt="image"
src="https://user-images.githubusercontent.com/34566234/235478676-6c2b5bc7-80bc-497d-a53d-e3ecff80538e.png">
The parameter value is of sql server type datetime but the DbType is
populated as `DbType.DateTimeOffset`. This upcasting cannot happen
correctly and so we are returned with 0 records. So, this approach won't
work as well.

**Conclusion:** We won't populate DbType for these sql server types as
it is not required and would prevent data loss and any unforeseen
exceptions.

## How was this tested?

- [x] Integration Tests
- [x] Unit Tests - The unit tests that were skipped before saying that
bytearray datatype is not supported are now running successfully for
MsSql.

## Sample Request(s)
Request method: POST
Request URL: https://localhost:5001/api/SupportedType/
Request:
```json
{
    "bytearray_types": null
}
```
Response:
```json
{
    "value": [
        {
            "typeid": 5001,
            "byte_types": null,
            "short_types": null,
            "int_types": null,
            "long_types": null,
            "string_types": null,
            "single_types": null,
            "float_types": null,
            "decimal_types": null,
            "boolean_types": null,
            "date_types": null,
            "datetime_types": null,
            "datetime2_types": null,
            "datetimeoffset_types": null,
            "smalldatetime_types": null,
            "bytearray_types": null,
            "guid_types": "27742da6-99f2-4e58-9fd8-4f143aea46b0"
        }
    ]
}
```

Author: ayush3797

src/Config/DatabaseObject.cs

@@ -1,6 +1,8 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
+using System.Data;
+
 namespace Azure.DataApiBuilder.Config
 {
     /// <summary>
@@ -105,11 +107,23 @@ public class StoredProcedureDefinition : SourceDefinition
         /// Key: parameter name, Value: ParameterDefinition object
         /// </summary>
         public Dictionary<string, ParameterDefinition> Parameters { get; set; } = new();
+
+        /// <inheritdoc/>
+        public override DbType? GetDbTypeForParam(string paramName)
+        {
+            if (Parameters.TryGetValue(paramName, out ParameterDefinition? paramDefinition))
+            {
+                return paramDefinition.DbType;
+            }
+
+            return null;
+        }
     }
 
     public class ParameterDefinition
     {
         public Type SystemType { get; set; } = null!;
+        public DbType? DbType { get; set; }
         public bool HasConfigDefault { get; set; }
         public object? ConfigDefaultValue { get; set; }
     }
@@ -153,6 +167,24 @@ public bool IsAnyColumnNullable(List<string> columnsToCheck)
                                  .Where(isNullable => isNullable == true)
                                  .Any();
         }
+
+        /// <summary>
+        /// Method to get the DbType for:
+        /// 1. column for table/view,
+        /// 2. parameter for stored procedure.
+        /// </summary>
+        /// <param name="paramName">The parameter whose DbType is to be determined.
+        /// For table/view paramName refers to the backingColumnName if aliases are used.</param>
+        /// <returns>DbType for the parameter.</returns>
+        public virtual DbType? GetDbTypeForParam(string paramName)
+        {
+            if (Columns.TryGetValue(paramName, out ColumnDefinition? columnDefinition))
+            {
+                return columnDefinition.DbType;
+            }
+
+            return null;
+        }
     }
 
     /// <summary>
@@ -178,6 +210,7 @@ public class ColumnDefinition
         /// The database type of this column mapped to the SystemType.
         /// </summary>
         public Type SystemType { get; set; } = typeof(object);
+        public DbType? DbType { get; set; }
         public bool HasDefault { get; set; }
         public bool IsAutoGenerated { get; set; }
         public bool IsNullable { get; set; }

src/Service.Tests/SqlTests/GraphQLSupportedTypesTests/MsSqlGQLSupportedTypesTests.cs

@@ -4,7 +4,6 @@
 using System.Collections.Generic;
 using System.Threading.Tasks;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
-using static Azure.DataApiBuilder.Service.GraphQLBuilder.GraphQLTypes.SupportedTypes;
 
 namespace Azure.DataApiBuilder.Service.Tests.SqlTests.GraphQLSupportedTypesTests
 {
@@ -34,19 +33,5 @@ ORDER BY id asc
                     INCLUDE_NULL_VALUES
             ";
         }
-
-        /// <summary>
-        /// Explicitly declaring a parameter for a bytearray type is not possible due to:
-        /// https://stackoverflow.com/questions/29254690/why-does-dbnull-value-require-a-proper-sqldbtype
-        /// </summary>
-        protected override bool IsSupportedType(string type)
-        {
-            if (type.Equals(BYTEARRAY_TYPE))
-            {
-                return false;
-            }
-
-            return true;
-        }
     }
 }

src/Service.Tests/SqlTests/RestApiTests/Insert/InsertApiTestBase.cs

@@ -60,6 +60,32 @@ await SetupAndRunRestApiTest(
             );
         }
 
+        /// <summary>
+        /// Perform insert test with bytearray column as NULL. This ensures that even though implicit conversion
+        /// between varchar to varbinary is not possible for MsSql (but it is possible for MySql & PgSql),
+        /// but since we are passing the DbType for the parameter, the database can explicitly convert it into varbinary.
+        /// </summary>
+        [TestMethod]
+        public virtual async Task InsertOneWithByteArrayTypeAsNull()
+        {
+            string requestBody = @"
+            {
+                ""bytearray_types"": null
+            }";
+
+            string expectedLocationHeader = $"typeid/{STARTING_ID_FOR_TEST_INSERTS}";
+            await SetupAndRunRestApiTest(
+                primaryKeyRoute: null,
+                queryString: null,
+                entityNameOrPath: _integrationTypeEntity,
+                sqlQuery: GetQuery("InsertOneInSupportedTypes"),
+                operationType: Config.Operation.Insert,
+                requestBody: requestBody,
+                expectedStatusCode: HttpStatusCode.Created,
+                expectedLocationHeader: expectedLocationHeader
+            );
+        }
+
         /// <summary>
         /// Tests insertion on simple/composite views.
         /// </summary>

src/Service.Tests/SqlTests/RestApiTests/Insert/MsSqlInsertApiTests.cs

@@ -27,6 +27,14 @@ public class MsSqlInsertApiTests : InsertApiTestBase
                 $"AND [publisher_id] = 1234 " +
                 $"FOR JSON PATH, INCLUDE_NULL_VALUES, WITHOUT_ARRAY_WRAPPER"
             },
+            {
+                "InsertOneInSupportedTypes",
+                $"SELECT [id] as [typeid], [byte_types], [short_types], [int_types], [long_types],string_types, [single_types], [float_types], " +
+                $"[decimal_types], [boolean_types], [date_types], [datetime_types], [datetime2_types], [datetimeoffset_types], [smalldatetime_types], " +
+                $"[bytearray_types], LOWER([guid_types]) as [guid_types] FROM { _integrationTypeTable } " +
+                $"WHERE [id] = { STARTING_ID_FOR_TEST_INSERTS } AND [bytearray_types] is NULL " +
+                $"FOR JSON PATH, INCLUDE_NULL_VALUES, WITHOUT_ARRAY_WRAPPER"
+            },
             {
                 "InsertOneInBooksViewAll",
                 $"SELECT [id], [title], [publisher_id] FROM { _simple_all_books } " +

src/Service.Tests/SqlTests/RestApiTests/Insert/MySqlInsertApiTests.cs

@@ -26,6 +26,17 @@ SELECT JSON_OBJECT('id', id, 'title', title, 'publisher_id', publisher_id) AS da
                     ) AS subq
                 "
             },
+            {
+                "InsertOneInSupportedTypes",
+                @"
+                    SELECT JSON_OBJECT('typeid', typeid,'bytearray_types', bytearray_types) AS data
+                    FROM (
+                        SELECT id as typeid, bytearray_types 
+                        FROM " + _integrationTypeTable + @"
+                        WHERE id = 5001 AND bytearray_types is NULL 
+                    ) AS subq
+                "
+            },
             {
                 "InsertOneUniqueCharactersTest",
                 @"

src/Service.Tests/SqlTests/RestApiTests/Insert/PostgreSqlInsertApiTests.cs

@@ -27,6 +27,18 @@ SELECT to_jsonb(subq) AS data
                     ) AS subq
                 "
             },
+            {
+                "InsertOneInSupportedTypes",
+                @"
+                    SELECT to_jsonb(subq) AS data
+                    FROM (
+                        SELECT id as typeid, short_types, int_types, long_types, string_types, single_types,
+                        float_types, decimal_types, boolean_types, datetime_types, bytearray_types, guid_types
+                        FROM " + _integrationTypeTable + @"
+                        WHERE id = " + STARTING_ID_FOR_TEST_INSERTS + @"
+                    ) AS subq
+                "
+            },
             {
                 "InsertOneUniqueCharactersTest",
                 @"

src/Service.Tests/Unittests/ODataASTVisitorUnitTests.cs

@@ -346,7 +346,8 @@ private static ODataASTVisitor CreateVisitor(
                 authorizationResolver,
                 _runtimeConfigProvider,
                 new GQLFilterParser(_sqlMetadataProvider),
-                null); // setting httpContext as null for the tests.
+                null) // setting httpContext as null for the tests.
+            { CallBase = true }; // setting CallBase = true enables calling the actual method on the mocked object without needing to mock the method behavior.
             return new ODataASTVisitor(structure.Object, _sqlMetadataProvider);
         }
 

src/Service.Tests/Unittests/SqlQueryExecutorUnitTests.cs

@@ -3,13 +3,15 @@
 
 using System;
 using System.Collections.Generic;
+using System.Data;
 using System.Data.Common;
 using System.Net;
 using System.Text.Json;
 using System.Threading.Tasks;
 using Azure.Core;
 using Azure.DataApiBuilder.Service.Configurations;
 using Azure.DataApiBuilder.Service.Exceptions;
+using Azure.DataApiBuilder.Service.Models;
 using Azure.DataApiBuilder.Service.Resolvers;
 using Azure.DataApiBuilder.Service.Tests.SqlTests;
 using Azure.Identity;
@@ -137,7 +139,7 @@ Mock<MsSqlQueryExecutor> queryExecutor
             queryExecutor.Setup(x => x.ExecuteQueryAgainstDbAsync(
                 It.IsAny<SqlConnection>(),
                 It.IsAny<string>(),
-                It.IsAny<IDictionary<string, object>>(),
+                It.IsAny<IDictionary<string, DbConnectionParam>>(),
                 It.IsAny<Func<DbDataReader, List<string>, Task<object>>>(),
                 It.IsAny<HttpContext>(),
                 It.IsAny<List<string>>()))
@@ -146,7 +148,7 @@ Mock<MsSqlQueryExecutor> queryExecutor
             // Call the actual ExecuteQueryAsync method.
             queryExecutor.Setup(x => x.ExecuteQueryAsync(
                 It.IsAny<string>(),
-                It.IsAny<IDictionary<string, object>>(),
+                It.IsAny<IDictionary<string, DbConnectionParam>>(),
                 It.IsAny<Func<DbDataReader, List<string>, Task<object>>>(),
                 It.IsAny<HttpContext>(),
                 It.IsAny<List<string>>())).CallBase();
@@ -155,7 +157,7 @@ Mock<MsSqlQueryExecutor> queryExecutor
             {
                 await queryExecutor.Object.ExecuteQueryAsync<object>(
                     sqltext: string.Empty,
-                    parameters: new Dictionary<string, object>(),
+                    parameters: new Dictionary<string, DbConnectionParam>(),
                     dataReaderHandler: null,
                     httpContext: null,
                     args: null);
@@ -189,7 +191,7 @@ Mock<MsSqlQueryExecutor> queryExecutor
             queryExecutor.SetupSequence(x => x.ExecuteQueryAgainstDbAsync(
                 It.IsAny<SqlConnection>(),
                 It.IsAny<string>(),
-                It.IsAny<IDictionary<string, object>>(),
+                It.IsAny<IDictionary<string, DbConnectionParam>>(),
                 It.IsAny<Func<DbDataReader, List<string>, Task<object>>>(),
                 It.IsAny<HttpContext>(),
                 It.IsAny<List<string>>()))
@@ -200,7 +202,7 @@ Mock<MsSqlQueryExecutor> queryExecutor
             // Call the actual ExecuteQueryAsync method.
             queryExecutor.Setup(x => x.ExecuteQueryAsync(
                 It.IsAny<string>(),
-                It.IsAny<IDictionary<string, object>>(),
+                It.IsAny<IDictionary<string, DbConnectionParam>>(),
                 It.IsAny<Func<DbDataReader, List<string>, Task<object>>>(),
                 It.IsAny<HttpContext>(),
                 It.IsAny<List<string>>())).CallBase();
@@ -209,7 +211,7 @@ Mock<MsSqlQueryExecutor> queryExecutor
 
             await queryExecutor.Object.ExecuteQueryAsync<object>(
                     sqltext: sqltext,
-                    parameters: new Dictionary<string, object>(),
+                    parameters: new Dictionary<string, DbConnectionParam>(),
                     dataReaderHandler: null,
                     args: null);
 

src/Service/Models/DbConnectionParam.cs

@@ -0,0 +1,30 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using System.Data;
+
+namespace Azure.DataApiBuilder.Service.Models
+{
+    /// <summary>
+    /// Represents a single parameter created for the database connection.
+    /// </summary>
+    public class DbConnectionParam
+    {
+        public DbConnectionParam(object? value, DbType? dbType = null)
+        {
+            Value = value;
+            DbType = dbType;
+        }
+
+        /// <summary>
+        /// Value of the parameter.
+        /// </summary>
+        public object? Value { get; set; }
+
+        // DbType of the parameter.
+        // This is being made nullable because GraphQL treats Sql Server types like datetime, datetimeoffset
+        // identically and then implicit conversion cannot happen.
+        // For more details refer: https://github.com/Azure/data-api-builder/pull/1442.
+        public DbType? DbType { get; set; }
+    }
+}

src/Service/Models/GraphQLFilterParsers.cs

@@ -204,7 +204,7 @@ public Predicate Parse(
                                     schemaName,
                                     sourceName,
                                     sourceAlias,
-                                    queryStructure.MakeParamWithValue)));
+                                    queryStructure.MakeDbConnectionParam)));
                     }
                 }
             }
@@ -300,7 +300,7 @@ private void HandleNestedFilterForSql(
             predicates.Push(new PredicateOperand(existsPredicate));
 
             // Add all parameters from the exists subquery to the main queryStructure.
-            foreach ((string key, object? value) in existsQuery.Parameters)
+            foreach ((string key, DbConnectionParam value) in existsQuery.Parameters)
             {
                 queryStructure.Parameters.Add(key, value);
             }
@@ -346,7 +346,7 @@ private static Predicate ParseScalarType(
             string schemaName,
             string tableName,
             string tableAlias,
-            Func<object, string> processLiterals)
+            Func<object, string?, string> processLiterals)
         {
             Column column = new(schemaName, tableName, columnName: name, tableAlias);
 
@@ -472,7 +472,7 @@ public static Predicate Parse(
             IInputField argumentSchema,
             Column column,
             List<ObjectFieldNode> fields,
-            Func<object, string> processLiterals)
+            Func<object, string?, string> processLiterals)
         {
             List<PredicateOperand> predicates = new();
 
@@ -542,7 +542,7 @@ public static Predicate Parse(
                 predicates.Push(new PredicateOperand(new Predicate(
                     new PredicateOperand(column),
                     op,
-                    new PredicateOperand(processLiteral ? $"{processLiterals(value)}" : value.ToString()))
+                    new PredicateOperand(processLiteral ? $"{processLiterals(value, column.ColumnName)}" : value.ToString()))
                 ));
             }