fix: unused index on jsonb/jsonb arrow filter

CHANGELOG.md

@@ -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

@@ -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,38 +529,41 @@ 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
   where
     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

@@ -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,14 +33,14 @@ data MutatePlan
       , updBody   :: Maybe LBS.ByteString
       , where_    :: [CoercibleLogicTree]
       , mutRange  :: NonnegRange
-      , mutOrder  :: [OrderTerm]
+      , mutOrder  :: [CoercibleOrderTerm]
       , returning :: [FieldName]
       , applyDefs :: Bool
       }
   | Delete
       { in_       :: QualifiedIdentifier
       , where_    :: [CoercibleLogicTree]
       , mutRange  :: NonnegRange
-      , mutOrder  :: [OrderTerm]
+      , mutOrder  :: [CoercibleOrderTerm]
       , returning :: [FieldName]
       }

src/PostgREST/Plan/ReadPlan.hs

@@ -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

@@ -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

@@ -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

@@ -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

@@ -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