Skip to content

Commit

Permalink
Fix a query optimisation regression
Browse files Browse the repository at this point in the history
Summary:
The recent changes exposed a regression with this query

```
0 where
  X1 = codemarkup.hack.HackFileEntityXRefLocations.2
    {file="www/RefClass.php"};
  codemarkup.hack.HackFileEntityXRefLocations.2
    {entity={decl={method=hack.MethodDeclaration.6 {}}}}=X1;
  X0=X1
```

This turns out to be pretty tricky to optimise. I'm adding yet more
flexibility to the final reordering pass that lets it pull out
lookups, which solves the problem in this case.

Reviewed By: iamirzhan

Differential Revision: D65059643

fbshipit-source-id: 879a869ea6d620b28c53454b084917fd0d8bfaa9
  • Loading branch information
Simon Marlow authored and facebook-github-bot committed Oct 28, 2024
1 parent 1e9fb02 commit b69cafd
Showing 1 changed file with 9 additions and 0 deletions.
9 changes: 9 additions & 0 deletions glean/db/Glean/Query/Reorder.hs
Original file line number Diff line number Diff line change
Expand Up @@ -586,13 +586,17 @@ reorderStmts stmts = iterate stmts []
choose _ [one] = ("only", (one, []))
choose scope stmts = fromMaybe (error "choose") $
firstResolved <|>
firstLookup <|>
firstNotUnresolved <|>
firstNotUnresolvedLenient <|>
fallback
where
firstResolved = ("resolved",) <$>
find (isResolvedFilter (ifBoundOnly scope) . fst) stmts'

firstLookup = ("lookup",) <$>
find (isLookup (ifBoundOnly scope) . fst) stmts'

-- try to find a statement that's already resolved without
-- adding any missing generators, and then try again allowing
-- unbound predicates to be resolved.
Expand Down Expand Up @@ -665,6 +669,11 @@ isReadyFilter scope stmt notFilter = case stmt of
appearInStmts
_ -> notFilter

isLookup :: InScope -> FlatStatement -> Bool
isLookup scope (FlatStatement _ lhs FactGenerator{})
| Just (Var _ v _) <- isVar lhs, inScopeBound scope v = True
isLookup _ _ = False

data InScope = InScope
{ unboundPredicates :: Bool
, isInScope :: Variable -> Bool
Expand Down

0 comments on commit b69cafd

Please sign in to comment.