MSSQL nodes aggregates & inherited roles

https://github.com/hasura/graphql-engine-mono/pull/1293

Co-authored-by: Chris Done <11019+chrisdone@users.noreply.github.com>
Co-authored-by: Abby Sassel <3883855+sassela@users.noreply.github.com>
GitOrigin-RevId: 776402dbbaf3d8166a62b1aaaf6abc7e584b3eb2

.circleci/test-server.sh

@@ -715,7 +715,7 @@ case "$SERVER_TEST_TO_RUN" in
       run_hge_with_args serve
       wait_for_port 8080
 
-      pytest -n 1 --hge-urls "$HGE_URL" --pg-urls "$HASURA_GRAPHQL_DATABASE_URL" --hge-key="$HASURA_GRAPHQL_ADMIN_SECRET"  --test-inherited-roles test_graphql_queries.py::TestGraphQLInheritedRoles
+      pytest -n 1 --hge-urls "$HGE_URL" --pg-urls "$HASURA_GRAPHQL_DATABASE_URL" --hge-key="$HASURA_GRAPHQL_ADMIN_SECRET"  --test-inherited-roles -k TestGraphQLInheritedRolesPostgres
       pytest --hge-urls="$HGE_URL" --pg-urls="$HASURA_GRAPHQL_DATABASE_URL" --hge-key="$HASURA_GRAPHQL_ADMIN_SECRET" --test-inherited-roles test_graphql_mutations.py::TestGraphQLInheritedRoles
 
       unset HASURA_GRAPHQL_EXPERIMENTAL_FEATURES
@@ -1101,6 +1101,7 @@ admin_users = postgres' > pgbouncer/pgbouncer.ini
     backend-mssql)
     echo -e "\n$(time_elapsed): <########## TEST GRAPHQL-ENGINE WITH SQL SERVER BACKEND ###########################################>\n"
     TEST_TYPE="no-auth"
+    export HASURA_GRAPHQL_EXPERIMENTAL_FEATURES="inherited_roles"
 
     run_hge_with_args serve
     wait_for_port 8080
@@ -1110,6 +1111,14 @@ admin_users = postgres' > pgbouncer/pgbouncer.ini
 
     pytest -n 1 --hge-urls "$HGE_URL" --pg-urls "$HASURA_GRAPHQL_DATABASE_URL" --backend mssql
 
+    # start inherited roles test
+    echo -e "\n$(time_elapsed): <########## TEST INHERITED-ROLES WITH SQL SERVER BACKEND ###########################################>\n"
+
+    pytest -n 1 --hge-urls "$HGE_URL" --pg-urls "$HASURA_GRAPHQL_DATABASE_URL" --test-inherited-roles -k TestGraphQLInheritedRolesMSSQL --backend mssql
+
+    unset HASURA_GRAPHQL_EXPERIMENTAL_FEATURES
+    # end inherited roles test
+
     kill_hge_servers
     ;;
     backend-citus)

CHANGELOG.md

@@ -37,6 +37,7 @@ NOTE: This only includes the diff between v2.0.0 and v2.0.0-beta.2
 
 ### Bug fixes and improvements
 
+- server: nodes aggregates and inherited roles support for SQL Server
 - server: remote relationships (database to remote schema joins) are now supported on SQL Server and BigQuery
 - server: BigQuery: switches to a single query generation from a dataloader approach. This should result in
   faster query responses.

server/src-lib/Hasura/Backends/MSSQL/FromIr.hs

@@ -1,3 +1,4 @@
+{-# LANGUAGE ViewPatterns #-}
 -- | Translate from the DML to the TSql dialect.
 
 module Hasura.Backends.MSSQL.FromIr
@@ -101,7 +102,7 @@ fromRootField =
   \case
     (IR.QDBSingleRow s)    -> mkSQLSelect IR.JASSingleObject s
     (IR.QDBMultipleRows s) -> mkSQLSelect IR.JASMultipleRows s
-    (IR.QDBAggregation s)  -> fromSelectAggregate s
+    (IR.QDBAggregation s)  -> fromSelectAggregate Nothing s
     (IR.QDBConnection _)   -> refute $ pure ConnectionsNotSupported
 
 --------------------------------------------------------------------------------
@@ -156,48 +157,171 @@ fromSelectRows annSelectG = do
         then StringifyNumbers
         else LeaveNumbersAlone
 
+
+mkNodesSelect :: Args -> Where -> Expression -> Top -> From -> [(Int, (IR.FieldName, [Projection]))] -> [(Int, [Projection])]
+mkNodesSelect Args{..} foreignKeyConditions filterExpression permissionBasedTop selectFrom nodes =
+  [ (index,
+      [ ExpressionProjection $ Aliased
+          { aliasedThing = SelectExpression $ Select
+              { selectProjections = projections
+              , selectTop = permissionBasedTop <> argsTop
+              , selectFrom = pure selectFrom
+              , selectJoins = argsJoins
+              , selectWhere = argsWhere <> Where [filterExpression] <> foreignKeyConditions
+              , selectFor =
+                  JsonFor ForJson {jsonCardinality = JsonArray, jsonRoot = NoRoot}
+              , selectOrderBy = argsOrderBy
+              , selectOffset = argsOffset
+              }
+          , aliasedAlias = IR.getFieldNameTxt fieldName
+          }
+      ] -- singleton
+    )
+  | (index, (fieldName, projections)) <- nodes ]
+
+
+--
+-- The idea here is that LIMIT/OFFSET and aggregates don't mix
+-- well. Therefore we have a nested query:
+--
+-- select sum(*), .. FROM (select * from x offset o limit l) p
+--
+-- That's why @projections@ appears on the outer, and is a
+-- @StarProjection@ for the inner. But the joins, conditions, top,
+-- offset are on the inner.
+--
+mkAggregateSelect :: Args -> Where -> From -> [(Int, (IR.FieldName, [Projection]))] -> [(Int, [Projection])]
+mkAggregateSelect Args {..} foreignKeyConditions selectFrom aggregates =
+  [ ( index
+    , [ ExpressionProjection $
+        Aliased
+          { aliasedThing =
+              JsonQueryExpression $
+              SelectExpression $
+              Select
+                { selectProjections = reproject aggSubselectName <$> projections
+                , selectTop = NoTop
+                , selectFrom = pure $
+                    FromSelect
+                      Aliased
+                        { aliasedAlias = aggSubselectName
+                        , aliasedThing =
+                            Select
+                              { selectProjections = pure StarProjection
+                              , selectTop = argsTop
+                              , selectFrom = pure selectFrom
+                              , selectJoins = argsJoins
+                              , selectWhere = argsWhere <> foreignKeyConditions
+                              , selectFor = NoFor
+                              , selectOrderBy = mempty
+                              , selectOffset = argsOffset
+                              }
+                        }
+                , selectJoins = mempty
+                , selectWhere = mempty
+                , selectFor =
+                    JsonFor
+                      ForJson
+                        {jsonCardinality = JsonSingleton, jsonRoot = NoRoot}
+                , selectOrderBy = mempty
+                , selectOffset = Nothing
+                }
+          , aliasedAlias = IR.getFieldNameTxt fieldName
+          }
+      ] -- singleton
+     )
+  | (index, (fieldName, projections)) <- aggregates
+  ]
+
+
+-- | Re-project projections in the aggSubselectName scope
+--
+-- For example,
+--
+-- [ AggregateProjection
+--     (Aliased {aliasedThing = CountAggregate StarCountable, aliasedAlias = "count"})
+-- , AggregateProjection
+--     (Aliased
+--       { aliasedThing = OpAggregate "sum"
+--           [ ColumnExpression
+--               (FieldName
+--                 { fieldName = "id"
+--                 , fieldNameEntity = "t_person1" -- <<<<< This needs to be `aggSubselectName`
+--                 })
+--           ]
+--       , aliasedAlias = "sum"
+--       })
+--
+reproject :: Text -> Projection -> Projection
+reproject label = \case
+  AggregateProjection (Aliased {aliasedThing = OpAggregate aggName expressions, ..}) ->
+    AggregateProjection (Aliased {aliasedThing = OpAggregate aggName (fixColumnEntity label <$> expressions), ..})
+  AggregateProjection (Aliased {aliasedThing = CountAggregate countableFieldnames, ..}) ->
+    AggregateProjection (Aliased {aliasedThing = CountAggregate $ fixEntity label <$> countableFieldnames, ..})
+  x -> x
+  where
+    fixColumnEntity entity = \case
+      ColumnExpression fName ->
+        ColumnExpression $ fixEntity entity fName
+      x -> x
+    fixEntity entity FieldName{..} = FieldName {fieldNameEntity = entity, ..}
+
+
 fromSelectAggregate
-  :: IR.AnnSelectG 'MSSQL (Const Void) (IR.TableAggregateFieldG 'MSSQL (Const Void)) Expression
+  :: Maybe (EntityAlias, HashMap ColumnName ColumnName)
+  -> IR.AnnSelectG 'MSSQL (Const Void) (IR.TableAggregateFieldG 'MSSQL (Const Void)) Expression
   -> FromIr TSQL.Select
