From 5f4cd66bbfe3efe365a3b329d11a90958513fc8c Mon Sep 17 00:00:00 2001 From: rakeshkky Date: Wed, 21 Nov 2018 16:45:42 +0530 Subject: [PATCH 1/2] support 'DISTINCT ON', close #1040 TODO:- Add tests and docs --- .../src-lib/Hasura/GraphQL/Resolve/Select.hs | 20 ++++- server/src-lib/Hasura/GraphQL/Schema.hs | 4 + server/src-lib/Hasura/Prelude.hs | 3 +- server/src-lib/Hasura/RQL/DML/Select.hs | 3 +- .../src-lib/Hasura/RQL/DML/Select/Internal.hs | 79 +++++++++++++------ 5 files changed, 80 insertions(+), 29 deletions(-) diff --git a/server/src-lib/Hasura/GraphQL/Resolve/Select.hs b/server/src-lib/Hasura/GraphQL/Resolve/Select.hs index ddd6433b1cb2f..f8f59db3d8a9f 100644 --- a/server/src-lib/Hasura/GraphQL/Resolve/Select.hs +++ b/server/src-lib/Hasura/GraphQL/Resolve/Select.hs @@ -78,6 +78,7 @@ fromSelSet f fldTy flds = fieldAsPath :: (MonadError QErr m) => Field -> m a -> m a fieldAsPath = nameAsPath . _fName + parseTableArgs :: (MonadError QErr m, MonadReader r m, Has FieldMap r, Has OrdByCtx r) => ((PGColType, PGColValue) -> m S.SQLExp) @@ -88,7 +89,24 @@ parseTableArgs f args = do let ordByExpM = NE.nonEmpty =<< ordByExpML limitExpM <- withArgM args "limit" parseLimit offsetExpM <- withArgM args "offset" $ asPGColVal >=> f - return $ RS.TableArgs whereExpM ordByExpM limitExpM offsetExpM + distOnColsML <- withArgM args "distinct_on" parseColumns + let distOnColsM = NE.nonEmpty =<< distOnColsML + mapM_ (validateDistOn ordByExpM) distOnColsM + return $ RS.TableArgs whereExpM ordByExpM limitExpM offsetExpM distOnColsM + where + validateDistOn Nothing _ = return () + validateDistOn (Just ordBys) cols = withPathK "args" $ do + let colsLen = length cols + initOrdBys = take colsLen $ toList ordBys + initOrdByCols = flip mapMaybe initOrdBys $ \ob -> + case obiColumn ob of + RS.AOCPG ci -> Just $ pgiName ci + _ -> Nothing + isValid = (colsLen == length initOrdByCols) + && all (`elem` initOrdByCols) (toList cols) + + unless isValid $ throwVE + "\"distinct_on\" columns must match initial \"order_by\" columns" fromField :: (MonadError QErr m, MonadReader r m, Has FieldMap r, Has OrdByCtx r) diff --git a/server/src-lib/Hasura/GraphQL/Schema.hs b/server/src-lib/Hasura/GraphQL/Schema.hs index 77c45602feac0..8666b6d3ae995 100644 --- a/server/src-lib/Hasura/GraphQL/Schema.hs +++ b/server/src-lib/Hasura/GraphQL/Schema.hs @@ -290,6 +290,7 @@ mkPGColFld (PGColInfo colName colTy isNullable) = -- where: table_bool_exp -- limit: Int -- offset: Int +-- distinct_on: [table_select_column!] mkSelArgs :: QualifiedTable -> [InpValInfo] mkSelArgs tn = [ InpValInfo (Just whereDesc) "where" $ G.toGT $ mkBoolExpTy tn @@ -297,12 +298,15 @@ mkSelArgs tn = , InpValInfo (Just offsetDesc) "offset" $ G.toGT $ mkScalarTy PGInteger , InpValInfo (Just orderByDesc) "order_by" $ G.toGT $ G.toLT $ G.toNT $ mkOrdByTy tn + , InpValInfo (Just distinctDesc) "distinct_on" $ G.toGT $ G.toLT $ + G.toNT $ mkSelColumnInpTy tn ] where whereDesc = "filter the rows returned" limitDesc = "limit the nuber of rows returned" offsetDesc = "skip the first n rows. Use only with order_by" orderByDesc = "sort the rows by one or more columns" + distinctDesc = "distinct select on columns" fromInpValL :: [InpValInfo] -> Map.HashMap G.Name InpValInfo fromInpValL = mapFromL _iviName diff --git a/server/src-lib/Hasura/Prelude.hs b/server/src-lib/Hasura/Prelude.hs index 66649e1c316a2..19e4d65aa47fa 100644 --- a/server/src-lib/Hasura/Prelude.hs +++ b/server/src-lib/Hasura/Prelude.hs @@ -13,7 +13,8 @@ import Data.Bool as M (bool) import Data.Either as M (lefts, partitionEithers, rights) import Data.Foldable as M (toList) import Data.Hashable as M (Hashable) -import Data.List as M (find, foldl', group, sort, sortBy) +import Data.List as M (find, foldl', group, sort, sortBy, + union) import Data.Maybe as M (catMaybes, fromMaybe, isJust, isNothing, listToMaybe, mapMaybe, maybeToList) diff --git a/server/src-lib/Hasura/RQL/DML/Select.hs b/server/src-lib/Hasura/RQL/DML/Select.hs index 6841a84345a18..0aea4013c868b 100644 --- a/server/src-lib/Hasura/RQL/DML/Select.hs +++ b/server/src-lib/Hasura/RQL/DML/Select.hs @@ -185,7 +185,8 @@ convSelectQ fieldInfoMap selPermInfo selQ prepValBuilder = do let tabFrom = TableFrom (spiTable selPermInfo) Nothing tabPerm = TablePerm (spiFilter selPermInfo) mPermLimit return $ AnnSelG annFlds tabFrom tabPerm $ - TableArgs wClause annOrdByM mQueryLimit (S.intToSQLExp <$> mQueryOffset) + TableArgs wClause annOrdByM mQueryLimit + (S.intToSQLExp <$> mQueryOffset) Nothing where mQueryOffset = sqOffset selQ diff --git a/server/src-lib/Hasura/RQL/DML/Select/Internal.hs b/server/src-lib/Hasura/RQL/DML/Select/Internal.hs index 3c25ed6ea06ce..5f2e9606782cc 100644 --- a/server/src-lib/Hasura/RQL/DML/Select/Internal.hs +++ b/server/src-lib/Hasura/RQL/DML/Select/Internal.hs @@ -5,6 +5,7 @@ module Hasura.RQL.DML.Select.Internal where +import Control.Arrow ((&&&)) import Data.Aeson.Types import Instances.TH.Lift () import Language.Haskell.TH.Syntax (Lift) @@ -85,14 +86,15 @@ data AnnFld data TableArgs = TableArgs - { _taWhere :: !(Maybe AnnBoolExpSQL) - , _taOrderBy :: !(Maybe (NE.NonEmpty AnnOrderByItem)) - , _taLimit :: !(Maybe Int) - , _taOffset :: !(Maybe S.SQLExp) + { _taWhere :: !(Maybe AnnBoolExpSQL) + , _taOrderBy :: !(Maybe (NE.NonEmpty AnnOrderByItem)) + , _taLimit :: !(Maybe Int) + , _taOffset :: !(Maybe S.SQLExp) + , _taDistCols :: !(Maybe (NE.NonEmpty PGCol)) } deriving (Show, Eq) noTableArgs :: TableArgs -noTableArgs = TableArgs Nothing Nothing Nothing Nothing +noTableArgs = TableArgs Nothing Nothing Nothing Nothing Nothing data PGColFld = PCFCol !PGCol @@ -155,17 +157,18 @@ type AnnSel = AnnSelG [(FieldName, AnnFld)] data BaseNode = BaseNode - { _bnPrefix :: !Iden - , _bnFrom :: !S.FromItem - , _bnWhere :: !S.BoolExp - , _bnOrderBy :: !(Maybe S.OrderByExp) - , _bnLimit :: !(Maybe Int) - , _bnOffset :: !(Maybe S.SQLExp) - - , _bnExtrs :: !(HM.HashMap S.Alias S.SQLExp) - , _bnObjRels :: !(HM.HashMap RelName RelNode) - , _bnArrRels :: !(HM.HashMap S.Alias RelNode) - , _bnAggs :: !(HM.HashMap S.Alias AggNode) + { _bnPrefix :: !Iden + , _bnDistinct :: !(Maybe S.DistinctExpr) + , _bnFrom :: !S.FromItem + , _bnWhere :: !S.BoolExp + , _bnOrderBy :: !(Maybe S.OrderByExp) + , _bnLimit :: !(Maybe Int) + , _bnOffset :: !(Maybe S.SQLExp) + + , _bnExtrs :: !(HM.HashMap S.Alias S.SQLExp) + , _bnObjRels :: !(HM.HashMap RelName RelNode) + , _bnArrRels :: !(HM.HashMap S.Alias RelNode) + , _bnAggs :: !(HM.HashMap S.Alias AggNode) } deriving (Show, Eq) @@ -354,7 +357,7 @@ processAnnOrderByCol pfx = \case ((nesAls, nesCol), nesNodeM) = processAnnOrderByCol relPfx rest qualCol = S.mkQIdenExp relPfx nesAls relBaseNode = - BaseNode relPfx (S.FISimple relTab Nothing) + BaseNode relPfx Nothing (S.FISimple relTab Nothing) (toSQLBoolExp (S.QualTable relTab) relFltr) Nothing Nothing Nothing (HM.singleton nesAls nesCol) @@ -365,9 +368,25 @@ processAnnOrderByCol pfx = \case , Just (rn, relNode) ) +processDistinctOnCol + :: Iden + -> NE.NonEmpty PGCol + -> ( S.DistinctExpr + -- additional column extractors + , [(S.Alias, S.SQLExp)] + ) +processDistinctOnCol pfx neCols = (distOnExp, colExtrs) + where + cols = toList neCols + distOnExp = S.DistinctOn $ map (S.SEIden . toIden . mkQColAls) cols + mkQCol c = S.mkQIdenExp (mkBaseTableAls pfx) $ toIden c + mkQColAls = S.Alias . mkBaseTableColAls pfx + colExtrs = flip map cols $ mkQColAls &&& mkQCol + + mkEmptyBaseNode :: Iden -> TableFrom -> BaseNode mkEmptyBaseNode pfx tableFrom = - BaseNode pfx fromItem (S.BELit True) Nothing Nothing Nothing + BaseNode pfx Nothing fromItem (S.BELit True) Nothing Nothing Nothing selOne HM.empty HM.empty HM.empty where selOne = HM.singleton (S.Alias $ pfx <> Iden "__one") (S.SEUnsafe "1") @@ -420,11 +439,11 @@ mkBaseNode :: Iden -> FieldName -> TableAggFld -> TableFrom -> TablePerm -> TableArgs -> BaseNode mkBaseNode pfx fldAls annSelFlds tableFrom tablePerm tableArgs = - BaseNode pfx fromItem finalWhere ordByExpM finalLimit offsetM + BaseNode pfx distExprM fromItem finalWhere ordByExpM finalLimit offsetM allExtrs allObjsWithOb allArrs aggs where TablePerm fltr permLimitM = tablePerm - TableArgs whereM orderByM limitM offsetM = tableArgs + TableArgs whereM orderByM limitM offsetM distM = tableArgs (allExtrs, allObjsWithOb, allArrs, aggs) = case annSelFlds of TAFNodes flds -> let selExtr = buildJsonObject pfx fldAls flds @@ -437,14 +456,14 @@ mkBaseNode pfx fldAls annSelFlds tableFrom tablePerm tableArgs = allObjs $ catMaybes $ maybe [] _3 procOrdByM aggItems = HM.fromList $ map mkAggItem $ mapMaybe (\(als, f) -> (als,) <$> getAggFld f) flds - in ( HM.fromList $ selExtr:obExtrs + in ( HM.fromList $ selExtr:obExtrs <> distExtrs , allObjRelsWithOb , allArrRels , aggItems ) TAFAgg aggFlds -> let extrs = concatMap (fetchExtrFromAggFld . snd) aggFlds - in ( HM.fromList $ extrs <> obExtrs + in ( HM.fromList $ extrs <> obExtrs <> distExtrs , HM.empty , HM.empty , HM.empty @@ -479,6 +498,10 @@ mkBaseNode pfx fldAls annSelFlds tableFrom tablePerm tableArgs = _2 (_, b, _) = b _3 (_, _, c) = c + distItemsM = processDistinctOnCol pfx <$> distM + distExprM = fst <$> distItemsM + distExtrs = fromMaybe [] (snd <$> distItemsM) + procOrdByM = unzip3 . map (processAnnOrderByItem pfx) . toList <$> orderByM ordByExpM = S.OrderByExp . _2 <$> procOrdByM @@ -525,14 +548,14 @@ annSelToBaseNode pfx fldAls annSel = mergeBaseNodes :: BaseNode -> BaseNode -> BaseNode mergeBaseNodes lNodeDet rNodeDet = - BaseNode pfx f whr ordBy limit offset + BaseNode pfx dExp f whr ordBy limit offset (HM.union lExtrs rExtrs) (HM.unionWith mergeRelNodes lObjs rObjs) (HM.union lArrs rArrs) (HM.union lAggs rAggs) where - (BaseNode pfx f whr ordBy limit offset lExtrs lObjs lArrs lAggs) = lNodeDet - (BaseNode _ _ _ _ _ _ rExtrs rObjs rArrs rAggs) = rNodeDet + (BaseNode pfx dExp f whr ordBy limit offset lExtrs lObjs lArrs lAggs) = lNodeDet + (BaseNode _ _ _ _ _ _ _ rExtrs rObjs rArrs rAggs) = rNodeDet -- should only be used to merge obj rel nodes mergeRelNodes :: RelNode -> RelNode -> RelNode @@ -574,15 +597,19 @@ mkJoinCond baseTableAls colMapn = S.BECompare S.SEQ (S.mkQIdenExp baseTableAls lCol) (S.mkSIdenExp rCol) baseNodeToSel :: S.BoolExp -> BaseNode -> S.Select -baseNodeToSel joinCond (BaseNode pfx fromItem whr ordByM limitM offsetM extrs objRels arrRels aggs) = +baseNodeToSel joinCond baseNode = S.mkSelect { S.selExtr = [S.Extractor e $ Just a | (a, e) <- HM.toList extrs] , S.selFrom = Just $ S.FromExp [joinedFrom] , S.selOrderBy = ordByM , S.selLimit = S.LimitExp . S.intToSQLExp <$> limitM , S.selOffset = S.OffsetExp <$> offsetM + , S.selDistinct = dExp } where + BaseNode pfx dExp fromItem whr ordByM limitM + offsetM extrs objRels arrRels aggs + = baseNode -- this is the table which is aliased as "pfx.base" baseSel = S.mkSelect { S.selExtr = [S.Extractor S.SEStar Nothing] From 25bd61de228dcbdc034e8df64abcacb3fe86fe84 Mon Sep 17 00:00:00 2001 From: rakeshkky Date: Thu, 22 Nov 2018 16:10:52 +0530 Subject: [PATCH 2/2] add tests and docs --- docs/graphql/manual/api-reference/query.rst | 27 +++++++- .../manual/queries/distinct-queries.rst | 63 +++++++++++++++++++ docs/graphql/manual/queries/index.rst | 2 + ...stinct_department_order_by_salary_asc.yaml | 32 ++++++++++ ...tinct_department_order_by_salary_desc.yaml | 33 ++++++++++ .../order_by/employee_distinct_fail.yaml | 22 +++++++ .../queries/graphql_query/order_by/setup.yaml | 46 ++++++++++++++ .../graphql_query/order_by/teardown.yaml | 5 ++ server/tests-py/test_graphql_queries.py | 9 +++ 9 files changed, 238 insertions(+), 1 deletion(-) create mode 100644 docs/graphql/manual/queries/distinct-queries.rst create mode 100644 server/tests-py/queries/graphql_query/order_by/employee_distinct_department_order_by_salary_asc.yaml create mode 100644 server/tests-py/queries/graphql_query/order_by/employee_distinct_department_order_by_salary_desc.yaml create mode 100644 server/tests-py/queries/graphql_query/order_by/employee_distinct_fail.yaml diff --git a/docs/graphql/manual/api-reference/query.rst b/docs/graphql/manual/api-reference/query.rst index 4d95effe402cd..5b23cf9e1e01b 100644 --- a/docs/graphql/manual/api-reference/query.rst +++ b/docs/graphql/manual/api-reference/query.rst @@ -225,7 +225,32 @@ Argument .. parsed-literal:: - WhereExp_ | OrderByExp_ | PaginationExp_ + DistinctOnExp_ | WhereExp_ | OrderByExp_ | PaginationExp_ + + +.. _DistinctOnExp: + +DistinctOnExp +************ + +.. parsed-literal:: + + distinct_on: [ TableSelectColumnEnum_ ] + +TableSelectColumnEnum +""""""""""""""""""""" + +.. code-block:: graphql + + #example table_select_column enum for "article" table + enum article_select_column { + id + title + content + author_id + is_published + } + .. _WhereExp: diff --git a/docs/graphql/manual/queries/distinct-queries.rst b/docs/graphql/manual/queries/distinct-queries.rst new file mode 100644 index 0000000000000..5140a594f9f2c --- /dev/null +++ b/docs/graphql/manual/queries/distinct-queries.rst @@ -0,0 +1,63 @@ +Distinct queries +================ + +You can fetch distinct columns using ``distinct_on`` argument. Initial ``order_by`` columns must +match ``distinct_on`` columns. Learn more about ``order_by`` :doc:`here `. + +.. code-block:: graphql + + employee ( + distinct_on: [employee_select_column] + order_by: [employee_order_by] + ): [employee]! + + #select column enum type for "employee" table + enum employee_select_column { + id + name + department + salary + } + +For example, fetch highest salaried employee from each department: + +.. graphiql:: + :view_only: + :query: + query { + employee( + distinct_on: [department] + order_by: [{department: asc}, {salary: desc}] + ){ + id + name + department + salary + } + } + :response: + { + "data": { + "employee": [ + { + "id": 5, + "name": "Kamila", + "department": "Engineering", + "salary": 4325 + }, + { + "id": 4, + "name": "Damien", + "department": "Product", + "salary": 3124 + }, + { + "id": 7, + "name": "Rickard", + "department": "Services", + "salary": 2223 + } + ] + } + } + diff --git a/docs/graphql/manual/queries/index.rst b/docs/graphql/manual/queries/index.rst index 3e8c889db0886..8eeda44897724 100644 --- a/docs/graphql/manual/queries/index.rst +++ b/docs/graphql/manual/queries/index.rst @@ -13,6 +13,7 @@ types, ``query_root`` and ``mutation_root`` respectively. For example, the auto- .. code-block:: graphql author ( + distinct_on: [author_select_column] where: author_bool_exp limit: Int offset: Int @@ -33,6 +34,7 @@ based on a typical author/article schema for reference. simple-object-queries nested-object-queries aggregation-queries + distinct-queries query-filters sorting pagination diff --git a/server/tests-py/queries/graphql_query/order_by/employee_distinct_department_order_by_salary_asc.yaml b/server/tests-py/queries/graphql_query/order_by/employee_distinct_department_order_by_salary_asc.yaml new file mode 100644 index 0000000000000..2f3b4284daed0 --- /dev/null +++ b/server/tests-py/queries/graphql_query/order_by/employee_distinct_department_order_by_salary_asc.yaml @@ -0,0 +1,32 @@ +description: Fetch lowest salaried employee from each department and sort results on department descending +url: /v1alpha1/graphql +status: 200 +response: + data: + employee: + - id: 2 + name: Zara + department: Services + salary: 1234 + - id: 6 + name: Dara + department: Product + salary: 1209 + - id: 1 + name: Kai + department: Engineering + salary: 2345 + +query: + query: | + query { + employee( + distinct_on: [department] + order_by: [{department: desc}, {salary: asc}] + ){ + id + name + department + salary + } + } diff --git a/server/tests-py/queries/graphql_query/order_by/employee_distinct_department_order_by_salary_desc.yaml b/server/tests-py/queries/graphql_query/order_by/employee_distinct_department_order_by_salary_desc.yaml new file mode 100644 index 0000000000000..9ae0c19e2a11a --- /dev/null +++ b/server/tests-py/queries/graphql_query/order_by/employee_distinct_department_order_by_salary_desc.yaml @@ -0,0 +1,33 @@ +description: Fetch highest salaried employee from each department +url: /v1alpha1/graphql +status: 200 +response: + data: + employee: + - id: 5 + name: Kamila + department: Engineering + salary: 4325 + - id: 4 + name: Damien + department: Product + salary: 3124 + - id: 7 + name: Rickard + department: Services + salary: 2223 + +query: + query: | + query { + employee( + distinct_on: [department] + order_by: [{department: asc}, {salary: desc}] + ){ + id + name + department + salary + } + } + diff --git a/server/tests-py/queries/graphql_query/order_by/employee_distinct_fail.yaml b/server/tests-py/queries/graphql_query/order_by/employee_distinct_fail.yaml new file mode 100644 index 0000000000000..17f8972c85657 --- /dev/null +++ b/server/tests-py/queries/graphql_query/order_by/employee_distinct_fail.yaml @@ -0,0 +1,22 @@ +description: Fetch highest salaried employee from each department (fail) +url: /v1alpha1/graphql +status: 400 +response: + errors: + - path: "$.selectionSet.employee.args" + error: '"distinct_on" columns must match initial "order_by" columns' + code: validation-failed + +query: + query: | + query { + employee( + distinct_on: [department] + order_by: [{salary: desc}] + ){ + id + name + department + salary + } + } diff --git a/server/tests-py/queries/graphql_query/order_by/setup.yaml b/server/tests-py/queries/graphql_query/order_by/setup.yaml index 5a32a74a17335..224d8c662140a 100644 --- a/server/tests-py/queries/graphql_query/order_by/setup.yaml +++ b/server/tests-py/queries/graphql_query/order_by/setup.yaml @@ -111,3 +111,49 @@ args: 2, true ) + +# Create employee table +- type: run_sql + args: + sql: | + CREATE TABLE employee ( + id SERIAL PRIMARY KEY, + name TEXT NOT NULL UNIQUE, + department TEXT, + salary INTEGER + ) +- type: track_table + args: + name: employee + schema: public + +# Insert data into employee table +- type: insert + args: + table: employee + objects: + - name: Kai + department: Engineering + salary: 2345 + - name: Zara + department: Services + salary: 1234 + - name: Kelsy + department: Services + salary: 2134 + - name: Damien + department: Product + salary: 3124 + - name: Kamila + department: Engineering + salary: 4325 + - name: Dara + department: Product + salary: 1209 + - name: Rickard + department: Services + salary: 2223 + - name: Bent + department: Engineering + salary: 4122 + diff --git a/server/tests-py/queries/graphql_query/order_by/teardown.yaml b/server/tests-py/queries/graphql_query/order_by/teardown.yaml index 843f55ab03649..f81e4070aa992 100644 --- a/server/tests-py/queries/graphql_query/order_by/teardown.yaml +++ b/server/tests-py/queries/graphql_query/order_by/teardown.yaml @@ -22,3 +22,8 @@ args: args: sql: | drop table contact + +- type: run_sql + args: + sql: | + drop table employee diff --git a/server/tests-py/test_graphql_queries.py b/server/tests-py/test_graphql_queries.py index d06a98636f689..c4e9ef1792c43 100644 --- a/server/tests-py/test_graphql_queries.py +++ b/server/tests-py/test_graphql_queries.py @@ -268,6 +268,15 @@ def test_articles_order_by_rel_author_id(self, hge_ctx): def test_articles_order_by_rel_author_rel_contact_phone(self, hge_ctx): check_query_f(hge_ctx, self.dir() + '/articles_order_by_rel_author_rel_contact_phone.yaml') + def test_employee_distinct_department_order_by_salary_desc(self, hge_ctx): + check_query_f(hge_ctx, self.dir() + '/employee_distinct_department_order_by_salary_desc.yaml') + + def test_employee_distinct_department_order_by_salary_asc(self, hge_ctx): + check_query_f(hge_ctx, self.dir() + '/employee_distinct_department_order_by_salary_asc.yaml') + + def test_employee_distinct_fail(self, hge_ctx): + check_query_f(hge_ctx, self.dir() + '/employee_distinct_fail.yaml') + @classmethod def dir(cls): return 'queries/graphql_query/order_by'