Introducing parameters' DbType before passing on to the database - MsSql

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

src/Service/Models/SqlQueryStructures.cs

@@ -362,6 +362,11 @@ public ulong Next()
             return _integer++;
         }
 
+        public ulong Current()
+        {
+            return _integer;
+        }
+
     }
 
     /// <summary>