-fromSelectAggregate annSelectG = do
-  selectFrom <-
-    case from of
-      IR.FromTable qualifiedObject -> fromQualifiedTable qualifiedObject
-      IR.FromFunction {}           -> refute $ pure FunctionNotSupported
-  fieldSources <-
-    runReaderT (traverse fromTableAggregateFieldG fields) (fromAlias selectFrom)
-  filterExpression <-
-    runReaderT (fromAnnBoolExp permFilter) (fromAlias selectFrom)
-  Args { argsOrderBy
-       , argsWhere
-       , argsJoins
-       , argsTop
-       , argsDistinct = Proxy
-       , argsOffset
-       } <- runReaderT (fromSelectArgsG args) (fromAlias selectFrom)
-  let selectProjections =
-        concatMap (toList . fieldSourceProjections) fieldSources
+fromSelectAggregate
+  mparentRelationship
+  IR.AnnSelectG
+    { _asnFields = (zip [0..] -> fields)
+    , _asnFrom = from
+    , _asnPerm = IR.TablePerm {_tpLimit = (maybe NoTop Top -> permissionBasedTop), _tpFilter = permFilter}
+    , _asnArgs = args
+    , _asnStrfyNum = (bool LeaveNumbersAlone StringifyNumbers -> stringifyNumbers)
+    }
+  = do
+  selectFrom <- case from of
+    IR.FromTable qualifiedObject -> fromQualifiedTable qualifiedObject
+    IR.FromFunction {}           -> refute $ pure FunctionNotSupported
+  -- Below: When we're actually a RHS of a query (of CROSS APPLY),
+  -- then we'll have a LHS table that we're joining on. So we get the
+  -- conditions expressions from the field mappings. The LHS table is
+  -- the entityAlias, and the RHS table is selectFrom.
+  mforeignKeyConditions <- fmap (Where . fromMaybe []) $ for mparentRelationship $
+    \(entityAlias, mapping) ->
+      runReaderT (fromMapping selectFrom mapping) entityAlias
+  filterExpression <- runReaderT (fromAnnBoolExp permFilter) (fromAlias selectFrom)
+  args'@Args{argsExistingJoins} <-
+    runReaderT (fromSelectArgsG args) (fromAlias selectFrom)
+  -- Although aggregates, exps and nodes could be handled in one list,
+  -- we need to separately treat the subselect expressions
+  expss :: [(Int, [Projection])] <- flip runReaderT (fromAlias selectFrom) $ sequence $ mapMaybe fromTableExpFieldG fields
+  nodes :: [(Int, (IR.FieldName, [Projection]))] <-
+    flip runReaderT (fromAlias selectFrom) $ sequence $ mapMaybe (fromTableNodesFieldG argsExistingJoins stringifyNumbers) fields
+  aggregates :: [(Int, (IR.FieldName, [Projection]))] <-
+    flip runReaderT (fromAlias selectFrom) $ sequence $ mapMaybe fromTableAggFieldG fields
   pure
     Select
