Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Config/DatabasePrimitives/DatabaseObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ public virtual SourceDefinition SourceDefinition
EntitySourceType.Table => ((DatabaseTable)this).TableDefinition,
EntitySourceType.View => ((DatabaseView)this).ViewDefinition,
EntitySourceType.StoredProcedure => ((DatabaseStoredProcedure)this).StoredProcedureDefinition,
EntitySourceType.Function => ((DatabaseStoredProcedure)this).StoredProcedureDefinition,
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Downcasting 'this' from DatabaseObject to DatabaseStoredProcedure introduces a dependency cycle between the two types.

Copilot uses AI. Check for mistakes.
_ => throw new Exception(
message: $"Unsupported EntitySourceType. It can either be Table,View, or Stored Procedure.")
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exception message references "Unsupported EntitySourceType. It can either be Table,View, or Stored Procedure" but should now include Function as a supported type since it was added to the EntitySourceType enum.

Suggested change
message: $"Unsupported EntitySourceType. It can either be Table,View, or Stored Procedure.")
message: $"Unsupported EntitySourceType. It can either be Table, View, Stored Procedure, or Function.")

Copilot uses AI. Check for mistakes.
};
Expand Down
3 changes: 2 additions & 1 deletion src/Config/ObjectModel/EntitySourceType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ public enum EntitySourceType
{
Table,
View,
[EnumMember(Value = "stored-procedure")] StoredProcedure
[EnumMember(Value = "stored-procedure")] StoredProcedure,
Function
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Function entity source type is missing the EnumMember attribute with a serialization value. For consistency with StoredProcedure which has [EnumMember(Value = "stored-procedure")], this should have an attribute like [EnumMember(Value = "function")].

Suggested change
Function
[EnumMember(Value = "function")] Function

Copilot uses AI. Check for mistakes.
}
6 changes: 4 additions & 2 deletions src/Core/Resolvers/BaseQueryStructure.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public virtual string MakeDbConnectionParam(object? value, string? paramName = n
string encodedParamName = GetEncodedParamName(Counter.Next());
if (!string.IsNullOrEmpty(paramName))
{
Parameters.Add(encodedParamName,
Parameters.Add(paramName,
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change from encodedParamName to paramName in the Parameters dictionary breaks the existing parameter encoding mechanism. This could cause parameter name collisions if the same parameter name is used multiple times, since the encoded names were designed to be unique. This change affects all database query structures, not just PostgreSQL functions.

Suggested change
Parameters.Add(paramName,
Parameters.Add(encodedParamName,

Copilot uses AI. Check for mistakes.
new(value,
dbType: GetUnderlyingSourceDefinition().GetDbTypeForParam(paramName),
sqlDbType: GetUnderlyingSourceDefinition().GetSqlDbTypeForParam(paramName)));
Comment on lines +124 to 127
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change from using encodedParamName to paramName in the Parameters dictionary could cause issues if paramName is null or empty. The original logic used encodedParamName which was guaranteed to have a value. This change may break scenarios where paramName is not provided.

Copilot uses AI. Check for mistakes.
Expand All @@ -131,7 +131,9 @@ public virtual string MakeDbConnectionParam(object? value, string? paramName = n
Parameters.Add(encodedParamName, new(value));
}

return encodedParamName;
return paramName == null ? encodedParamName : $"{PARAM_NAME_PREFIX}{paramName}";


}

/// <summary>
Expand Down
85 changes: 80 additions & 5 deletions src/Core/Resolvers/PostgresQueryBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,59 @@ public string Build(SqlDeleteStructure structure)
/// </summary>
public string Build(SqlExecuteStructure structure)
{
throw new NotImplementedException();
return $"SELECT * FROM {QuoteIdentifier(structure.DatabaseObject.SchemaName)}.{QuoteIdentifier(structure.DatabaseObject.Name)}({BuildProcedureParameterList(structure.ProcedureParameters)})";
}
private string BuildProcedureParameterList(Dictionary<string, object> parameters)
{
if (parameters == null || parameters.Count == 0)
{
return "";
}

List<string> parameterList = new();
foreach (KeyValuePair<string, object> param in parameters)
{
parameterList.Add($"{QuoteIdentifier(param.Key)} := {FormatParameterValue(param.Value)}");
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The named parameter syntax used here (parameter_name := value) is PostgreSQL-specific. However, this implementation doesn't use parameterized queries, which means the values are being embedded directly into the SQL string. This approach is vulnerable to SQL injection attacks. The implementation should use proper parameterized queries instead of string formatting the values directly into the SQL.

Copilot uses AI. Check for mistakes.
}

return string.Join(", ", parameterList);
}

#pragma warning disable CA1822 // Mark members as static
private string? FormatParameterValue(object value)
#pragma warning restore CA1822 // Mark members as static
Comment on lines +131 to +133
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pragma directives around FormatParameterValue are inconsistent with their purpose. The method is marked with pragma to suppress CA1822 (Mark members as static), but then the method implementation shows it doesn't use instance state and could indeed be static. Either make the method static or remove the pragma suppressions if there's a valid reason it needs to be an instance method.

Suggested change
#pragma warning disable CA1822 // Mark members as static
private string? FormatParameterValue(object value)
#pragma warning restore CA1822 // Mark members as static
private static string? FormatParameterValue(object value)

Copilot uses AI. Check for mistakes.
{
if (value == null)
{
return "NULL";
}

if (value is string || value is char)
{
if (value == null)
{
value = string.Empty;
}
Comment on lines +142 to +145
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This null check is redundant as it immediately follows another null check at line 126 that would have already returned "NULL" if value was null.

Suggested change
if (value == null)
{
value = string.Empty;
}

Copilot uses AI. Check for mistakes.
Comment on lines +142 to +145
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is unreachable code here. The null check on line 135 already returns "NULL" if value is null, so lines 142-145 will never execute because value cannot be null at that point. This check should be removed.

Suggested change
if (value == null)
{
value = string.Empty;
}

Copilot uses AI. Check for mistakes.
// Handle string values, escaping single quotes
#pragma warning disable CS8602 // Dereference of a possibly null reference.
return $"{value.ToString().Replace("'", "''")}";
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String values are not being properly quoted. The current implementation replaces single quotes but doesn't wrap the string in single quotes. This will result in invalid SQL syntax.

Suggested change
return $"{value.ToString().Replace("'", "''")}";
return $"'{value.ToString().Replace("'", "''")}'";

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String values are not properly quoted in the SQL function call. Line 148 escapes single quotes but doesn't wrap the value in quotes. This will cause SQL syntax errors. The return statement should be wrapped in single quotes.

Suggested change
return $"{value.ToString().Replace("'", "''")}";
return $"'{value.ToString().Replace("'", "''")}'";

Copilot uses AI. Check for mistakes.
#pragma warning restore CS8602 // Dereference of a possibly null reference.
Comment on lines +142 to +149
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The suppression of CS8602 (Dereference of a possibly null reference) is unnecessary because the null check on line 142-145 already handles the null case (though that code is unreachable due to the earlier null check on line 135-138). After removing the unreachable code, this pragma can also be removed since value.ToString() will not be null at this point.

Suggested change
if (value == null)
{
value = string.Empty;
}
// Handle string values, escaping single quotes
#pragma warning disable CS8602 // Dereference of a possibly null reference.
return $"{value.ToString().Replace("'", "''")}";
#pragma warning restore CS8602 // Dereference of a possibly null reference.
// Handle string values, escaping single quotes
return $"{value.ToString().Replace("'", "''")}";

Copilot uses AI. Check for mistakes.
}
Comment on lines +140 to +150
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pragma warning disable/restore for CS8602 is unnecessary here. If value can be null at this point, the proper solution is to add a null check or use the null-conditional operator, not suppress the warning.

Suggested change
if (value is string || value is char)
{
if (value == null)
{
value = string.Empty;
}
// Handle string values, escaping single quotes
#pragma warning disable CS8602 // Dereference of a possibly null reference.
return $"{value.ToString().Replace("'", "''")}";
#pragma warning restore CS8602 // Dereference of a possibly null reference.
}
if (value is string)
{
// Handle string values, escaping single quotes
}
else if (value is char)
{
// Handle char values, escaping single quotes
return $"{value.ToString().Replace("'", "''")}";

Copilot uses AI. Check for mistakes.

if (value is bool)
{
// Handle boolean values
return (bool)value ? "TRUE" : "FALSE";
}

if (value is DateTime)
{
// Handle DateTime values
return $"'{((DateTime)value).ToString("yyyy-MM-dd HH:mm:ss")}'";
}

// Handle numeric and other types
return value == null ? string.Empty : value.ToString();
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FormatParameterValue method directly embeds parameter values into the SQL query string, creating a SQL injection vulnerability. Named parameters should use proper parameterization instead of inline value formatting.

Suggested change
return value == null ? string.Empty : value.ToString();
throw new NotSupportedException("Direct formatting of parameter values is not supported. Use parameterized queries instead.");

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Condition is always false because of ... == ....

Suggested change
return value == null ? string.Empty : value.ToString();
return value.ToString();

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ternary expression on line 165 is redundant. If value is null, the method already returns "NULL" at line 137. The null check here is unnecessary.

Suggested change
return value == null ? string.Empty : value.ToString();
return value.ToString();

Copilot uses AI. Check for mistakes.
}

public string Build(SqlUpsertQueryStructure structure)
Expand Down Expand Up @@ -227,13 +279,36 @@ private string MakeSelectColumns(SqlQueryStructure structure)

return string.Join(", ", builtColumns);
}

/// <inheritdoc/>
public string BuildStoredProcedureResultDetailsQuery(string databaseObjectName)
public string BuildStoredProcedureResultDetailsQuery(string procedureName)
{
throw new NotImplementedException();
// This query retrieves the details of the result set for a given stored procedure

string query = $@"
SELECT
p.parameter_name AS name,
p.data_type AS system_type_name,
CASE
WHEN p.parameter_mode = 'IN' THEN FALSE
ELSE TRUE
END AS is_nullable
FROM
information_schema.parameters p
JOIN
information_schema.routines r
ON p.specific_name = r.specific_name
WHERE
r.routine_schema = 'public'
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The routine_schema is hardcoded to 'public' instead of using a parameter. This will cause the query to fail for functions in non-public schemas. The query should use a parameterized value like the function name.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The schema name is hardcoded to 'public' in this query. This should use a parameter or variable to support schemas other than the default 'public' schema, similar to how it's done elsewhere in the codebase.

Copilot uses AI. Check for mistakes.
and p.parameter_mode = 'OUT'
AND r.routine_name = '{procedureName}'
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The procedureName parameter is directly concatenated into the SQL query string, making this code vulnerable to SQL injection attacks. The parameter should be properly escaped or parameterized.

Copilot uses AI. Check for mistakes.
ORDER BY
p.ordinal_position";



return query;
}


/// <inheritdoc/>
public string BuildQueryToGetReadOnlyColumns(string schemaParamName, string tableParamName)
{
Expand Down
119 changes: 116 additions & 3 deletions src/Core/Services/MetadataProviders/MySqlMetadataProvider.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.Collections;
using System.Data;
using Azure.DataApiBuilder.Config.DatabasePrimitives;
using Azure.DataApiBuilder.Core.Configurations;
Expand Down Expand Up @@ -128,12 +129,124 @@ protected override DatabaseTable GenerateDbTable(string schemaName, string table
}

/// <summary>
/// Takes a string version of a MySql data type and returns its .NET common language runtime (CLR) counterpart
/// TODO: For MySql stored procedure support, this needs to be implemented.
/// Takes a string version of a PostgreSql data type and returns its .NET common language runtime (CLR) counterpart
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there are changes related to MySQL as well, can you move them in a separate PR?

Since, the PR title says it's for Postgres

/// TODO: For PostgreSql stored procedure support, this needs to be implemented.
Comment on lines +132 to +133
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment incorrectly refers to "PostgreSql" instead of "MySql". This method implements type mapping for MySql, not PostgreSql.

Suggested change
/// Takes a string version of a PostgreSql data type and returns its .NET common language runtime (CLR) counterpart
/// TODO: For PostgreSql stored procedure support, this needs to be implemented.
/// Takes a string version of a MySql data type and returns its .NET common language runtime (CLR) counterpart
/// TODO: For MySql stored procedure support, this needs to be implemented.

Copilot uses AI. Check for mistakes.
/// </summary>
public override Type SqlToCLRType(string sqlType)
{
throw new NotImplementedException();
switch (sqlType.ToLower())
{
case "boolean":
case "bool":
return typeof(bool);

case "smallint":
return typeof(short);

case "integer":
case "int":
return typeof(int);

case "bigint":
return typeof(long);

case "real":
return typeof(float);

case "double precision":
return typeof(double);

case "numeric":
case "decimal":
return typeof(decimal);

case "money":
return typeof(decimal);

case "character varying":
case "varchar":
case "character":
case "char":
case "text":
return typeof(string);

case "bytea":
return typeof(byte[]);

case "date":
return typeof(DateTime);

case "timestamp":
case "timestamp without time zone":
return typeof(DateTime);

case "timestamp with time zone":
return typeof(DateTimeOffset);

case "time":
case "time without time zone":
return typeof(TimeSpan);

case "time with time zone":
return typeof(DateTimeOffset);

case "interval":
return typeof(TimeSpan);

case "uuid":
return typeof(Guid);

case "json":
case "jsonb":
return typeof(string);

case "xml":
return typeof(string);

case "inet":
return typeof(System.Net.IPAddress);

case "cidr":
return typeof(ValueTuple<System.Net.IPAddress, int>);

case "macaddr":
return typeof(System.Net.NetworkInformation.PhysicalAddress);

case "bit":
case "bit varying":
return typeof(BitArray);

case "point":
return typeof((double, double));

case "line":
return typeof(string); // Implement a custom type if needed

case "lseg":
return typeof((double, double)[]);

case "box":
return typeof((double, double)[]);

case "path":
return typeof((double, double)[]);

case "polygon":
return typeof((double, double)[]);

case "circle":
return typeof((double, double, double));

case "tsvector":
return typeof(string); // Implement a custom type if needed

case "tsquery":
return typeof(string); // Implement a custom type if needed

Comment on lines +139 to +245
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SqlToCLRType implementation in MySqlMetadataProvider contains PostgreSQL-specific type mappings (like "boolean", "double precision", "bytea", "jsonb", "inet", "cidr", "macaddr", "bit varying", "tsvector", "tsquery") rather than MySQL type mappings. MySQL uses different type names such as "tinyint", "mediumint", "blob", "longtext", "datetime", etc. This implementation appears to have been incorrectly copied from the PostgreSQL provider.

Suggested change
case "boolean":
case "bool":
return typeof(bool);
case "smallint":
return typeof(short);
case "integer":
case "int":
return typeof(int);
case "bigint":
return typeof(long);
case "real":
return typeof(float);
case "double precision":
return typeof(double);
case "numeric":
case "decimal":
return typeof(decimal);
case "money":
return typeof(decimal);
case "character varying":
case "varchar":
case "character":
case "char":
case "text":
return typeof(string);
case "bytea":
return typeof(byte[]);
case "date":
return typeof(DateTime);
case "timestamp":
case "timestamp without time zone":
return typeof(DateTime);
case "timestamp with time zone":
return typeof(DateTimeOffset);
case "time":
case "time without time zone":
return typeof(TimeSpan);
case "time with time zone":
return typeof(DateTimeOffset);
case "interval":
return typeof(TimeSpan);
case "uuid":
return typeof(Guid);
case "json":
case "jsonb":
return typeof(string);
case "xml":
return typeof(string);
case "inet":
return typeof(System.Net.IPAddress);
case "cidr":
return typeof(ValueTuple<System.Net.IPAddress, int>);
case "macaddr":
return typeof(System.Net.NetworkInformation.PhysicalAddress);
case "bit":
case "bit varying":
return typeof(BitArray);
case "point":
return typeof((double, double));
case "line":
return typeof(string); // Implement a custom type if needed
case "lseg":
return typeof((double, double)[]);
case "box":
return typeof((double, double)[]);
case "path":
return typeof((double, double)[]);
case "polygon":
return typeof((double, double)[]);
case "circle":
return typeof((double, double, double));
case "tsvector":
return typeof(string); // Implement a custom type if needed
case "tsquery":
return typeof(string); // Implement a custom type if needed
case "tinyint":
return typeof(sbyte); // MySQL tinyint is signed by default
case "tinyint unsigned":
return typeof(byte);
case "bool":
case "boolean":
return typeof(bool);
case "smallint":
return typeof(short);
case "smallint unsigned":
return typeof(ushort);
case "mediumint":
return typeof(int);
case "mediumint unsigned":
return typeof(uint);
case "int":
case "integer":
return typeof(int);
case "int unsigned":
case "integer unsigned":
return typeof(uint);
case "bigint":
return typeof(long);
case "bigint unsigned":
return typeof(ulong);
case "float":
return typeof(float);
case "double":
case "double precision":
return typeof(double);
case "decimal":
case "dec":
case "fixed":
case "numeric":
return typeof(decimal);
case "bit":
return typeof(ulong); // MySQL BIT can be up to 64 bits
case "char":
case "nchar":
case "varchar":
case "nvarchar":
case "text":
case "tinytext":
case "mediumtext":
case "longtext":
return typeof(string);
case "binary":
case "varbinary":
case "blob":
case "tinyblob":
case "mediumblob":
case "longblob":
return typeof(byte[]);
case "enum":
case "set":
return typeof(string);
case "date":
case "datetime":
case "timestamp":
return typeof(DateTime);
case "time":
return typeof(TimeSpan);
case "year":
return typeof(int);
case "json":
return typeof(string);

Copilot uses AI. Check for mistakes.
default:
throw new NotSupportedException($"The SQL type '{sqlType}' is not supported.");
}
Comment on lines +137 to +248
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SqlToCLRType implementation in MySqlMetadataProvider uses PostgreSQL data types instead of MySQL data types. MySQL has different type names (e.g., TINYINT, MEDIUMINT, LONGTEXT, BLOB types, etc.) and this implementation will not work correctly for MySQL. This is a copy-paste error from the PostgreSQL implementation.

Copilot uses AI. Check for mistakes.
}

}
}
Loading