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

perf: fix space leaks by downgrading fuzzyset to v0.2.4 #3339

Merged
merged 2 commits into from
Apr 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 9 additions & 2 deletions nix/overlays/haskell-packages.nix
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,15 @@ let
}
{ };

# Marked as broken (?)
fuzzyset = lib.markUnbroken prev.fuzzyset;
# Before upgrading fuzzyset to 0.3, check: https://github.com/PostgREST/postgrest/issues/3329
fuzzyset =
prev.callHackageDirect
{
pkg = "fuzzyset";
ver = "0.2.4";
sha256 = "sha256-lpkrTFcR0B4rT/P6x7ui31Twgq7BBj6KIvjKyqXKdpc=";
}
{ };

postgresql-libpq = lib.dontCheck
(prev.postgresql-libpq_0_10_0_0.override {
Expand Down
2 changes: 1 addition & 1 deletion postgrest.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ library
, directory >= 1.2.6 && < 1.4
, either >= 4.4.1 && < 5.1
, extra >= 1.7.0 && < 2.0
, fuzzyset >= 0.3.0
, fuzzyset >= 0.2.4 && < 0.3
, gitrev >= 1.2 && < 1.4
, hasql >= 1.6.1.1 && < 1.7
, hasql-dynamic-statements >= 0.3.1 && < 0.4
Expand Down
10 changes: 5 additions & 5 deletions src/PostgREST/Error.hs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ module PostgREST.Error
import qualified Data.Aeson as JSON
import qualified Data.ByteString.Char8 as BS
import qualified Data.CaseInsensitive as CI
import qualified Data.FuzzySet.Simple as Fuzzy
import qualified Data.FuzzySet as Fuzzy
import qualified Data.HashMap.Strict as HM
import qualified Data.Map.Internal as M
import qualified Data.Text as T
Expand Down Expand Up @@ -293,9 +293,9 @@ noRelBetweenHint parent child schema allRels = ("Perhaps you meant '" <>) <$>
findParent = HM.lookup (QualifiedIdentifier schema parent, schema) allRels
fuzzySetOfParents = Fuzzy.fromList [qiName (fst p) | p <- HM.keys allRels, snd p == schema]
fuzzySetOfChildren = Fuzzy.fromList [qiName (relForeignTable c) | c <- fromMaybe [] findParent]
suggestParent = snd <$> Fuzzy.findOne parent fuzzySetOfParents
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.find child fuzzySetOfChildren, fst k < 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
Expand Down Expand Up @@ -345,8 +345,8 @@ noRpcHint schema procName params allProcs overloadedProcs =
-- E.g. ["val", "param", "name"] into "(name, param, val)"
listToText = ("(" <>) . (<> ")") . T.intercalate ", " . sort
possibleProcs
| null overloadedProcs = snd <$> Fuzzy.findOne procName fuzzySetOfProcs
| otherwise = (procName <>) . snd <$> Fuzzy.findOne (listToText params) fuzzySetOfParams
| null overloadedProcs = Fuzzy.getOne fuzzySetOfProcs procName
| otherwise = (procName <>) <$> Fuzzy.getOne fuzzySetOfParams (listToText params)

compressedRel :: Relationship -> JSON.Value
-- An ambiguousness error cannot happen for computed relationships TODO refactor so this mempty is not needed
Expand Down
2 changes: 1 addition & 1 deletion stack.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ nix:

extra-deps:
- configurator-pg-0.2.7
- fuzzyset-0.3.1
- fuzzyset-0.2.4
- hasql-notifications-0.2.1.0
- hasql-pool-0.10
- megaparsec-9.2.2
Expand Down
8 changes: 4 additions & 4 deletions stack.yaml.lock
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ packages:
original:
hackage: configurator-pg-0.2.7
- completed:
hackage: fuzzyset-0.3.1@sha256:344e16deedb43da5cabae558450e5710843cff7ac2f3073be9db453c6f3a3fb7,2373
hackage: fuzzyset-0.2.4@sha256:f1b6de8bf33277bf6255207541d65028f1f1ea93af5541b654c86b5674995485,1618
pantry-tree:
sha256: 89e22c2ce70a7c7c4b599ffe2546cda5699e4f8f15410eac49f39af8c4c381c8
size: 643
sha256: cee68e8d88f530e9e0588b81b260236936fe3318ef9a66e9f43f680b4cd5f76e
size: 574
original:
hackage: fuzzyset-0.3.1
hackage: fuzzyset-0.2.4
- completed:
hackage: hasql-notifications-0.2.1.0@sha256:c9c0ba3ac0866142836d2d0a08a0d10a945bcbe1c163ee1ef6aed407d046a24e,2022
pantry-tree:
Expand Down
18 changes: 18 additions & 0 deletions test/io/test_big_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,21 @@ def test_openapi_in_big_schema(defaultenv):
with run(env=env) as postgrest:
response = postgrest.session.get("/")
assert response.status_code == 200


# See: https://github.com/PostgREST/postgrest/issues/3329
def test_should_not_fail_with_stack_overflow(defaultenv):
Copy link
Member Author

Choose a reason for hiding this comment

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

This test fails on the main branch with "stack overflow".

"requesting a non-existent relationship should not fail with stack overflow due to fuzzy search of candidates"

env = {
**defaultenv,
"PGRST_DB_SCHEMAS": "apflora",
"PGRST_DB_POOL": "2",
"PGRST_DB_ANON_ROLE": "postgrest_test_anonymous",
}

with run(env=env, wait_max_seconds=30) as postgrest:
response = postgrest.session.get("/unknown-table?select=unknown-rel(*)")
assert response.status_code == 400
data = response.json()
assert data["code"] == "PGRST200"
Loading