Skip to content

Full text search follow up#375

Merged
roji merged 4 commits intonpgsql:devfrom
rwasef1830:full-text-search-follow-up
Apr 30, 2018
Merged

Full text search follow up#375
roji merged 4 commits intonpgsql:devfrom
rwasef1830:full-text-search-follow-up

Conversation

@rwasef1830
Copy link
Copy Markdown
Contributor

@roji This is a follow up to #315 that implements a few missing methods and operators for full text search and resolves issue #373:

  1. tsvector concat operator (Concat extension method on NpgsqlTsVector)
  2. ts_delete function (single lexeme and array of lexemes overload).
  3. ts_filter function
  4. array_to_tsvector function.

There is a caveat however, due to limitations in EF Core, all functions that take array parameters can only accept constant values (ie: the array must be computable outside the query). Attempts to use an array with elements that come from range variables from tables will result in NotSupportedException being thrown from deep within EF Core itself.

@roji
Copy link
Copy Markdown
Member

roji commented Apr 28, 2018

@rwasef1830 thanks, I'll review and merge ASAP. Regarding the array literal caveat, is it possible for you to check for non-constant values and to throw an informative message? This way users will at least know exactly what's going?

@rwasef1830
Copy link
Copy Markdown
Contributor Author

rwasef1830 commented Apr 28, 2018

@roji The exception thrown is NotSupportedException. The stack trace of one of them (in case of ArrayToTsVector) is:

   at Microsoft.EntityFrameworkCore.NpgsqlFullTextSearchDbFunctionsExtensions.ArrayToTsVector(DbFunctions _, String[] lexemes)
   at lambda_method(Closure , QueryContext , ValueBuffer )
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.ProjectionShaper.TypedProjectionShaper`3.Shape(QueryContext queryContext, ValueBuffer& valueBuffer)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryingEnumerable`1.Enumerator.BufferlessMoveNext(DbContext _, Boolean buffer)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.NpgsqlExecutionStrategy.Execute[TState,TResult](TState state, Func`3 operation, Func`3 verifySucceeded)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryingEnumerable`1.Enumerator.MoveNext()
   at System.Linq.Enumerable.First[TSource](IEnumerable`1 source)
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.ResultEnumerable`1.GetEnumerator()
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.ExceptionInterceptor`1.EnumeratorExceptionInterceptor.MoveNext()

It is coming outside of Npgsql code. EF Core decides to client evaluate ArrayToTsVector and then calls it directly which throws NotSupportedException. In order to fix this, would need to go through Linq expression visitors and handle the case only to throw an exception.

Last time I tried to do this I found my handle new array init expression visitor getting called for internal EF structures in other unrelated unit tests and I wasn't able to find a way to distinguish between array init expressions that EF Core expects to not get handled and array init expressions that do. It was messy and fragile so I went back on that idea.

@roji
Copy link
Copy Markdown
Member

roji commented Apr 28, 2018

@rwasef1830 ok, that does sound like too much trouble. I'll review a bit later and merge.

I don't recall anymore if there's an issue open on the EF Core repo for these array issues, so that they're at least tracked?

@roji roji merged commit 80d41f2 into npgsql:dev Apr 30, 2018
@rwasef1830 rwasef1830 deleted the full-text-search-follow-up branch April 30, 2018 11:23
austindrenski pushed a commit to austindrenski/npgsql-efcore.pg that referenced this pull request May 13, 2018
* tsvector concat operator support.
* ts_delete function support.
* ts_filter function support.
* array_to_tsvector function support (table columns in array are not supported).
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.

2 participants