From a2892ab1dde3405467704dda2ecce28013a90268 Mon Sep 17 00:00:00 2001 From: Laurence Isla Date: Thu, 12 Jun 2025 19:15:32 -0500 Subject: [PATCH] fix: regression that makes fts not work on domain types based on tsvector --- CHANGELOG.md | 1 + src/PostgREST/Plan.hs | 16 ++++++++-------- src/PostgREST/Plan/Types.hs | 3 ++- src/PostgREST/Query/QueryBuilder.hs | 2 +- test/spec/Feature/Query/QuerySpec.hs | 14 ++++++++++++++ test/spec/Feature/Query/RpcSpec.hs | 16 ++++++++++++++++ test/spec/fixtures/data.sql | 7 ++++--- test/spec/fixtures/schema.sql | 10 +++++++++- 8 files changed, 55 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f4e904bd..adbc01ffa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - Fix `max-affected` preference not failing with RPC when `handling=strict` by @taimoorzaeem in #4100 - Fix a property definition's type in OpenAPI not showing the correct base type of a recursive domain by @laurenceisla in #4136 +- Fix regression that makes full-text search not work on domain types based on `tsvector` by @laurenceisla in #4135 ## [13.0.2] - 2025-06-02 diff --git a/src/PostgREST/Plan.hs b/src/PostgREST/Plan.hs index a5cb25192..f808476d5 100644 --- a/src/PostgREST/Plan.hs +++ b/src/PostgREST/Plan.hs @@ -278,7 +278,7 @@ data ResolverContext = ResolverContext } resolveColumnField :: Column -> Maybe ToTsVector -> CoercibleField -resolveColumnField col toTsV = CoercibleField (colName col) mempty False toTsV (colNominalType col) Nothing (colDefault col) False +resolveColumnField col toTsV = CoercibleField (colName col) mempty False toTsV (colNominalType col) (colType col) Nothing (colDefault col) False resolveTableFieldName :: Table -> FieldName -> Maybe ToTsVector -> CoercibleField resolveTableFieldName table fieldName toTsV= @@ -291,12 +291,12 @@ resolveTypeOrUnknown ResolverContext{..} (fn, jp) toTsV = case res of -- types that are already json/jsonb don't need to be converted with `to_jsonb` for using arrow operators `data->attr` -- this prevents indexes not applying https://github.com/PostgREST/postgrest/issues/2594 - cf@CoercibleField{cfIRType="json"} -> cf{cfJsonPath=jp, cfToJson=False} - cf@CoercibleField{cfIRType="jsonb"} -> cf{cfJsonPath=jp, cfToJson=False} + cf@CoercibleField{cfIRType="json"} -> cf{cfJsonPath=jp, cfToJson=False} + cf@CoercibleField{cfIRType="jsonb"} -> cf{cfJsonPath=jp, cfToJson=False} -- Do not apply to_tsvector to tsvector types - cf@CoercibleField{cfIRType="tsvector"} -> cf{cfJsonPath=jp, cfToJson=True, cfToTsVector=Nothing} + cf@CoercibleField{cfBaseType="tsvector"} -> cf{cfJsonPath=jp, cfToJson=True, cfToTsVector=Nothing} -- other types will get converted `to_jsonb(col)->attr`, even unknown types - cf -> cf{cfJsonPath=jp, cfToJson=True} + cf -> cf{cfJsonPath=jp, cfToJson=True} where res = fromMaybe (unknownField fn jp) $ HM.lookup qi tables >>= Just . (\t -> resolveTableFieldName t fn toTsV) @@ -891,7 +891,7 @@ addRelatedOrders (Node rp@ReadPlan{order,from} forest) = do -- where_ = [ -- CoercibleStmnt ( -- CoercibleFilter { --- field = CoercibleField {cfName = "projects", cfJsonPath = [], cfToJson=False, cfToTsVector = Nothing, cfIRType = "", cfTransform = Nothing, cfDefault = Nothing, cfFullRow = False}, +-- field = CoercibleField {cfName = "projects", cfJsonPath = [], cfToJson=False, cfToTsVector = Nothing, cfIRType = "", cfBaseType = "", cfTransform = Nothing, cfDefault = Nothing, cfFullRow = False}, -- opExpr = op -- } -- ) @@ -907,7 +907,7 @@ addRelatedOrders (Node rp@ReadPlan{order,from} forest) = do -- Don't do anything to the filter if there's no embedding (a subtree) on projects. Assume it's a normal filter. -- -- >>> ReadPlan.where_ . rootLabel <$> addNullEmbedFilters (readPlanTree nullOp []) --- Right [CoercibleStmnt (CoercibleFilter {field = CoercibleField {cfName = "projects", cfJsonPath = [], cfToJson = False, cfToTsVector = Nothing, cfIRType = "", cfTransform = Nothing, cfDefault = Nothing, cfFullRow = False}, opExpr = OpExpr True (Is IsNull)})] +-- Right [CoercibleStmnt (CoercibleFilter {field = CoercibleField {cfName = "projects", cfJsonPath = [], cfToJson = False, cfToTsVector = Nothing, cfIRType = "", cfBaseType = "", cfTransform = Nothing, cfDefault = Nothing, cfFullRow = False}, opExpr = OpExpr True (Is IsNull)})] -- -- If there's an embedding on projects, then change the filter to use the internal aggregate name (`clients_projects_1`) so the filter can succeed later. -- @@ -926,7 +926,7 @@ addNullEmbedFilters (Node rp@ReadPlan{where_=curLogic} forest) = do newNullFilters rPlans = \case (CoercibleExpr b lOp trees) -> CoercibleExpr b lOp <$> (newNullFilters rPlans `traverse` trees) - flt@(CoercibleStmnt (CoercibleFilter (CoercibleField fld [] _ _ _ _ _ _) opExpr)) -> + flt@(CoercibleStmnt (CoercibleFilter CoercibleField{cfName=fld, cfJsonPath=[]} opExpr)) -> let foundRP = find (\ReadPlan{relName, relAlias} -> fld == fromMaybe relName relAlias) rPlans in case (foundRP, opExpr) of (Just ReadPlan{relAggAlias}, OpExpr b (Is IsNull)) -> Right $ CoercibleStmnt $ CoercibleFilterNullEmbed b relAggAlias diff --git a/src/PostgREST/Plan/Types.hs b/src/PostgREST/Plan/Types.hs index 59bf52e36..92eedf37b 100644 --- a/src/PostgREST/Plan/Types.hs +++ b/src/PostgREST/Plan/Types.hs @@ -44,13 +44,14 @@ data CoercibleField = CoercibleField , cfToJson :: Bool , cfToTsVector :: Maybe ToTsVector -- ^ If the field should be converted using to_tsvector(, ) , cfIRType :: Text -- ^ The native Postgres type of the field, the intermediate (IR) type before mapping. + , cfBaseType :: Text -- ^ The base type of the field in case of domains, or just the type otherwise (without modifiers in case of pg_catalog types) , cfTransform :: Maybe TransformerProc -- ^ The optional mapping from irType -> targetType. , cfDefault :: Maybe Text , cfFullRow :: Bool -- ^ True if the field represents the whole selected row. Used in spread rels: instead of COUNT(*), it does a COUNT() in order to not mix with other spreaded resources. } deriving (Eq, Show) unknownField :: FieldName -> JsonPath -> CoercibleField -unknownField name path = CoercibleField name path False Nothing "" Nothing Nothing False +unknownField name path = CoercibleField name path False Nothing "" "" Nothing Nothing False -- | Like an API request LogicTree, but with coercible field information. data CoercibleLogicTree diff --git a/src/PostgREST/Query/QueryBuilder.hs b/src/PostgREST/Query/QueryBuilder.hs index 2e07cc321..5a65f56a5 100644 --- a/src/PostgREST/Query/QueryBuilder.hs +++ b/src/PostgREST/Query/QueryBuilder.hs @@ -182,7 +182,7 @@ callPlanToQuery (FunctionCall qi params arguments returnsScalar returnsSetOfScal KeyParams [] -> "FROM " <> callIt mempty KeyParams prms -> case arguments of DirectArgs args -> "FROM " <> callIt (fmtArgs prms args) - JsonArgs json -> fromJsonBodyF json ((\p -> CoercibleField (ppName p) mempty False Nothing (ppTypeMaxLength p) Nothing Nothing False) <$> prms) False True False <> ", " <> + JsonArgs json -> fromJsonBodyF json ((\p -> CoercibleField (ppName p) mempty False Nothing (ppTypeMaxLength p) mempty Nothing Nothing False) <$> prms) False True False <> ", " <> "LATERAL " <> callIt (fmtParams prms) callIt :: SQL.Snippet -> SQL.Snippet diff --git a/test/spec/Feature/Query/QuerySpec.hs b/test/spec/Feature/Query/QuerySpec.hs index 43cb46723..ca51eaca3 100644 --- a/test/spec/Feature/Query/QuerySpec.hs +++ b/test/spec/Feature/Query/QuerySpec.hs @@ -294,6 +294,20 @@ spec = do ]|] { matchHeaders = [matchContentTypeJson] } + it "works when the column type is a tsvector domain" $ do + get "tsearch_to_tsvector?select=text_search_domain&text_search_domain=fts(simple).of" `shouldRespondWith` + [json| [ + {"text_search_domain":"'do':7 'fun':5 'impossible':9 'it':1 'kind':3 'of':4 's':2 'the':8 'to':6"} + ]|] + { matchHeaders = [matchContentTypeJson] } + + it "works when the column type is a recursive tsvector domain" $ do + get "tsearch_to_tsvector?select=text_search_rec_domain&text_search_rec_domain=fts(simple).of" `shouldRespondWith` + [json| [ + {"text_search_rec_domain":"'do':7 'fun':5 'impossible':9 'it':1 'kind':3 'of':4 's':2 'the':8 'to':6"} + ]|] + { matchHeaders = [matchContentTypeJson] } + context "text and json columns" $ do it "finds matches with to_tsquery" $ do get "/tsearch_to_tsvector?select=text_search&text_search=fts.impossible" `shouldRespondWith` diff --git a/test/spec/Feature/Query/RpcSpec.hs b/test/spec/Feature/Query/RpcSpec.hs index b78c2de9e..c3745e0f2 100644 --- a/test/spec/Feature/Query/RpcSpec.hs +++ b/test/spec/Feature/Query/RpcSpec.hs @@ -998,6 +998,22 @@ spec = |] { matchHeaders = [matchContentTypeJson] } + it "should work with filters that use the fts operator when the column type is a tsvector domain" $ + get "/rpc/get_tsearch_to_tsvector?select=text_search_domain&text_search_domain=fts(simple).impossible" `shouldRespondWith` + [json|[ + {"text_search_domain":"'do':7 'fun':5 'impossible':9 'it':1 'kind':3 'of':4 's':2 'the':8 'to':6"}, + {"text_search_domain":"'amusant':5 'c':1 'de':6 'est':2 'faire':7 'impossible':9 'l':8 'peu':4 'un':3"}] + |] + { matchHeaders = [matchContentTypeJson] } + + it "should work with filters that use the fts operator when the column type is a recursive tsvector domain" $ + get "/rpc/get_tsearch_to_tsvector?select=text_search_rec_domain&text_search_rec_domain=fts(simple).impossible" `shouldRespondWith` + [json|[ + {"text_search_rec_domain":"'do':7 'fun':5 'impossible':9 'it':1 'kind':3 'of':4 's':2 'the':8 'to':6"}, + {"text_search_rec_domain":"'amusant':5 'c':1 'de':6 'est':2 'faire':7 'impossible':9 'l':8 'peu':4 'un':3"}] + |] + { matchHeaders = [matchContentTypeJson] } + it "should work with the phraseto_tsquery function" $ get "/rpc/get_tsearch?text_search_vector=phfts(english).impossible" `shouldRespondWith` [json|[{"text_search_vector":"'fun':5 'imposs':9 'kind':3"}]|] diff --git a/test/spec/fixtures/data.sql b/test/spec/fixtures/data.sql index 624040210..b0304ae01 100644 --- a/test/spec/fixtures/data.sql +++ b/test/spec/fixtures/data.sql @@ -957,15 +957,16 @@ INSERT INTO tsearch_to_tsvector(text_search) VALUES ('C''est un peu amusant de f INSERT INTO tsearch_to_tsvector(text_search) VALUES ('Es ist eine Art Spaß, das Unmögliche zu machen'); UPDATE tsearch_to_tsvector SET jsonb_search = jsonb_build_object('text_search', text_search); - +UPDATE tsearch_to_tsvector SET text_search_domain = to_tsvector('simple', text_search); +UPDATE tsearch_to_tsvector SET text_search_rec_domain = to_tsvector('simple', text_search); TRUNCATE TABLE artists CASCADE; INSERT INTO artists VALUES (1, 'duster'), (2, 'black country, new road'), (3, 'bjork'); TRUNCATE TABLE albums CASCADE; -INSERT INTO albums +INSERT INTO albums VALUES (1, 'stratosphere', 1), - (2, 'ants from up above',2), + (2, 'ants from up above',2), (3, 'vespertine',3), (4, 'contemporary movement', 1); diff --git a/test/spec/fixtures/schema.sql b/test/spec/fixtures/schema.sql index 770c1e5d1..e9e1e70ac 100644 --- a/test/spec/fixtures/schema.sql +++ b/test/spec/fixtures/schema.sql @@ -3757,9 +3757,17 @@ create table "Surr_Gen_Default_Upsert" ( extra text ); +create domain tsvector_not_null as tsvector + constraint "tsvector is required" check (value is not null); + +create domain tsvector_not_empty as tsvector_not_null + constraint "tsvector is required and not empty" check (value <> ''); + create table tsearch_to_tsvector ( text_search text, - jsonb_search jsonb + jsonb_search jsonb, + text_search_domain tsvector_not_null default '', + text_search_rec_domain tsvector_not_empty default '.' ); create function test.get_tsearch_to_tsvector() returns setof test.tsearch_to_tsvector AS $$