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.

Sign up for GitHub

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

Jump to bottom

Add suggestions with fuzzy text search when no relationship is found #2583

Merged
merged 8 commits into from
Dec 15, 2022
Merged

Add suggestions with fuzzy text search when no relationship is found #2583

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d957415
Add simple suggestion when no relationship is found
laurenceisla Dec 10, 2022
d0e2ba7
Add doctests and clean code
laurenceisla Dec 12, 2022
51774f5
Add spectests
laurenceisla Dec 13, 2022
cce368e
Add changelog
laurenceisla Dec 14, 2022
e15459e
Add changelog for #2575
laurenceisla Dec 14, 2022
f6dbada
Use Schema instead of Text
laurenceisla Dec 14, 2022
f69f314
Clarify comment on function
laurenceisla Dec 14, 2022
cf1fc3e
Update CHANGELOG.md
steve-chavez Dec 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
### Fixed

- #2565, Fix bad M2M embedding on RPC - @steve-chavez

- #2575, Replace misleading error message when no function is found with a hint containing functions/parameters names suggestions - @laurenceisla
- #2569, Replace misleading error message when no relationship is found with a hint containing parent/child names suggestions - @laurenceisla
## [10.1.1] - 2022-11-08

### Fixed
Expand Down
5 changes: 3 additions & 2 deletions src/PostgREST/ApiRequest/Types.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ import PostgREST.MediaType (MediaType (..))
import PostgREST.SchemaCache.Identifiers (FieldName,
QualifiedIdentifier)
import PostgREST.SchemaCache.Proc (ProcDescription (..))
import PostgREST.SchemaCache.Relationship (Relationship)
import PostgREST.SchemaCache.Relationship (Relationship,
RelationshipsMap)

import Protolude

Expand Down Expand Up @@ -72,7 +73,7 @@ data ApiRequestError
| InvalidRpcMethod ByteString
| LimitNoOrderError
| NotFound
| NoRelBetween Text Text Text
| NoRelBetween Text Text (Maybe Text) Text RelationshipsMap
| NoRpc Text Text [Text] Bool MediaType Bool [QualifiedIdentifier] [ProcDescription]
| NotEmbedded Text
| ParseRequestError Text Text
Expand Down
59 changes: 54 additions & 5 deletions src/PostgREST/Error.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ module PostgREST.Error
import qualified Data.Aeson as JSON
import qualified Data.ByteString.Char8 as BS
import qualified Data.FuzzySet as Fuzzy
import qualified Data.HashMap.Strict as HM
import qualified Data.Text as T
import qualified Data.Text.Encoding as T
import qualified Data.Text.Encoding.Error as T
Expand All @@ -36,12 +37,14 @@ import PostgREST.ApiRequest.Types (ApiRequestError (..),
import PostgREST.MediaType (MediaType (..))
import qualified PostgREST.MediaType as MediaType

import PostgREST.SchemaCache.Identifiers (QualifiedIdentifier (..))
import PostgREST.SchemaCache.Identifiers (QualifiedIdentifier (..),
Schema)
import PostgREST.SchemaCache.Proc (ProcDescription (..),
ProcParam (..))
import PostgREST.SchemaCache.Relationship (Cardinality (..),
Junction (..),
Relationship (..))
Relationship (..),
RelationshipsMap)
import Protolude


Expand Down Expand Up @@ -174,11 +177,11 @@ instance JSON.ToJSON ApiRequestError where
"details" .= ("Only is null or not is null filters are allowed on embedded resources":: Text),
"hint" .= JSON.Null]

