-
Notifications
You must be signed in to change notification settings - Fork 260
Regex replace and count #3062
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Regex replace and count #3062
Changes from 12 commits
4bfdf0d
ff41eda
ac9b36d
d6a3efe
fc4e3bb
0a2debd
ff79e36
908aba9
98113c3
f854f30
382d1bc
4b83cdc
bfc73bb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,224 @@ | ||
| using System.Diagnostics.CodeAnalysis; | ||
| using System.Text.RegularExpressions; | ||
| using Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal; | ||
| using ExpressionExtensions = Microsoft.EntityFrameworkCore.Query.ExpressionExtensions; | ||
|
|
||
| namespace Npgsql.EntityFrameworkCore.PostgreSQL.Query.ExpressionTranslators.Internal; | ||
|
|
||
| /// <summary> | ||
| /// Translates Regex method calls into their corresponding PostgreSQL equivalent for database-side processing. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// http://www.postgresql.org/docs/current/static/functions-matching.html | ||
| /// </remarks> | ||
| public class NpgsqlRegexTranslator : IMethodCallTranslator | ||
| { | ||
| private static readonly MethodInfo IsMatch = | ||
| typeof(Regex).GetRuntimeMethod(nameof(Regex.IsMatch), [typeof(string), typeof(string)])!; | ||
|
|
||
| private static readonly MethodInfo IsMatchWithRegexOptions = | ||
| typeof(Regex).GetRuntimeMethod(nameof(Regex.IsMatch), [typeof(string), typeof(string), typeof(RegexOptions)])!; | ||
|
|
||
| private static readonly MethodInfo Replace = | ||
| typeof(Regex).GetRuntimeMethod(nameof(Regex.Replace), [typeof(string), typeof(string), typeof(string)])!; | ||
|
|
||
| private static readonly MethodInfo ReplaceWithRegexOptions = | ||
| typeof(Regex).GetRuntimeMethod(nameof(Regex.Replace), [typeof(string), typeof(string), typeof(string), typeof(RegexOptions)])!; | ||
|
|
||
| private static readonly MethodInfo Count = | ||
| typeof(Regex).GetRuntimeMethod(nameof(Regex.Count), [typeof(string), typeof(string)])!; | ||
|
|
||
| private static readonly MethodInfo CountWithRegexOptions = | ||
| typeof(Regex).GetRuntimeMethod(nameof(Regex.Count), [typeof(string), typeof(string), typeof(RegexOptions)])!; | ||
|
|
||
| private const RegexOptions UnsupportedRegexOptions = RegexOptions.RightToLeft | RegexOptions.ECMAScript; | ||
|
|
||
| private readonly NpgsqlSqlExpressionFactory _sqlExpressionFactory; | ||
| private readonly bool _supportRegexCount; | ||
| private readonly NpgsqlTypeMappingSource _typeMappingSource; | ||
|
|
||
| /// <summary> | ||
| /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to | ||
| /// the same compatibility standards as public APIs. It may be changed or removed without notice in | ||
| /// any release. You should only use it directly in your code with extreme caution and knowing that | ||
| /// doing so can result in application failures when updating to a new Entity Framework Core release. | ||
| /// </summary> | ||
| public NpgsqlRegexTranslator( | ||
| NpgsqlTypeMappingSource typeMappingSource, | ||
| NpgsqlSqlExpressionFactory sqlExpressionFactory, | ||
| bool supportRegexCount) | ||
| { | ||
| _sqlExpressionFactory = sqlExpressionFactory; | ||
| _supportRegexCount = supportRegexCount; | ||
| _typeMappingSource = typeMappingSource; | ||
| } | ||
|
|
||
| /// <inheritdoc /> | ||
| public SqlExpression? Translate( | ||
| SqlExpression? instance, | ||
| MethodInfo method, | ||
| IReadOnlyList<SqlExpression> arguments, | ||
| IDiagnosticsLogger<DbLoggerCategory.Query> logger) | ||
| => TranslateIsMatch(instance, method, arguments, logger) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: no need to have parameters for instance, logger if they're unused (this is all private code, which we can change in the future if we need to) |
||
| ?? TranslateReplace(method, arguments, logger) | ||
| ?? TranslateCount(method, arguments, logger); | ||
|
|
||
| private SqlExpression? TranslateIsMatch( | ||
| SqlExpression? instance, | ||
| MethodInfo method, | ||
| IReadOnlyList<SqlExpression> arguments, | ||
| IDiagnosticsLogger<DbLoggerCategory.Query> logger) | ||
| { | ||
| if (method != IsMatch && method != IsMatchWithRegexOptions) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| var (input, pattern) = (arguments[0], arguments[1]); | ||
| var typeMapping = ExpressionExtensions.InferTypeMapping(input, pattern); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know the code was this way before, but it's incorrect - the type of the arguments/operands isn't related, and PG doesn't infer one from the other (for example, a |
||
|
|
||
| return TryGetRegexOptions(arguments, 2, out var regexOptions) | ||
| ? _sqlExpressionFactory.RegexMatch( | ||
| _sqlExpressionFactory.ApplyTypeMapping(input, typeMapping), | ||
| _sqlExpressionFactory.ApplyTypeMapping(pattern, typeMapping), | ||
| regexOptions.Value) | ||
| : null; | ||
| } | ||
|
|
||
| private SqlExpression? TranslateReplace( | ||
| MethodInfo method, | ||
| IReadOnlyList<SqlExpression> arguments, | ||
| IDiagnosticsLogger<DbLoggerCategory.Query> logger) | ||
| { | ||
| if (method != Replace && method != ReplaceWithRegexOptions) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| var (input, pattern, replacement) = (arguments[0], arguments[1], arguments[2]); | ||
| var typeMapping = ExpressionExtensions.InferTypeMapping(input, pattern, replacement); | ||
|
|
||
| if (!TryGetRegexOptions(arguments, 3, out var regexOptions)) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| List<SqlExpression> passingArguments = [ | ||
| _sqlExpressionFactory.ApplyTypeMapping(input, typeMapping), | ||
| _sqlExpressionFactory.ApplyTypeMapping(pattern, typeMapping), | ||
| _sqlExpressionFactory.ApplyTypeMapping(replacement, typeMapping) | ||
| ]; | ||
|
|
||
| if (TranslateOptions(regexOptions.Value) is { Length: not 0 } translatedOptions) | ||
| { | ||
| passingArguments.Add(_sqlExpressionFactory.Constant(translatedOptions)); | ||
| } | ||
|
|
||
| return _sqlExpressionFactory.Function( | ||
| "regexp_replace", | ||
| passingArguments, | ||
| nullable: true, | ||
| Enumerable.Repeat(true, passingArguments.Count), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: use TrueArrays |
||
| typeof(string), | ||
| _typeMappingSource.FindMapping(typeof(string))); | ||
| } | ||
|
|
||
| private SqlExpression? TranslateCount( | ||
| MethodInfo method, | ||
| IReadOnlyList<SqlExpression> arguments, | ||
| IDiagnosticsLogger<DbLoggerCategory.Query> logger) | ||
| { | ||
| if (!_supportRegexCount || (method != Count && method != CountWithRegexOptions)) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| var (input, pattern) = (arguments[0], arguments[1]); | ||
| var typeMapping = ExpressionExtensions.InferTypeMapping(input, pattern); | ||
|
|
||
| if (!TryGetRegexOptions(arguments, 2, out var regexOptions)) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| List<SqlExpression> passingArguments = [ | ||
| _sqlExpressionFactory.ApplyTypeMapping(input, typeMapping), | ||
| _sqlExpressionFactory.ApplyTypeMapping(pattern, typeMapping) | ||
| ]; | ||
|
|
||
| if (TranslateOptions(regexOptions.Value) is { Length: not 0 } translatedOptions) | ||
| { | ||
| passingArguments.AddRange([ | ||
| //starting position has to be set to use the options in postgres | ||
| _sqlExpressionFactory.Constant(1), | ||
| _sqlExpressionFactory.Constant(translatedOptions) | ||
| ]); | ||
| } | ||
|
|
||
| return _sqlExpressionFactory.Function( | ||
| "regexp_count", | ||
| passingArguments, | ||
| nullable: true, | ||
| Enumerable.Repeat(true, passingArguments.Count), | ||
| typeof(int?), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be |
||
| _typeMappingSource.FindMapping(typeof(int))); | ||
| } | ||
|
|
||
| private static bool TryGetRegexOptions( | ||
| IReadOnlyList<SqlExpression> arguments, | ||
| int minArguments, | ||
| [NotNullWhen(true)] out RegexOptions? options) | ||
| { | ||
| if (arguments.Count == minArguments) | ||
| { | ||
| options = RegexOptions.None; | ||
| } | ||
| else if (arguments[minArguments] is SqlConstantExpression { Value: RegexOptions regexOptions } | ||
| && (regexOptions & UnsupportedRegexOptions) is 0) | ||
| { | ||
| options = regexOptions; | ||
| } | ||
| else | ||
| { | ||
| options = null; | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| private static string TranslateOptions(RegexOptions options) | ||
| { | ||
| if (options is RegexOptions.Singleline) | ||
| { | ||
| return string.Empty; | ||
| } | ||
|
|
||
| string? result; | ||
|
|
||
| if (options.HasFlag(RegexOptions.Multiline)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: this is a conceptual switch over options |
||
| { | ||
| result = "n"; | ||
| }else if(!options.HasFlag(RegexOptions.Singleline)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: proper spacing |
||
| { | ||
| result = "p"; | ||
| } | ||
| else | ||
| { | ||
| result = string.Empty; | ||
| } | ||
|
|
||
| if (options.HasFlag(RegexOptions.IgnoreCase)) | ||
| { | ||
| result += "i"; | ||
| } | ||
|
|
||
| if (options.HasFlag(RegexOptions.IgnorePatternWhitespace)) | ||
| { | ||
| result += "x"; | ||
| } | ||
|
|
||
| return result; | ||
| } | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: empty line |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI we're starting to move away from this way of matching things in method/member translators; this lookup of MethodInfos via reflection causes them to always get preserved when using trimming, regardless of whether the user actually needs them or not (and therefore bloats the final binary). Instead, it's better to check the declaring type, and then simply check the method name as a string.
I'll rewrite to do this.