Skip to content

Commit 0829d85

Browse files
ayush3797Aniruddh25abhishekkumamsseantleonard
authored
Handling datetime types (#1636)
## Why make this change? Currently, we are incorrectly handling the datetime datatypes. A snippet from `BaseSqlQueryStructure` class: <img width="509" alt="image" src="https://github.com/Azure/data-api-builder/assets/34566234/bc126221-f392-41cf-b1e3-767eefb2a54a"> As evident from the highlighted portion, even for `DateTime` .NET type, we are parsing it with `DateTimeOffset` which is wrong. Just for an example, if you execute a GET request on the `stocks_price` table which has `(categoryid,pieceid,instant)` as PK, you would not get any result back even when the record exists for that PK in the table. Why? Because even though instant is a `datetime` column, it gets parsed as `datetimeoffset` column. This is shown in the next 2 images. <img width="642" alt="image" src="https://github.com/Azure/data-api-builder/assets/34566234/24cd9003-1d30-4713-aa54-5953584ff574"> <img width="323" alt="image" src="https://github.com/Azure/data-api-builder/assets/34566234/bac63094-adba-4544-814d-43e3661536d3"> Also, the existing tests (like the ones fixed in `MsSqlGraphQLPaginationTests` and `GraphQLSupportedTypesTestsBase`) were wrongly written, in which values in the range of `datetime2` were supplied for `datetime` column. ## What is this change? 1. Correctly return value using `DateTime` field of the `DateTimeOffset` parsed value in `BaseSqlQueryStructure` class, 2. Populate the DbType of the datetime parameters when they are sent to the database so that the database knows the exact type of the parameter. Eg. by default, if you send a `datetime2` param without a DbType, it is assumed to be a `datetime` and an exception is thrown incorrectly by the database. This is done using the map `TypeHelper._timeSqlDbTypeToDbType`. It should be noted 4 sql server types - `date`, `smalldatetime`, `datetime`, `datetime2` map to the same .NET system type `System.DateTime`. The existing `TypeHelper._systemTypeToDbTypeMap` would not be of any use to us because it has mapping from system type to DbType and all the above 3 sql server types have the same system type. So, we need to rely on the actual mapping from sql server type to DbType which is: `datetime -> DbType.DateTime, smalldatetime -> DbType.DateTime, datetime2 -> DbType.DateTime2` `date -> DbType.Date` 3. Fix the existing tests. ## How was this tested? - [x] Integration Tests - Added test for getting single record by PK having a `datetime` column as a PK column to `FindApiTestBase`. - [x] Unit Tests - Added filter/orderby and insert tests to `GraphQLSupportedTypesTestsBase.QueryTypeColumnFilterAndOrderByDateTime` and `GraphQLSupportedTypesTestsBase.InsertIntoTypeColumn`for datetime types - `datetime2`, `smalldatetime`, `date`. ## Sample Request(s) - Example REST request to demonstrate modifications <img width="651" alt="image" src="https://github.com/Azure/data-api-builder/assets/34566234/9e150a5b-63ce-4c44-b674-e2e4cb2f88b8"> - Example GQL request to demonstrate modifications <img width="850" alt="image" src="https://github.com/Azure/data-api-builder/assets/34566234/28c70b5a-fb2b-4dbe-9641-bab25823425e"> --------- Co-authored-by: Aniruddh Munde <anmunde@microsoft.com> Co-authored-by: Abhishek Kumar <abhishekkuma@microsoft.com> Co-authored-by: abhishekkumams <102276754+abhishekkumams@users.noreply.github.com> Co-authored-by: Sean Leonard <sean.leonard@microsoft.com>
1 parent da8b47b commit 0829d85

35 files changed

+602
-257
lines changed

config-generators/mssql-commands.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ add Broker --config "dab-config.MsSql.json" --source brokers --permissions "anon
1111
add WebsiteUser --config "dab-config.MsSql.json" --source website_users --permissions "anonymous:create,read,delete,update"
1212
add SupportedType --config "dab-config.MsSql.json" --source type_table --permissions "anonymous:create,read,delete,update"
1313
add stocks_price --config "dab-config.MsSql.json" --source stocks_price --permissions "authenticated:create,read,update,delete"
14+
update stocks_price --config "dab-config.MsSql.json" --permissions "anonymous:read"
1415
update stocks_price --config "dab-config.MsSql.json" --permissions "TestNestedFilterFieldIsNull_ColumnForbidden:read" --fields.exclude "price"
1516
update stocks_price --config "dab-config.MsSql.json" --permissions "TestNestedFilterFieldIsNull_EntityReadForbidden:create"
1617
add Tree --config "dab-config.MsSql.json" --source trees --permissions "anonymous:create,read,update,delete"

config-generators/mysql-commands.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ update BookWebsitePlacement --config "dab-config.MySql.json" --permissions "auth
9898
update BookWebsitePlacement --config "dab-config.MySql.json" --permissions "authenticated:delete" --fields.include "*" --policy-database "@claims.userId eq @item.id"
9999
update Author --config "dab-config.MySql.json" --permissions "authenticated:create,read,update,delete" --rest true --graphql true
100100
update WebsiteUser --config "dab-config.MySql.json" --permissions "authenticated:create,read,delete,update" --rest false --graphql "websiteUser:websiteUsers"
101-
update stocks_price --config "dab-config.MySql.json" --permissions "authenticated:create,read,delete,update" --rest false
101+
update stocks_price --config "dab-config.MySql.json" --permissions "authenticated:create,read,delete,update" --rest true
102102
update Comic --config "dab-config.MySql.json" --permissions "authenticated:create,read,update,delete" --rest true --graphql true --relationship myseries --target.entity series --cardinality one
103103
update series --config "dab-config.MySql.json" --relationship comics --target.entity Comic --cardinality many
104104
update Broker --config "dab-config.MySql.json" --permissions "authenticated:create,update,read,delete" --graphql false

config-generators/postgresql-commands.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ add Broker --config "dab-config.PostgreSql.json" --source brokers --permissions
1010
add WebsiteUser --config "dab-config.PostgreSql.json" --source website_users --permissions "anonymous:create,read,delete,update"
1111
add SupportedType --config "dab-config.PostgreSql.json" --source type_table --permissions "anonymous:create,read,delete,update"
1212
add stocks_price --config "dab-config.PostgreSql.json" --source stocks_price --permissions "authenticated:create,read,update,delete"
13+
update stocks_price --config "dab-config.PostgreSql.json" --permissions "anonymous:read"
1314
update stocks_price --config "dab-config.PostgreSql.json" --permissions "TestNestedFilterFieldIsNull_ColumnForbidden:read" --fields.exclude "price"
1415
update stocks_price --config "dab-config.PostgreSql.json" --permissions "TestNestedFilterFieldIsNull_EntityReadForbidden:create"
1516
add Tree --config "dab-config.PostgreSql.json" --source trees --permissions "anonymous:create,read,update,delete"

src/Core/Models/GraphQLFilterParsers.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,7 @@ public static Predicate Parse(
564564
new PredicateOperand(column),
565565
op,
566566
new PredicateOperand(processLiteral ? $"{processLiterals(value, column.ColumnName)}" : value.ToString()))
567-
));
567+
));
568568
}
569569