toJSON (NoRelBetween parent child schema) = JSON.object [
toJSON (NoRelBetween parent child embedHint schema allRels) = JSON.object [
"code" .= SchemaCacheErrorCode00,
"message" .= ("Could not find a relationship between '" <> parent <> "' and '" <> child <> "' in the schema cache" :: Text),
"details" .= JSON.Null,
"hint" .= ("Verify that '" <> parent <> "' and '" <> child <> "' exist in the schema '" <> schema <> "' and that there is a foreign key relationship between them. If a new relationship was created, try reloading the schema cache." :: Text)]
"details" .= ("Searched for a foreign key relationship between '" <> parent <> "' and '" <> child <> maybe mempty ("' using the hint '" <>) embedHint <> "' in the schema '" <> schema <> "', but no matches were found."),
"hint" .= noRelBetweenHint parent child schema allRels]

toJSON (AmbiguousRelBetween parent child rels) = JSON.object [
"code" .= SchemaCacheErrorCode01,
Expand Down Expand Up @@ -214,6 +217,52 @@ instance JSON.ToJSON ApiRequestError where
"details" .= JSON.Null,
"hint" .= ("Try renaming the parameters or the function itself in the database so function overloading can be resolved" :: Text)]

-- |
-- If no relationship is found then:
--
-- Looks for parent suggestions if parent not found
-- Looks for child suggestions if parent is found but child is not
-- Gives no suggestions if both are found (it means that there is a problem with the embed hint)
--
-- >>> :set -Wno-missing-fields
-- >>> let qi t = QualifiedIdentifier "api" t
-- >>> let rel ft = Relationship{relForeignTable = qi ft}
-- >>> let rels = HM.fromList [((qi "films", "api"), [rel "directors", rel "roles", rel "actors"])]
--
-- >>> noRelBetweenHint "film" "directors" "api" rels
-- Just "Perhaps you meant 'films' instead of 'film'."
--
-- >>> noRelBetweenHint "films" "role" "api" rels
-- Just "Perhaps you meant 'roles' instead of 'role'."
--
-- >>> noRelBetweenHint "films" "role" "api" rels
-- Just "Perhaps you meant 'roles' instead of 'role'."
--
-- >>> noRelBetweenHint "films" "actors" "api" rels
-- Nothing
--
-- >>> noRelBetweenHint "noclosealternative" "roles" "api" rels
-- Nothing
--
-- >>> noRelBetweenHint "films" "noclosealternative" "api" rels
-- Nothing
--
-- >>> noRelBetweenHint "films" "noclosealternative" "noclosealternative" rels
-- Nothing
--
noRelBetweenHint :: Text -> Text -> Schema -> RelationshipsMap -> Maybe Text
noRelBetweenHint parent child schema allRels = ("Perhaps you meant '" <>) <$>
if isJust findParent
then (<> "' instead of '" <> child <> "'.") <$> suggestChild
else (<> "' instead of '" <> parent <> "'.") <$> suggestParent
where
findParent = HM.lookup (QualifiedIdentifier schema parent, schema) allRels
fuzzySetOfParents = Fuzzy.fromList [qiName (fst p) | p <- HM.keys allRels, snd p == schema]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this is expensive if allRels is big, in which case perhaps the hint could be disabled by looking at the map length?

Maybe this could be tested with the apflora schema, by doing time curl .. on a request with bad names.

Copy link
Member Author

@laurenceisla laurenceisla Dec 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In apflora, allRels has a size of 305. These are the times that I get:

  1. Relationship is found (no fuzzy search):

    $ curl -w "%{time_total}s" -s "http://localhost:3000/ziel?select=ap(*)"

    Response:

    []

    Times:

    0.002485s
0.002125s

  2. Relationship not found (fuzzy search for parent)

    $ curl -w "%{time_total}s" -s "http://localhost:3000/ziels?select=ap(*)"

    Response:

    {
    "code": "PGRST200",
    "details": "Searched for a foreign key relationship between 'ziels' and 'ap' in the schema 'apflora', but no matches were found.",
    "hint": "Perhaps you meant 'ziel' instead of 'ziels'.",
    "message": "Could not find a relationship between 'ziels' and 'ap' in the schema cache"
}

    Times:

    0.072776s
