replace SELECT FROM with SELECT 1 FROM to fix count aggregates for cockroachdb

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5787
GitOrigin-RevId: f5f01a64ccffa36c93a0a88228ac927fbd3aa168

server/src-lib/Hasura/App.hs

@@ -3,6 +3,7 @@
 {-# LANGUAGE UndecidableInstances #-}
 {-# LANGUAGE ViewPatterns #-}
 
+-- | Imported by 'server/src-exec/Main.hs'.
 module Hasura.App
   ( ExitCode (DatabaseMigrationError, DowngradeProcessError, MetadataCleanError, MetadataExportError, SchemaCacheInitError),
     ExitException (ExitException),
@@ -189,6 +190,7 @@ throwErrJExit reason = liftIO . throwIO . ExitException reason . BLC.toStrict .
 -- TODO(SOLOMON): Move Into `Hasura.Server.Init`. Unable to do so
 -- currently due `throwErrExit`.
 
+-- | Parse cli arguments to graphql-engine executable.
 parseArgs :: EnabledLogTypes impl => IO (HGEOptions (ServeOptions impl))
 parseArgs = do
   rawHGEOpts <- execParser opts

server/src-lib/Hasura/Backends/Postgres/SQL/DML.hs

@@ -49,6 +49,7 @@ module Hasura.Backends.Postgres.SQL.DML
     TypeAnn (TypeAnn),
     ValuesExp (ValuesExp),
     WhereFrag (WhereFrag),
+    dummySelectList,
     applyJsonBuildArray,
     applyJsonBuildObj,
     applyRowToJson,
@@ -152,6 +153,11 @@ mkSelect =
     Nothing
     Nothing
 
+-- | A dummy select list to avoid an empty select list, which doesn't work for cockroach db.
+--   This is just the value @1@ without an alias.
+dummySelectList :: [Extractor]
+dummySelectList = [Extractor (SEUnsafe "1") Nothing]
+
 newtype LimitExp
   = LimitExp SQLExp
   deriving (Show, Eq, NFData, Data, Cacheable, Hashable)
@@ -907,7 +913,7 @@ mkExists :: FromItem -> BoolExp -> BoolExp
 mkExists fromItem whereFrag =
   BEExists
     mkSelect
-      { selExtr = [Extractor (SEUnsafe "1") Nothing],
+      { selExtr = dummySelectList,
         selFrom = Just $ FromExp $ pure fromItem,
         selWhere = Just $ WhereFrag whereFrag
       }

server/src-lib/Hasura/Backends/Postgres/Translate/Select/Aggregate.hs

@@ -33,6 +33,9 @@ selectAggregateQuerySQL ::
   Query
 selectAggregateQuerySQL = fromBuilder . toSQL . mkAggregateSelect
 
+-- | We process aggregate queries differently because the types of aggregate queries are different.
+--   In the '_asnFields' field of an 'AnnSelectG', we will have a 'TableAggregateFieldG' instead
+--   of an 'AnnFieldG'.
 mkAggregateSelect ::
   forall pgKind.
   (Backend ('Postgres pgKind), PostgresAnnotatedFieldJSON pgKind) =>
@@ -43,7 +46,9 @@ mkAggregateSelect annAggSel =
         runWriter $
           flip runReaderT strfyNum $
             processAnnAggregateSelect sourcePrefixes rootFieldName annAggSel
+      -- select the relevant columns and subquery we want to aggregate
       selectNode = SelectNode nodeExtractors joinTree
+      -- aggregate the results into a top-level return value
       arrayNode = MultiRowSelectNode [topExtractor] selectNode
    in renameIdentifiers $
         generateSQLSelectFromArrayNode selectSource arrayNode $ BELit True

server/src-lib/Hasura/Backends/Postgres/Translate/Select/Internal/GenerateSelect.hs

@@ -31,14 +31,21 @@ import Hasura.Prelude
 import Hasura.RQL.IR.Select (ConnectionSlice (SliceFirst, SliceLast))
 
 generateSQLSelect ::
-  -- | Pre join condition
+  -- | Pre join condition for lateral joins
   S.BoolExp ->
   SelectSource ->
   SelectNode ->
   S.Select
 generateSQLSelect joinCondition selectSource selectNode =
   S.mkSelect
-    { S.selExtr = [S.Extractor e $ Just a | (a, e) <- HM.toList extractors],
+    { S.selExtr =
+        case [S.Extractor e $ Just a | (a, e) <- HM.toList extractors] of
+          -- If the select list is empty we will generated code which looks like this:
+          -- > SELECT FROM ...
+          -- This works for postgres, but not for cockroach, which expects a non-empty
+          -- select list. So we add a dummy value:
+          [] -> S.dummySelectList -- SELECT 1 FROM ...
+          exts -> exts,
       S.selFrom = Just $ S.FromExp [joinedFrom],
       S.selOrderBy = nodeOrderBy,
       S.selLimit = S.LimitExp . S.intToSQLExp <$> _ssLimit nodeSlicing,
@@ -128,22 +135,23 @@ generateSQLSelect joinCondition selectSource selectNode =
               }
        in S.mkLateralFromItem select alias
 
+-- | Create a select query
 generateSQLSelectFromArrayNode ::
   SelectSource ->
   MultiRowSelectNode ->
   S.BoolExp ->
   S.Select
-generateSQLSelectFromArrayNode selectSource arraySelectNode joinCondition =
+generateSQLSelectFromArrayNode selectSource (MultiRowSelectNode topExtractors selectNode) joinCondition =
   S.mkSelect
     { S.selExtr = topExtractors,
-      S.selFrom = Just $ S.FromExp [selectFrom]
+      S.selFrom =
+        Just $
+          S.FromExp
+            [ S.mkSelFromItem
+                (generateSQLSelect joinCondition selectSource selectNode)
+                $ S.toTableAlias $ _ssPrefix selectSource
+            ]
     }
-  where
-    MultiRowSelectNode topExtractors selectNode = arraySelectNode
-    selectFrom =
-      S.mkSelFromItem
-        (generateSQLSelect joinCondition selectSource selectNode)
-        $ S.toTableAlias $ _ssPrefix selectSource
 
 mkJoinCond :: S.TableAlias -> HashMap PGCol PGCol -> S.BoolExp
 mkJoinCond baseTablepfx colMapn =

server/src-lib/Hasura/Backends/Postgres/Translate/Types.hs

@@ -133,7 +133,7 @@ data SelectNode = SelectNode
   { _snExtractors :: HM.HashMap Postgres.ColumnAlias Postgres.SQLExp,
     _snJoinTree :: JoinTree
   }
-  deriving stock (Eq)
+  deriving stock (Eq, Show)
 
 instance Semigroup SelectNode where
   SelectNode lExtrs lJoinTree <> SelectNode rExtrs rJoinTree =
@@ -168,7 +168,7 @@ data ObjectRelationSource = ObjectRelationSource
     _orsRelationMapping :: HM.HashMap Postgres.PGCol Postgres.PGCol,
     _orsSelectSource :: ObjectSelectSource
   }
-  deriving (Generic)
+  deriving (Generic, Show)
 
 instance Hashable ObjectRelationSource
 
@@ -179,7 +179,7 @@ data ArrayRelationSource = ArrayRelationSource
     _arsRelationMapping :: HM.HashMap Postgres.PGCol Postgres.PGCol,
     _arsSelectSource :: SelectSource
   }
-  deriving (Generic)
+  deriving (Generic, Show)
 
 instance Hashable ArrayRelationSource
 
@@ -189,7 +189,7 @@ data MultiRowSelectNode = MultiRowSelectNode
   { _mrsnTopExtractors :: [Postgres.Extractor],
     _mrsnSelectNode :: SelectNode
   }
-  deriving stock (Eq)
+  deriving stock (Eq, Show)
 
 instance Semigroup MultiRowSelectNode where
   MultiRowSelectNode lTopExtrs lSelNode <> MultiRowSelectNode rTopExtrs rSelNode =
@@ -214,7 +214,7 @@ data ArrayConnectionSource = ArrayConnectionSource
     _acsSlice :: Maybe ConnectionSlice,
     _acsSource :: SelectSource
   }
-  deriving (Generic)
+  deriving (Generic, Show)
 
 deriving instance Eq ArrayConnectionSource
 
@@ -226,7 +226,7 @@ data JoinTree = JoinTree
     _jtArrayConnections :: HM.HashMap ArrayConnectionSource MultiRowSelectNode,
     _jtComputedFieldTableSets :: HM.HashMap ComputedFieldTableSetSource MultiRowSelectNode
   }
-  deriving stock (Eq)
+  deriving stock (Eq, Show)
 
 instance Semigroup JoinTree where
   JoinTree lObjs lArrs lArrConns lCfts <> JoinTree rObjs rArrs rArrConns rCfts =

server/tests-hspec/Test/Queries/AggregationSpec.hs

@@ -12,6 +12,7 @@ import Data.Aeson (Value)
 import Data.List.NonEmpty qualified as NE
 import Harness.Backend.BigQuery qualified as BigQuery
 import Harness.Backend.Citus qualified as Citus
+import Harness.Backend.Cockroach qualified as Cockroach
 import Harness.Backend.Postgres qualified as Postgres
 import Harness.Backend.Sqlserver qualified as Sqlserver
 import Harness.GraphqlEngine (postGraphql)
@@ -50,6 +51,17 @@ spec = do
                 [ Citus.setupTablesAction schema testEnv
                 ]
             },
+          (Fixture.fixture $ Fixture.Backend Fixture.Cockroach)
+            { Fixture.setupTeardown = \(testEnv, _) ->
+                [ Cockroach.setupTablesAction schema testEnv
+                ],
+              Fixture.customOptions =
+                Just $
+                  Fixture.defaultOptions
+                    { Fixture.stringifyNumbers = True,
+                      Fixture.skipTests = Just "Cockroach disabled pending oid 114 fix https://github.com/hasura/graphql-engine-mono/issues/5984"
+                    }
+            },
           (Fixture.fixture $ Fixture.Backend Fixture.SQLServer)
             { Fixture.setupTeardown = \(testEnv, _) ->
                 [ Sqlserver.setupTablesAction schema testEnv
@@ -205,3 +217,32 @@ tests opts = do
             |]
 
       actual `shouldBe` expected
+
+    it "Fetch aggregated count only" \testEnvironment -> do
+      let schemaName :: Schema.SchemaName
+          schemaName = Schema.getSchemaName testEnvironment
+
+      let expected :: Value
+          expected =
+            [interpolateYaml|
+              data:
+                #{schemaName}_article_aggregate:
+                  aggregate:
+                    count: 4
+            |]
+
+          actual :: IO Value
+          actual =
+            postGraphql
+              testEnvironment
+              [graphql|
+                query {
+                  #{schemaName}_article_aggregate {
+                   aggregate {
+                      count
+                    }
+                  }
+                }
+              |]
+
+      actual `shouldBe` expected