From 9097bb6e3ac52c18c1aa038e0cba6b993c5cdc71 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Tue, 18 Apr 2023 17:07:37 +0530 Subject: [PATCH 01/22] Initial progress --- src/Config/DatabaseObject.cs | 3 + src/Service/Models/GraphQLFilterParsers.cs | 6 +- src/Service/Models/SqlQueryStructures.cs | 5 ++ src/Service/Parsers/ODataASTVisitor.cs | 22 +++++-- src/Service/Resolvers/BaseQueryStructure.cs | 2 +- src/Service/Resolvers/CosmosQueryStructure.cs | 7 +++ src/Service/Resolvers/QueryExecutor.cs | 1 - .../BaseSqlQueryStructure.cs | 19 +++++- .../SqlDeleteQueryStructure.cs | 2 +- .../SqlExecuteQueryStructure.cs | 6 +- .../SqlInsertQueryStructure.cs | 4 +- .../Sql Query Structures/SqlQueryStructure.cs | 6 +- .../SqlUpdateQueryStructure.cs | 4 +- .../SqlUpsertQueryStructure.cs | 4 +- .../MetadataProviders/SqlMetadataProvider.cs | 60 ++++++++++++++++++- 15 files changed, 126 insertions(+), 25 deletions(-) diff --git a/src/Config/DatabaseObject.cs b/src/Config/DatabaseObject.cs index 84f313c152..f6aec1cbf1 100644 --- a/src/Config/DatabaseObject.cs +++ b/src/Config/DatabaseObject.cs @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Data; + namespace Azure.DataApiBuilder.Config { /// @@ -178,6 +180,7 @@ public class ColumnDefinition /// The database type of this column mapped to the SystemType. /// 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; } diff --git a/src/Service/Models/GraphQLFilterParsers.cs b/src/Service/Models/GraphQLFilterParsers.cs index c303bb5276..e008dc1543 100644 --- a/src/Service/Models/GraphQLFilterParsers.cs +++ b/src/Service/Models/GraphQLFilterParsers.cs @@ -346,7 +346,7 @@ private static Predicate ParseScalarType( string schemaName, string tableName, string tableAlias, - Func processLiterals) + Func processLiterals) { Column column = new(schemaName, tableName, columnName: name, tableAlias); @@ -472,7 +472,7 @@ public static Predicate Parse( IInputField argumentSchema, Column column, List fields, - Func processLiterals) + Func processLiterals) { List 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())) )); } diff --git a/src/Service/Models/SqlQueryStructures.cs b/src/Service/Models/SqlQueryStructures.cs index adbf74e3aa..ffe40061b9 100644 --- a/src/Service/Models/SqlQueryStructures.cs +++ b/src/Service/Models/SqlQueryStructures.cs @@ -362,6 +362,11 @@ public ulong Next() return _integer++; } + public ulong Current() + { + return _integer; + } + } /// diff --git a/src/Service/Parsers/ODataASTVisitor.cs b/src/Service/Parsers/ODataASTVisitor.cs index 7553ee75de..54afbe04c7 100644 --- a/src/Service/Parsers/ODataASTVisitor.cs +++ b/src/Service/Parsers/ODataASTVisitor.cs @@ -37,6 +37,17 @@ public override string Visit(BinaryOperatorNode nodeIn) // In order traversal but add parens to maintain order of logical operations string left = nodeIn.Left.Accept(this); string right = nodeIn.Right.Accept(this); + + if (nodeIn.Left.GetType() == typeof(SingleValuePropertyAccessNode) && nodeIn.Right.GetType() == typeof(ConstantNode) || + nodeIn.Left.GetType() == typeof(ConstantNode) && nodeIn.Right.GetType() == typeof(SingleValuePropertyAccessNode)) + { + SingleValuePropertyAccessNode propertyNode = nodeIn.Left.GetType() == typeof(SingleValuePropertyAccessNode) ? + (SingleValuePropertyAccessNode)nodeIn.Left : (SingleValuePropertyAccessNode)nodeIn.Right; + string? paramName = $"{BaseQueryStructure.PARAM_NAME_PREFIX}param{_struct.Counter.Current() - 1}"; + _metadataProvider.TryGetBackingColumn(_struct.EntityName, propertyNode.Property.Name, out string? backingColumnName); + _struct.ParamToDbTypeMap.Add(paramName, _struct.GetUnderlyingSourceDefinition().Columns[backingColumnName!].DbType); + } + return CreateResult(nodeIn.OperatorKind, left, right); } @@ -79,13 +90,16 @@ public override string Visit(SingleValuePropertyAccessNode nodeIn) /// String representing param that holds given value. public override string Visit(ConstantNode nodeIn) { - if (nodeIn.TypeReference is null) + if (nodeIn.TypeReference is not null) { - // Represents a NULL value, we support NULL in queries so return "NULL" here - return "NULL"; + return $"{_struct.MakeParamWithValue(GetParamWithSystemType(nodeIn.Value.ToString()!, nodeIn.TypeReference))}"; } - return $"{_struct.MakeParamWithValue(GetParamWithSystemType(nodeIn.Value.ToString()!, nodeIn.TypeReference))}"; + // This is necessary because we want to create a mapping between column name and its corresponding param name. + _struct.MakeParamWithValue(null); + + // Represents a NULL value, we support NULL in queries so return "NULL" here + return "NULL"; } /// diff --git a/src/Service/Resolvers/BaseQueryStructure.cs b/src/Service/Resolvers/BaseQueryStructure.cs index ec7461f6e5..56041f7d59 100644 --- a/src/Service/Resolvers/BaseQueryStructure.cs +++ b/src/Service/Resolvers/BaseQueryStructure.cs @@ -109,7 +109,7 @@ public BaseQueryStructure( /// Add parameter to Parameters and return the name associated with it /// /// Value to be assigned to parameter, which can be null for nullable columns. - public string MakeParamWithValue(object? value) + public virtual string MakeParamWithValue(object? value, string? columnName = null) { string paramName = $"{PARAM_NAME_PREFIX}param{Counter.Next()}"; Parameters.Add(paramName, value); diff --git a/src/Service/Resolvers/CosmosQueryStructure.cs b/src/Service/Resolvers/CosmosQueryStructure.cs index d8e323a13b..17e450bf9a 100644 --- a/src/Service/Resolvers/CosmosQueryStructure.cs +++ b/src/Service/Resolvers/CosmosQueryStructure.cs @@ -47,6 +47,13 @@ public CosmosQueryStructure( Init(parameters); } + public override string MakeParamWithValue(object? value, string? columnName = null) + { + string paramName = $"{PARAM_NAME_PREFIX}param{Counter.Next()}"; + Parameters.Add(paramName, value); + return paramName; + } + private static IEnumerable GenerateQueryColumns(SelectionSetNode selectionSet, DocumentNode document, string tableName) { foreach (ISelectionNode selectionNode in selectionSet.Selections) diff --git a/src/Service/Resolvers/QueryExecutor.cs b/src/Service/Resolvers/QueryExecutor.cs index ea94220a36..1f18b0a5f3 100644 --- a/src/Service/Resolvers/QueryExecutor.cs +++ b/src/Service/Resolvers/QueryExecutor.cs @@ -154,7 +154,6 @@ await ExecuteQueryAgainstDbAsync(conn, // Add query to send user data from DAB to the underlying database to enable additional security the user might have configured // at the database level. string sessionParamsQuery = GetSessionParamsQuery(httpContext, parameters); - //"EXEC sp_set_session_context 'roles', 'Anonymous', @read_only =1 ;"; cmd.CommandText = sessionParamsQuery + sqltext; if (parameters is not null) diff --git a/src/Service/Resolvers/Sql Query Structures/BaseSqlQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/BaseSqlQueryStructure.cs index 00b3ef35cd..6902284140 100644 --- a/src/Service/Resolvers/Sql Query Structures/BaseSqlQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/BaseSqlQueryStructure.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Data; using System.IO; using System.Linq; using System.Net; @@ -27,6 +28,9 @@ namespace Azure.DataApiBuilder.Service.Resolvers /// public abstract class BaseSqlQueryStructure : BaseQueryStructure { + + public Dictionary ParamToDbTypeMap { get; set; } = new(); + /// /// All tables/views that should be in the FROM clause of the query. /// All these objects are linked via an INNER JOIN. @@ -83,6 +87,18 @@ public BaseSqlQueryStructure( } } + public override string MakeParamWithValue(object? value, string? columnName = null) + { + string paramName = $"{PARAM_NAME_PREFIX}param{Counter.Next()}"; + Parameters.Add(paramName, value); + if (!string.IsNullOrEmpty(columnName)) + { + ParamToDbTypeMap.Add(paramName, GetUnderlyingSourceDefinition().Columns[columnName].DbType); + } + + return paramName; + } + /// /// For UPDATE (OVERWRITE) operation /// Adds result of (SourceDefinition.Columns minus MutationFields) to UpdateOperations with null values @@ -111,7 +127,7 @@ public void AddNullifiedUnspecifiedFields( Predicate predicate = new( new PredicateOperand(new Column(tableSchema: DatabaseObject.SchemaName, tableName: DatabaseObject.Name, leftoverColumn)), PredicateOperation.Equal, - new PredicateOperand($"{MakeParamWithValue(value: null)}") + new PredicateOperand($"{MakeParamWithValue(null, leftoverColumn)}") ); updateOperations.Add(predicate); @@ -549,6 +565,5 @@ protected object GetParamAsSystemType(string fieldValue, string fieldName, Type } } - } } diff --git a/src/Service/Resolvers/Sql Query Structures/SqlDeleteQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/SqlDeleteQueryStructure.cs index 53f974ae2b..e435c29b59 100644 --- a/src/Service/Resolvers/Sql Query Structures/SqlDeleteQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/SqlDeleteQueryStructure.cs @@ -53,7 +53,7 @@ public SqlDeleteStructure( Predicates.Add(new Predicate( new PredicateOperand(new Column(DatabaseObject.SchemaName, DatabaseObject.Name, backingColumn!)), PredicateOperation.Equal, - new PredicateOperand($"{MakeParamWithValue(GetParamAsSystemType(param.Value.ToString()!, backingColumn!, GetColumnSystemType(backingColumn!)))}") + new PredicateOperand($"{MakeParamWithValue(GetParamAsSystemType(param.Value.ToString()!, backingColumn!, GetColumnSystemType(backingColumn!)), backingColumn)}") )); } } diff --git a/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs index f854d0fdb9..21a428f956 100644 --- a/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs @@ -48,11 +48,11 @@ public SqlExecuteStructure( if (requestParamValue is not null) { Type systemType = GetUnderlyingStoredProcedureDefinition().Parameters[paramKey].SystemType!; - parametrizedName = MakeParamWithValue(GetParamAsSystemType(requestParamValue.ToString()!, paramKey, systemType)); + parametrizedName = MakeParamWithValue(GetParamAsSystemType(requestParamValue.ToString()!, paramKey, systemType), paramKey); } else { - parametrizedName = MakeParamWithValue(value: null); + parametrizedName = MakeParamWithValue(null, paramKey); } ProcedureParameters.Add(paramKey, $"{parametrizedName}"); @@ -62,7 +62,7 @@ public SqlExecuteStructure( // Fill with default value from runtime config if (paramDefinition.HasConfigDefault) { - string parameterizedName = MakeParamWithValue(paramDefinition.ConfigDefaultValue); + string parameterizedName = MakeParamWithValue(paramDefinition.ConfigDefaultValue, paramKey); ProcedureParameters.Add(paramKey, $"{parameterizedName}"); } else diff --git a/src/Service/Resolvers/Sql Query Structures/SqlInsertQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/SqlInsertQueryStructure.cs index cba846fc5c..4f078ad183 100644 --- a/src/Service/Resolvers/Sql Query Structures/SqlInsertQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/SqlInsertQueryStructure.cs @@ -105,11 +105,11 @@ private void PopulateColumnsAndParams(string columnName, object? value) if (value is not null) { paramName = MakeParamWithValue( - GetParamAsSystemType(value.ToString()!, columnName, GetColumnSystemType(columnName))); + GetParamAsSystemType(value.ToString()!, columnName, GetColumnSystemType(columnName)), columnName); } else { - paramName = MakeParamWithValue(value: null); + paramName = MakeParamWithValue(null, columnName); } Values.Add($"{paramName}"); diff --git a/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs index 23ae6f5599..4350cd7b52 100644 --- a/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs @@ -458,7 +458,7 @@ private void AddPrimaryKeyPredicates(IDictionary queryParams) columnName: columnName, tableAlias: SourceAlias)), PredicateOperation.Equal, - new PredicateOperand($"{MakeParamWithValue(parameter.Value)}") + new PredicateOperand($"{MakeParamWithValue(parameter.Value, columnName)}") )); } } @@ -479,7 +479,7 @@ public void AddPaginationPredicate(IEnumerable afterJsonValues column.TableAlias = SourceAlias; column.ParamName = column.Value is not null ? MakeParamWithValue(GetParamAsSystemType(column.Value!.ToString()!, column.ColumnName, GetColumnSystemType(column.ColumnName))) : - MakeParamWithValue(value: null); + MakeParamWithValue(null, column.ColumnName); } PaginationMetadata.PaginationPredicate = new KeysetPaginationPredicate(afterJsonValues.ToList()); @@ -501,7 +501,7 @@ private void PopulateParamsAndPredicates(string field, string backingColumn, obj if (value != null) { parameterName = MakeParamWithValue( - GetParamAsSystemType(value.ToString()!, backingColumn, GetColumnSystemType(backingColumn))); + GetParamAsSystemType(value.ToString()!, backingColumn, GetColumnSystemType(backingColumn)), backingColumn); Predicates.Add(new Predicate( new PredicateOperand(new Column(DatabaseObject.SchemaName, DatabaseObject.Name, backingColumn, SourceAlias)), op, diff --git a/src/Service/Resolvers/Sql Query Structures/SqlUpdateQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/SqlUpdateQueryStructure.cs index e10aba4bde..0e7ffeb707 100644 --- a/src/Service/Resolvers/Sql Query Structures/SqlUpdateQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/SqlUpdateQueryStructure.cs @@ -173,7 +173,7 @@ private Predicate CreatePredicateForParam(KeyValuePair param) new PredicateOperand( new Column(tableSchema: DatabaseObject.SchemaName, tableName: DatabaseObject.Name, backingColumn!)), PredicateOperation.Equal, - new PredicateOperand($"{MakeParamWithValue(null)}") + new PredicateOperand($"{MakeParamWithValue(null, backingColumn)}") ); } else @@ -182,7 +182,7 @@ private Predicate CreatePredicateForParam(KeyValuePair param) new PredicateOperand( new Column(tableSchema: DatabaseObject.SchemaName, tableName: DatabaseObject.Name, param.Key)), PredicateOperation.Equal, - new PredicateOperand($"{MakeParamWithValue(GetParamAsSystemType(param.Value.ToString()!, param.Key, GetColumnSystemType(param.Key)))}")); + new PredicateOperand($"{MakeParamWithValue(GetParamAsSystemType(param.Value.ToString()!, param.Key, GetColumnSystemType(param.Key)), param.Key)}")); } return predicate; diff --git a/src/Service/Resolvers/Sql Query Structures/SqlUpsertQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/SqlUpsertQueryStructure.cs index 500190cea6..189e6ad8dd 100644 --- a/src/Service/Resolvers/Sql Query Structures/SqlUpsertQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/SqlUpsertQueryStructure.cs @@ -123,11 +123,11 @@ private void PopulateColumns( string paramIdentifier; if (param.Value is not null) { - paramIdentifier = MakeParamWithValue(GetParamAsSystemType(param.Value.ToString()!, backingColumn!, GetColumnSystemType(backingColumn!))); + paramIdentifier = MakeParamWithValue(GetParamAsSystemType(param.Value.ToString()!, backingColumn!, GetColumnSystemType(backingColumn!)), backingColumn); } else { - paramIdentifier = MakeParamWithValue(null); + paramIdentifier = MakeParamWithValue(null, backingColumn); } ColumnToParam.Add(backingColumn!, paramIdentifier); diff --git a/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs b/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs index d6d124ca97..54c9a56799 100644 --- a/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs +++ b/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs @@ -18,6 +18,7 @@ using Azure.DataApiBuilder.Service.Models; using Azure.DataApiBuilder.Service.Parsers; using Azure.DataApiBuilder.Service.Resolvers; +using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; using static Azure.DataApiBuilder.Service.GraphQLBuilder.GraphQLNaming; @@ -45,6 +46,47 @@ public abstract class SqlMetadataProvider : // to their corresponding entity names defined in the config. public Dictionary GraphQLStoredProcedureExposedNameToEntityNameMap { get; set; } = new(); + public Dictionary typeMap = new() + { + [typeof(byte)] = DbType.Byte, + [typeof(sbyte)] = DbType.SByte, + [typeof(short)] = DbType.Int16, + [typeof(ushort)] = DbType.UInt16, + [typeof(int)] = DbType.Int32, + [typeof(uint)] = DbType.UInt32, + [typeof(long)] = DbType.Int64, + [typeof(ulong)] = DbType.UInt64, + [typeof(float)] = DbType.Single, + [typeof(double)] = DbType.Double, + [typeof(decimal)] = DbType.Decimal, + [typeof(bool)] = DbType.Boolean, + [typeof(string)] = DbType.String, + [typeof(char)] = DbType.StringFixedLength, + [typeof(Guid)] = DbType.Guid, + [typeof(DateTime)] = DbType.DateTime, + [typeof(DateTimeOffset)] = DbType.DateTimeOffset, + [typeof(TimeSpan)] = DbType.Time, + [typeof(byte[])] = DbType.Binary, + [typeof(byte?)] = DbType.Byte, + [typeof(sbyte?)] = DbType.SByte, + [typeof(short?)] = DbType.Int16, + [typeof(ushort?)] = DbType.UInt16, + [typeof(int?)] = DbType.Int32, + [typeof(uint?)] = DbType.UInt32, + [typeof(long?)] = DbType.Int64, + [typeof(ulong?)] = DbType.UInt64, + [typeof(float?)] = DbType.Single, + [typeof(double?)] = DbType.Double, + [typeof(decimal?)] = DbType.Decimal, + [typeof(bool?)] = DbType.Boolean, + [typeof(char?)] = DbType.StringFixedLength, + [typeof(Guid?)] = DbType.Guid, + [typeof(DateTime?)] = DbType.DateTime, + [typeof(DateTimeOffset?)] = DbType.DateTimeOffset, + [typeof(TimeSpan?)] = DbType.Time, + [typeof(object)] = DbType.Object + }; + // Contains all the referencing and referenced columns for each pair // of referencing and referenced tables. public Dictionary? PairToFkDefinition { get; set; } @@ -1000,11 +1042,13 @@ private async Task PopulateSourceDefinitionAsync( subStatusCode: DataApiBuilderException.SubStatusCodes.ErrorInInitialization); } + Type systemTypeOfColumn = (Type)columnInfoFromAdapter["DataType"]; ColumnDefinition column = new() { IsNullable = (bool)columnInfoFromAdapter["AllowDBNull"], IsAutoGenerated = (bool)columnInfoFromAdapter["IsAutoIncrement"], - SystemType = (Type)columnInfoFromAdapter["DataType"] + SystemType = systemTypeOfColumn, + DbType = GetDbTypeFromSystemType(systemTypeOfColumn, columnName) }; // Tests may try to add the same column simultaneously @@ -1021,6 +1065,20 @@ private async Task PopulateSourceDefinitionAsync( columnsInTable); } + private DbType GetDbTypeFromSystemType(Type systemType, string columnName) + { + if (!typeMap.TryGetValue(systemType, out DbType dbType)) + { + throw new DataApiBuilderException( + message: $"Unsupported data type encountered for column {columnName}.", + statusCode: HttpStatusCode.InternalServerError, + subStatusCode: DataApiBuilderException.SubStatusCodes.NotSupported + ); + } + + return dbType; + } + /// /// Determine whether the provided field of a GraphQL enabled entity meets GraphQL reserved name requirements. /// Criteria: From 65507bb27fd5e184682d0428980e49a02ea7dab6 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Tue, 18 Apr 2023 18:45:17 +0530 Subject: [PATCH 02/22] Changing param dict to store {paramName,{paramValue,DbType}} --- .../Unittests/SqlQueryExecutorUnitTests.cs | 13 +++++++------ src/Service/Models/GraphQLFilterParsers.cs | 3 ++- src/Service/Resolvers/BaseQueryStructure.cs | 16 +++++++++++++--- src/Service/Resolvers/CosmosQueryEngine.cs | 9 +++++---- src/Service/Resolvers/CosmosQueryStructure.cs | 7 ------- src/Service/Resolvers/IQueryExecutor.cs | 5 +++-- src/Service/Resolvers/MsSqlQueryExecutor.cs | 6 ++++-- src/Service/Resolvers/QueryExecutor.cs | 15 ++++++++++----- .../BaseSqlQueryStructure.cs | 12 ------------ .../Sql Query Structures/SqlQueryStructure.cs | 3 ++- src/Service/Resolvers/SqlMutationEngine.cs | 6 +++--- .../MetadataProviders/MySqlMetadataProvider.cs | 8 ++++---- .../MetadataProviders/SqlMetadataProvider.cs | 11 +++++------ 13 files changed, 58 insertions(+), 56 deletions(-) diff --git a/src/Service.Tests/Unittests/SqlQueryExecutorUnitTests.cs b/src/Service.Tests/Unittests/SqlQueryExecutorUnitTests.cs index 41543d460b..95daa121cf 100644 --- a/src/Service.Tests/Unittests/SqlQueryExecutorUnitTests.cs +++ b/src/Service.Tests/Unittests/SqlQueryExecutorUnitTests.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Data; using System.Data.Common; using System.Net; using System.Text.Json; @@ -137,7 +138,7 @@ Mock queryExecutor queryExecutor.Setup(x => x.ExecuteQueryAgainstDbAsync( It.IsAny(), It.IsAny(), - It.IsAny>(), + It.IsAny>>(), It.IsAny, Task>>(), It.IsAny(), It.IsAny>())) @@ -146,7 +147,7 @@ Mock queryExecutor // Call the actual ExecuteQueryAsync method. queryExecutor.Setup(x => x.ExecuteQueryAsync( It.IsAny(), - It.IsAny>(), + It.IsAny>>(), It.IsAny, Task>>(), It.IsAny(), It.IsAny>())).CallBase(); @@ -155,7 +156,7 @@ Mock queryExecutor { await queryExecutor.Object.ExecuteQueryAsync( sqltext: string.Empty, - parameters: new Dictionary(), + parameters: new Dictionary>(), dataReaderHandler: null, httpContext: null, args: null); @@ -189,7 +190,7 @@ Mock queryExecutor queryExecutor.SetupSequence(x => x.ExecuteQueryAgainstDbAsync( It.IsAny(), It.IsAny(), - It.IsAny>(), + It.IsAny>>(), It.IsAny, Task>>(), It.IsAny(), It.IsAny>())) @@ -200,7 +201,7 @@ Mock queryExecutor // Call the actual ExecuteQueryAsync method. queryExecutor.Setup(x => x.ExecuteQueryAsync( It.IsAny(), - It.IsAny>(), + It.IsAny>>(), It.IsAny, Task>>(), It.IsAny(), It.IsAny>())).CallBase(); @@ -209,7 +210,7 @@ Mock queryExecutor await queryExecutor.Object.ExecuteQueryAsync( sqltext: sqltext, - parameters: new Dictionary(), + parameters: new Dictionary>(), dataReaderHandler: null, args: null); diff --git a/src/Service/Models/GraphQLFilterParsers.cs b/src/Service/Models/GraphQLFilterParsers.cs index e008dc1543..dc1cc348b0 100644 --- a/src/Service/Models/GraphQLFilterParsers.cs +++ b/src/Service/Models/GraphQLFilterParsers.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Data; using System.Linq; using System.Net; using Azure.DataApiBuilder.Config; @@ -300,7 +301,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, Tuple value) in existsQuery.Parameters) { queryStructure.Parameters.Add(key, value); } diff --git a/src/Service/Resolvers/BaseQueryStructure.cs b/src/Service/Resolvers/BaseQueryStructure.cs index 56041f7d59..b1413dda26 100644 --- a/src/Service/Resolvers/BaseQueryStructure.cs +++ b/src/Service/Resolvers/BaseQueryStructure.cs @@ -1,7 +1,9 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System; using System.Collections.Generic; +using System.Data; using Azure.DataApiBuilder.Auth; using Azure.DataApiBuilder.Config; using Azure.DataApiBuilder.Service.GraphQLBuilder; @@ -51,7 +53,7 @@ public class BaseQueryStructure /// /// Parameters values required to execute the query. /// - public Dictionary Parameters { get; set; } + public Dictionary> Parameters { get; set; } /// /// Predicates that should filter the result set of the query. @@ -109,10 +111,18 @@ public BaseQueryStructure( /// Add parameter to Parameters and return the name associated with it /// /// Value to be assigned to parameter, which can be null for nullable columns. - public virtual string MakeParamWithValue(object? value, string? columnName = null) + public string MakeParamWithValue(object? value, string? columnName = null) { string paramName = $"{PARAM_NAME_PREFIX}param{Counter.Next()}"; - Parameters.Add(paramName, value); + if (!string.IsNullOrEmpty(columnName)) + { + Parameters.Add(paramName, new(value, GetUnderlyingSourceDefinition().Columns[columnName].DbType)); + } + else + { + Parameters.Add(paramName, new(value, null)); + } + return paramName; } diff --git a/src/Service/Resolvers/CosmosQueryEngine.cs b/src/Service/Resolvers/CosmosQueryEngine.cs index 0aa68bde75..cf6fee70a0 100644 --- a/src/Service/Resolvers/CosmosQueryEngine.cs +++ b/src/Service/Resolvers/CosmosQueryEngine.cs @@ -4,6 +4,7 @@ # nullable disable using System; using System.Collections.Generic; +using System.Data; using System.Linq; using System.Text; using System.Text.Json; @@ -71,9 +72,9 @@ public async Task> ExecuteAsync( Container container = _clientProvider.Client.GetDatabase(structure.Database).GetContainer(structure.Container); (string idValue, string partitionKeyValue) = await GetIdAndPartitionKey(parameters, container, structure); - foreach (KeyValuePair parameterEntry in structure.Parameters) + foreach (KeyValuePair> parameterEntry in structure.Parameters) { - querySpec = querySpec.WithParameter(parameterEntry.Key, parameterEntry.Value); + querySpec = querySpec.WithParameter(parameterEntry.Key, parameterEntry.Value.Item1); } if (!string.IsNullOrEmpty(partitionKeyValue)) @@ -154,9 +155,9 @@ public async Task, IMetadata>> ExecuteListAsync( Container container = _clientProvider.Client.GetDatabase(structure.Database).GetContainer(structure.Container); QueryDefinition querySpec = new(_queryBuilder.Build(structure)); - foreach (KeyValuePair parameterEntry in structure.Parameters) + foreach (KeyValuePair> parameterEntry in structure.Parameters) { - querySpec = querySpec.WithParameter(parameterEntry.Key, parameterEntry.Value); + querySpec = querySpec.WithParameter(parameterEntry.Key, parameterEntry.Value.Item1); } FeedIterator resultSetIterator = container.GetItemQueryIterator(querySpec); diff --git a/src/Service/Resolvers/CosmosQueryStructure.cs b/src/Service/Resolvers/CosmosQueryStructure.cs index 17e450bf9a..d8e323a13b 100644 --- a/src/Service/Resolvers/CosmosQueryStructure.cs +++ b/src/Service/Resolvers/CosmosQueryStructure.cs @@ -47,13 +47,6 @@ public CosmosQueryStructure( Init(parameters); } - public override string MakeParamWithValue(object? value, string? columnName = null) - { - string paramName = $"{PARAM_NAME_PREFIX}param{Counter.Next()}"; - Parameters.Add(paramName, value); - return paramName; - } - private static IEnumerable GenerateQueryColumns(SelectionSetNode selectionSet, DocumentNode document, string tableName) { foreach (ISelectionNode selectionNode in selectionSet.Selections) diff --git a/src/Service/Resolvers/IQueryExecutor.cs b/src/Service/Resolvers/IQueryExecutor.cs index 7aea5b8598..b3eb82def3 100644 --- a/src/Service/Resolvers/IQueryExecutor.cs +++ b/src/Service/Resolvers/IQueryExecutor.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Data; using System.Data.Common; using System.Text.Json.Nodes; using System.Threading.Tasks; @@ -30,7 +31,7 @@ public interface IQueryExecutor /// An object formed using the results of the query as returned by the given handler. public Task ExecuteQueryAsync( string sqltext, - IDictionary parameters, + IDictionary> parameters, Func?, Task>? dataReaderHandler, HttpContext? httpContext = null, List? args = null); @@ -111,6 +112,6 @@ public Task> GetResultProperties( /// Current user httpContext. /// Dictionary of parameters/value required to execute the query. /// empty string / query to set session parameters for the connection. - public string GetSessionParamsQuery(HttpContext? httpContext, IDictionary parameters); + public string GetSessionParamsQuery(HttpContext? httpContext, IDictionary> parameters); } } diff --git a/src/Service/Resolvers/MsSqlQueryExecutor.cs b/src/Service/Resolvers/MsSqlQueryExecutor.cs index 43c16b8310..ae39e9d20f 100644 --- a/src/Service/Resolvers/MsSqlQueryExecutor.cs +++ b/src/Service/Resolvers/MsSqlQueryExecutor.cs @@ -1,7 +1,9 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System; using System.Collections.Generic; +using System.Data; using System.Data.Common; using System.Security.Claims; using System.Text; @@ -168,7 +170,7 @@ await AzureCredential.GetTokenAsync( /// Dictionary of parameters/value required to execute the query. /// empty string / query to set session parameters for the connection. /// - public override string GetSessionParamsQuery(HttpContext? httpContext, IDictionary parameters) + public override string GetSessionParamsQuery(HttpContext? httpContext, IDictionary> parameters) { if (httpContext is null || !_isSessionContextEnabled) { @@ -186,7 +188,7 @@ public override string GetSessionParamsQuery(HttpContext? httpContext, IDictiona foreach ((string claimType, Claim claim) in sessionParams) { string paramName = $"{SESSION_PARAM_NAME}{counter.Next()}"; - parameters.Add(paramName, claim.Value); + parameters.Add(paramName, new(claim.Value, null)); // Append statement to set read only param value - can be set only once for a connection. string statementToSetReadOnlyParam = "EXEC sp_set_session_context " + $"'{claimType}', " + paramName + ", @read_only = 1;"; sessionMapQuery = sessionMapQuery.Append(statementToSetReadOnlyParam); diff --git a/src/Service/Resolvers/QueryExecutor.cs b/src/Service/Resolvers/QueryExecutor.cs index 1f18b0a5f3..1a8c273f72 100644 --- a/src/Service/Resolvers/QueryExecutor.cs +++ b/src/Service/Resolvers/QueryExecutor.cs @@ -66,7 +66,7 @@ public QueryExecutor(DbExceptionParser dbExceptionParser, /// public virtual async Task ExecuteQueryAsync( string sqltext, - IDictionary parameters, + IDictionary> parameters, Func?, Task>? dataReaderHandler, HttpContext? httpContext = null, List? args = null) @@ -142,7 +142,7 @@ await ExecuteQueryAgainstDbAsync(conn, public virtual async Task ExecuteQueryAgainstDbAsync( TConnection conn, string sqltext, - IDictionary parameters, + IDictionary> parameters, Func?, Task>? dataReaderHandler, HttpContext? httpContext, List? args = null) @@ -158,11 +158,16 @@ await ExecuteQueryAgainstDbAsync(conn, cmd.CommandText = sessionParamsQuery + sqltext; if (parameters is not null) { - foreach (KeyValuePair parameterEntry in parameters) + foreach (KeyValuePair> parameterEntry in parameters) { DbParameter parameter = cmd.CreateParameter(); parameter.ParameterName = parameterEntry.Key; - parameter.Value = parameterEntry.Value ?? DBNull.Value; + parameter.Value = parameterEntry.Value.Item1 ?? DBNull.Value; + if (parameterEntry.Value.Item2 is not null) + { + parameter.DbType = (DbType)parameterEntry.Value.Item2; + } + cmd.Parameters.Add(parameter); } } @@ -190,7 +195,7 @@ await ExecuteQueryAgainstDbAsync(conn, } /// - public virtual string GetSessionParamsQuery(HttpContext? httpContext, IDictionary parameters) + public virtual string GetSessionParamsQuery(HttpContext? httpContext, IDictionary> parameters) { return string.Empty; } diff --git a/src/Service/Resolvers/Sql Query Structures/BaseSqlQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/BaseSqlQueryStructure.cs index 6902284140..7040aea7dd 100644 --- a/src/Service/Resolvers/Sql Query Structures/BaseSqlQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/BaseSqlQueryStructure.cs @@ -87,18 +87,6 @@ public BaseSqlQueryStructure( } } - public override string MakeParamWithValue(object? value, string? columnName = null) - { - string paramName = $"{PARAM_NAME_PREFIX}param{Counter.Next()}"; - Parameters.Add(paramName, value); - if (!string.IsNullOrEmpty(columnName)) - { - ParamToDbTypeMap.Add(paramName, GetUnderlyingSourceDefinition().Columns[columnName].DbType); - } - - return paramName; - } - /// /// For UPDATE (OVERWRITE) operation /// Adds result of (SourceDefinition.Columns minus MutationFields) to UpdateOperations with null values diff --git a/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs index 4350cd7b52..616187140f 100644 --- a/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Data; using System.Linq; using System.Net; using Azure.DataApiBuilder.Auth; @@ -663,7 +664,7 @@ private void AddGraphQLFields(IReadOnlyList selections, RuntimeC // pass the parameters of the subquery to the current query so upmost query has all the // parameters of the query tree and it can pass them to the database query executor - foreach (KeyValuePair parameter in subquery.Parameters) + foreach (KeyValuePair> parameter in subquery.Parameters) { Parameters.Add(parameter.Key, parameter.Value); } diff --git a/src/Service/Resolvers/SqlMutationEngine.cs b/src/Service/Resolvers/SqlMutationEngine.cs index 920b82c557..1c753cdf11 100644 --- a/src/Service/Resolvers/SqlMutationEngine.cs +++ b/src/Service/Resolvers/SqlMutationEngine.cs @@ -435,7 +435,7 @@ private async Task IMiddlewareContext? context = null) { string queryString; - Dictionary queryParameters; + Dictionary> queryParameters; switch (operationType) { case Config.Operation.Insert: @@ -597,7 +597,7 @@ private async Task?> IDictionary parameters) { string queryString; - Dictionary queryParameters; + Dictionary> queryParameters; SqlDeleteStructure deleteStructure = new( entityName, _sqlMetadataProvider, @@ -632,7 +632,7 @@ private async Task RestRequestContext context) { string queryString; - Dictionary queryParameters; + Dictionary> queryParameters; Config.Operation operationType = context.OperationType; string entityName = context.EntityName; diff --git a/src/Service/Services/MetadataProviders/MySqlMetadataProvider.cs b/src/Service/Services/MetadataProviders/MySqlMetadataProvider.cs index 4e434b145b..9058ea6930 100644 --- a/src/Service/Services/MetadataProviders/MySqlMetadataProvider.cs +++ b/src/Service/Services/MetadataProviders/MySqlMetadataProvider.cs @@ -70,13 +70,13 @@ protected override async Task GetColumnsAsync( /// For MySql, the table name is only a 2 part name. /// The database name from the connection string needs to be used instead of schemaName. /// - protected override Dictionary + protected override Dictionary> GetForeignKeyQueryParams( string[] schemaNames, string[] tableNames) { MySqlConnectionStringBuilder connBuilder = new(ConnectionString); - Dictionary parameters = new(); + Dictionary> parameters = new(); string[] databaseNameParams = BaseSqlQueryBuilder.CreateParams( @@ -89,12 +89,12 @@ protected override async Task GetColumnsAsync( for (int i = 0; i < schemaNames.Count(); ++i) { - parameters.Add(databaseNameParams[i], connBuilder.Database); + parameters.Add(databaseNameParams[i], new(connBuilder.Database, DbType.String)); } for (int i = 0; i < tableNames.Count(); ++i) { - parameters.Add(tableNameParams[i], tableNames[i]); + parameters.Add(tableNameParams[i], new(tableNames[i], DbType.String)); } return parameters; diff --git a/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs b/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs index 54c9a56799..063fc601d0 100644 --- a/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs +++ b/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs @@ -18,7 +18,6 @@ using Azure.DataApiBuilder.Service.Models; using Azure.DataApiBuilder.Service.Parsers; using Azure.DataApiBuilder.Service.Resolvers; -using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; using static Azure.DataApiBuilder.Service.GraphQLBuilder.GraphQLNaming; @@ -534,12 +533,12 @@ protected virtual DatabaseTable GenerateDbTable(string schemaName, string tableN /// /// /// The dictionary populated with parameters. - protected virtual Dictionary + protected virtual Dictionary> GetForeignKeyQueryParams( string[] schemaNames, string[] tableNames) { - Dictionary parameters = new(); + Dictionary> parameters = new(); string[] schemaNameParams = BaseSqlQueryBuilder.CreateParams( kindOfParam: BaseSqlQueryBuilder.SCHEMA_NAME_PARAM, @@ -551,12 +550,12 @@ protected virtual DatabaseTable GenerateDbTable(string schemaName, string tableN for (int i = 0; i < schemaNames.Count(); ++i) { - parameters.Add(schemaNameParams[i], schemaNames[i]); + parameters.Add(schemaNameParams[i], new(schemaNames[i], DbType.String)); } for (int i = 0; i < tableNames.Count(); ++i) { - parameters.Add(tableNameParams[i], tableNames[i]); + parameters.Add(tableNameParams[i], new(tableNames[i], DbType.String)); } return parameters; @@ -1319,7 +1318,7 @@ private async Task PopulateForeignKeyDefinitionAsync() // Build the parameters dictionary for the foreign key info query // consisting of all schema names and table names. - Dictionary parameters = + Dictionary> parameters = GetForeignKeyQueryParams( schemaNames.ToArray(), tableNames.ToArray()); From 0a9487f391a9906f04685224774eaef63b394a21 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Tue, 18 Apr 2023 19:18:03 +0530 Subject: [PATCH 03/22] reverting change --- .../Sql Query Structures/SqlExecuteQueryStructure.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs index 21a428f956..2ea8a5c50f 100644 --- a/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs @@ -48,11 +48,11 @@ public SqlExecuteStructure( if (requestParamValue is not null) { Type systemType = GetUnderlyingStoredProcedureDefinition().Parameters[paramKey].SystemType!; - parametrizedName = MakeParamWithValue(GetParamAsSystemType(requestParamValue.ToString()!, paramKey, systemType), paramKey); + parametrizedName = MakeParamWithValue(GetParamAsSystemType(requestParamValue.ToString()!, paramKey, systemType)); } else { - parametrizedName = MakeParamWithValue(null, paramKey); + parametrizedName = MakeParamWithValue(null); } ProcedureParameters.Add(paramKey, $"{parametrizedName}"); From 3ad1b7a35669a703a47687047a75a29903381e27 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Tue, 18 Apr 2023 19:53:12 +0530 Subject: [PATCH 04/22] Adding DbType to params for MsSql --- src/Service/Resolvers/QueryExecutor.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Service/Resolvers/QueryExecutor.cs b/src/Service/Resolvers/QueryExecutor.cs index 1a8c273f72..0ad4295b74 100644 --- a/src/Service/Resolvers/QueryExecutor.cs +++ b/src/Service/Resolvers/QueryExecutor.cs @@ -11,6 +11,7 @@ using System.Text.Json; using System.Text.Json.Nodes; using System.Threading.Tasks; +using Azure.DataApiBuilder.Config; using Azure.DataApiBuilder.Service.Configurations; using Azure.DataApiBuilder.Service.Exceptions; using Azure.DataApiBuilder.Service.Models; @@ -163,7 +164,7 @@ await ExecuteQueryAgainstDbAsync(conn, DbParameter parameter = cmd.CreateParameter(); parameter.ParameterName = parameterEntry.Key; parameter.Value = parameterEntry.Value.Item1 ?? DBNull.Value; - if (parameterEntry.Value.Item2 is not null) + if (parameterEntry.Value.Item2 is not null && ConfigProvider.GetRuntimeConfiguration().DatabaseType is DatabaseType.mssql) { parameter.DbType = (DbType)parameterEntry.Value.Item2; } From faa240bc4923fdec75a7dc118e5ac9462e5a9e36 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Tue, 18 Apr 2023 21:09:17 +0530 Subject: [PATCH 05/22] fixing mssql test --- src/Service.Tests/Unittests/SqlQueryExecutorUnitTests.cs | 4 ++-- src/Service/Parsers/ODataASTVisitor.cs | 3 --- src/Service/Resolvers/CosmosQueryEngine.cs | 4 ++-- src/Service/Services/MetadataProviders/SqlMetadataProvider.cs | 4 ++-- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/Service.Tests/Unittests/SqlQueryExecutorUnitTests.cs b/src/Service.Tests/Unittests/SqlQueryExecutorUnitTests.cs index 95daa121cf..e1699b2288 100644 --- a/src/Service.Tests/Unittests/SqlQueryExecutorUnitTests.cs +++ b/src/Service.Tests/Unittests/SqlQueryExecutorUnitTests.cs @@ -156,7 +156,7 @@ Mock queryExecutor { await queryExecutor.Object.ExecuteQueryAsync( sqltext: string.Empty, - parameters: new Dictionary>(), + parameters: new Dictionary>(), dataReaderHandler: null, httpContext: null, args: null); @@ -210,7 +210,7 @@ Mock queryExecutor await queryExecutor.Object.ExecuteQueryAsync( sqltext: sqltext, - parameters: new Dictionary>(), + parameters: new Dictionary>(), dataReaderHandler: null, args: null); diff --git a/src/Service/Parsers/ODataASTVisitor.cs b/src/Service/Parsers/ODataASTVisitor.cs index 54afbe04c7..8ff30ef041 100644 --- a/src/Service/Parsers/ODataASTVisitor.cs +++ b/src/Service/Parsers/ODataASTVisitor.cs @@ -95,9 +95,6 @@ public override string Visit(ConstantNode nodeIn) return $"{_struct.MakeParamWithValue(GetParamWithSystemType(nodeIn.Value.ToString()!, nodeIn.TypeReference))}"; } - // This is necessary because we want to create a mapping between column name and its corresponding param name. - _struct.MakeParamWithValue(null); - // Represents a NULL value, we support NULL in queries so return "NULL" here return "NULL"; } diff --git a/src/Service/Resolvers/CosmosQueryEngine.cs b/src/Service/Resolvers/CosmosQueryEngine.cs index cf6fee70a0..294dfaa57a 100644 --- a/src/Service/Resolvers/CosmosQueryEngine.cs +++ b/src/Service/Resolvers/CosmosQueryEngine.cs @@ -72,7 +72,7 @@ public async Task> ExecuteAsync( Container container = _clientProvider.Client.GetDatabase(structure.Database).GetContainer(structure.Container); (string idValue, string partitionKeyValue) = await GetIdAndPartitionKey(parameters, container, structure); - foreach (KeyValuePair> parameterEntry in structure.Parameters) + foreach (KeyValuePair> parameterEntry in structure.Parameters) { querySpec = querySpec.WithParameter(parameterEntry.Key, parameterEntry.Value.Item1); } @@ -155,7 +155,7 @@ public async Task, IMetadata>> ExecuteListAsync( Container container = _clientProvider.Client.GetDatabase(structure.Database).GetContainer(structure.Container); QueryDefinition querySpec = new(_queryBuilder.Build(structure)); - foreach (KeyValuePair> parameterEntry in structure.Parameters) + foreach (KeyValuePair> parameterEntry in structure.Parameters) { querySpec = querySpec.WithParameter(parameterEntry.Key, parameterEntry.Value.Item1); } diff --git a/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs b/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs index 063fc601d0..3bd65f0f72 100644 --- a/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs +++ b/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs @@ -62,7 +62,7 @@ public abstract class SqlMetadataProvider : [typeof(string)] = DbType.String, [typeof(char)] = DbType.StringFixedLength, [typeof(Guid)] = DbType.Guid, - [typeof(DateTime)] = DbType.DateTime, + [typeof(DateTime)] = DbType.DateTimeOffset, [typeof(DateTimeOffset)] = DbType.DateTimeOffset, [typeof(TimeSpan)] = DbType.Time, [typeof(byte[])] = DbType.Binary, @@ -80,7 +80,7 @@ public abstract class SqlMetadataProvider : [typeof(bool?)] = DbType.Boolean, [typeof(char?)] = DbType.StringFixedLength, [typeof(Guid?)] = DbType.Guid, - [typeof(DateTime?)] = DbType.DateTime, + [typeof(DateTime?)] = DbType.DateTimeOffset, [typeof(DateTimeOffset?)] = DbType.DateTimeOffset, [typeof(TimeSpan?)] = DbType.Time, [typeof(object)] = DbType.Object From 2e9c9f70a43aebbdc2a2b7970be0dfc0e323ee38 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Wed, 19 Apr 2023 00:39:11 +0530 Subject: [PATCH 06/22] Removing dbtype for datetime/datimeoffset since not required and causing problem for gql --- src/Config/DatabaseObject.cs | 2 +- .../MsSqlGQLSupportedTypesTests.cs | 14 -------------- src/Service/Parsers/ODataASTVisitor.cs | 2 +- .../MetadataProviders/SqlMetadataProvider.cs | 14 ++------------ 4 files changed, 4 insertions(+), 28 deletions(-) diff --git a/src/Config/DatabaseObject.cs b/src/Config/DatabaseObject.cs index f6aec1cbf1..7ab0087f1e 100644 --- a/src/Config/DatabaseObject.cs +++ b/src/Config/DatabaseObject.cs @@ -180,7 +180,7 @@ public class ColumnDefinition /// The database type of this column mapped to the SystemType. /// public Type SystemType { get; set; } = typeof(object); - public DbType DbType { get; set; } + public DbType? DbType { get; set; } public bool HasDefault { get; set; } public bool IsAutoGenerated { get; set; } public bool IsNullable { get; set; } diff --git a/src/Service.Tests/SqlTests/GraphQLSupportedTypesTests/MsSqlGQLSupportedTypesTests.cs b/src/Service.Tests/SqlTests/GraphQLSupportedTypesTests/MsSqlGQLSupportedTypesTests.cs index 6db7872cdc..9fcaaf11a6 100644 --- a/src/Service.Tests/SqlTests/GraphQLSupportedTypesTests/MsSqlGQLSupportedTypesTests.cs +++ b/src/Service.Tests/SqlTests/GraphQLSupportedTypesTests/MsSqlGQLSupportedTypesTests.cs @@ -34,19 +34,5 @@ ORDER BY id asc INCLUDE_NULL_VALUES "; } - - /// - /// 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 - /// - protected override bool IsSupportedType(string type) - { - if (type.Equals(BYTEARRAY_TYPE)) - { - return false; - } - - return true; - } } } diff --git a/src/Service/Parsers/ODataASTVisitor.cs b/src/Service/Parsers/ODataASTVisitor.cs index 8ff30ef041..b2ba2ac5b2 100644 --- a/src/Service/Parsers/ODataASTVisitor.cs +++ b/src/Service/Parsers/ODataASTVisitor.cs @@ -45,7 +45,7 @@ public override string Visit(BinaryOperatorNode nodeIn) (SingleValuePropertyAccessNode)nodeIn.Left : (SingleValuePropertyAccessNode)nodeIn.Right; string? paramName = $"{BaseQueryStructure.PARAM_NAME_PREFIX}param{_struct.Counter.Current() - 1}"; _metadataProvider.TryGetBackingColumn(_struct.EntityName, propertyNode.Property.Name, out string? backingColumnName); - _struct.ParamToDbTypeMap.Add(paramName, _struct.GetUnderlyingSourceDefinition().Columns[backingColumnName!].DbType); + _struct.Parameters[paramName] = new(_struct.Parameters[paramName].Item1, _struct.GetUnderlyingSourceDefinition().Columns[backingColumnName!].DbType); } return CreateResult(nodeIn.OperatorKind, left, right); diff --git a/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs b/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs index 3bd65f0f72..1368315869 100644 --- a/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs +++ b/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs @@ -62,9 +62,6 @@ public abstract class SqlMetadataProvider : [typeof(string)] = DbType.String, [typeof(char)] = DbType.StringFixedLength, [typeof(Guid)] = DbType.Guid, - [typeof(DateTime)] = DbType.DateTimeOffset, - [typeof(DateTimeOffset)] = DbType.DateTimeOffset, - [typeof(TimeSpan)] = DbType.Time, [typeof(byte[])] = DbType.Binary, [typeof(byte?)] = DbType.Byte, [typeof(sbyte?)] = DbType.SByte, @@ -80,9 +77,6 @@ public abstract class SqlMetadataProvider : [typeof(bool?)] = DbType.Boolean, [typeof(char?)] = DbType.StringFixedLength, [typeof(Guid?)] = DbType.Guid, - [typeof(DateTime?)] = DbType.DateTimeOffset, - [typeof(DateTimeOffset?)] = DbType.DateTimeOffset, - [typeof(TimeSpan?)] = DbType.Time, [typeof(object)] = DbType.Object }; @@ -1064,15 +1058,11 @@ private async Task PopulateSourceDefinitionAsync( columnsInTable); } - private DbType GetDbTypeFromSystemType(Type systemType, string columnName) + private DbType? GetDbTypeFromSystemType(Type systemType, string columnName) { if (!typeMap.TryGetValue(systemType, out DbType dbType)) { - throw new DataApiBuilderException( - message: $"Unsupported data type encountered for column {columnName}.", - statusCode: HttpStatusCode.InternalServerError, - subStatusCode: DataApiBuilderException.SubStatusCodes.NotSupported - ); + return null; } return dbType; From 923a602e6033ab7ca61642685585b2c7ffd4ea35 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Wed, 19 Apr 2023 01:04:32 +0530 Subject: [PATCH 07/22] Fixing cosmos tests --- src/Service/Resolvers/BaseQueryStructure.cs | 2 +- src/Service/Resolvers/CosmosQueryStructure.cs | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Service/Resolvers/BaseQueryStructure.cs b/src/Service/Resolvers/BaseQueryStructure.cs index b1413dda26..77bbb66316 100644 --- a/src/Service/Resolvers/BaseQueryStructure.cs +++ b/src/Service/Resolvers/BaseQueryStructure.cs @@ -111,7 +111,7 @@ public BaseQueryStructure( /// Add parameter to Parameters and return the name associated with it /// /// Value to be assigned to parameter, which can be null for nullable columns. - public string MakeParamWithValue(object? value, string? columnName = null) + public virtual string MakeParamWithValue(object? value, string? columnName = null) { string paramName = $"{PARAM_NAME_PREFIX}param{Counter.Next()}"; if (!string.IsNullOrEmpty(columnName)) diff --git a/src/Service/Resolvers/CosmosQueryStructure.cs b/src/Service/Resolvers/CosmosQueryStructure.cs index d8e323a13b..efcb91aed3 100644 --- a/src/Service/Resolvers/CosmosQueryStructure.cs +++ b/src/Service/Resolvers/CosmosQueryStructure.cs @@ -47,6 +47,14 @@ public CosmosQueryStructure( Init(parameters); } + /// + public override string MakeParamWithValue(object? value, string? columnName = null) + { + string paramName = $"{PARAM_NAME_PREFIX}param{Counter.Next()}"; + Parameters.Add(paramName, new(value, null)); + return paramName; + } + private static IEnumerable GenerateQueryColumns(SelectionSetNode selectionSet, DocumentNode document, string tableName) { foreach (ISelectionNode selectionNode in selectionSet.Selections) From 20deea69b08ebf99a071337735fefd3e1f31d925 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Wed, 19 Apr 2023 01:10:11 +0530 Subject: [PATCH 08/22] build failure fixes --- .../GraphQLSupportedTypesTests/MsSqlGQLSupportedTypesTests.cs | 1 - .../Resolvers/Sql Query Structures/SqlQueryStructure.cs | 2 +- src/Service/Services/MetadataProviders/SqlMetadataProvider.cs | 4 ++-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Service.Tests/SqlTests/GraphQLSupportedTypesTests/MsSqlGQLSupportedTypesTests.cs b/src/Service.Tests/SqlTests/GraphQLSupportedTypesTests/MsSqlGQLSupportedTypesTests.cs index 9fcaaf11a6..62e9b42b11 100644 --- a/src/Service.Tests/SqlTests/GraphQLSupportedTypesTests/MsSqlGQLSupportedTypesTests.cs +++ b/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 { diff --git a/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs index 616187140f..8f5aab642b 100644 --- a/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs @@ -664,7 +664,7 @@ private void AddGraphQLFields(IReadOnlyList selections, RuntimeC // pass the parameters of the subquery to the current query so upmost query has all the // parameters of the query tree and it can pass them to the database query executor - foreach (KeyValuePair> parameter in subquery.Parameters) + foreach (KeyValuePair> parameter in subquery.Parameters) { Parameters.Add(parameter.Key, parameter.Value); } diff --git a/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs b/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs index 1368315869..ac961c9730 100644 --- a/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs +++ b/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs @@ -1041,7 +1041,7 @@ private async Task PopulateSourceDefinitionAsync( IsNullable = (bool)columnInfoFromAdapter["AllowDBNull"], IsAutoGenerated = (bool)columnInfoFromAdapter["IsAutoIncrement"], SystemType = systemTypeOfColumn, - DbType = GetDbTypeFromSystemType(systemTypeOfColumn, columnName) + DbType = GetDbTypeFromSystemType(systemTypeOfColumn) }; // Tests may try to add the same column simultaneously @@ -1058,7 +1058,7 @@ private async Task PopulateSourceDefinitionAsync( columnsInTable); } - private DbType? GetDbTypeFromSystemType(Type systemType, string columnName) + private DbType? GetDbTypeFromSystemType(Type systemType) { if (!typeMap.TryGetValue(systemType, out DbType dbType)) { From b3357b8a95b2d1cf5f1765e3a261c80c285471a4 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Wed, 19 Apr 2023 01:14:46 +0530 Subject: [PATCH 09/22] formatting fix --- src/Service/Resolvers/MsSqlQueryExecutor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Service/Resolvers/MsSqlQueryExecutor.cs b/src/Service/Resolvers/MsSqlQueryExecutor.cs index ae39e9d20f..f786104fa6 100644 --- a/src/Service/Resolvers/MsSqlQueryExecutor.cs +++ b/src/Service/Resolvers/MsSqlQueryExecutor.cs @@ -170,7 +170,7 @@ await AzureCredential.GetTokenAsync( /// Dictionary of parameters/value required to execute the query. /// empty string / query to set session parameters for the connection. /// - public override string GetSessionParamsQuery(HttpContext? httpContext, IDictionary> parameters) + public override string GetSessionParamsQuery(HttpContext? httpContext, IDictionary> parameters) { if (httpContext is null || !_isSessionContextEnabled) { From 7021fbc603ae66b3fd1a3d8e571a6e70ddae1078 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Wed, 19 Apr 2023 02:03:14 +0530 Subject: [PATCH 10/22] fixing tests --- src/Service.Tests/Unittests/ODataASTVisitorUnitTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Service.Tests/Unittests/ODataASTVisitorUnitTests.cs b/src/Service.Tests/Unittests/ODataASTVisitorUnitTests.cs index c8e6313ce4..8cb36bfaaf 100644 --- a/src/Service.Tests/Unittests/ODataASTVisitorUnitTests.cs +++ b/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 }; return new ODataASTVisitor(structure.Object, _sqlMetadataProvider); } From fa7d67e18f9f107ecc727f8a5ce6f795a6e8c30b Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Wed, 19 Apr 2023 11:31:37 +0530 Subject: [PATCH 11/22] Adding helper class for type resolution --- src/Service/Services/DbTypeHelper.cs | 65 +++++++++++++++++++ .../MetadataProviders/SqlMetadataProvider.cs | 47 +------------- 2 files changed, 66 insertions(+), 46 deletions(-) create mode 100644 src/Service/Services/DbTypeHelper.cs diff --git a/src/Service/Services/DbTypeHelper.cs b/src/Service/Services/DbTypeHelper.cs new file mode 100644 index 0000000000..0877bc37ed --- /dev/null +++ b/src/Service/Services/DbTypeHelper.cs @@ -0,0 +1,65 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Collections.Generic; +using System.Data; +using System; + +namespace Azure.DataApiBuilder.Service.Services +{ + /// + /// Helper class used to resolve the underlying DbType for the parameter for its given SystemType. + /// + public static class DbTypeHelper + { + private static Dictionary _systemTypeToDbTypeMap = new() + { + [typeof(byte)] = DbType.Byte, + [typeof(sbyte)] = DbType.SByte, + [typeof(short)] = DbType.Int16, + [typeof(ushort)] = DbType.UInt16, + [typeof(int)] = DbType.Int32, + [typeof(uint)] = DbType.UInt32, + [typeof(long)] = DbType.Int64, + [typeof(ulong)] = DbType.UInt64, + [typeof(float)] = DbType.Single, + [typeof(double)] = DbType.Double, + [typeof(decimal)] = DbType.Decimal, + [typeof(bool)] = DbType.Boolean, + [typeof(string)] = DbType.String, + [typeof(char)] = DbType.StringFixedLength, + [typeof(Guid)] = DbType.Guid, + [typeof(byte[])] = DbType.Binary, + [typeof(byte?)] = DbType.Byte, + [typeof(sbyte?)] = DbType.SByte, + [typeof(short?)] = DbType.Int16, + [typeof(ushort?)] = DbType.UInt16, + [typeof(int?)] = DbType.Int32, + [typeof(uint?)] = DbType.UInt32, + [typeof(long?)] = DbType.Int64, + [typeof(ulong?)] = DbType.UInt64, + [typeof(float?)] = DbType.Single, + [typeof(double?)] = DbType.Double, + [typeof(decimal?)] = DbType.Decimal, + [typeof(bool?)] = DbType.Boolean, + [typeof(char?)] = DbType.StringFixedLength, + [typeof(Guid?)] = DbType.Guid, + [typeof(object)] = DbType.Object + }; + + /// + /// Returns the DbType for given system type. + /// + /// The system type for which the DbType is to be determined. + /// DbType for the given system tyep. + public static DbType? GetDbTypeFromSystemType(Type systemType) + { + if (!_systemTypeToDbTypeMap.TryGetValue(systemType, out DbType dbType)) + { + return null; + } + + return dbType; + } + } +} diff --git a/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs b/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs index ac961c9730..ff871cac89 100644 --- a/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs +++ b/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs @@ -45,41 +45,6 @@ public abstract class SqlMetadataProvider : // to their corresponding entity names defined in the config. public Dictionary GraphQLStoredProcedureExposedNameToEntityNameMap { get; set; } = new(); - public Dictionary typeMap = new() - { - [typeof(byte)] = DbType.Byte, - [typeof(sbyte)] = DbType.SByte, - [typeof(short)] = DbType.Int16, - [typeof(ushort)] = DbType.UInt16, - [typeof(int)] = DbType.Int32, - [typeof(uint)] = DbType.UInt32, - [typeof(long)] = DbType.Int64, - [typeof(ulong)] = DbType.UInt64, - [typeof(float)] = DbType.Single, - [typeof(double)] = DbType.Double, - [typeof(decimal)] = DbType.Decimal, - [typeof(bool)] = DbType.Boolean, - [typeof(string)] = DbType.String, - [typeof(char)] = DbType.StringFixedLength, - [typeof(Guid)] = DbType.Guid, - [typeof(byte[])] = DbType.Binary, - [typeof(byte?)] = DbType.Byte, - [typeof(sbyte?)] = DbType.SByte, - [typeof(short?)] = DbType.Int16, - [typeof(ushort?)] = DbType.UInt16, - [typeof(int?)] = DbType.Int32, - [typeof(uint?)] = DbType.UInt32, - [typeof(long?)] = DbType.Int64, - [typeof(ulong?)] = DbType.UInt64, - [typeof(float?)] = DbType.Single, - [typeof(double?)] = DbType.Double, - [typeof(decimal?)] = DbType.Decimal, - [typeof(bool?)] = DbType.Boolean, - [typeof(char?)] = DbType.StringFixedLength, - [typeof(Guid?)] = DbType.Guid, - [typeof(object)] = DbType.Object - }; - // Contains all the referencing and referenced columns for each pair // of referencing and referenced tables. public Dictionary? PairToFkDefinition { get; set; } @@ -1041,7 +1006,7 @@ private async Task PopulateSourceDefinitionAsync( IsNullable = (bool)columnInfoFromAdapter["AllowDBNull"], IsAutoGenerated = (bool)columnInfoFromAdapter["IsAutoIncrement"], SystemType = systemTypeOfColumn, - DbType = GetDbTypeFromSystemType(systemTypeOfColumn) + DbType = DbTypeHelper.GetDbTypeFromSystemType(systemTypeOfColumn) }; // Tests may try to add the same column simultaneously @@ -1058,16 +1023,6 @@ private async Task PopulateSourceDefinitionAsync( columnsInTable); } - private DbType? GetDbTypeFromSystemType(Type systemType) - { - if (!typeMap.TryGetValue(systemType, out DbType dbType)) - { - return null; - } - - return dbType; - } - /// /// Determine whether the provided field of a GraphQL enabled entity meets GraphQL reserved name requirements. /// Criteria: From f70d9e8daec441fc193a081e44e7cc7e3990c535 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Wed, 19 Apr 2023 15:35:50 +0530 Subject: [PATCH 12/22] refactor --- src/Config/DatabaseObject.cs | 29 +++++++++++++++++++ src/Service/Resolvers/BaseQueryStructure.cs | 12 ++++---- .../SqlExecuteQueryStructure.cs | 4 +-- .../MetadataProviders/SqlMetadataProvider.cs | 6 ++-- 4 files changed, 41 insertions(+), 10 deletions(-) diff --git a/src/Config/DatabaseObject.cs b/src/Config/DatabaseObject.cs index 7ab0087f1e..9603069dfc 100644 --- a/src/Config/DatabaseObject.cs +++ b/src/Config/DatabaseObject.cs @@ -107,11 +107,23 @@ public class StoredProcedureDefinition : SourceDefinition /// Key: parameter name, Value: ParameterDefinition object /// public Dictionary Parameters { get; set; } = new(); + + /// + 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; } } @@ -155,6 +167,23 @@ public bool IsAnyColumnNullable(List columnsToCheck) .Where(isNullable => isNullable == true) .Any(); } + + /// + /// Method to get the DbType for: + /// 1. column for table/view, + /// 2. parameter for stored procedure. + /// + /// The parameter whose DbType is to be determined. + /// + public virtual DbType? GetDbTypeForParam(string paramName) + { + if (Columns.TryGetValue(paramName, out ColumnDefinition? columnDefinition)) + { + return columnDefinition.DbType; + } + + return null; + } } /// diff --git a/src/Service/Resolvers/BaseQueryStructure.cs b/src/Service/Resolvers/BaseQueryStructure.cs index 77bbb66316..9b06d72a43 100644 --- a/src/Service/Resolvers/BaseQueryStructure.cs +++ b/src/Service/Resolvers/BaseQueryStructure.cs @@ -111,19 +111,19 @@ public BaseQueryStructure( /// Add parameter to Parameters and return the name associated with it /// /// Value to be assigned to parameter, which can be null for nullable columns. - public virtual string MakeParamWithValue(object? value, string? columnName = null) + public virtual string MakeParamWithValue(object? value, string? paramName = null) { - string paramName = $"{PARAM_NAME_PREFIX}param{Counter.Next()}"; - if (!string.IsNullOrEmpty(columnName)) + string encodedParamName = $"{PARAM_NAME_PREFIX}param{Counter.Next()}"; + if (!string.IsNullOrEmpty(paramName)) { - Parameters.Add(paramName, new(value, GetUnderlyingSourceDefinition().Columns[columnName].DbType)); + Parameters.Add(encodedParamName, new(value, GetUnderlyingSourceDefinition().GetDbTypeForParam(paramName))); } else { - Parameters.Add(paramName, new(value, null)); + Parameters.Add(encodedParamName, new(value, null)); } - return paramName; + return encodedParamName; } /// diff --git a/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs index 2ea8a5c50f..f854d0fdb9 100644 --- a/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs @@ -52,7 +52,7 @@ public SqlExecuteStructure( } else { - parametrizedName = MakeParamWithValue(null); + parametrizedName = MakeParamWithValue(value: null); } ProcedureParameters.Add(paramKey, $"{parametrizedName}"); @@ -62,7 +62,7 @@ public SqlExecuteStructure( // Fill with default value from runtime config if (paramDefinition.HasConfigDefault) { - string parameterizedName = MakeParamWithValue(paramDefinition.ConfigDefaultValue, paramKey); + string parameterizedName = MakeParamWithValue(paramDefinition.ConfigDefaultValue); ProcedureParameters.Add(paramKey, $"{parameterizedName}"); } else diff --git a/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs b/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs index ff871cac89..f15c8d6356 100644 --- a/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs +++ b/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs @@ -310,12 +310,14 @@ private async Task FillSchemaForStoredProcedureAsync( // For each row/parameter, add an entry to StoredProcedureDefinition.Parameters dictionary foreach (DataRow row in parameterMetadata.Rows) { + // row["DATA_TYPE"] has value type string so a direct cast to System.Type is not supported. + Type systemType = SqlToCLRType((string)row["DATA_TYPE"]); // Add to parameters dictionary without the leading @ sign storedProcedureDefinition.Parameters.TryAdd(((string)row["PARAMETER_NAME"])[1..], new() { - // row["DATA_TYPE"] has value type string so a direct cast to System.Type is not supported. - SystemType = SqlToCLRType((string)row["DATA_TYPE"]), + SystemType = systemType, + DbType = DbTypeHelper.GetDbTypeFromSystemType(systemType) } ); } From 034d55b0a4721a180e403de1bcb0935f1e8e6031 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Wed, 19 Apr 2023 16:59:05 +0530 Subject: [PATCH 13/22] Fixing import order --- src/Service/Services/DbTypeHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Service/Services/DbTypeHelper.cs b/src/Service/Services/DbTypeHelper.cs index 0877bc37ed..d7cbc30c1e 100644 --- a/src/Service/Services/DbTypeHelper.cs +++ b/src/Service/Services/DbTypeHelper.cs @@ -1,9 +1,9 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System; using System.Collections.Generic; using System.Data; -using System; namespace Azure.DataApiBuilder.Service.Services { From e90090f39a4da5e785bd6681529b7561c0b49ae2 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Mon, 24 Apr 2023 13:34:58 +0530 Subject: [PATCH 14/22] adding param type for SPs --- .../Sql Query Structures/SqlExecuteQueryStructure.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs index f854d0fdb9..3fcf64ac1f 100644 --- a/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs @@ -40,6 +40,7 @@ public SqlExecuteStructure( ProcedureParameters = new(); foreach ((string paramKey, ParameterDefinition paramDefinition) in storedProcedureDefinition.Parameters) { + Type systemType = GetUnderlyingStoredProcedureDefinition().Parameters[paramKey].SystemType!; // Populate with request param if able if (requestParams.TryGetValue(paramKey, out object? requestParamValue)) { @@ -47,12 +48,11 @@ public SqlExecuteStructure( string? parametrizedName = null; if (requestParamValue is not null) { - Type systemType = GetUnderlyingStoredProcedureDefinition().Parameters[paramKey].SystemType!; - parametrizedName = MakeParamWithValue(GetParamAsSystemType(requestParamValue.ToString()!, paramKey, systemType)); + parametrizedName = MakeParamWithValue(GetParamAsSystemType(requestParamValue.ToString()!, paramKey, systemType), paramKey); } else { - parametrizedName = MakeParamWithValue(value: null); + parametrizedName = MakeParamWithValue(null, paramKey); } ProcedureParameters.Add(paramKey, $"{parametrizedName}"); @@ -62,7 +62,8 @@ public SqlExecuteStructure( // Fill with default value from runtime config if (paramDefinition.HasConfigDefault) { - string parameterizedName = MakeParamWithValue(paramDefinition.ConfigDefaultValue); + object? value = paramDefinition.ConfigDefaultValue == null ? null : GetParamAsSystemType(paramDefinition.ConfigDefaultValue!.ToString()!, paramKey, systemType); + string parameterizedName = MakeParamWithValue(value, paramKey); ProcedureParameters.Add(paramKey, $"{parameterizedName}"); } else From 3e693f6ad1da9d72ce11046f6f776e6dbfff89df Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Mon, 24 Apr 2023 15:14:25 +0530 Subject: [PATCH 15/22] single->real --- .../Services/MetadataProviders/MsSqlMetadataProvider.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Service/Services/MetadataProviders/MsSqlMetadataProvider.cs b/src/Service/Services/MetadataProviders/MsSqlMetadataProvider.cs index 6b651bde6a..4e97ae6252 100644 --- a/src/Service/Services/MetadataProviders/MsSqlMetadataProvider.cs +++ b/src/Service/Services/MetadataProviders/MsSqlMetadataProvider.cs @@ -43,14 +43,13 @@ public override Type SqlToCLRType(string sqlType) switch (sqlType) { case "bigint": - case "real": - return typeof(long); case "numeric": return typeof(decimal); case "bit": return typeof(bool); case "smallint": return typeof(short); + case "real": case "decimal": case "smallmoney": case "money": From acc38132e774baa1a3c8cdd1a13d5078beecc19d Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Wed, 26 Apr 2023 10:16:38 +0530 Subject: [PATCH 16/22] test for null bytearray --- .../RestApiTests/Insert/InsertApiTestBase.cs | 27 +++++++++++++++++++ .../Insert/MsSqlInsertApiTests.cs | 8 ++++++ .../Insert/MySqlInsertApiTests.cs | 11 ++++++++ .../Insert/PostgreSqlInsertApiTests.cs | 12 +++++++++ 4 files changed, 58 insertions(+) diff --git a/src/Service.Tests/SqlTests/RestApiTests/Insert/InsertApiTestBase.cs b/src/Service.Tests/SqlTests/RestApiTests/Insert/InsertApiTestBase.cs index 9ffd888209..9ecc8f5eeb 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Insert/InsertApiTestBase.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Insert/InsertApiTestBase.cs @@ -60,6 +60,33 @@ await SetupAndRunRestApiTest( ); } + /// + /// 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. + /// + /// + [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 + ); + } + /// /// Tests insertion on simple/composite views. /// diff --git a/src/Service.Tests/SqlTests/RestApiTests/Insert/MsSqlInsertApiTests.cs b/src/Service.Tests/SqlTests/RestApiTests/Insert/MsSqlInsertApiTests.cs index a56a78ded8..024e22f028 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Insert/MsSqlInsertApiTests.cs +++ b/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 } " + diff --git a/src/Service.Tests/SqlTests/RestApiTests/Insert/MySqlInsertApiTests.cs b/src/Service.Tests/SqlTests/RestApiTests/Insert/MySqlInsertApiTests.cs index a1558df56c..517fc32136 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Insert/MySqlInsertApiTests.cs +++ b/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", @" diff --git a/src/Service.Tests/SqlTests/RestApiTests/Insert/PostgreSqlInsertApiTests.cs b/src/Service.Tests/SqlTests/RestApiTests/Insert/PostgreSqlInsertApiTests.cs index d1c278ecde..59dbcde730 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Insert/PostgreSqlInsertApiTests.cs +++ b/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", @" From 586b9c33d9d76884c5e4e80855afa0a77ba724e2 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Fri, 28 Apr 2023 12:19:42 +0530 Subject: [PATCH 17/22] Adding class for param, delegating responsibility for adding DbType to individual db executor class --- .../Unittests/SqlQueryExecutorUnitTests.cs | 13 ++++--- src/Service/Models/DbConnectionParam.cs | 27 +++++++++++++ src/Service/Models/GraphQLFilterParsers.cs | 3 +- src/Service/Parsers/ODataASTVisitor.cs | 38 +++++++++++++++---- src/Service/Resolvers/BaseQueryStructure.cs | 6 +-- src/Service/Resolvers/CosmosQueryEngine.cs | 9 ++--- src/Service/Resolvers/CosmosQueryStructure.cs | 2 +- src/Service/Resolvers/IQueryExecutor.cs | 12 ++++-- src/Service/Resolvers/MsSqlQueryExecutor.cs | 14 +++++-- src/Service/Resolvers/QueryExecutor.cs | 22 ++++++----- .../Sql Query Structures/SqlQueryStructure.cs | 2 +- src/Service/Resolvers/SqlMutationEngine.cs | 6 +-- .../MySqlMetadataProvider.cs | 5 ++- .../MetadataProviders/SqlMetadataProvider.cs | 6 +-- 14 files changed, 115 insertions(+), 50 deletions(-) create mode 100644 src/Service/Models/DbConnectionParam.cs diff --git a/src/Service.Tests/Unittests/SqlQueryExecutorUnitTests.cs b/src/Service.Tests/Unittests/SqlQueryExecutorUnitTests.cs index e1699b2288..5d2f0a9184 100644 --- a/src/Service.Tests/Unittests/SqlQueryExecutorUnitTests.cs +++ b/src/Service.Tests/Unittests/SqlQueryExecutorUnitTests.cs @@ -11,6 +11,7 @@ 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; @@ -138,7 +139,7 @@ Mock queryExecutor queryExecutor.Setup(x => x.ExecuteQueryAgainstDbAsync( It.IsAny(), It.IsAny(), - It.IsAny>>(), + It.IsAny>(), It.IsAny, Task>>(), It.IsAny(), It.IsAny>())) @@ -147,7 +148,7 @@ Mock queryExecutor // Call the actual ExecuteQueryAsync method. queryExecutor.Setup(x => x.ExecuteQueryAsync( It.IsAny(), - It.IsAny>>(), + It.IsAny>(), It.IsAny, Task>>(), It.IsAny(), It.IsAny>())).CallBase(); @@ -156,7 +157,7 @@ Mock queryExecutor { await queryExecutor.Object.ExecuteQueryAsync( sqltext: string.Empty, - parameters: new Dictionary>(), + parameters: new Dictionary(), dataReaderHandler: null, httpContext: null, args: null); @@ -190,7 +191,7 @@ Mock queryExecutor queryExecutor.SetupSequence(x => x.ExecuteQueryAgainstDbAsync( It.IsAny(), It.IsAny(), - It.IsAny>>(), + It.IsAny>(), It.IsAny, Task>>(), It.IsAny(), It.IsAny>())) @@ -201,7 +202,7 @@ Mock queryExecutor // Call the actual ExecuteQueryAsync method. queryExecutor.Setup(x => x.ExecuteQueryAsync( It.IsAny(), - It.IsAny>>(), + It.IsAny>(), It.IsAny, Task>>(), It.IsAny(), It.IsAny>())).CallBase(); @@ -210,7 +211,7 @@ Mock queryExecutor await queryExecutor.Object.ExecuteQueryAsync( sqltext: sqltext, - parameters: new Dictionary>(), + parameters: new Dictionary(), dataReaderHandler: null, args: null); diff --git a/src/Service/Models/DbConnectionParam.cs b/src/Service/Models/DbConnectionParam.cs new file mode 100644 index 0000000000..1b1512b72b --- /dev/null +++ b/src/Service/Models/DbConnectionParam.cs @@ -0,0 +1,27 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Data; + +namespace Azure.DataApiBuilder.Service.Models +{ + /// + /// Represents a single parameter created for the database connection. + /// + public class DbConnectionParam + { + public DbConnectionParam(object? value, DbType? dbType = null) + { + Value = value; + DbType = dbType; + } + + /// + /// Value of the parameter. + /// + public object? Value { get; set; } + + // DbType of the parameter. + public DbType? DbType { get; set; } + } +} diff --git a/src/Service/Models/GraphQLFilterParsers.cs b/src/Service/Models/GraphQLFilterParsers.cs index dc1cc348b0..0257d9b776 100644 --- a/src/Service/Models/GraphQLFilterParsers.cs +++ b/src/Service/Models/GraphQLFilterParsers.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Data; using System.Linq; using System.Net; using Azure.DataApiBuilder.Config; @@ -301,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, Tuple value) in existsQuery.Parameters) + foreach ((string key, DbConnectionParam value) in existsQuery.Parameters) { queryStructure.Parameters.Add(key, value); } diff --git a/src/Service/Parsers/ODataASTVisitor.cs b/src/Service/Parsers/ODataASTVisitor.cs index b2ba2ac5b2..957a297771 100644 --- a/src/Service/Parsers/ODataASTVisitor.cs +++ b/src/Service/Parsers/ODataASTVisitor.cs @@ -38,14 +38,9 @@ public override string Visit(BinaryOperatorNode nodeIn) string left = nodeIn.Left.Accept(this); string right = nodeIn.Right.Accept(this); - if (nodeIn.Left.GetType() == typeof(SingleValuePropertyAccessNode) && nodeIn.Right.GetType() == typeof(ConstantNode) || - nodeIn.Left.GetType() == typeof(ConstantNode) && nodeIn.Right.GetType() == typeof(SingleValuePropertyAccessNode)) + if (IsSimpleBinaryExpression(nodeIn)) { - SingleValuePropertyAccessNode propertyNode = nodeIn.Left.GetType() == typeof(SingleValuePropertyAccessNode) ? - (SingleValuePropertyAccessNode)nodeIn.Left : (SingleValuePropertyAccessNode)nodeIn.Right; - string? paramName = $"{BaseQueryStructure.PARAM_NAME_PREFIX}param{_struct.Counter.Current() - 1}"; - _metadataProvider.TryGetBackingColumn(_struct.EntityName, propertyNode.Property.Name, out string? backingColumnName); - _struct.Parameters[paramName] = new(_struct.Parameters[paramName].Item1, _struct.GetUnderlyingSourceDefinition().Columns[backingColumnName!].DbType); + PopulateDbTypeForPropery(nodeIn); } return CreateResult(nodeIn.OperatorKind, left, right); @@ -262,5 +257,34 @@ private static string GetFilterPredicateOperator(UnaryOperatorKind op) throw new ArgumentException($"Uknown Predicate Operation of {op}"); } } + + /// + /// Helper method to populate the DbType for the property referenced in the OData filter. + /// Since this nodes are processed in a postorder fashion, the current BinaryOperatorNode has already been processed, + /// and the parameter is already created for the property. + /// We just need to populate the DbType. + /// + /// Binary operator node< + private void PopulateDbTypeForPropery(BinaryOperatorNode nodeIn) + { + SingleValuePropertyAccessNode propertyNode = nodeIn.Left.GetType() == typeof(SingleValuePropertyAccessNode) ? + (SingleValuePropertyAccessNode)nodeIn.Left : (SingleValuePropertyAccessNode)nodeIn.Right; + string? paramName = $"{BaseQueryStructure.PARAM_NAME_PREFIX}param{_struct.Counter.Current() - 1}"; + _metadataProvider.TryGetBackingColumn(_struct.EntityName, propertyNode.Property.Name, out string? backingColumnName); + _struct.Parameters[paramName].DbType = _struct.GetUnderlyingSourceDefinition().Columns[backingColumnName!].DbType; + } + + /// + /// Helper method to determine if the BinaryOperatorNode represents a simple binary expression - + /// SingleValuePropertyAccessNode followed by ConstantNode or vice versa. + /// Eg. id gt 5, 'DAB' ne name, etc. + /// + /// Binary operator node + /// Whether BinaryOperatorNode represents a simple binary expression. + private static bool IsSimpleBinaryExpression(BinaryOperatorNode nodeIn) + { + return nodeIn.Left.GetType() == typeof(SingleValuePropertyAccessNode) && nodeIn.Right.GetType() == typeof(ConstantNode) || + nodeIn.Left.GetType() == typeof(ConstantNode) && nodeIn.Right.GetType() == typeof(SingleValuePropertyAccessNode); + } } } diff --git a/src/Service/Resolvers/BaseQueryStructure.cs b/src/Service/Resolvers/BaseQueryStructure.cs index 9b06d72a43..14d81d699c 100644 --- a/src/Service/Resolvers/BaseQueryStructure.cs +++ b/src/Service/Resolvers/BaseQueryStructure.cs @@ -1,9 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using System; using System.Collections.Generic; -using System.Data; using Azure.DataApiBuilder.Auth; using Azure.DataApiBuilder.Config; using Azure.DataApiBuilder.Service.GraphQLBuilder; @@ -53,7 +51,7 @@ public class BaseQueryStructure /// /// Parameters values required to execute the query. /// - public Dictionary> Parameters { get; set; } + public Dictionary Parameters { get; set; } /// /// Predicates that should filter the result set of the query. @@ -120,7 +118,7 @@ public virtual string MakeParamWithValue(object? value, string? paramName = null } else { - Parameters.Add(encodedParamName, new(value, null)); + Parameters.Add(encodedParamName, new(value)); } return encodedParamName; diff --git a/src/Service/Resolvers/CosmosQueryEngine.cs b/src/Service/Resolvers/CosmosQueryEngine.cs index 294dfaa57a..02594b1d75 100644 --- a/src/Service/Resolvers/CosmosQueryEngine.cs +++ b/src/Service/Resolvers/CosmosQueryEngine.cs @@ -4,7 +4,6 @@ # nullable disable using System; using System.Collections.Generic; -using System.Data; using System.Linq; using System.Text; using System.Text.Json; @@ -72,9 +71,9 @@ public async Task> ExecuteAsync( Container container = _clientProvider.Client.GetDatabase(structure.Database).GetContainer(structure.Container); (string idValue, string partitionKeyValue) = await GetIdAndPartitionKey(parameters, container, structure); - foreach (KeyValuePair> parameterEntry in structure.Parameters) + foreach (KeyValuePair parameterEntry in structure.Parameters) { - querySpec = querySpec.WithParameter(parameterEntry.Key, parameterEntry.Value.Item1); + querySpec = querySpec.WithParameter(parameterEntry.Key, parameterEntry.Value.Value); } if (!string.IsNullOrEmpty(partitionKeyValue)) @@ -155,9 +154,9 @@ public async Task, IMetadata>> ExecuteListAsync( Container container = _clientProvider.Client.GetDatabase(structure.Database).GetContainer(structure.Container); QueryDefinition querySpec = new(_queryBuilder.Build(structure)); - foreach (KeyValuePair> parameterEntry in structure.Parameters) + foreach (KeyValuePair parameterEntry in structure.Parameters) { - querySpec = querySpec.WithParameter(parameterEntry.Key, parameterEntry.Value.Item1); + querySpec = querySpec.WithParameter(parameterEntry.Key, parameterEntry.Value.Value); } FeedIterator resultSetIterator = container.GetItemQueryIterator(querySpec); diff --git a/src/Service/Resolvers/CosmosQueryStructure.cs b/src/Service/Resolvers/CosmosQueryStructure.cs index efcb91aed3..a932aad471 100644 --- a/src/Service/Resolvers/CosmosQueryStructure.cs +++ b/src/Service/Resolvers/CosmosQueryStructure.cs @@ -51,7 +51,7 @@ public CosmosQueryStructure( public override string MakeParamWithValue(object? value, string? columnName = null) { string paramName = $"{PARAM_NAME_PREFIX}param{Counter.Next()}"; - Parameters.Add(paramName, new(value, null)); + Parameters.Add(paramName, new(value)); return paramName; } diff --git a/src/Service/Resolvers/IQueryExecutor.cs b/src/Service/Resolvers/IQueryExecutor.cs index b3eb82def3..8d217ca778 100644 --- a/src/Service/Resolvers/IQueryExecutor.cs +++ b/src/Service/Resolvers/IQueryExecutor.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Data; using System.Data.Common; using System.Text.Json.Nodes; using System.Threading.Tasks; @@ -31,7 +30,7 @@ public interface IQueryExecutor /// An object formed using the results of the query as returned by the given handler. public Task ExecuteQueryAsync( string sqltext, - IDictionary> parameters, + IDictionary parameters, Func?, Task>? dataReaderHandler, HttpContext? httpContext = null, List? args = null); @@ -112,6 +111,13 @@ public Task> GetResultProperties( /// Current user httpContext. /// Dictionary of parameters/value required to execute the query. /// empty string / query to set session parameters for the connection. - public string GetSessionParamsQuery(HttpContext? httpContext, IDictionary> parameters); + public string GetSessionParamsQuery(HttpContext? httpContext, IDictionary parameters); + + /// + /// Helper method to populate DbType for parameter. Currently DbType(s) for parameters are only populated for MsSql. + /// + /// Entry corresponding to current database parameter to be created. + /// Parameter sent to database. + public void PopulateDbTypeForParameter(KeyValuePair parameterEntry, DbParameter parameter); } } diff --git a/src/Service/Resolvers/MsSqlQueryExecutor.cs b/src/Service/Resolvers/MsSqlQueryExecutor.cs index f786104fa6..488d2f21c5 100644 --- a/src/Service/Resolvers/MsSqlQueryExecutor.cs +++ b/src/Service/Resolvers/MsSqlQueryExecutor.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using System; using System.Collections.Generic; using System.Data; using System.Data.Common; @@ -170,7 +169,7 @@ await AzureCredential.GetTokenAsync( /// Dictionary of parameters/value required to execute the query. /// empty string / query to set session parameters for the connection. /// - public override string GetSessionParamsQuery(HttpContext? httpContext, IDictionary> parameters) + public override string GetSessionParamsQuery(HttpContext? httpContext, IDictionary parameters) { if (httpContext is null || !_isSessionContextEnabled) { @@ -188,7 +187,7 @@ public override string GetSessionParamsQuery(HttpContext? httpContext, IDictiona foreach ((string claimType, Claim claim) in sessionParams) { string paramName = $"{SESSION_PARAM_NAME}{counter.Next()}"; - parameters.Add(paramName, new(claim.Value, null)); + parameters.Add(paramName, new(claim.Value)); // Append statement to set read only param value - can be set only once for a connection. string statementToSetReadOnlyParam = "EXEC sp_set_session_context " + $"'{claimType}', " + paramName + ", @read_only = 1;"; sessionMapQuery = sessionMapQuery.Append(statementToSetReadOnlyParam); @@ -196,5 +195,14 @@ public override string GetSessionParamsQuery(HttpContext? httpContext, IDictiona return sessionMapQuery.ToString(); } + + /// + public override void PopulateDbTypeForParameter(KeyValuePair parameterEntry, DbParameter parameter) + { + if (parameterEntry.Value.DbType is not null) + { + parameter.DbType = (DbType)parameterEntry.Value.DbType; + } + } } } diff --git a/src/Service/Resolvers/QueryExecutor.cs b/src/Service/Resolvers/QueryExecutor.cs index 0ad4295b74..3574d50b18 100644 --- a/src/Service/Resolvers/QueryExecutor.cs +++ b/src/Service/Resolvers/QueryExecutor.cs @@ -67,7 +67,7 @@ public QueryExecutor(DbExceptionParser dbExceptionParser, /// public virtual async Task ExecuteQueryAsync( string sqltext, - IDictionary> parameters, + IDictionary parameters, Func?, Task>? dataReaderHandler, HttpContext? httpContext = null, List? args = null) @@ -143,7 +143,7 @@ await ExecuteQueryAgainstDbAsync(conn, public virtual async Task ExecuteQueryAgainstDbAsync( TConnection conn, string sqltext, - IDictionary> parameters, + IDictionary parameters, Func?, Task>? dataReaderHandler, HttpContext? httpContext, List? args = null) @@ -159,16 +159,12 @@ await ExecuteQueryAgainstDbAsync(conn, cmd.CommandText = sessionParamsQuery + sqltext; if (parameters is not null) { - foreach (KeyValuePair> parameterEntry in parameters) + foreach (KeyValuePair parameterEntry in parameters) { DbParameter parameter = cmd.CreateParameter(); parameter.ParameterName = parameterEntry.Key; - parameter.Value = parameterEntry.Value.Item1 ?? DBNull.Value; - if (parameterEntry.Value.Item2 is not null && ConfigProvider.GetRuntimeConfiguration().DatabaseType is DatabaseType.mssql) - { - parameter.DbType = (DbType)parameterEntry.Value.Item2; - } - + parameter.Value = parameterEntry.Value.Value ?? DBNull.Value; + PopulateDbTypeForParameter(parameterEntry, parameter); cmd.Parameters.Add(parameter); } } @@ -196,11 +192,17 @@ await ExecuteQueryAgainstDbAsync(conn, } /// - public virtual string GetSessionParamsQuery(HttpContext? httpContext, IDictionary> parameters) + public virtual string GetSessionParamsQuery(HttpContext? httpContext, IDictionary parameters) { return string.Empty; } + /// + public virtual void PopulateDbTypeForParameter(KeyValuePair parameterEntry, DbParameter parameter) + { + return; + } + /// public virtual async Task SetManagedIdentityAccessTokenIfAnyAsync(DbConnection conn) { diff --git a/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs index 8f5aab642b..767ec5f083 100644 --- a/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs @@ -664,7 +664,7 @@ private void AddGraphQLFields(IReadOnlyList selections, RuntimeC // pass the parameters of the subquery to the current query so upmost query has all the // parameters of the query tree and it can pass them to the database query executor - foreach (KeyValuePair> parameter in subquery.Parameters) + foreach (KeyValuePair parameter in subquery.Parameters) { Parameters.Add(parameter.Key, parameter.Value); } diff --git a/src/Service/Resolvers/SqlMutationEngine.cs b/src/Service/Resolvers/SqlMutationEngine.cs index 1c753cdf11..dfb8acdc86 100644 --- a/src/Service/Resolvers/SqlMutationEngine.cs +++ b/src/Service/Resolvers/SqlMutationEngine.cs @@ -435,7 +435,7 @@ private async Task IMiddlewareContext? context = null) { string queryString; - Dictionary> queryParameters; + Dictionary queryParameters; switch (operationType) { case Config.Operation.Insert: @@ -597,7 +597,7 @@ private async Task?> IDictionary parameters) { string queryString; - Dictionary> queryParameters; + Dictionary queryParameters; SqlDeleteStructure deleteStructure = new( entityName, _sqlMetadataProvider, @@ -632,7 +632,7 @@ private async Task RestRequestContext context) { string queryString; - Dictionary> queryParameters; + Dictionary queryParameters; Config.Operation operationType = context.OperationType; string entityName = context.EntityName; diff --git a/src/Service/Services/MetadataProviders/MySqlMetadataProvider.cs b/src/Service/Services/MetadataProviders/MySqlMetadataProvider.cs index 9058ea6930..a119d75801 100644 --- a/src/Service/Services/MetadataProviders/MySqlMetadataProvider.cs +++ b/src/Service/Services/MetadataProviders/MySqlMetadataProvider.cs @@ -8,6 +8,7 @@ using System.Threading.Tasks; using Azure.DataApiBuilder.Config; using Azure.DataApiBuilder.Service.Configurations; +using Azure.DataApiBuilder.Service.Models; using Azure.DataApiBuilder.Service.Resolvers; using Microsoft.Extensions.Logging; using MySqlConnector; @@ -70,13 +71,13 @@ protected override async Task GetColumnsAsync( /// For MySql, the table name is only a 2 part name. /// The database name from the connection string needs to be used instead of schemaName. /// - protected override Dictionary> + protected override Dictionary GetForeignKeyQueryParams( string[] schemaNames, string[] tableNames) { MySqlConnectionStringBuilder connBuilder = new(ConnectionString); - Dictionary> parameters = new(); + Dictionary parameters = new(); string[] databaseNameParams = BaseSqlQueryBuilder.CreateParams( diff --git a/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs b/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs index f15c8d6356..921f8ac1aa 100644 --- a/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs +++ b/src/Service/Services/MetadataProviders/SqlMetadataProvider.cs @@ -494,12 +494,12 @@ protected virtual DatabaseTable GenerateDbTable(string schemaName, string tableN /// /// /// The dictionary populated with parameters. - protected virtual Dictionary> + protected virtual Dictionary GetForeignKeyQueryParams( string[] schemaNames, string[] tableNames) { - Dictionary> parameters = new(); + Dictionary parameters = new(); string[] schemaNameParams = BaseSqlQueryBuilder.CreateParams( kindOfParam: BaseSqlQueryBuilder.SCHEMA_NAME_PARAM, @@ -1265,7 +1265,7 @@ private async Task PopulateForeignKeyDefinitionAsync() // Build the parameters dictionary for the foreign key info query // consisting of all schema names and table names. - Dictionary> parameters = + Dictionary parameters = GetForeignKeyQueryParams( schemaNames.ToArray(), tableNames.ToArray()); From b5effd5d5132540baec8b970fa7fb99d620dee68 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Fri, 28 Apr 2023 12:42:34 +0530 Subject: [PATCH 18/22] Renaming func and nits --- src/Service/Models/GraphQLFilterParsers.cs | 2 +- src/Service/Parsers/ODataASTVisitor.cs | 4 ++-- src/Service/Resolvers/BaseQueryStructure.cs | 3 ++- src/Service/Resolvers/CosmosQueryStructure.cs | 10 +++++----- src/Service/Resolvers/QueryExecutor.cs | 1 - .../Sql Query Structures/BaseSqlQueryStructure.cs | 2 +- .../Sql Query Structures/SqlDeleteQueryStructure.cs | 2 +- .../Sql Query Structures/SqlExecuteQueryStructure.cs | 6 +++--- .../Sql Query Structures/SqlInsertQueryStructure.cs | 4 ++-- .../Sql Query Structures/SqlQueryStructure.cs | 10 +++++----- .../Sql Query Structures/SqlUpdateQueryStructure.cs | 4 ++-- .../Sql Query Structures/SqlUpsertQueryStructure.cs | 4 ++-- 12 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/Service/Models/GraphQLFilterParsers.cs b/src/Service/Models/GraphQLFilterParsers.cs index 0257d9b776..f5cd06eede 100644 --- a/src/Service/Models/GraphQLFilterParsers.cs +++ b/src/Service/Models/GraphQLFilterParsers.cs @@ -204,7 +204,7 @@ public Predicate Parse( schemaName, sourceName, sourceAlias, - queryStructure.MakeParamWithValue))); + queryStructure.MakeDbConnectionParam))); } } } diff --git a/src/Service/Parsers/ODataASTVisitor.cs b/src/Service/Parsers/ODataASTVisitor.cs index 957a297771..92918331e3 100644 --- a/src/Service/Parsers/ODataASTVisitor.cs +++ b/src/Service/Parsers/ODataASTVisitor.cs @@ -87,7 +87,7 @@ public override string Visit(ConstantNode nodeIn) { if (nodeIn.TypeReference is not null) { - return $"{_struct.MakeParamWithValue(GetParamWithSystemType(nodeIn.Value.ToString()!, nodeIn.TypeReference))}"; + return $"{_struct.MakeDbConnectionParam(GetParamWithSystemType(nodeIn.Value.ToString()!, nodeIn.TypeReference))}"; } // Represents a NULL value, we support NULL in queries so return "NULL" here @@ -260,7 +260,7 @@ private static string GetFilterPredicateOperator(UnaryOperatorKind op) /// /// Helper method to populate the DbType for the property referenced in the OData filter. - /// Since this nodes are processed in a postorder fashion, the current BinaryOperatorNode has already been processed, + /// Since the nodes are processed in a postorder fashion, the current BinaryOperatorNode has already been processed, /// and the parameter is already created for the property. /// We just need to populate the DbType. /// diff --git a/src/Service/Resolvers/BaseQueryStructure.cs b/src/Service/Resolvers/BaseQueryStructure.cs index 14d81d699c..80d1f8b409 100644 --- a/src/Service/Resolvers/BaseQueryStructure.cs +++ b/src/Service/Resolvers/BaseQueryStructure.cs @@ -109,7 +109,8 @@ public BaseQueryStructure( /// Add parameter to Parameters and return the name associated with it /// /// Value to be assigned to parameter, which can be null for nullable columns. - public virtual string MakeParamWithValue(object? value, string? paramName = null) + /// The name of the parameter - column name for table/views or parameter name for stored procedures. + public virtual string MakeDbConnectionParam(object? value, string? paramName = null) { string encodedParamName = $"{PARAM_NAME_PREFIX}param{Counter.Next()}"; if (!string.IsNullOrEmpty(paramName)) diff --git a/src/Service/Resolvers/CosmosQueryStructure.cs b/src/Service/Resolvers/CosmosQueryStructure.cs index a932aad471..83c0aaa5e4 100644 --- a/src/Service/Resolvers/CosmosQueryStructure.cs +++ b/src/Service/Resolvers/CosmosQueryStructure.cs @@ -48,11 +48,11 @@ public CosmosQueryStructure( } /// - public override string MakeParamWithValue(object? value, string? columnName = null) + public override string MakeDbConnectionParam(object? value, string? columnName = null) { - string paramName = $"{PARAM_NAME_PREFIX}param{Counter.Next()}"; - Parameters.Add(paramName, new(value)); - return paramName; + string encodedParamName = $"{PARAM_NAME_PREFIX}param{Counter.Next()}"; + Parameters.Add(encodedParamName, new(value)); + return encodedParamName; } private static IEnumerable GenerateQueryColumns(SelectionSetNode selectionSet, DocumentNode document, string tableName) @@ -187,7 +187,7 @@ private void Init(IDictionary queryParams) Predicates.Add(new Predicate( new PredicateOperand(new Column(tableSchema: string.Empty, _containerAlias, parameter.Key)), PredicateOperation.Equal, - new PredicateOperand($"{MakeParamWithValue(parameter.Value)}") + new PredicateOperand($"{MakeDbConnectionParam(parameter.Value)}") )); } } diff --git a/src/Service/Resolvers/QueryExecutor.cs b/src/Service/Resolvers/QueryExecutor.cs index 3574d50b18..01135c5993 100644 --- a/src/Service/Resolvers/QueryExecutor.cs +++ b/src/Service/Resolvers/QueryExecutor.cs @@ -11,7 +11,6 @@ using System.Text.Json; using System.Text.Json.Nodes; using System.Threading.Tasks; -using Azure.DataApiBuilder.Config; using Azure.DataApiBuilder.Service.Configurations; using Azure.DataApiBuilder.Service.Exceptions; using Azure.DataApiBuilder.Service.Models; diff --git a/src/Service/Resolvers/Sql Query Structures/BaseSqlQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/BaseSqlQueryStructure.cs index 7040aea7dd..84d8f82919 100644 --- a/src/Service/Resolvers/Sql Query Structures/BaseSqlQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/BaseSqlQueryStructure.cs @@ -115,7 +115,7 @@ public void AddNullifiedUnspecifiedFields( Predicate predicate = new( new PredicateOperand(new Column(tableSchema: DatabaseObject.SchemaName, tableName: DatabaseObject.Name, leftoverColumn)), PredicateOperation.Equal, - new PredicateOperand($"{MakeParamWithValue(null, leftoverColumn)}") + new PredicateOperand($"{MakeDbConnectionParam(value: null, leftoverColumn)}") ); updateOperations.Add(predicate); diff --git a/src/Service/Resolvers/Sql Query Structures/SqlDeleteQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/SqlDeleteQueryStructure.cs index e435c29b59..e6244d20fc 100644 --- a/src/Service/Resolvers/Sql Query Structures/SqlDeleteQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/SqlDeleteQueryStructure.cs @@ -53,7 +53,7 @@ public SqlDeleteStructure( Predicates.Add(new Predicate( new PredicateOperand(new Column(DatabaseObject.SchemaName, DatabaseObject.Name, backingColumn!)), PredicateOperation.Equal, - new PredicateOperand($"{MakeParamWithValue(GetParamAsSystemType(param.Value.ToString()!, backingColumn!, GetColumnSystemType(backingColumn!)), backingColumn)}") + new PredicateOperand($"{MakeDbConnectionParam(GetParamAsSystemType(param.Value.ToString()!, backingColumn!, GetColumnSystemType(backingColumn!)), backingColumn)}") )); } } diff --git a/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs index 3fcf64ac1f..ba7a831390 100644 --- a/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs @@ -48,11 +48,11 @@ public SqlExecuteStructure( string? parametrizedName = null; if (requestParamValue is not null) { - parametrizedName = MakeParamWithValue(GetParamAsSystemType(requestParamValue.ToString()!, paramKey, systemType), paramKey); + parametrizedName = MakeDbConnectionParam(GetParamAsSystemType(requestParamValue.ToString()!, paramKey, systemType), paramKey); } else { - parametrizedName = MakeParamWithValue(null, paramKey); + parametrizedName = MakeDbConnectionParam(null, paramKey); } ProcedureParameters.Add(paramKey, $"{parametrizedName}"); @@ -63,7 +63,7 @@ public SqlExecuteStructure( if (paramDefinition.HasConfigDefault) { object? value = paramDefinition.ConfigDefaultValue == null ? null : GetParamAsSystemType(paramDefinition.ConfigDefaultValue!.ToString()!, paramKey, systemType); - string parameterizedName = MakeParamWithValue(value, paramKey); + string parameterizedName = MakeDbConnectionParam(value, paramKey); ProcedureParameters.Add(paramKey, $"{parameterizedName}"); } else diff --git a/src/Service/Resolvers/Sql Query Structures/SqlInsertQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/SqlInsertQueryStructure.cs index 4f078ad183..f2b7d40173 100644 --- a/src/Service/Resolvers/Sql Query Structures/SqlInsertQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/SqlInsertQueryStructure.cs @@ -104,12 +104,12 @@ private void PopulateColumnsAndParams(string columnName, object? value) if (value is not null) { - paramName = MakeParamWithValue( + paramName = MakeDbConnectionParam( GetParamAsSystemType(value.ToString()!, columnName, GetColumnSystemType(columnName)), columnName); } else { - paramName = MakeParamWithValue(null, columnName); + paramName = MakeDbConnectionParam(null, columnName); } Values.Add($"{paramName}"); diff --git a/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs index 767ec5f083..8a7da187ff 100644 --- a/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs @@ -459,7 +459,7 @@ private void AddPrimaryKeyPredicates(IDictionary queryParams) columnName: columnName, tableAlias: SourceAlias)), PredicateOperation.Equal, - new PredicateOperand($"{MakeParamWithValue(parameter.Value, columnName)}") + new PredicateOperand($"{MakeDbConnectionParam(parameter.Value, columnName)}") )); } } @@ -479,8 +479,8 @@ public void AddPaginationPredicate(IEnumerable afterJsonValues { column.TableAlias = SourceAlias; column.ParamName = column.Value is not null ? - MakeParamWithValue(GetParamAsSystemType(column.Value!.ToString()!, column.ColumnName, GetColumnSystemType(column.ColumnName))) : - MakeParamWithValue(null, column.ColumnName); + MakeDbConnectionParam(GetParamAsSystemType(column.Value!.ToString()!, column.ColumnName, GetColumnSystemType(column.ColumnName))) : + MakeDbConnectionParam(null, column.ColumnName); } PaginationMetadata.PaginationPredicate = new KeysetPaginationPredicate(afterJsonValues.ToList()); @@ -501,7 +501,7 @@ private void PopulateParamsAndPredicates(string field, string backingColumn, obj string parameterName; if (value != null) { - parameterName = MakeParamWithValue( + parameterName = MakeDbConnectionParam( GetParamAsSystemType(value.ToString()!, backingColumn, GetColumnSystemType(backingColumn)), backingColumn); Predicates.Add(new Predicate( new PredicateOperand(new Column(DatabaseObject.SchemaName, DatabaseObject.Name, backingColumn, SourceAlias)), @@ -847,7 +847,7 @@ private void ParametrizeColumns() { foreach (LabelledColumn column in Columns) { - ColumnLabelToParam.Add(column.Label, $"{MakeParamWithValue(column.Label)}"); + ColumnLabelToParam.Add(column.Label, $"{MakeDbConnectionParam(column.Label)}"); } } } diff --git a/src/Service/Resolvers/Sql Query Structures/SqlUpdateQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/SqlUpdateQueryStructure.cs index 0e7ffeb707..db50d2fc70 100644 --- a/src/Service/Resolvers/Sql Query Structures/SqlUpdateQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/SqlUpdateQueryStructure.cs @@ -173,7 +173,7 @@ private Predicate CreatePredicateForParam(KeyValuePair param) new PredicateOperand( new Column(tableSchema: DatabaseObject.SchemaName, tableName: DatabaseObject.Name, backingColumn!)), PredicateOperation.Equal, - new PredicateOperand($"{MakeParamWithValue(null, backingColumn)}") + new PredicateOperand($"{MakeDbConnectionParam(null, backingColumn)}") ); } else @@ -182,7 +182,7 @@ private Predicate CreatePredicateForParam(KeyValuePair param) new PredicateOperand( new Column(tableSchema: DatabaseObject.SchemaName, tableName: DatabaseObject.Name, param.Key)), PredicateOperation.Equal, - new PredicateOperand($"{MakeParamWithValue(GetParamAsSystemType(param.Value.ToString()!, param.Key, GetColumnSystemType(param.Key)), param.Key)}")); + new PredicateOperand($"{MakeDbConnectionParam(GetParamAsSystemType(param.Value.ToString()!, param.Key, GetColumnSystemType(param.Key)), param.Key)}")); } return predicate; diff --git a/src/Service/Resolvers/Sql Query Structures/SqlUpsertQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/SqlUpsertQueryStructure.cs index 189e6ad8dd..7017238c5f 100644 --- a/src/Service/Resolvers/Sql Query Structures/SqlUpsertQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/SqlUpsertQueryStructure.cs @@ -123,11 +123,11 @@ private void PopulateColumns( string paramIdentifier; if (param.Value is not null) { - paramIdentifier = MakeParamWithValue(GetParamAsSystemType(param.Value.ToString()!, backingColumn!, GetColumnSystemType(backingColumn!)), backingColumn); + paramIdentifier = MakeDbConnectionParam(GetParamAsSystemType(param.Value.ToString()!, backingColumn!, GetColumnSystemType(backingColumn!)), backingColumn); } else { - paramIdentifier = MakeParamWithValue(null, backingColumn); + paramIdentifier = MakeDbConnectionParam(null, backingColumn); } ColumnToParam.Add(backingColumn!, paramIdentifier); From 437033f6a20c769ed462b564f1251612590e35f6 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Mon, 1 May 2023 19:43:06 +0530 Subject: [PATCH 19/22] nits --- src/Service/Parsers/ODataASTVisitor.cs | 4 ++-- src/Service/Resolvers/MsSqlQueryExecutor.cs | 2 +- src/Service/Resolvers/QueryExecutor.cs | 1 + src/Service/Services/DbTypeHelper.cs | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Service/Parsers/ODataASTVisitor.cs b/src/Service/Parsers/ODataASTVisitor.cs index 92918331e3..d8262838fd 100644 --- a/src/Service/Parsers/ODataASTVisitor.cs +++ b/src/Service/Parsers/ODataASTVisitor.cs @@ -40,7 +40,7 @@ public override string Visit(BinaryOperatorNode nodeIn) if (IsSimpleBinaryExpression(nodeIn)) { - PopulateDbTypeForPropery(nodeIn); + PopulateDbTypeForProperty(nodeIn); } return CreateResult(nodeIn.OperatorKind, left, right); @@ -265,7 +265,7 @@ private static string GetFilterPredicateOperator(UnaryOperatorKind op) /// We just need to populate the DbType. /// /// Binary operator node< - private void PopulateDbTypeForPropery(BinaryOperatorNode nodeIn) + private void PopulateDbTypeForProperty(BinaryOperatorNode nodeIn) { SingleValuePropertyAccessNode propertyNode = nodeIn.Left.GetType() == typeof(SingleValuePropertyAccessNode) ? (SingleValuePropertyAccessNode)nodeIn.Left : (SingleValuePropertyAccessNode)nodeIn.Right; diff --git a/src/Service/Resolvers/MsSqlQueryExecutor.cs b/src/Service/Resolvers/MsSqlQueryExecutor.cs index 488d2f21c5..9b0bf49e1a 100644 --- a/src/Service/Resolvers/MsSqlQueryExecutor.cs +++ b/src/Service/Resolvers/MsSqlQueryExecutor.cs @@ -199,7 +199,7 @@ public override string GetSessionParamsQuery(HttpContext? httpContext, IDictiona /// public override void PopulateDbTypeForParameter(KeyValuePair parameterEntry, DbParameter parameter) { - if (parameterEntry.Value.DbType is not null) + if (parameterEntry.Value is not null && parameterEntry.Value.DbType is not null) { parameter.DbType = (DbType)parameterEntry.Value.DbType; } diff --git a/src/Service/Resolvers/QueryExecutor.cs b/src/Service/Resolvers/QueryExecutor.cs index 01135c5993..345324cf15 100644 --- a/src/Service/Resolvers/QueryExecutor.cs +++ b/src/Service/Resolvers/QueryExecutor.cs @@ -199,6 +199,7 @@ public virtual string GetSessionParamsQuery(HttpContext? httpContext, IDictionar /// public virtual void PopulateDbTypeForParameter(KeyValuePair parameterEntry, DbParameter parameter) { + // DbType for parameter is currently only populated for MsSql which has its own overridden implementation. return; } diff --git a/src/Service/Services/DbTypeHelper.cs b/src/Service/Services/DbTypeHelper.cs index d7cbc30c1e..57a63b6fd1 100644 --- a/src/Service/Services/DbTypeHelper.cs +++ b/src/Service/Services/DbTypeHelper.cs @@ -51,7 +51,7 @@ public static class DbTypeHelper /// Returns the DbType for given system type. /// /// The system type for which the DbType is to be determined. - /// DbType for the given system tyep. + /// DbType for the given system type. public static DbType? GetDbTypeFromSystemType(Type systemType) { if (!_systemTypeToDbTypeMap.TryGetValue(systemType, out DbType dbType)) From e8ca84aa09cad39f0eebf0393a7f30eca0c731cc Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Tue, 2 May 2023 00:09:43 +0530 Subject: [PATCH 20/22] adding comments --- src/Config/DatabaseObject.cs | 5 +++-- .../SqlTests/RestApiTests/Insert/InsertApiTestBase.cs | 1 - src/Service.Tests/Unittests/ODataASTVisitorUnitTests.cs | 2 +- src/Service/Models/DbConnectionParam.cs | 3 +++ src/Service/Parsers/ODataASTVisitor.cs | 3 +++ src/Service/Resolvers/IQueryExecutor.cs | 2 +- 6 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/Config/DatabaseObject.cs b/src/Config/DatabaseObject.cs index 9603069dfc..03b4b6fa8b 100644 --- a/src/Config/DatabaseObject.cs +++ b/src/Config/DatabaseObject.cs @@ -173,8 +173,9 @@ public bool IsAnyColumnNullable(List columnsToCheck) /// 1. column for table/view, /// 2. parameter for stored procedure. /// - /// The parameter whose DbType is to be determined. - /// + /// The parameter whose DbType is to be determined. + /// For table/view paramName refers to the backingColumnName if aliases are used. + /// DbType for the parameter. public virtual DbType? GetDbTypeForParam(string paramName) { if (Columns.TryGetValue(paramName, out ColumnDefinition? columnDefinition)) diff --git a/src/Service.Tests/SqlTests/RestApiTests/Insert/InsertApiTestBase.cs b/src/Service.Tests/SqlTests/RestApiTests/Insert/InsertApiTestBase.cs index 9ecc8f5eeb..cfdfb2bb00 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Insert/InsertApiTestBase.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Insert/InsertApiTestBase.cs @@ -65,7 +65,6 @@ await SetupAndRunRestApiTest( /// 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. /// - /// [TestMethod] public virtual async Task InsertOneWithByteArrayTypeAsNull() { diff --git a/src/Service.Tests/Unittests/ODataASTVisitorUnitTests.cs b/src/Service.Tests/Unittests/ODataASTVisitorUnitTests.cs index 8cb36bfaaf..212ec95ed2 100644 --- a/src/Service.Tests/Unittests/ODataASTVisitorUnitTests.cs +++ b/src/Service.Tests/Unittests/ODataASTVisitorUnitTests.cs @@ -347,7 +347,7 @@ private static ODataASTVisitor CreateVisitor( _runtimeConfigProvider, new GQLFilterParser(_sqlMetadataProvider), null) // setting httpContext as null for the tests. - { CallBase = true }; + { 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); } diff --git a/src/Service/Models/DbConnectionParam.cs b/src/Service/Models/DbConnectionParam.cs index 1b1512b72b..1de9b9b7aa 100644 --- a/src/Service/Models/DbConnectionParam.cs +++ b/src/Service/Models/DbConnectionParam.cs @@ -22,6 +22,9 @@ public DbConnectionParam(object? value, DbType? dbType = null) 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; } } } diff --git a/src/Service/Parsers/ODataASTVisitor.cs b/src/Service/Parsers/ODataASTVisitor.cs index d8262838fd..1253c3dcdf 100644 --- a/src/Service/Parsers/ODataASTVisitor.cs +++ b/src/Service/Parsers/ODataASTVisitor.cs @@ -40,6 +40,9 @@ public override string Visit(BinaryOperatorNode nodeIn) if (IsSimpleBinaryExpression(nodeIn)) { + // Whenever we encounter a simple binary expression like "@item.name ne 'DAB'", we know that we would have just added a parameter for 'name', + // and are coming back to the root node after traversing the left and right child. + // Thats when we need to populate the DbType for the parameter. PopulateDbTypeForProperty(nodeIn); } diff --git a/src/Service/Resolvers/IQueryExecutor.cs b/src/Service/Resolvers/IQueryExecutor.cs index 8d217ca778..c63979f07e 100644 --- a/src/Service/Resolvers/IQueryExecutor.cs +++ b/src/Service/Resolvers/IQueryExecutor.cs @@ -114,7 +114,7 @@ public Task> GetResultProperties( public string GetSessionParamsQuery(HttpContext? httpContext, IDictionary parameters); /// - /// Helper method to populate DbType for parameter. Currently DbType(s) for parameters are only populated for MsSql. + /// Helper method to populate DbType for parameter. Currently DbTypes for parameters are only populated for MsSql. /// /// Entry corresponding to current database parameter to be created. /// Parameter sent to database. From 404b8ab194cb561ada00e85829bf203599a67055 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Tue, 2 May 2023 00:13:26 +0530 Subject: [PATCH 21/22] reverting changes --- src/Service/Parsers/ODataASTVisitor.cs | 8 ++++---- .../Sql Query Structures/SqlExecuteQueryStructure.cs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Service/Parsers/ODataASTVisitor.cs b/src/Service/Parsers/ODataASTVisitor.cs index 1253c3dcdf..da32352c74 100644 --- a/src/Service/Parsers/ODataASTVisitor.cs +++ b/src/Service/Parsers/ODataASTVisitor.cs @@ -88,13 +88,13 @@ public override string Visit(SingleValuePropertyAccessNode nodeIn) /// String representing param that holds given value. public override string Visit(ConstantNode nodeIn) { - if (nodeIn.TypeReference is not null) + if (nodeIn.TypeReference is null) { - return $"{_struct.MakeDbConnectionParam(GetParamWithSystemType(nodeIn.Value.ToString()!, nodeIn.TypeReference))}"; + // Represents a NULL value, we support NULL in queries so return "NULL" here + return "NULL"; } - // Represents a NULL value, we support NULL in queries so return "NULL" here - return "NULL"; + return $"{_struct.MakeDbConnectionParam(GetParamWithSystemType(nodeIn.Value.ToString()!, nodeIn.TypeReference))}"; } /// diff --git a/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs index ba7a831390..7839742f53 100644 --- a/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/SqlExecuteQueryStructure.cs @@ -52,7 +52,7 @@ public SqlExecuteStructure( } else { - parametrizedName = MakeDbConnectionParam(null, paramKey); + parametrizedName = MakeDbConnectionParam(value: null, paramKey); } ProcedureParameters.Add(paramKey, $"{parametrizedName}"); From 45117f19b782074c09e8422798d00be7fc7b5dc8 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Tue, 2 May 2023 11:16:25 +0530 Subject: [PATCH 22/22] centralising encoded param name creation --- src/Service/Parsers/ODataASTVisitor.cs | 2 +- src/Service/Resolvers/BaseQueryStructure.cs | 12 +++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Service/Parsers/ODataASTVisitor.cs b/src/Service/Parsers/ODataASTVisitor.cs index da32352c74..a730e5a737 100644 --- a/src/Service/Parsers/ODataASTVisitor.cs +++ b/src/Service/Parsers/ODataASTVisitor.cs @@ -272,7 +272,7 @@ private void PopulateDbTypeForProperty(BinaryOperatorNode nodeIn) { SingleValuePropertyAccessNode propertyNode = nodeIn.Left.GetType() == typeof(SingleValuePropertyAccessNode) ? (SingleValuePropertyAccessNode)nodeIn.Left : (SingleValuePropertyAccessNode)nodeIn.Right; - string? paramName = $"{BaseQueryStructure.PARAM_NAME_PREFIX}param{_struct.Counter.Current() - 1}"; + string? paramName = BaseQueryStructure.GetEncodedParamName(_struct.Counter.Current() - 1); _metadataProvider.TryGetBackingColumn(_struct.EntityName, propertyNode.Property.Name, out string? backingColumnName); _struct.Parameters[paramName].DbType = _struct.GetUnderlyingSourceDefinition().Columns[backingColumnName!].DbType; } diff --git a/src/Service/Resolvers/BaseQueryStructure.cs b/src/Service/Resolvers/BaseQueryStructure.cs index 80d1f8b409..48e7c5232e 100644 --- a/src/Service/Resolvers/BaseQueryStructure.cs +++ b/src/Service/Resolvers/BaseQueryStructure.cs @@ -112,7 +112,7 @@ public BaseQueryStructure( /// The name of the parameter - column name for table/views or parameter name for stored procedures. public virtual string MakeDbConnectionParam(object? value, string? paramName = null) { - string encodedParamName = $"{PARAM_NAME_PREFIX}param{Counter.Next()}"; + string encodedParamName = GetEncodedParamName(Counter.Next()); if (!string.IsNullOrEmpty(paramName)) { Parameters.Add(encodedParamName, new(value, GetUnderlyingSourceDefinition().GetDbTypeForParam(paramName))); @@ -125,6 +125,16 @@ public virtual string MakeDbConnectionParam(object? value, string? paramName = n return encodedParamName; } + /// + /// Helper method to create encoded parameter name. + /// + /// The counter value used as a suffix in the encoded parameter name. + /// Encoded parameter name. + public static string GetEncodedParamName(ulong counterValue) + { + return $"{PARAM_NAME_PREFIX}param{counterValue}"; + } + /// /// Creates a unique table alias. ///