0.056077s

  3. Relationship not found (fuzzy search for child)

    $ curl -w "%{time_total}s" -s "http://localhost:3000/ziel?select=app(*)"

    Response:

    {
    "code": "PGRST200",
    "details": "Searched for a foreign key relationship between 'ziel' and 'app' in the schema 'apflora', but no matches were found.",
    "hint": "Perhaps you meant 'ap' instead of 'app'.",
    "message": "Could not find a relationship between 'ziel' and 'app' in the schema cache"
}

    Times:

    0.035553s
0.027014s

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(fuzzy search for parent)
0.072776s
0.056077s
(fuzzy search for child)
0.035553s
0.027014s

So parent takes longer than child because it searches on the full allRels, while child only searches on the relationships that have parent. Makes sense.

fuzzySetOfChildren = Fuzzy.fromList [qiName (relForeignTable c) | c <- fromMaybe [] findParent]
suggestParent = Fuzzy.getOne fuzzySetOfParents parent
-- Do not give suggestion if the child is found in the relations (weight = 1.0)
suggestChild = headMay [snd k | k <- Fuzzy.get fuzzySetOfChildren child, fst k < 1.0]

-- |
-- If no function is found with the given name, it does a fuzzy search to all the functions
-- in the same schema and shows the best match as hint.
Expand Down
2 changes: 1 addition & 1 deletion src/PostgREST/Plan.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ getJoinConditions tblAlias parentAlias Relationship{relTable=qi,relForeignTable=
findRel :: Schema -> RelationshipsMap -> NodeName -> NodeName -> Maybe Hint -> Either ApiRequestError Relationship
findRel schema allRels origin target hint =
case rels of
[] -> Left $ NoRelBetween origin target schema
[] -> Left $ NoRelBetween origin target hint schema allRels
[r] -> Right r
rs -> Left $ AmbiguousRelBetween origin target rs
where
Expand Down
12 changes: 6 additions & 6 deletions test/spec/Feature/Query/EmbedDisambiguationSpec.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,10 @@ spec =
it "fails if the fk is not known" $
get "/message?select=id,sender:person!space(name)&id=lt.4" `shouldRespondWith`
[json|{
"hint":"Verify that 'message' and 'person' exist in the schema 'test' and that there is a foreign key relationship between them. If a new relationship was created, try reloading the schema cache.",
"hint":null,
"message":"Could not find a relationship between 'message' and 'person' in the schema cache",
"code": "PGRST200",
"details": null}|]
"details":"Searched for a foreign key relationship between 'message' and 'person' using the hint 'space' in the schema 'test', but no matches were found."}|]
{ matchStatus = 400
, matchHeaders = [matchContentTypeJson] }

Expand Down Expand Up @@ -507,21 +507,21 @@ spec =
it "doesn't work if the junction is only internal" $
get "/end_1?select=end_2(*)" `shouldRespondWith`
[json|{
"hint":"Verify that 'end_1' and 'end_2' exist in the schema 'test' and that there is a foreign key relationship between them. If a new relationship was created, try reloading the schema cache.",
"hint": null,
"message":"Could not find a relationship between 'end_1' and 'end_2' in the schema cache",
"code":"PGRST200",
"details": null}|]
"details": "Searched for a foreign key relationship between 'end_1' and 'end_2' in the schema 'test', but no matches were found."}|]
{ matchStatus = 400
, matchHeaders = [matchContentTypeJson] }
it "shouldn't try to embed if the private junction has an exposed homonym" $
-- ensures the "invalid reference to FROM-clause entry for table "rollen" error doesn't happen.
-- Ref: https://github.com/PostgREST/postgrest/issues/1587#issuecomment-734995669
get "/schauspieler?select=filme(*)" `shouldRespondWith`
[json|{
"hint":"Verify that 'schauspieler' and 'filme' exist in the schema 'test' and that there is a foreign key relationship between them. If a new relationship was created, try reloading the schema cache.",
"hint":null,
"message":"Could not find a relationship between 'schauspieler' and 'filme' in the schema cache",
"code":"PGRST200",
"details": null}|]
"details":"Searched for a foreign key relationship between 'schauspieler' and 'filme' in the schema 'test', but no matches were found."}|]
{ matchStatus = 400
, matchHeaders = [matchContentTypeJson] }

