Skip to content
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

Support for int IndexOf(string value, int startIndex) #1625

Closed
jespersh opened this issue Mar 24, 2022 · 8 comments · Fixed by #1645
Closed

Support for int IndexOf(string value, int startIndex) #1625

jespersh opened this issue Mar 24, 2022 · 8 comments · Fixed by #1645

Comments

@jespersh
Copy link

Currently

We Currently have an implementation of int IndexOf(string value) calling column.IndexOf("#") <= 1 that gets (roughly) translated into `

LOCATE('#', `j`.`column`) - 1) > 1

Implementing int IndexOf(string value, int startIndex) means we can use the form LOCATE('#',column,X)=0 for column.IndexOf("#", X) == -1.
A way to currently achieve this implementation is to use Substring(1).IndexOf("#") == -1 which however results in a fairly interesting:

(LOCATE('#', SUBSTRING(`j`.`column`, 1 + 1, CHAR_LENGTH(`j`.`column`))) - 1) = -1
@mguinness
Copy link
Collaborator

Yes, currently int string.IndexOf(string value, int startIndex) isn't translated into LOCATE(substr,str,pos) yet. This is a community based project and we welcome code contributions via pull requests.

You can take a look at the existing implementation as a guide to adding the overload in MySqlStringComparisonMethodTranslator.

@LeaFrock
Copy link
Contributor

@mguinness I wanna contribute to this. Would you please help me review and accomplish my PR #1645 ?

@mguinness
Copy link
Collaborator

@ajcvickers @roji Is there a reason why the start index parameter of IndexOf() isn't translated for SQL Server or Postgres? Is it because there is a workaround, like substring?

@roji
Copy link

roji commented May 11, 2022

@mguinness I don't think there's any particular reason - probably just nobody asked for it... I can see overloads of CHARINDEX (SQL Server) and STRPOS (PG) which accept the additional starting position parameter.

@roji
Copy link

roji commented May 16, 2022

@mguinness @lauxjpn @LeaFrock it turns out this actually was implemented for SQL Server in dotnet/efcore#26623, for 7.0.0-preview.1. There's a specification test for it as well, but since it was using a top-most projection, the test can client-eval and always passes regardless of whether a provider implements translation support or not. I opened dotnet/efcore#28031 to improve the tests.

Note that SQLite and PostgreSQL do not have a function which accepts a start position parameter.

@LeaFrock
Copy link
Contributor

LeaFrock commented May 16, 2022

Thanks for the PR from @roji , and based on it, I've created another PR #1652 to optimize the previous translation.

@mguinness Would you please add stringValue.IndexOf(value,startIndex) into the wiki of String funtions ? I don't know how to update it.

@mguinness
Copy link
Collaborator

Wiki updated as requested. Since you're a collaborator, you should see an Edit button on the page to allow you to edit the markdown.

@LeaFrock
Copy link
Contributor

Wiki updated as requested. Since you're a collaborator, you should see an Edit button on the page to allow you to edit the markdown.

Thanks for your update. I'm just a normal contributor 😄 and the edit button is hidden to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants