jimmy/postgrest
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: use index on jsonb/jsonb arrow filter/order

Browse Source
This commit is contained in:
steve-chavez
2023-07-12 14:51:21 -05:00
committed by Steve Chavez
parent e332f038ef
commit 28d5278d62
10 changed files with 114 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CHANGELOG.md
+1
View File
@@ -29,6 +29,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
- #2834, Fix compilation on Ubuntu by being compatible with GHC 9.0.2 - @steve-chavez
- #2840, Fix `Prefer: missing=default` with DOMAIN default values - @steve-chavez
- #2849, Fix HEAD unnecessarily executing aggregates - @steve-chavez
- #2594, Fix unused index on jsonb/jsonb arrow filter and order (``/bets?data->>contractId=eq.1`` and ``/bets?order=data->>contractId``) - @steve-chavez
## [11.1.0] - 2023-06-07
src/PostgREST/Plan.hs
+25 -19
View File
@@ -219,7 +219,7 @@ data ResolverContext = ResolverContext
}
resolveColumnField :: Column -> CoercibleField
resolveColumnField col = CoercibleField (colName col) mempty (colNominalType col) Nothing (colDefault col)
resolveColumnField col = CoercibleField (colName col) mempty False (colNominalType col) Nothing (colDefault col)
resolveTableFieldName :: Table -> FieldName -> CoercibleField
resolveTableFieldName table fieldName =
@@ -228,11 +228,14 @@ resolveTableFieldName table fieldName =
resolveTableField :: Table -> Field -> CoercibleField
resolveTableField table (fieldName, []) = resolveTableFieldName table fieldName
-- If the field is known and a JSON path is given, always assume the JSON type. But don't assume a type for entirely unknown fields.
resolveTableField table (fieldName, jp) =
case resolveTableFieldName table fieldName of
cf@CoercibleField{cfIRType=""} -> cf{cfJsonPath=jp}
cf -> cf{cfJsonPath=jp, cfIRType="json"}
-- 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}
cf@CoercibleField{cfIRType="jsonb"} -> cf{cfJsonPath=jp}
-- other types will get converted `to_jsonb(col)->attr`
cf -> cf{cfJsonPath=jp, cfToJson=True}
-- | Resolve a type within the context based on the given field name and JSON path. Although there are situations where failure to resolve a field is considered an error (see `resolveOrError`), there are also situations where we allow it (RPC calls). If it should be an error and `resolveOrError` doesn't fit, ensure to check the `cfIRType` isn't empty.
resolveTypeOrUnknown :: ResolverContext -> Field -> CoercibleField
@@ -289,7 +292,7 @@ readPlan qi@QualifiedIdentifier{..} AppConfig{configDbMaxRows} SchemaCache{dbTab
addRels qiSchema (iAction apiRequest) dbRelationships Nothing =<<
addLogicTrees ctx apiRequest =<<
addRanges apiRequest =<<
addOrders apiRequest =<<
addOrders ctx apiRequest =<<
addFilters ctx apiRequest (initReadRequest ctx $ QueryParams.qsSelect $ iQueryParams apiRequest)
-- Build the initial read plan tree
@@ -526,8 +529,8 @@ addFilters ctx ApiRequest{..} rReq =
addFilterToNode =
updateNode (\flt (Node q@ReadPlan{from=fromTable, where_=lf} f) -> Node q{ReadPlan.where_=addFilterToLogicForest (resolveFilter ctx{qi=fromTable} flt) lf} f)
addOrders :: ApiRequest -> ReadPlanTree -> Either ApiRequestError ReadPlanTree
addOrders ApiRequest{..} rReq =
addOrders :: ResolverContext -> ApiRequest -> ReadPlanTree -> Either ApiRequestError ReadPlanTree
addOrders ctx ApiRequest{..} rReq =
case iAction of
ActionMutate _ -> Right rReq
_ -> foldr addOrderToNode (Right rReq) qsOrder
@@ -535,29 +538,32 @@ addOrders ApiRequest{..} rReq =
QueryParams.QueryParams{..} = iQueryParams
addOrderToNode :: (EmbedPath, [OrderTerm]) -> Either ApiRequestError ReadPlanTree -> Either ApiRequestError ReadPlanTree
addOrderToNode = updateNode (\o (Node q f) -> Node q{order=o} f)
addOrderToNode = updateNode (\o (Node q f) -> Node q{order=resolveOrder ctx <$> o} f)
resolveOrder :: ResolverContext -> OrderTerm -> CoercibleOrderTerm
resolveOrder _ (OrderRelationTerm a b c d) = CoercibleOrderRelationTerm a b c d
resolveOrder ctx (OrderTerm fld dir nulls) = CoercibleOrderTerm (resolveTypeOrUnknown ctx fld) dir nulls
-- Validates that the related resource on the order is an embedded resource,
-- e.g. if `clients` is inside the `select` in /projects?order=clients(id)&select=*,clients(*),
-- and if it's a to-one relationship, it adds the right alias to the OrderRelationTerm so the generated query can succeed.
-- TODO might be clearer if there's an additional intermediate type
addRelatedOrders :: ReadPlanTree -> Either ApiRequestError ReadPlanTree
addRelatedOrders (Node rp@ReadPlan{order,from} forest) = do
newOrder <- getRelOrder `traverse` order
newOrder <- newRelOrder `traverse` order
Node rp{order=newOrder} <$> addRelatedOrders `traverse` forest
where
getRelOrder ot@OrderTerm{} = Right ot
getRelOrder ot@OrderRelationTerm{otRelation} =
let foundRP = rootLabel <$> find (\(Node ReadPlan{relName, relAlias} _) -> otRelation == fromMaybe relName relAlias) forest in
newRelOrder cot@CoercibleOrderTerm{} = Right cot
newRelOrder cot@CoercibleOrderRelationTerm{coRelation} =
let foundRP = rootLabel <$> find (\(Node ReadPlan{relName, relAlias} _) -> coRelation == fromMaybe relName relAlias) forest in
case foundRP of
Just ReadPlan{relName,relAlias,relAggAlias,relToParent} ->
let isToOne = relIsToOne <$> relToParent
name = fromMaybe relName relAlias in
if isToOne == Just True
then Right $ ot{otRelation=relAggAlias}
then Right $ cot{coRelation=relAggAlias}
else Left $ RelatedOrderNotToOne (qiName from) name
Nothing ->
Left $ NotEmbedded otRelation
Left $ NotEmbedded coRelation
-- | Searches for null filters on embeds, e.g. `projects=not.is.null` on `GET /clients?select=*,projects(*)&projects=not.is.null`
--
@@ -598,7 +604,7 @@ addRelatedOrders (Node rp@ReadPlan{order,from} forest) = do
-- where_ = [
-- CoercibleStmnt (
-- CoercibleFilter {
-- field = CoercibleField {cfName = "projects", cfJsonPath = [], cfIRType = "", cfTransform = Nothing, cfDefault = Nothing},
-- field = CoercibleField {cfName = "projects", cfJsonPath = [], cfToJson=False, cfIRType = "", cfTransform = Nothing, cfDefault = Nothing},
-- opExpr = op
-- }
-- )
@@ -613,7 +619,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 = [], cfIRType = "", cfTransform = Nothing, cfDefault = Nothing}, opExpr = OpExpr True (Is TriNull)})]
-- Right [CoercibleStmnt (CoercibleFilter {field = CoercibleField {cfName = "projects", cfJsonPath = [], cfToJson = False, cfIRType = "", cfTransform = Nothing, cfDefault = Nothing}, opExpr = OpExpr True (Is TriNull)})]
--
-- 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.
--
@@ -637,7 +643,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 fld [] _ _ _ _) opExpr)) ->
let foundRP = find (\ReadPlan{relName, relAlias} -> fld == fromMaybe relName relAlias) rPlans in
case (foundRP, opExpr) of
(Just ReadPlan{relAggAlias}, OpExpr b (Is TriNull)) -> Right $ CoercibleStmnt $ CoercibleFilterNullEmbed b relAggAlias
@@ -726,7 +732,7 @@ mutatePlan mutation qi ApiRequest{iPreferences=Preferences{..}, ..} SchemaCache{
tbl = HM.lookup qi dbTables
pkCols = maybe mempty tablePKCols tbl
logic = map (resolveLogicTree ctx . snd) qsLogic
rootOrder = maybe [] snd $ find (\(x, _) -> null x) qsOrder
rootOrder = resolveOrder ctx <$> maybe [] snd (find (\(x, _) -> null x) qsOrder)
combinedLogic = foldr (addFilterToLogicForest . resolveFilter ctx) logic qsFiltersRoot
body = payRaw <$> iPayload -- the body is assumed to be json at this stage(ApiRequest validates)
applyDefaults = preferMissing == Just ApplyDefaults
src/PostgREST/Plan/MutatePlan.hs
+4 -4
View File
@@ -6,9 +6,9 @@ where
import qualified Data.ByteString.Lazy as LBS
import PostgREST.ApiRequest.Preferences (PreferResolution)
import PostgREST.ApiRequest.Types (OrderTerm)
import PostgREST.Plan.Types (CoercibleField,
CoercibleLogicTree)
CoercibleLogicTree,
CoercibleOrderTerm)
import PostgREST.RangeQuery (NonnegRange)
import PostgREST.SchemaCache.Identifiers (FieldName,
QualifiedIdentifier)
@@ -33,7 +33,7 @@ data MutatePlan
, updBody :: Maybe LBS.ByteString
, where_ :: [CoercibleLogicTree]
, mutRange :: NonnegRange
, mutOrder :: [OrderTerm]
, mutOrder :: [CoercibleOrderTerm]
, returning :: [FieldName]
, applyDefs :: Bool
}
@@ -41,6 +41,6 @@ data MutatePlan
{ in_ :: QualifiedIdentifier
, where_ :: [CoercibleLogicTree]
, mutRange :: NonnegRange
, mutOrder :: [OrderTerm]
, mutOrder :: [CoercibleOrderTerm]
, returning :: [FieldName]
}
src/PostgREST/Plan/ReadPlan.hs
+4 -4
View File
@@ -7,10 +7,10 @@ module PostgREST.Plan.ReadPlan
import Data.Tree (Tree (..))
import PostgREST.ApiRequest.Types (Alias, Cast, Depth, Hint,
JoinType, NodeName,
OrderTerm)
JoinType, NodeName)
import PostgREST.Plan.Types (CoercibleField (..),
CoercibleLogicTree)
CoercibleLogicTree,
CoercibleOrderTerm)
import PostgREST.RangeQuery (NonnegRange)
import PostgREST.SchemaCache.Identifiers (FieldName,
QualifiedIdentifier)
@@ -32,7 +32,7 @@ data ReadPlan = ReadPlan
, from :: QualifiedIdentifier
, fromAlias :: Maybe Alias
, where_ :: [CoercibleLogicTree]
, order :: [OrderTerm]
, order :: [CoercibleOrderTerm]
, range_ :: NonnegRange
, relName :: NodeName
, relToParent :: Maybe Relationship
src/PostgREST/Plan/Types.hs
+19 -2
View File
@@ -4,9 +4,11 @@ module PostgREST.Plan.Types
, CoercibleLogicTree(..)
, CoercibleFilter(..)
, TransformerProc
, CoercibleOrderTerm(..)
) where
import PostgREST.ApiRequest.Types (JsonPath, LogicOperator, OpExpr)
import PostgREST.ApiRequest.Types (Field, JsonPath, LogicOperator,
OpExpr, OrderDirection, OrderNulls)
import PostgREST.SchemaCache.Identifiers (FieldName)
@@ -28,13 +30,14 @@ type TransformerProc = Text
data CoercibleField = CoercibleField
{ cfName :: FieldName
, cfJsonPath :: JsonPath
, cfToJson :: Bool
, cfIRType :: Text -- ^ The native Postgres type of the field, the intermediate (IR) type before mapping.
, cfTransform :: Maybe TransformerProc -- ^ The optional mapping from irType -> targetType.
, cfDefault :: Maybe Text
} deriving (Eq, Show)
unknownField :: FieldName -> JsonPath -> CoercibleField
unknownField name path = CoercibleField name path "" Nothing Nothing
unknownField name path = CoercibleField name path False "" Nothing Nothing
-- | Like an API request LogicTree, but with coercible field information.
data CoercibleLogicTree
@@ -48,3 +51,17 @@ data CoercibleFilter = CoercibleFilter
}
| CoercibleFilterNullEmbed Bool FieldName
deriving (Eq, Show)
data CoercibleOrderTerm
= CoercibleOrderTerm
{ coField :: CoercibleField
, coDirection :: Maybe OrderDirection
, coNullOrder :: Maybe OrderNulls
}
| CoercibleOrderRelationTerm
{ coRelation :: FieldName
, coRelTerm :: Field
, coDirection :: Maybe OrderDirection
, coNullOrder :: Maybe OrderNulls
}
deriving (Eq, Show)
src/PostgREST/Query/QueryBuilder.hs
+3 -3
View File
@@ -138,7 +138,7 @@ mutatePlanToQuery (Update mainQi uCols body logicForest range ordts returnings a
emptyBodyReturnedColumns = if null returnings then "NULL" else intercalateSnippet ", " (pgFmtColumn (QualifiedIdentifier mempty $ qiName mainQi) <$> returnings)
nonRangeCols = intercalateSnippet ", " (pgFmtIdent . cfName <> const " = " <> pgFmtColumn (QualifiedIdentifier mempty "pgrst_body") . cfName <$> uCols)
rangeCols = intercalateSnippet ", " ((\col -> pgFmtIdent (cfName col) <> " = (SELECT " <> pgFmtIdent (cfName col) <> " FROM pgrst_update_body) ") <$> uCols)
(whereRangeIdF, rangeIdF) = mutRangeF mainQi (fst . otTerm <$> ordts)
(whereRangeIdF, rangeIdF) = mutRangeF mainQi (cfName . coField <$> ordts)
mutatePlanToQuery (Delete mainQi logicForest range ordts returnings)
| range == allRange =
@@ -161,7 +161,7 @@ mutatePlanToQuery (Delete mainQi logicForest range ordts returnings)
where
whereLogic = if null logicForest then mempty else " WHERE " <> intercalateSnippet " AND " (pgFmtLogicTree mainQi <$> logicForest)
(whereRangeIdF, rangeIdF) = mutRangeF mainQi (fst . otTerm <$> ordts)
(whereRangeIdF, rangeIdF) = mutRangeF mainQi (cfName . coField <$> ordts)
callPlanToQuery :: CallPlan -> PgVersion -> SQL.Snippet
callPlanToQuery (FunctionCall qi params args returnsScalar returnsSetOfScalar returnsCompositeAlias returnings) pgVer =
@@ -171,7 +171,7 @@ callPlanToQuery (FunctionCall qi params args returnsScalar returnsSetOfScalar re
fromCall = case params of
OnePosParam prm -> "FROM " <> callIt (singleParameter args $ encodeUtf8 $ ppType prm)
KeyParams [] -> "FROM " <> callIt mempty
KeyParams prms -> fromJsonBodyF args ((\p -> CoercibleField (ppName p) mempty (ppType p) Nothing Nothing) <$> prms) False True False <> ", " <>
KeyParams prms -> fromJsonBodyF args ((\p -> CoercibleField (ppName p) mempty False (ppType p) Nothing Nothing) <$> prms) False True False <> ", " <>
"LATERAL " <> callIt (fmtParams prms)
callIt :: SQL.Snippet -> SQL.Snippet
src/PostgREST/Query/SqlFragment.hs
+10 -11
View File
@@ -61,7 +61,6 @@ import PostgREST.ApiRequest.Types (Alias, Cast,
Operation (..),
OrderDirection (..),
OrderNulls (..),
OrderTerm (..),
QuantOperator (..),
SimpleOperator (..),
TrileanVal (..))
@@ -71,6 +70,7 @@ import PostgREST.Plan.ReadPlan (JoinCondition (..))
import PostgREST.Plan.Types (CoercibleField (..),
CoercibleFilter (..),
CoercibleLogicTree (..),
CoercibleOrderTerm (..),
unknownField)
import PostgREST.RangeQuery (NonnegRange, allRange,
rangeLimit, rangeOffset)
@@ -241,10 +241,9 @@ pgFmtCallUnary :: Text -> SQL.Snippet -> SQL.Snippet
pgFmtCallUnary f x = SQL.sql (encodeUtf8 f) <> "(" <> x <> ")"
pgFmtField :: QualifiedIdentifier -> CoercibleField -> SQL.Snippet
pgFmtField table CoercibleField{cfName=fn, cfJsonPath=[]} = pgFmtColumn table fn
-- Using to_jsonb instead of to_json to avoid missing operator errors when filtering:
-- "operator does not exist: json = unknown"
pgFmtField table CoercibleField{cfName=fn, cfJsonPath=jp} = "to_jsonb(" <> pgFmtColumn table fn <> ")" <> pgFmtJsonPath jp
pgFmtField table CoercibleField{cfName=fn, cfJsonPath=[]} = pgFmtColumn table fn
pgFmtField table CoercibleField{cfName=fn, cfToJson=doToJson, cfJsonPath=jp} | doToJson = "to_jsonb(" <> pgFmtColumn table fn <> ")" <> pgFmtJsonPath jp
| otherwise = pgFmtColumn table fn <> pgFmtJsonPath jp
-- Select the value of a named element from a table, applying its optional coercion mapping if any.
pgFmtTableCoerce :: QualifiedIdentifier -> CoercibleField -> SQL.Snippet
@@ -299,16 +298,16 @@ fromJsonBodyF body fields includeSelect includeLimitOne includeDefaults =
else ("pgrst_uniform_json.val", "json_typeof", "json_build_array", "json_array_elements", "json_to_recordset")
jsonPlaceHolder = SQL.encoderAndParam (HE.nullable $ if includeDefaults then HE.jsonbLazyBytes else HE.jsonLazyBytes) body
pgFmtOrderTerm :: QualifiedIdentifier -> OrderTerm -> SQL.Snippet
pgFmtOrderTerm :: QualifiedIdentifier -> CoercibleOrderTerm -> SQL.Snippet
pgFmtOrderTerm qi ot =
fmtOTerm ot <> " " <>
SQL.sql (BS.unwords [
maybe mempty direction $ otDirection ot,
maybe mempty nullOrder $ otNullOrder ot])
maybe mempty direction $ coDirection ot,
maybe mempty nullOrder $ coNullOrder ot])
where
fmtOTerm = \case
OrderTerm{otTerm=(fn, jp)} -> pgFmtField qi (unknownField fn jp)
OrderRelationTerm{otRelation, otRelTerm=(fn, jp)} -> pgFmtField (QualifiedIdentifier mempty otRelation) (unknownField fn jp)
CoercibleOrderTerm{coField=cof} -> pgFmtField qi cof
CoercibleOrderRelationTerm{coRelation, coRelTerm=(fn, jp)} -> pgFmtField (QualifiedIdentifier mempty coRelation) (unknownField fn jp)
direction OrderAsc = "ASC"
direction OrderDesc = "DESC"
@@ -446,7 +445,7 @@ mutRangeF mainQi rangeId =
, intercalateSnippet ", " (pgFmtColumn mainQi <$> rangeId)
)
orderF :: QualifiedIdentifier -> [OrderTerm] -> SQL.Snippet
orderF :: QualifiedIdentifier -> [CoercibleOrderTerm] -> SQL.Snippet
orderF _ [] = mempty
orderF qi ordts = "ORDER BY " <> intercalateSnippet ", " (pgFmtOrderTerm qi <$> ordts)
test/spec/Feature/Query/JsonOperatorSpec.hs
+2 -2
View File
@@ -103,7 +103,7 @@ spec actualPgVersion = describe "json and jsonb operators" $ do
it "can get array of objects" $ do
get "/json_arr?select=data->0->>a&id=in.(5,6)" `shouldRespondWith`
[json| [{"a":"A"}, {"a":"[1, 2, 3]"}] |]
[json|[{"a":"A"}, {"a":"[1,2,3]"}]|]
{ matchHeaders = [matchContentTypeJson] }
get "/json_arr?select=data->0->a->>2&id=in.(5,6)" `shouldRespondWith`
[json| [{"a":null}, {"a":"3"}] |]
@@ -275,7 +275,7 @@ spec actualPgVersion = describe "json and jsonb operators" $ do
[json| [{"data":8}, {"data":7}] |]
{ matchHeaders = [matchContentTypeJson] }
get "/json_arr?select=data->-2->>a&id=in.(5,6)" `shouldRespondWith`
[json| [{"a":"A"}, {"a":"[1, 2, 3]"}] |]
[json| [{"a":"A"}, {"a":"[1,2,3]"}] |]
{ matchHeaders = [matchContentTypeJson] }
it "can filter with negative indexes" $ do
test/spec/Feature/Query/PlanSpec.hs
+37
View File
@@ -348,6 +348,43 @@ spec actualPgVersion = do
liftIO $ do
resBody `shouldSatisfy` (\t -> not $ T.isInfixOf "getitemrange" (decodeUtf8 $ LBS.toStrict t))
context "index usage" $ do
it "should use an index for a json arrow operator filter" $ do
r <- request methodGet "/bets?data_json->>contractId=eq.1"
[(hAccept, "application/vnd.pgrst.plan")] ""
let resBody = simpleBody r
liftIO $ do
resBody `shouldSatisfy` (\t -> T.isInfixOf "Index Cond" (decodeUtf8 $ LBS.toStrict t))
it "should use an index for a jsonb arrow operator filter" $ do
r <- request methodGet "/bets?data_jsonb->>contractId=eq.1"
[(hAccept, "application/vnd.pgrst.plan")] ""
let resBody = simpleBody r
liftIO $ do
resBody `shouldSatisfy` (\t -> T.isInfixOf "Index" (decodeUtf8 $ LBS.toStrict t))
it "should use an index for ordering on a json arrow operator" $ do
r <- request methodGet "/bets?order=data_json->>contractId"
[(hAccept, "application/vnd.pgrst.plan")] ""
let resBody = simpleBody r
liftIO $ do
resBody `shouldSatisfy` (\t -> T.isInfixOf "Index" (decodeUtf8 $ LBS.toStrict t))
it "should use an index for ordering on a jsonb arrow operator" $ do
r <- request methodGet "/bets?order=data_jsonb->>contractId"
[(hAccept, "application/vnd.pgrst.plan")] ""
let resBody = simpleBody r
liftIO $ do
resBody `shouldSatisfy` (\t -> T.isInfixOf "Index" (decodeUtf8 $ LBS.toStrict t))
disabledSpec :: SpecWith ((), Application)
disabledSpec =
it "doesn't work if db-plan-enabled=false(the default)" $ do
test/spec/fixtures/schema.sql
Vendored
+9
View File
@@ -3293,3 +3293,12 @@ create table evil_friends(
id devil_int
, name text
);
create table bets (
id int
, data_json json
, data_jsonb jsonb
);
create index bets_data_json on bets ((data_json ->>'contractId'));
create index bets_data_jsonb on bets ((data_jsonb ->>'contractId'));