Expand Down
16 changes: 8 additions & 8 deletions test/spec/Feature/Query/QuerySpec.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,8 +590,8 @@ spec actualPgVersion = do
it "cannot request partitions as children from a partitioned table" $
get "/car_models?id=in.(1,2,4)&select=id,name,car_model_sales_202101(id)&order=id.asc" `shouldRespondWith`
[json|
{"hint":"Verify that 'car_models' and 'car_model_sales_202101' exist in the schema 'test' and that there is a foreign key relationship between them. If a new relationship was created, try reloading the schema cache.",
"details":null,
{"hint":"Perhaps you meant 'car_model_sales' instead of 'car_model_sales_202101'.",
"details":"Searched for a foreign key relationship between 'car_models' and 'car_model_sales_202101' in the schema 'test', but no matches were found.",
"code":"PGRST200",
"message":"Could not find a relationship between 'car_models' and 'car_model_sales_202101' in the schema cache"} |]
{ matchStatus = 400
Expand All @@ -601,8 +601,8 @@ spec actualPgVersion = do
it "cannot request a partitioned table as parent from a partition" $
get "/car_model_sales_202101?select=id,name,car_models(id,name)&order=id.asc" `shouldRespondWith`
[json|
{"hint":"Verify that 'car_model_sales_202101' and 'car_models' exist in the schema 'test' and that there is a foreign key relationship between them. If a new relationship was created, try reloading the schema cache.",
"details":null,
{"hint":"Perhaps you meant 'car_model_sales' instead of 'car_model_sales_202101'.",
"details":"Searched for a foreign key relationship between 'car_model_sales_202101' and 'car_models' in the schema 'test', but no matches were found.",
"code":"PGRST200",
"message":"Could not find a relationship between 'car_model_sales_202101' and 'car_models' in the schema cache"} |]
{ matchStatus = 400
Expand All @@ -612,8 +612,8 @@ spec actualPgVersion = do
it "cannot request a partition as parent from a partitioned table" $
get "/car_model_sales?id=in.(1,3,4)&select=id,name,car_models_default(id,name)&order=id.asc" `shouldRespondWith`
[json|
{"hint":"Verify that 'car_model_sales' and 'car_models_default' exist in the schema 'test' and that there is a foreign key relationship between them. If a new relationship was created, try reloading the schema cache.",
"details":null,
{"hint":"Perhaps you meant 'car_models' instead of 'car_models_default'.",
"details":"Searched for a foreign key relationship between 'car_model_sales' and 'car_models_default' in the schema 'test', but no matches were found.",
"code":"PGRST200",
"message":"Could not find a relationship between 'car_model_sales' and 'car_models_default' in the schema cache"} |]
{ matchStatus = 400
Expand All @@ -623,8 +623,8 @@ spec actualPgVersion = do
it "cannot request partitioned tables as children from a partition" $
get "/car_models_default?select=id,name,car_model_sales(id,name)&order=id.asc" `shouldRespondWith`
[json|
{"hint":"Verify that 'car_models_default' and 'car_model_sales' exist in the schema 'test' and that there is a foreign key relationship between them. If a new relationship was created, try reloading the schema cache.",
"details":null,
{"hint":"Perhaps you meant 'car_model_sales' instead of 'car_models_default'.",
"details":"Searched for a foreign key relationship between 'car_models_default' and 'car_model_sales' in the schema 'test', but no matches were found.",
"code":"PGRST200",
"message":"Could not find a relationship between 'car_models_default' and 'car_model_sales' in the schema cache"} |]
{ matchStatus = 400
Expand Down