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

Full text querying support in LINQ (tsvector) #1003

Closed
wants to merge 3 commits into from
Closed

Full text querying support in LINQ (tsvector) #1003

wants to merge 3 commits into from

Conversation

rwasef1830
Copy link
Contributor

Here's my stab at implementing a limited form of what is described in http://blog.devart.com/using-postgresql-full-text-search-in-entity-framework.html in Npgsql. This is related to #999

So far, I've got to_tsquery, plainto_tsquery, and to_tsvector working. Will try with more functions as time permits.

Let me know if I messed something up.

…via static methods on PgSqlTextFunctions and exposed on NpgsqlProviderManifest.GetStoreFunctions() for EF6.
…name and add support for to_tsquery. Make the function registration reflection based.
@Emill
Copy link
Contributor

Emill commented Mar 24, 2016

Nice!
With this we can extend to not only having full text search functions but all kinds of other postgresql functionality.

A few things:

  • Use the Operator class in VisitedExpression so that parenthesis are inserted correctly according to the lexical rules. I guess @@ falls under the "any other operator" precedence, so new Operator("@@", 10, 8).
  • I think Npgsql.EntityFramework6 would be a better namespace to put PgSqlTextFunctions in than Npgsql.Linq. I'd also rather chose the name NpgsqlTextFunctions.
  • It's kind of hacky to have the prototypes return a string and take strings as argument even though they really are of tsquery, tsvector types. But I see devart does this as well... EF leaves no good choices here. A question is if it's possible to use "object" as type instead?
  • I think you don't need DbFunctionStoreName. Use the second argument (name) of DbFunction instead as the name of the function in postgres.

@roji
Copy link
Member

roji commented Mar 24, 2016

Unfortunately no time on my side to give this a full review, but:

  • The diff shows the entire csproj being changed, probably a newline/whitespace issue, can you please fix?
  • When this PR is ready, you'll also need to prepare a PR for the develop branch (master is 3.0, develop is 3.1)

@roji
Copy link
Member

roji commented Mar 24, 2016

One more note - it would be great to have this kind of functionality in the EF7 provider as well.

@rwasef1830
Copy link
Contributor Author

Thanks for the guidance, about the csproj file, the only thing that changed is the addition of <Compile Include="Linq\PgSqlTextFunctions.cs" /> and <Compile Include="Linq\DbFunctionStoreNameAttribute.cs" /> in the other commits.

Any way to fix it without blowing away the changeset and redoing and/or messing up the current pull request ? Sorry, a bit of a noob here. This happened before in #1002 and I had to blow the branch and redo it.

@rwasef1830
Copy link
Contributor Author

Ah, I found out what's wrong, I'm using windows line endings whereas the project is using unix. Sorry, about that. Wish I could fix this without opening a new pull request.

@roji
Copy link
Member

roji commented Mar 24, 2016

@rwasef1830 you don't have to open a new pull request, just push-force your changes on the branch you already have (git push -f).

@rwasef1830
Copy link
Contributor Author

@Emill If both the EF7 and EF6 providers will have support for this, then PgSqlTextFunctions (or NpgsqlTextFunctions) cannot be in EntityFramework6.Npgsql.

Should I put in Npgsql itself then ? But it would get confusing if it is only used in the context of EF, and it would feel dirty to have a "common" project. What do you think ?

As for the parameter types, I tried to have the parameter and return types properly typed as NpgsqlTsVector and NpgsqlTsQuery, but I wasn't able to find a working EdmType incantation that would make EF accept it (always getting an exception that no function overload could match), so I had to resort to using string. I am not familiar with the visceral internals of EF, so I would love some suggestions here.

As for the DbFunctionStoreName, I think it is needed so that I don't have to name the static method itself the same name as the postgres name (trying to adhere to C# naming conventions), and to have the function definitions generated by reflection to reduce the effort needed to add new functions. It does feel hacky though...

@roji
Copy link
Member

roji commented Mar 29, 2016

Regarding PgSqlTextFunctions and EF6/EF7, I think the right way is to duplicate this in both the EF6 and EF7 projects. It definitely doesn't belong in Npgsql itself and there may even be variations (even if down the road) between what we can do in the EF versions.

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

Successfully merging this pull request may close these issues.

None yet

3 participants