570570
return GQLFilterParser.MakeChainPredicate(predicates, PredicateOperation.AND);

src/Core/Resolvers/Sql Query Structures/BaseSqlQueryStructure.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ protected static object ParseParamAsSystemType(string param, Type systemType)
361361
"Double" => double.Parse(param),
362362
"Decimal" => decimal.Parse(param),
363363
"Boolean" => bool.Parse(param),
364-
"DateTime" => DateTimeOffset.Parse(param, DateTimeFormatInfo.InvariantInfo, DateTimeStyles.AssumeUniversal),
364+
"DateTime" => DateTimeOffset.Parse(param, DateTimeFormatInfo.InvariantInfo, DateTimeStyles.AssumeUniversal).DateTime,
365365
"DateTimeOffset" => DateTimeOffset.Parse(param, DateTimeFormatInfo.InvariantInfo, DateTimeStyles.AssumeUniversal),
366366
"Date" => DateOnly.Parse(param),
367367
"Guid" => Guid.Parse(param),

src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,7 @@ public void AddPaginationPredicate(IEnumerable<PaginationColumn> afterJsonValues
475475
{
476476
column.TableAlias = SourceAlias;
477477
column.ParamName = column.Value is not null ?
478-
MakeDbConnectionParam(GetParamAsSystemType(column.Value!.ToString()!, column.ColumnName, GetColumnSystemType(column.ColumnName))) :
478+
MakeDbConnectionParam(GetParamAsSystemType(column.Value!.ToString()!, column.ColumnName, GetColumnSystemType(column.ColumnName)), column.ColumnName) :
479479
MakeDbConnectionParam(null, column.ColumnName);
480480
}
481481

src/Core/Services/MetadataProviders/MsSqlMetadataProvider.cs

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,20 @@
22
// Licensed under the MIT License.
33

44
using System.Data;
5+
using System.Data.Common;
6+
using System.Net;
57
using System.Text.Json;
68
using System.Text.Json.Nodes;
79
using Azure.DataApiBuilder.Config.DatabasePrimitives;
10+
using Azure.DataApiBuilder.Config.ObjectModel;
811
using Azure.DataApiBuilder.Core.Configurations;
912
using Azure.DataApiBuilder.Core.Models;
1013
using Azure.DataApiBuilder.Core.Resolvers;
1114
using Azure.DataApiBuilder.Core.Resolvers.Factories;
15+
using Azure.DataApiBuilder.Service.Exceptions;
1216
using Microsoft.Data.SqlClient;
1317
using Microsoft.Extensions.Logging;
18+
using static Azure.DataApiBuilder.Service.GraphQLBuilder.GraphQLNaming;
1419

1520
namespace Azure.DataApiBuilder.Core.Services
1621
{
@@ -80,5 +85,156 @@ public override async Task PopulateTriggerMetadataForTable(string entityName, st
8085
}
8186
}
8287
}
88+
89+
/// <inheritdoc/>
90+
protected override void PopulateColumnDefinitionWithHasDefaultAndDbType(
91+
SourceDefinition sourceDefinition,
92+
DataTable allColumnsInTable)
93+
{
94+
foreach (DataRow columnInfo in allColumnsInTable.Rows)
95+
{
96+
string columnName = (string)columnInfo["COLUMN_NAME"];
97+
bool hasDefault =
98+
Type.GetTypeCode(columnInfo["COLUMN_DEFAULT"].GetType()) != TypeCode.DBNull;
99+
if (sourceDefinition.Columns.TryGetValue(columnName, out ColumnDefinition? columnDefinition))
100+
{
101+
columnDefinition.HasDefault = hasDefault;
102+
103+
if (hasDefault)
104+
{
105+
columnDefinition.DefaultValue = columnInfo["COLUMN_DEFAULT"];
106+
}
107+
108+
columnDefinition.DbType = TypeHelper.GetDbTypeFromSystemType(columnDefinition.SystemType);
109+
if (columnDefinition.SystemType == typeof(DateTime) || columnDefinition.SystemType == typeof(DateTimeOffset))
110+
{
111+
// MsSql types like date,smalldatetime,datetime,datetime2 are mapped to the same .NET type of DateTime.
112+
// Thus to determine the actual dbtype, we use the underlying MsSql type instead of the .NET type.
113+
DbType dbType;
114+
string sqlType = (string)columnInfo["DATA_TYPE"];
115+
if (TryResolveDbType(sqlType, out dbType))
116+
{
117+
columnDefinition.DbType = dbType;
118+
}
119+
}
120+
}
121+
}
122+
}
123+
124+
/// <inheritdoc/>
125+
protected override async Task FillSchemaForStoredProcedureAsync(
126+
Entity procedureEntity,
127+
string entityName,
128+
string schemaName,
129+
string storedProcedureSourceName,
130+
StoredProcedureDefinition storedProcedureDefinition)
131+
{
132+
using DbConnection conn = new SqlConnection();
133+
conn.ConnectionString = ConnectionString;
134+
await QueryExecutor.SetManagedIdentityAccessTokenIfAnyAsync(conn, _dataSourceName);
135+
await conn.OpenAsync();
136+
137+
string[] procedureRestrictions = new string[NUMBER_OF_RESTRICTIONS];
138+
139+
// To restrict the parameters for the current stored procedure, specify its name
140+
procedureRestrictions[0] = conn.Database;
141+
procedureRestrictions[1] = schemaName;
142+
procedureRestrictions[2] = storedProcedureSourceName;
143+
144+
DataTable procedureMetadata = await conn.GetSchemaAsync(collectionName: "Procedures", restrictionValues: procedureRestrictions);
145+
146+
// Stored procedure does not exist in DB schema
147+
if (procedureMetadata.Rows.Count == 0)
148+
{
149+
throw new DataApiBuilderException(
150+
message: $"No stored procedure definition found for the given database object {storedProcedureSourceName}",
151+
statusCode: HttpStatusCode.ServiceUnavailable,
152+
subStatusCode: DataApiBuilderException.SubStatusCodes.ErrorInInitialization);
153+
}
154+
155+
// Each row in the procedureParams DataTable corresponds to a single parameter
156+
DataTable parameterMetadata = await conn.GetSchemaAsync(collectionName: "ProcedureParameters", restrictionValues: procedureRestrictions);
157+
158+
// For each row/parameter, add an entry to StoredProcedureDefinition.Parameters dictionary
159+
foreach (DataRow row in parameterMetadata.Rows)
160+
{
161+
// row["DATA_TYPE"] has value type string so a direct cast to System.Type is not supported.
162+
// See https://learn.microsoft.com/en-us/dotnet/framework/data/adonet/sql-server-data-type-mappings
163+
string sqlType = (string)row["DATA_TYPE"];
164+
Type systemType = SqlToCLRType(sqlType);
165+
ParameterDefinition paramDefinition = new()
166+
{
167+
SystemType = systemType,
168+
DbType = TypeHelper.GetDbTypeFromSystemType(systemType)
169+
};
170+
171+
if (paramDefinition.SystemType == typeof(DateTime) || paramDefinition.SystemType == typeof(DateTimeOffset))
172+
{
173+
// MsSql types like date,smalldatetime,datetime,datetime2 are mapped to the same .NET type of DateTime.
174+
// Thus to determine the actual dbtype, we use the underlying MsSql type instead of the .NET type.
175+
DbType dbType;
176+
if (TryResolveDbType(sqlType, out dbType))
177+
{
178+
paramDefinition.DbType = dbType;
179+
}
180+
}
181+
182+
// Add to parameters dictionary without the leading @ sign
183+
storedProcedureDefinition.Parameters.TryAdd(((string)row["PARAMETER_NAME"])[1..], paramDefinition);
184+
}
185+
186+
// Loop through parameters specified in config, throw error if not found in schema
187+
// else set runtime config defined default values.
188+
// Note: we defer type checking of parameters specified in config until request time
189+
Dictionary<string, object>? configParameters = procedureEntity.Source.Parameters;
190+
if (configParameters is not null)
191+
{
192+
foreach ((string configParamKey, object configParamValue) in configParameters)
193+
{
194+
if (!storedProcedureDefinition.Parameters.TryGetValue(configParamKey, out ParameterDefinition? parameterDefinition))
195+
{
196+
throw new DataApiBuilderException(
197+
message: $"Could not find parameter \"{configParamKey}\" specified in config for procedure \"{schemaName}.{storedProcedureSourceName}\"",
198+
statusCode: HttpStatusCode.ServiceUnavailable,
199+
subStatusCode: DataApiBuilderException.SubStatusCodes.ErrorInInitialization);
200+
}
201+
else
202+
{
203+
parameterDefinition.HasConfigDefault = true;
204+
parameterDefinition.ConfigDefaultValue = configParamValue?.ToString();
205+
}
206+
}
207+
}
208+
209+
// Generating exposed stored-procedure query/mutation name and adding to the dictionary mapping it to its entity name.
210+
GraphQLStoredProcedureExposedNameToEntityNameMap.TryAdd(GenerateStoredProcedureGraphQLFieldName(entityName, procedureEntity), entityName);
211+
}
212+
213+
/// <summary>
214+
/// Takes a string version of a sql date/time type and returns its corresponding DbType.
215+
/// </summary>
216+
/// <param name="sqlDbTypeName">Name of the sqlDbType.<</param>
217+
/// <param name="dbType">DbType of the parameter corresponding to its sqlDbTypeName.</param>
218+
/// <returns>Returns true when the given sqlDbTypeName datetime type is supported by DAB and resolve it to its corresponding DbType, else false.</returns>
219+
private bool TryResolveDbType(string sqlDbTypeName, out DbType dbType)
220+
{
221+
if (Enum.TryParse(sqlDbTypeName, ignoreCase: true, out SqlDbType sqlDbType))
222+
{
223+
// For MsSql, all the date time types i.e. date, smalldatetime, datetime, datetime2 map to System.DateTime system type.
224+
// Hence we cannot directly determine the DbType from the system type.
225+
// However, to make sure that the database correctly interprets these datatypes, it is necessary to correctly
226+
// populate the DbTypes.
227+
return TypeHelper.TryGetDbTypeFromSqlDbDateTimeType(sqlDbType, out dbType);
228+
}
229+
else
230+
{
231+
// This code should never be hit because every sqlDbTypeName must have a corresponding sqlDbType.
232+
// However, when a new data type is introduced in MsSql which maps to .NET type of DateTime, this code block
233+
// will be hit. Returning false instead of throwing an exception in that case prevents the engine from crashing.
234+
_logger.LogWarning("Could not determine DbType for SqlDb type of {sqlDbTypeName}", sqlDbTypeName);
235+
dbType = 0;
236+
return false;
237+
}
238+
}
83239
}
84240
}

src/Core/Services/MetadataProviders/SqlMetadataProvider.cs

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public abstract class SqlMetadataProvider<ConnectionT, DataAdapterT, CommandT> :
5050

5151
protected IQueryExecutor QueryExecutor { get; }
5252

53-
private const int NUMBER_OF_RESTRICTIONS = 4;
53+
protected const int NUMBER_OF_RESTRICTIONS = 4;
5454

5555
protected string ConnectionString { get; init; }
5656

@@ -320,7 +320,7 @@ private void LogPrimaryKeys()
320320
/// <summary>
321321
/// Verify that the stored procedure exists in the database schema, then populate its database object parameters accordingly
322322
/// </summary>
323-
private async Task FillSchemaForStoredProcedureAsync(
323+
protected virtual async Task FillSchemaForStoredProcedureAsync(
324324
Entity procedureEntity,
325325
string entityName,
326326
string schemaName,
@@ -374,15 +374,16 @@ private async Task FillSchemaForStoredProcedureAsync(
374374
{
375375
// row["DATA_TYPE"] has value type string so a direct cast to System.Type is not supported.
376376
// See https://learn.microsoft.com/en-us/dotnet/framework/data/adonet/sql-server-data-type-mappings
377-
Type systemType = SqlToCLRType((string)row["DATA_TYPE"]);
377+
string sqlType = (string)row["DATA_TYPE"];
378+
Type systemType = SqlToCLRType(sqlType);
379+
ParameterDefinition paramDefinition = new()
380+
{
381+
SystemType = systemType,
382+
DbType = TypeHelper.GetDbTypeFromSystemType(systemType)
383+
};
384+
378385
// Add to parameters dictionary without the leading @ sign
379-
storedProcedureDefinition.Parameters.TryAdd(((string)row["PARAMETER_NAME"])[1..],
380-
new()
381-
{
382-
SystemType = systemType,
383-
DbType = TypeHelper.GetDbTypeFromSystemType(systemType)
384-
}
385-
);
386+
storedProcedureDefinition.Parameters.TryAdd(((string)row["PARAMETER_NAME"])[1..], paramDefinition);
386387
}
387388

388389
// Loop through parameters specified in config, throw error if not found in schema
@@ -1097,13 +1098,11 @@ private async Task PopulateSourceDefinitionAsync(
10971098
subStatusCode: DataApiBuilderException.SubStatusCodes.ErrorInInitialization);
10981099
}
10991100

1100-
Type systemTypeOfColumn = (Type)columnInfoFromAdapter["DataType"];
11011101
ColumnDefinition column = new()
11021102
{
11031103
IsNullable = (bool)columnInfoFromAdapter["AllowDBNull"],
11041104
IsAutoGenerated = (bool)columnInfoFromAdapter["IsAutoIncrement"],
1105-
SystemType = systemTypeOfColumn,
1106-
DbType = TypeHelper.GetDbTypeFromSystemType(systemTypeOfColumn),
1105+
SystemType = (Type)columnInfoFromAdapter["DataType"],
11071106
// An auto-increment column is also considered as a read-only column. For other types of read-only columns,
11081107
// the flag is populated later via PopulateColumnDefinitionsWithReadOnlyFlag() method.
11091108
IsReadOnly = (bool)columnInfoFromAdapter["IsAutoIncrement"]
@@ -1117,7 +1116,8 @@ private async Task PopulateSourceDefinitionAsync(
11171116
}
11181117

11191118
DataTable columnsInTable = await GetColumnsAsync(schemaName, tableName);
1120-
PopulateColumnDefinitionWithHasDefault(
1119+
1120+
PopulateColumnDefinitionWithHasDefaultAndDbType(
11211121
sourceDefinition,
11221122
columnsInTable);
11231123

@@ -1400,9 +1400,9 @@ protected virtual async Task<DataTable> GetColumnsAsync(
14001400
}
14011401

14021402
/// <summary>
1403-
/// Populates the column definition with HasDefault property.
1403+
/// Helper method to populate the column definition with HasDefault and DbType properties.
14041404
/// </summary>
1405-
private static void PopulateColumnDefinitionWithHasDefault(
1405+
protected virtual void PopulateColumnDefinitionWithHasDefaultAndDbType(
14061406
SourceDefinition sourceDefinition,
14071407
DataTable allColumnsInTable)
14081408
{
@@ -1419,6 +1419,8 @@ private static void PopulateColumnDefinitionWithHasDefault(
14191419
{
14201420
columnDefinition.DefaultValue = columnInfo["COLUMN_DEFAULT"];
14211421
}
1422+
1423+
columnDefinition.DbType = TypeHelper.GetDbTypeFromSystemType(columnDefinition.SystemType);
14221424
}
14231425
}
14241426
}

src/Core/Services/ResolverMiddleware.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
using Microsoft.AspNetCore.Http;
1919
using Microsoft.Extensions.Primitives;
2020
using NodaTime.Text;
21-
using static Azure.DataApiBuilder.Service.GraphQLBuilder.GraphQLTypes.SupportedTypes;
21+
using static Azure.DataApiBuilder.Service.GraphQLBuilder.GraphQLTypes.SupportedHotChocolateTypes;
2222

2323
namespace Azure.DataApiBuilder.Core.Services
2424
{

0 commit comments

Comments
 (0)