Cosmos: Add translator for Regex.IsMatch method #28121
Conversation
There was a problem hiding this comment.
Thanks, it's nice to see this!
In addition to the below, please take a look at the regex options as suggested by @AndriySvyryd (#28078 (comment)) - there seems to be at least some correspondence with the .NET API here.
Note that we'd only support constant regex options, which we can interpret at compile-time and translate to the appropriate Cosmos constant string. The PostgreSQL translator may provide some help.
| AssertSql( | ||
| @"SELECT c | ||
| FROM root c | ||
| WHERE ((c[""Discriminator""] = ""Customer"") AND RegexMatch(c[""CustomerID""], ""^T"", """"))"); |
There was a problem hiding this comment.
do we need to print last argument if options are none?
There was a problem hiding this comment.
IMO, yes. If Cosmos allows it and a user specifically indicated None, then why not. Clear behavior.
Of course I can change it if you want
There was a problem hiding this comment.
I'd go with Smit's suggestion, always better to emit clearer, more concise SQL. Added to #28139.
There was a problem hiding this comment.
ok, please assign 28139 to me
| { | ||
| await AssertQuery( | ||
| async, | ||
| ss => ss.Set<Customer>().Where(o => Regex.IsMatch(o.CustomerID, "^T", RegexOptions.IgnoreCase | RegexOptions.IgnorePatternWhitespace)), |
There was a problem hiding this comment.
Should we consider case where regex options are passed as a variable here so it won't be constant. We can force "constantization" in parameter extraction through evaluatablefilter.
There was a problem hiding this comment.
Right, we definitely can... Added that to #28139.
| WHERE ((c[""Discriminator""] = ""Customer"") AND RegexMatch(c[""CustomerID""], ""^T"", ""ix""))"); | ||
| } | ||
|
|
||
| [Fact] |
There was a problem hiding this comment.
ConditionalTheory
sync/async both version
Use AssertQuery syntax
cc: @roji
Add translator for Regex.IsMatch method to REGEXMATCH
Fixes #28078
Please review
Thank you in advance