-      { selectProjections
-      , selectTop = permissionBasedTop <> argsTop
-      , selectFrom = Just selectFrom
-      , selectJoins = argsJoins <> mapMaybe fieldSourceJoin fieldSources
-      , selectWhere = argsWhere <> Where [filterExpression]
-      , selectFor =
-          JsonFor ForJson {jsonCardinality = JsonSingleton, jsonRoot = Root "aggregate"}
-      , selectOrderBy = argsOrderBy
-      , selectOffset = argsOffset
+      { selectProjections =
+          concatMap snd $ sortBy (comparing fst) $
+            expss
+            <> mkNodesSelect     args' mforeignKeyConditions filterExpression permissionBasedTop selectFrom nodes
+            <> mkAggregateSelect args' mforeignKeyConditions selectFrom aggregates
+      , selectTop = NoTop
+      , selectFrom = pure $ FromOpenJson $ Aliased
+          { aliasedThing = OpenJson
+              { openJsonExpression = ValueExpression $ ODBC.TextValue "[0]"
+              , openJsonWith = Nothing
+              }
+          , aliasedAlias = existsFieldName
+          }
+      , selectJoins = mempty -- JOINs and WHEREs are only relevant in subselects
+      , selectWhere = mempty
+      , selectFor = JsonFor ForJson {jsonCardinality = JsonSingleton, jsonRoot = NoRoot}
+      , selectOrderBy = Nothing
+      , selectOffset = Nothing
       }
-  where
-    IR.AnnSelectG { _asnFields = fields
-                  , _asnFrom = from
-                  , _asnPerm = perm
-                  , _asnArgs = args
-                  , _asnStrfyNum = _num -- TODO: Do we ignore this for aggregates?
-                  } = annSelectG
-    IR.TablePerm {_tpLimit = mPermLimit, _tpFilter = permFilter} = perm
-    permissionBasedTop = maybe NoTop Top mPermLimit
 
 
 --------------------------------------------------------------------------------
@@ -376,11 +500,8 @@ unfurlAnnOrderByElement =
 --------------------------------------------------------------------------------
 -- Conversion functions
 
-tableNameText :: {-PG.QualifiedObject-} TableName -> Text
+tableNameText :: TableName -> Text
 tableNameText (TableName {tableName}) = tableName
--- tableNameText qualifiedObject = qname
---   where
---     PG.QualifiedObject {qName = PG.TableName qname} = qualifiedObject
 
 -- | This is really the start where you query the base table,
 -- everything else is joins attached to it.
@@ -390,15 +511,9 @@ fromQualifiedTable schemadTableName@(TableName{tableName}) = do
   pure
     (FromQualifiedTable
        (Aliased
-          { aliasedThing =
-             schemadTableName {-TableName {tableName = qname, tableNameSchema = schemaName}-}
+          { aliasedThing = schemadTableName
           , aliasedAlias = alias
           }))
-  -- where
-  --   PG.QualifiedObject { qSchema = PG.SchemaName schemaName
-  --                        -- TODO: Consider many x.y.z. in schema name.
-  --                       , qName = PG.TableName qname
-  --                       } = qualifiedObject
 
 fromTableName :: TableName -> FromIr EntityAlias
 fromTableName TableName{tableName} = do
@@ -508,25 +623,46 @@ data FieldSource
   | AggregateFieldSource [Aliased Aggregate]
   deriving (Eq, Show)
 
-fromTableAggregateFieldG ::
-     (IR.FieldName, IR.TableAggregateFieldG 'MSSQL (Const Void) Expression) -> ReaderT EntityAlias FromIr FieldSource
-fromTableAggregateFieldG (IR.FieldName name, field) =
-  case field of
-    IR.TAFAgg (aggregateFields :: [(IR.FieldName, IR.AggregateField 'MSSQL)]) -> do
-      aggregates <-
-        for aggregateFields \(fieldName, aggregateField) ->
-          fromAggregateField aggregateField <&> \aliasedThing ->
-            Aliased {aliasedAlias = IR.getFieldNameTxt fieldName, ..}
-      pure (AggregateFieldSource aggregates)
-    IR.TAFExp text ->
-      pure
-        (ExpressionFieldSource
-           Aliased
-             { aliasedThing = TSQL.ValueExpression (ODBC.TextValue text)
-             , aliasedAlias = name
-             })
-    IR.TAFNodes {} ->
-      refute (pure NodesUnsupportedForNow)
+-- | Get FieldSource from a TAFExp type table aggregate field
+fromTableExpFieldG :: -- TODO: Convert function to be similar to Nodes function
+  (Int, (IR.FieldName, IR.TableAggregateFieldG 'MSSQL (Const Void) Expression)) ->
+  Maybe (ReaderT EntityAlias FromIr (Int, [Projection]))
+fromTableExpFieldG = \case
+  (index, (IR.FieldName name, IR.TAFExp text)) -> Just $
+    pure $
+      (index, fieldSourceProjections $
+        ExpressionFieldSource
+          Aliased
+            { aliasedThing = TSQL.ValueExpression (ODBC.TextValue text)
+            , aliasedAlias = name
+            })
+  _ -> Nothing
+
+fromTableAggFieldG ::
+  (Int, (IR.FieldName, IR.TableAggregateFieldG 'MSSQL (Const Void) Expression)) ->
+  Maybe (ReaderT EntityAlias FromIr (Int, (IR.FieldName, [Projection])))
+fromTableAggFieldG = \case
+  (index, (fieldName, IR.TAFAgg (aggregateFields :: [(IR.FieldName, IR.AggregateField 'MSSQL)]))) -> Just do
+    aggregates <-
+      for aggregateFields \(fieldName', aggregateField) ->
+        fromAggregateField aggregateField <&> \aliasedThing ->
+          Aliased {aliasedAlias = IR.getFieldNameTxt fieldName', ..}
+    pure (index, (fieldName, fieldSourceProjections $ AggregateFieldSource aggregates))
+  _ -> Nothing
+
+
+fromTableNodesFieldG ::
+  Map TableName EntityAlias ->
+  StringifyNumbers ->
+  (Int, (IR.FieldName, IR.TableAggregateFieldG 'MSSQL (Const Void) Expression)) ->
+  Maybe (ReaderT EntityAlias FromIr (Int, (IR.FieldName, [Projection])))
+fromTableNodesFieldG argsExistingJoins stringifyNumbers = \case
+  (index, (fieldName, IR.TAFNodes () (annFieldsG :: [(IR.FieldName, IR.AnnFieldG 'MSSQL (Const Void) Expression)]))) -> Just do
+    fieldSources' <- fromAnnFieldsG argsExistingJoins stringifyNumbers `traverse` annFieldsG
+    let nodesProjections' :: [Projection] = concatMap fieldSourceProjections fieldSources'
+    pure (index, (fieldName, nodesProjections'))
+  _ -> Nothing
+
 
 fromAggregateField :: IR.AggregateField 'MSSQL -> ReaderT EntityAlias FromIr Aggregate
 fromAggregateField aggregateField =
@@ -623,11 +759,16 @@ fromAnnColumnField _stringifyNumbers annColumnField = do
   -- WKT format
   if typ == (IR.ColumnScalar GeometryType) || typ == (IR.ColumnScalar GeographyType)
      then pure $ MethodExpression (ColumnExpression fieldName) "STAsText" []
-     else pure (ColumnExpression fieldName)
+     else case caseBoolExpMaybe of
+            Nothing -> pure (ColumnExpression fieldName)
+            Just ex -> do
+                ex' <- (traverse fromAnnBoolExpFld >=> fromGBoolExp) (coerce ex)
+                pure (ConditionalProjection ex' fieldName)
   where
     IR.AnnColumnField { _acfInfo = IR.ColumnInfo{pgiColumn=pgCol,pgiType=typ}
                       , _acfAsText = _asText :: Bool
                       , _acfOp = _ :: Maybe (IR.ColumnOp 'MSSQL) -- TODO: What's this?
+                      , _acfCaseBoolExpression = caseBoolExpMaybe
                       } = annColumnField
 
 -- | This is where a field name "foo" is resolved to a fully qualified
@@ -763,11 +904,11 @@ fromArrayAggregateSelectG
   -> ReaderT EntityAlias FromIr Join
 fromArrayAggregateSelectG annRelationSelectG = do
   fieldName <- lift (fromRelName aarRelationshipName)
-  sel <- lift (fromSelectAggregate annSelectG)
-  joinSelect <-
-    do foreignKeyConditions <- selectFromMapping sel mapping
-       pure
-         sel {selectWhere = Where foreignKeyConditions <> selectWhere sel}
+  joinSelect <- do
+    lhsEntityAlias <- ask
+    -- With this, the foreign key relations are injected automatically
+    -- at the right place by fromSelectAggregate.
+    lift (fromSelectAggregate (pure (lhsEntityAlias, mapping)) annSelectG)
   alias <- lift (generateEntityAlias (ArrayAggregateTemplate fieldName))
   pure
     Join
@@ -778,7 +919,7 @@ fromArrayAggregateSelectG annRelationSelectG = do
       }
   where
     IR.AnnRelationSelectG { aarRelationshipName
-                          , aarColumnMapping = mapping :: HashMap ColumnName ColumnName-- PG.PGCol PG.PGCol
+                          , aarColumnMapping = mapping :: HashMap ColumnName ColumnName
                           , aarAnnSelect = annSelectG
                           } = annRelationSelectG
 
@@ -943,6 +1084,9 @@ jsonFieldName = "json"
 aggFieldName :: Text
 aggFieldName = "agg"
 
+aggSubselectName :: Text
+aggSubselectName = "agg_sub"
+
 existsFieldName :: Text
 existsFieldName = "exists_placeholder"
 
@@ -976,6 +1120,7 @@ generateEntityAlias template = do
 fromAlias :: From -> EntityAlias
 fromAlias (FromQualifiedTable Aliased {aliasedAlias}) = EntityAlias aliasedAlias
 fromAlias (FromOpenJson Aliased {aliasedAlias})       = EntityAlias aliasedAlias
+fromAlias (FromSelect Aliased {aliasedAlias})         = EntityAlias aliasedAlias
 
 columnNameToFieldName :: ColumnName -> EntityAlias -> FieldName
 columnNameToFieldName (ColumnName fieldName) EntityAlias {entityAliasText = fieldNameEntity} =

server/src-lib/Hasura/Backends/MSSQL/Instances/Execute.hs

@@ -169,7 +169,7 @@ multiplexRootReselect variables rootReselect =
                 OpenJson
                   { openJsonExpression =
                       ValueExpression (ODBC.TextValue $ lbsToTxt $ J.encode variables)
-                  , openJsonWith =
+                  , openJsonWith = Just $
                       NE.fromList
                         [ UuidField resultIdAlias (Just $ IndexPath RootPath 0)
                         , JsonField resultVarsAlias (Just $ IndexPath RootPath 1)
@@ -293,7 +293,7 @@ validateVariables sourceConfig sessionVariableValues prepState = do
 
         canaryQuery = if null projAll
                       then Nothing
-                      else Just $ renderQuery select {
+                      else Just $ renderQuery emptySelect {
                                       selectProjections = projAll,
                                       selectFrom = sessionOpenJson occSessionVars
                                       }
@@ -325,7 +325,7 @@ validateVariables sourceConfig sessionVariableValues prepState = do
                 (
                   OpenJson
                     (ValueExpression $ ODBC.TextValue $ lbsToTxt $ J.encode occSessionVars)
-                    (sessField <$> fields)
+                    (pure (sessField <$> fields))
                 )
                 "session"
 
@@ -334,4 +334,3 @@ validateVariables sourceConfig sessionVariableValues prepState = do
 
       sessionReference :: Text -> Aliased Expression
       sessionReference var = Aliased (ColumnExpression (TSQL.FieldName var "session")) var
-

server/src-lib/Hasura/Backends/MSSQL/Plan.hs

@@ -44,23 +44,7 @@ planQuery sessionVariables queryDB = do
   sel <-
     runValidate (runFromIr (fromRootField rootField))
     `onLeft` (throw400 NotSupported . tshow)
-  pure $
-    sel
-      { selectFor =
-          case selectFor sel of
-            NoFor           -> NoFor
-            JsonFor forJson ->
-              JsonFor forJson {jsonRoot =
-                                 case jsonRoot forJson of
-                                   NoRoot -> Root "root"
-                                   -- Keep whatever's there if already
-                                   -- specified. In the case of an
-                                   -- aggregate query, the root will
-                                   -- be specified "aggregate", for
-                                   -- example.
-                                   keep   -> keep
-                              }
-      }
+  pure sel
 
 -- | Prepare a value without any query planning; we just execute the
 -- query with the values embedded.

server/src-lib/Hasura/Backends/MSSQL/ToQuery.hs

@@ -59,7 +59,6 @@ instance IsString Printer where
 instance ToJSON Expression where
   toJSON = toJSON . T.toTxt . toQueryFlat . fromExpression
 
-
 --------------------------------------------------------------------------------
 -- Printer generators
 
@@ -109,6 +108,10 @@ fromExpression =
       "(" <+> fromExpression e <+> ")." <+>
       fromString (show op) <+>
       "(" <+> fromExpression str <+> ") = 1"
+    ConditionalProjection expression fieldName ->
+      "(CASE WHEN(" <+>
+      fromExpression expression <+>
+      ") THEN " <+> fromFieldName fieldName <+> " ELSE NULL END)"
 
 fromOp :: Op -> Printer
 fromOp =
@@ -289,7 +292,7 @@ fromFor =
   \case
     NoFor -> ""
     JsonFor ForJson {jsonCardinality} ->
-      "FOR JSON PATH" <+>
+      "FOR JSON PATH, INCLUDE_NULL_VALUES" <+>
       case jsonCardinality of
         JsonArray     -> ""
         JsonSingleton -> ", WITHOUT_ARRAY_WRAPPER"
@@ -327,9 +330,33 @@ fromCountable =
 fromWhere :: Where -> Printer
 fromWhere =
   \case
-    Where expressions ->
-      "WHERE " <+>
-      IndentPrinter 6 (fromExpression (AndExpression expressions))
+    Where expressions
+      | Just whereExp <- collapseWhere (AndExpression expressions) ->
+        "WHERE " <+> IndentPrinter 6 (fromExpression whereExp)
+      | otherwise -> ""
+
+-- Drop useless examples like this from the output:
+--
+-- WHERE (((1<>1))
+--       AND ((1=1)))
+--       AND ((1=1))
+--
+-- And
+--
+-- WHERE ((1<>1))
+--
+-- They're redundant, but make the output less readable.
+collapseWhere :: Expression -> Maybe Expression
+collapseWhere = go
+  where
+    go =
+      \case
+        ValueExpression (BoolValue True) -> Nothing
+        AndExpression xs ->
+          case mapMaybe go xs of
+            [] -> Nothing
+            ys -> pure (AndExpression ys)
+        e -> pure e
 
 fromFrom :: From -> Printer
 fromFrom =
@@ -337,6 +364,7 @@ fromFrom =
     FromQualifiedTable aliasedQualifiedTableName ->
       fromAliased (fmap fromTableName aliasedQualifiedTableName)
     FromOpenJson openJson -> fromAliased (fmap fromOpenJson openJson)
+    FromSelect select -> fromAliased (fmap (parens . fromSelect) select)
 
 fromOpenJson :: OpenJson -> Printer
 fromOpenJson OpenJson {openJsonExpression, openJsonWith} =
@@ -344,13 +372,14 @@ fromOpenJson OpenJson {openJsonExpression, openJsonWith} =
     NewlinePrinter
     [ "OPENJSON(" <+>
       IndentPrinter 9 (fromExpression openJsonExpression) <+> ")"
-    , "WITH (" <+>
-      IndentPrinter
-        5
-        (SepByPrinter
-           ("," <+> NewlinePrinter)
-           (toList (fmap fromJsonFieldSpec openJsonWith))) <+>
-      ")"
+    , case openJsonWith of
+        Nothing -> ""
+        Just openJsonWith' -> "WITH (" <+>
+          IndentPrinter
+            5
+            (SepByPrinter
+               ("," <+> NewlinePrinter)
+               (fmap fromJsonFieldSpec $ toList openJsonWith')) <+> ")"
     ]
 
 fromJsonFieldSpec :: JsonFieldSpec -> Printer
@@ -385,6 +414,9 @@ truePrinter = "(1=1)"
 falsePrinter :: Printer
 falsePrinter = "(1<>1)"
 
+parens :: Printer -> Printer
+parens p = "(" <+> IndentPrinter 1 p <+> ")"
+
 -- | Wrap a select with things needed when using FOR JSON.
 wrapFor :: For -> Printer -> Printer
 wrapFor for' inner = nullToArray

server/src-lib/Hasura/Backends/MSSQL/Types/Internal.hs

@@ -82,8 +82,8 @@ data Select = Select
   , selectOffset      :: !(Maybe Expression)
   }
 
-select :: Select
-select =
+emptySelect :: Select
+emptySelect =
   Select
     { selectFrom        = Nothing
     , selectTop         = NoTop
@@ -191,6 +191,7 @@ data Expression
   | ListExpression [Expression]
   | STOpExpression SpatialOp Expression Expression
   | CastExpression Expression Text
+  | ConditionalProjection Expression FieldName
 
 data JsonPath
   = RootPath
@@ -206,14 +207,16 @@ data Countable name
   = StarCountable
   | NonNullFieldCountable (NonEmpty name)
   | DistinctCountable (NonEmpty name)
+deriving instance Functor Countable
 
 data From
   = FromQualifiedTable (Aliased TableName)
   | FromOpenJson (Aliased OpenJson)
+  | FromSelect (Aliased Select)
 
 data OpenJson = OpenJson
   { openJsonExpression :: Expression
-  , openJsonWith       :: NonEmpty JsonFieldSpec
+  , openJsonWith       :: Maybe (NonEmpty JsonFieldSpec)
   }
 
 data JsonFieldSpec

server/tests-py/queries/explain/permissions_query_mssql.yaml

@@ -3,11 +3,15 @@ url: /v1/graphql/explain
 status: 200
 response:
   - field: user
-    sql:
-      "SELECT ISNULL((SELECT [t_user1].[id] AS [id],\n       [t_user1].[name] AS\
-      \ [name],\n       [t_user1].[age] AS [age]\nFROM [dbo].[user] AS [t_user1]\nWHERE\
-      \ ((((([t_user1].[id]) = ((N'1')))\n      OR ((([t_user1].[id]) IS NULL)\n  \
-      \    AND (((N'1')) IS NULL)))))\nFOR JSON PATH), '[]')"
+    sql: |-
+      SELECT ISNULL((SELECT [t_user1].[id] AS [id],
+             [t_user1].[name] AS [name],
+             [t_user1].[age] AS [age]
+      FROM [dbo].[user] AS [t_user1]
+      WHERE ((((([t_user1].[id]) = ((N'1')))
+            OR ((([t_user1].[id]) IS NULL)
+            AND (((N'1')) IS NULL)))))
+      FOR JSON PATH, INCLUDE_NULL_VALUES), '[]')
 query:
   user:
     X-Hasura-Role: user

server/tests-py/queries/explain/simple_query_mssql.yaml

@@ -3,9 +3,12 @@ url: /v1/graphql/explain
 status: 200
 response:
   - field: user
-    sql:
-      "SELECT ISNULL((SELECT [t_user1].[id] AS [id],\n       [t_user1].[name] AS\
-      \ [name],\n       [t_user1].[age] AS [age]\nFROM [dbo].[user] AS [t_user1]\nWHERE ((1=1))\nFOR JSON PATH), '[]')"
+    sql: |-
+      SELECT ISNULL((SELECT [t_user1].[id] AS [id],
+             [t_user1].[name] AS [name],
+             [t_user1].[age] AS [age]
+      FROM [dbo].[user] AS [t_user1]
+      FOR JSON PATH, INCLUDE_NULL_VALUES), '[]')
 query:
   query:
     query: |

server/tests-py/queries/graphql_query/basic/nodes_aggregates_conditions_mssql.yaml

@@ -0,0 +1,61 @@
+- description: Simple GraphQL object query on author, excercising multiple operations
+  url: /v1/graphql
+  status: 200
+  response:
+    data:
+      person_aggregate:
+        bar:
+        - id: 3
+          name: ' clarke '
+        - id: 2
+          name: ' Clarke '
+        foo:
+          count: 2
+          sum: 7
+  query:
+    query: |
+      query {
+        person_aggregate(offset: 1, order_by: {id: desc}, where: {id: {_gte: 2}}) {
+          bar:nodes {
+            id
+            name
+          }
+          foo:aggregate {
+            count
+            sum {
+              id
+            }
+          }
+        }
+      }
+
+
+- description: test that count aggregate works as expected
+  url: /v1/graphql
+  status: 200
+  response:
+    data:
+      author:
+      - articles_aggregate:
+          aggregate:
+            count: 2
+            max: 2
+      - articles_aggregate:
+          aggregate:
+            count: 1
+            max: 3
+  query:
+    query: |
+      query {
+        author {
+          articles_aggregate {
+            aggregate {
+              count(columns: author_id)
+              max {
+                id
+              }
+            }
+          }
+        }
+      }
+

server/tests-py/queries/graphql_query/basic/nodes_aggregates_mssql.yaml

@@ -0,0 +1,38 @@
+description: Simple GraphQL object query on author, excercising multiple operations
+url: /v1/graphql
+status: 200
+response:
+  data:
+    person_aggregate:
+      welp:
+        count: 4
+        sum: 10
+        min: 1
+      blah:
+      - id: 1
+        name: John\
+      - id: 2
+        name: ' Clarke '
+      - id: 3
+        name: ' clarke '
+      - id: 4
+        name: null
+query:
+  query: |
+    query {
+      person_aggregate {
+        welp: aggregate {
+          count
+          sum {
+            id
+          }
+          min {
+            id
+          }
+        }
+        blah: nodes {
+          id
+          name
+        }
+      }
+    }

server/tests-py/queries/graphql_query/basic/schema_setup_mssql.yaml

@@ -5,6 +5,15 @@ args:
   args:
     source: mssql
     sql: |
+
+        DROP TABLE IF EXISTS test_types;
+        DROP TABLE IF EXISTS article;
+        DROP TABLE IF EXISTS author;
+        DROP TABLE IF EXISTS person;
+        DROP TABLE IF EXISTS [user];
+        DROP TABLE IF EXISTS article_multi;
+        DROP TABLE IF EXISTS author_multi;
+
         CREATE TABLE test_types
         (
             c1_smallint smallint,
@@ -175,7 +184,8 @@ args:
         VALUES
             ('John\'),
             (' Clarke '),
-            (' clarke ');
+            (' clarke '),
+            (NULL);
 
         CREATE TABLE author_multi
         (

server/tests-py/queries/graphql_query/basic/schema_teardown_mssql.yaml

@@ -9,6 +9,6 @@ args:
       drop table article;
       drop table author;
       drop table [user];
-      --  TODO: https://github.com/hasura/graphql-engine-mono/issues/1435
-      -- drop table article_multi;
-      -- drop table author_multi;
+      drop table person;
+      drop table article_multi;
+      drop table author_multi;

server/tests-py/queries/graphql_query/basic/teardown_mssql.yaml

@@ -13,14 +13,14 @@ args:
   args:
     source: mssql
     table:
-      name: author
+      name: article
     cascade: true
 
 - type: mssql_untrack_table
   args:
     source: mssql
     table:
-      name: article
+      name: author
     cascade: true
 
 - type: mssql_untrack_table
@@ -40,13 +40,9 @@ args:
 - type: mssql_untrack_table
   args:
     source: mssql
-    table:
-      name: article_multi
+    table: author_multi
+    relationship: articles
     cascade: true
 
-- type: mssql_untrack_table
-  args:
-    source: mssql
-    table:
-      name: author_multi
-    cascade: true
+
+

server/tests-py/queries/graphql_query/boolexp/basic/select_bools_mssql.yaml

@@ -4,10 +4,7 @@ url: /v1/graphql
 status: 200
 response:
   data:
-    author:
-    - name: Author 1
-    - name: Author 2
-    - name: Author 3
+    author: []
 
 query:
   query: |

server/tests-py/queries/graphql_query/permissions/inherited_roles_mssql/basic_inherited_roles.yaml

@@ -0,0 +1,201 @@
+- description: test that when a role doesn't have access to a certain column in a row, it should throw an error
+  url: /v1/graphql
+  status: 200
+  headers:
+    X-Hasura-Role: editor
+  response:
+    errors:
+    - extensions:
+        path: $.selectionSet.author.selectionSet.followers
+        code: validation-failed
+      message: "field \"followers\" not found in type: 'author'"
+  query:
+    query: |
+      query {
+        author {
+          id
+          name
+          followers
+        }
+      }
+
+
+- description: test that only data and related data pertaining to the allowed rows is retrieved per role
+  url: /v1/graphql
+  status: 200
+  headers:
+    X-Hasura-Role: author
+    x-hasura-author-id: '1'
+  response:
+    data:
+      author:
+      - id: 1
+        name: J.K.Rowling
+        articles:
+        - content: content 1
+          title: title 1
+        - content: content 3
+          title: title 3
+  query:
+    query: |
+      query {
+        author {
+          id
+          name
+          articles {
+            content
+            title
+          }
+        }
+      }
+
+
+- description: test that role with no permissions on a table cannot access the table
+  url: /v1/graphql
+  status: 200
+  headers:
+    X-Hasura-Role: guest
+  response:
+    errors:
+    - extensions:
+        path: $.selectionSet.author
+        code: validation-failed
+      message: "field \"author\" not found in type: 'query_root'"
+  query:
+    query: |
+      query {
+        author {
+          id
+          articles {
+            content
+            title
+          }
+        }
+      }
+
+
+- description: test that role with specific permissions on a table cannot access the nonpermitted columns
+  url: /v1/graphql
+  status: 200
+  headers:
+    X-Hasura-Role: guest
+  response:
+   errors:
+   - extensions:
+       path: $.selectionSet.article.selectionSet.content
+       code: validation-failed
+     message: "field \"content\" not found in type: 'article'"
+  query:
+    query: |
+      query {
+        article {
+          content
+          title
+        }
+      }
+
+
+- description: test that a role with specific permissions cannot use nonpermitted columns for `where` conditions
+  url: /v1/graphql
+  status: 200
+  headers:
+    X-Hasura-Role: guest
+  response:
+    errors:
+    - extensions:
+        path: $.selectionSet.article.args.where.id
+        code: validation-failed
+      message: "field \"id\" not found in type: 'article_bool_exp'"
+  query:
+    query: |
+      query {
+        article (where: {id: {_lt: 3}}) {
+          title
+        }
+      }
+
+
+- description: test that role with filtered permissions is able to retrieve only allowed rows even across relationships
+  url: /v1/graphql
+  status: 200
+  headers:
+    X-Hasura-Role: author
+    X-Hasura-Author-Id: '1'
+  response:
+    data:
+      article:
+      - id: 1
+        title: title 1
+        author:
+          id: 1
+          name: J.K.Rowling
+      - id: 3
+        title: title 3
+        author:
+          id: 1
+          name: J.K.Rowling
+  query:
+    query: |
+      query {
+        article {
+          id
+          title
+          author {
+            id
+            name
+          }
+        }
+      }
+
+
+- description: test that nulls are obtained for columns that inherited role is not permitted to access
+  url: /v1/graphql
+  status: 200
+  headers:
+    X-Hasura-Role: author_and_editor
+    X-Hasura-Author-Id: '1'
+  response:
+    data:
+      author:
+      - id: 1
+        name: J.K.Rowling
+        followers: 1234
+      - id: 2
+        name: Paulo Coelho
+        followers: null
+      - id: 3
+        name: Murakami
+        followers: null
+  query:
+    query: |
+      query {
+        author {
+          id
+          name
+          followers
+        }
+      }
+
+
+- description: test that inherited roles work with limit-based select permissions for nodes
+  url: /v1/graphql
+  status: 200
+  headers:
+    x-hasura-role: limited_retrieval
+  query:
+    query: |
+      query {
+        article_aggregate {
+          nodes {
+            title
+          }
+        }
+      }
+  response:
+    data:
+      article_aggregate:
+        nodes:
+          - title: title 1
+          - title: title 2
+
+

server/tests-py/queries/graphql_query/permissions/inherited_roles_mssql/inherited_role_with_some_roles_having_no_permissions.yaml

@@ -0,0 +1,37 @@
+description: |
+  Suppose an inherited role `ir1` is created out of role1, role2 and role3.
+  role1 and role2 have some select permissions configured for a Table T and
+  role3 doesn't have any select permissions configured for T. In such cases,
+  the inherited role `ir1` should work as if the inherited role is created out
+  of only role1 and role2 or the inherited role's permissions should be only
+  constructed out of the permissions which exist for the underlying roles. In
+  this case, the `guest` role doesn't have select permissions configured for the
+  table `author`
+url: /v1/graphql
+status: 200
+response:
+  data:
+    author:
+    - id: 1
+      name: J.K.Rowling
+      followers: 1234
+    - id: 2
+      name: Paulo Coelho
+      followers: null
+    - id: 3
+      name: Murakami
+      followers: null
+headers:
+  X-Hasura-Role: author_editor_guest_inherited_role
+  X-Hasura-Author-Id: '1'
+  X-Hasura-Editor-Id: '1'
+query:
+  query: |
+    query {
+      author {
+        id
+        name
+        followers
+      }
+    }
+

server/tests-py/queries/graphql_query/permissions/inherited_roles_mssql/schema_setup_mssql.yaml

@@ -0,0 +1,34 @@
+type: bulk
+args:
+- type: mssql_run_sql
+  args:
+    source: mssql
+    sql: |
+
+      DROP TABLE IF EXISTS article; -- article first because of foreign key constraint
+      DROP TABLE IF EXISTS author;
+
+      CREATE TABLE author (
+        id int identity(1,1) primary key,
+        name nvarchar(255),
+        followers int
+      );
+
+      CREATE TABLE article (
+        id int identity(1,1)  primary key,
+        title nvarchar(255),
+        content nvarchar(255),
+        author_id int foreign key references author(id)
+      );
+
+      insert into author (name, followers) values
+      ('J.K.Rowling', 1234),
+      ('Paulo Coelho', 123),
+      ('Murakami', 12);
+
+      insert into article (title, content, author_id) values
+      ('title 1', 'content 1', 1),
+      ('title 2', 'content 2', 2),
+      ('title 3', 'content 3', 1),
+      ('title 4', 'content 4', 3),
+      ('title 5', 'content 5', 2);

server/tests-py/queries/graphql_query/permissions/inherited_roles_mssql/schema_teardown_mssql.yaml

@@ -0,0 +1,10 @@
+type: bulk
+args:
+- type: mssql_run_sql
+  args:
+    source: mssql
+    cascade: true
+    sql: |
+      DROP TABLE article;
+      DROP TABLE author;
+

server/tests-py/queries/graphql_query/permissions/inherited_roles_mssql/setup_mssql.yaml

@@ -0,0 +1,123 @@
+type: bulk
+args:
+
+# Tables
+- type: mssql_track_table
+  args:
+    source: mssql
+    table: author
+
+- type: mssql_track_table
+  args:
+    source: mssql
+    table: article
+
+# Relationships
+- type: mssql_create_object_relationship
+  args:
+    source: mssql
+    table: article
+    name: author
+    using:
+      foreign_key_constraint_on: author_id
+
+- type: mssql_create_array_relationship
+  args:
+    source: mssql
+    table: author
+    name: articles
+    using:
+      foreign_key_constraint_on:
+        table: article
+        column: author_id
+
+# Permissions
+- type: mssql_create_select_permission
+  args:
+    role: author
+    source: mssql
+    table: author
+    permission:
+      columns:
+        - id
+        - name
+        - followers
+      allow_aggregations: false
+      filter:
+        id: X-Hasura-Author-Id
+
+- type: mssql_create_select_permission
+  args:
+    role: author
+    table: article
+    source: mssql
+    permission:
+      columns: "*"
+      allow_aggregations: true
+      filter:
+        author_id: X-Hasura-Author-Id
+
+- type: mssql_create_select_permission
+  args:
+    role: editor
+    table: author
+    source: mssql
+    permission:
+      columns:
+        - id
+        - name
+      allow_aggregations: true
+      filter: {}
+
+- type: mssql_create_select_permission
+  args:
+    role: editor
+    table: article
+    source: mssql
+    permission:
+      columns: "*"
+      filter: {}
+
+- type: mssql_create_select_permission
+  args:
+    role: guest
+    table: article
+    source: mssql
+    permission:
+      columns:
+        - title
+      allow_aggregations: true
+      filter: {}
+
+- type: mssql_create_select_permission
+  args:
+    role: limited_retrieval
+    table: article
+    source: mssql
+    permission:
+      columns:
+        - id
+        - title
+        - content
+      allow_aggregations: true
+      limit: 2
+      filter: {}
+
+# Roles
+- type: add_inherited_role
+  args:
+    role_name: author_editor_guest_inherited_role
+    source: mssql
+    role_set:
+      - author
+      - editor
+      - guest
+
+- type: add_inherited_role
+  args:
+    role_name: author_and_editor
+    source: mssql
+    role_set:
+      - author
+      - editor
+

server/tests-py/queries/graphql_query/permissions/inherited_roles_mssql/teardown_mssql.yaml

@@ -0,0 +1,32 @@
+type: bulk
+args:
+
+  - type: drop_inherited_role
+    args:
+      role_name: author_editor_guest_inherited_role
+
+  - type: drop_inherited_role
+    args:
+      role_name: author_and_editor
+
+  - type: mssql_drop_relationship
+    args:
+      source: mssql
+      table: article
+      relationship: author
+
+  - type: mssql_drop_relationship
+    args:
+      source: mssql
+      table: author
+      relationship: articles
+
+  - type: mssql_untrack_table
+    args:
+      source: mssql
+      table: article
+
+  - type: mssql_untrack_table
+    args:
+      source: mssql
+      table: author

server/tests-py/test_graphql_queries.py

@@ -173,6 +173,7 @@ class TestGraphQLQueryBasicCommon:
 @pytest.mark.parametrize("backend", ['mssql'])
 @usefixtures('per_class_tests_db_state')
 class TestGraphQLQueryBasicMSSQL:
+
     def test_select_various_mssql_types(self, hge_ctx, transport):
         check_query_f(hge_ctx, self.dir() + '/select_query_test_types_mssql.yaml', transport)
 
@@ -182,6 +183,12 @@ class TestGraphQLQueryBasicMSSQL:
     def test_select_query_user_col_change(self, hge_ctx, transport):
         check_query_f(hge_ctx, self.dir() + "/select_query_user_col_change_mssql.yaml")
 
+    def test_nodes_aggregates_mssql(self, hge_ctx, transport):
+        check_query_f(hge_ctx, self.dir() + "/nodes_aggregates_mssql.yaml", transport)
+
+    def test_nodes_aggregates_conditions_mssql(self, hge_ctx, transport):
+        check_query_f(hge_ctx, self.dir() + "/nodes_aggregates_conditions_mssql.yaml", transport)
+
     @classmethod
     def dir(cls):
         return 'queries/graphql_query/basic'
@@ -493,7 +500,6 @@ class TestGraphQLQueryBoolExpBasicMSSQL:
     def test_uuid_test_in_uuid_col(self, hge_ctx, transport):
         check_query_f(hge_ctx, self.dir() + '/select_uuid_test_in_uuid_col_mssql.yaml', transport)
 
-    @pytest.mark.skip(reason="TODO: https://github.com/hasura/graphql-engine-mono/issues/1438")
     def test_bools(self, hge_ctx, transport):
         check_query_f(hge_ctx, self.dir() + '/select_bools_mssql.yaml', transport)
 
@@ -582,7 +588,7 @@ class TestGraphqlQueryPermissions:
 
 @pytest.mark.parametrize('transport', ['http', 'websocket'])
 @use_inherited_roles_fixtures
-class TestGraphQLInheritedRoles:
+class TestGraphQLInheritedRolesPostgres:
 
     @classmethod
     def dir(cls):
@@ -596,6 +602,21 @@ class TestGraphQLInheritedRoles:
     def test_inherited_role_when_some_roles_may_not_have_permission_configured(self, hge_ctx, transport):
         check_query_f(hge_ctx, self.dir() + '/inherited_role_with_some_roles_having_no_permissions.yaml')
 
+@pytest.mark.parametrize('transport', ['http', 'websocket'])
+@pytest.mark.parametrize('backend', ['mssql'])
+@usefixtures('per_backend_tests', 'inherited_role_fixtures', 'per_class_tests_db_state')
+class TestGraphQLInheritedRolesMSSQL:
+
+    @classmethod
+    def dir(cls):
+        return 'queries/graphql_query/permissions/inherited_roles_mssql'
+
+    def test_basic_inherited_role(self, hge_ctx, transport):
+        check_query_f(hge_ctx, self.dir() + '/basic_inherited_roles.yaml')
+
+    def test_inherited_role_when_some_roles_may_not_have_permission_configured(self, hge_ctx, transport):
+        check_query_f(hge_ctx, self.dir() + '/inherited_role_with_some_roles_having_no_permissions.yaml')
+
 
 @pytest.mark.parametrize("transport", ['http', 'websocket', 'subscription'])
 @pytest.mark.parametrize("backend", ['postgres', 'mssql'])