server/postgres: LIMIT 1 on object relationships

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3489
Co-authored-by: Abby Sassel <3883855+sassela@users.noreply.github.com>
GitOrigin-RevId: f09a75508545cdbc34cf8728fad95bbb00bde018

.circleci/server-upgrade-downgrade/run.sh

@@ -198,6 +198,8 @@ get_server_upgrade_tests() {
 		--deselect test_schema_stitching.py::TestAddRemoteSchemaTbls::test_add_conflicting_table \
 		--deselect test_events.py \
 		--deselect test_graphql_queries.py::TestGraphQLQueryFunctions \
+		--deselect test_graphql_queries.py::TestGraphQLExplainCommon::test_limit_orderby_relationship_query \
+		--deselect test_graphql_queries.py::TestGraphQLExplainCommon::test_limit_offset_orderby_relationship_query \
 		  1>/dev/null 2>/dev/null
 	set +x
 	# Choose the subset of jobs to run based on possible parallelism in this buildkite job
@@ -235,6 +237,8 @@ run_server_upgrade_pytest() {
 			--deselect test_graphql_mutations.py::TestGraphqlInsertPermission::test_user_with_no_backend_privilege \
 			--deselect test_graphql_mutations.py::TestGraphqlMutationCustomSchema::test_update_article \
 			--deselect test_graphql_queries.py::TestGraphQLQueryEnums::test_introspect_user_role \
+			--deselect test_graphql_queries.py::TestGraphQLExplainCommon::test_limit_orderby_relationship_query \
+			--deselect test_graphql_queries.py::TestGraphQLExplainCommon::test_limit_offset_orderby_relationship_query \
 			-v $tests_to_run
 		set +x
 		cd -

CHANGELOG.md

@@ -115,6 +115,7 @@ count (
 - server: implement update mutations for MS SQL Server (closes #7834)
 - server: support nested output object types in actions (#4796)
 - server: action webhook requests now include a User-Agent header (fix #8070)
+- server: postgres: return a single entry per row (selected randomly) when an object relationship is misconfigured one-to-many (fix #7936).
 - console: action/event trigger transforms are now called REST connectors
 - console: fix list of tables (and schemas) being unsorted when creating a new trigger event (fix #6391)
 - console: fix custom field names breaking browse table sorting and the pre-populating of the edit row form

server/graphql-engine.cabal

@@ -995,6 +995,7 @@ test-suite tests-hspec
       Test.HelloWorldSpec
       Test.LimitOffsetSpec
       Test.ObjectRelationshipsSpec
+      Test.ObjectRelationshipsLimitSpec
       Test.OrderingSpec
       Test.ServiceLivenessSpec
       Test.ViewsSpec

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

@@ -149,7 +149,18 @@ instance Hashable ObjectSelectSource
 
 objectSelectSourceToSelectSource :: ObjectSelectSource -> SelectSource
 objectSelectSourceToSelectSource ObjectSelectSource {..} =
-  SelectSource _ossPrefix _ossFrom _ossWhere noSortingAndSlicing
+  SelectSource _ossPrefix _ossFrom _ossWhere sortingAndSlicing
+  where
+    sortingAndSlicing = SortingAndSlicing noSorting limit1
+    noSorting = NoSorting Nothing
+    -- We specify 'LIMIT 1' here to mitigate misconfigured object relationships with an
+    -- unexpected one-to-many/many-to-many relationship, instead of the expected one-to-one/many-to-one relationship.
+    -- Because we can't detect this misconfiguration statically (it depends on the data),
+    -- we force a single (or null) result instead by adding 'LIMIT 1'.
+    -- Which result is returned might be non-deterministic (though only in misconfigured cases).
+    -- Proper one-to-one/many-to-one object relationships should not be semantically affected by this.
+    -- See: https://github.com/hasura/graphql-engine/issues/7936
+    limit1 = SelectSlicing (Just 1) Nothing
 
 data ObjectRelationSource = ObjectRelationSource
   { _orsRelationshipName :: !RelName,

server/tests-hspec/Test/ObjectRelationshipsLimitSpec.hs

@@ -0,0 +1,325 @@
+{-# LANGUAGE QuasiQuotes #-}
+{-# LANGUAGE RecordWildCards #-}
+
+-- | Testing manual, misconfigured object relationships.
+--   Specifically, having manual relationships with one-to-many relationships.
+--   Test case for bug reported at https://github.com/hasura/graphql-engine/issues/7936
+module Test.ObjectRelationshipsLimitSpec (spec) where
+
+import Harness.Backend.Postgres as Postgres
+import Harness.GraphqlEngine qualified as GraphqlEngine
+import Harness.Quoter.Graphql
+import Harness.Quoter.Sql
+import Harness.Quoter.Yaml
+import Harness.State (State)
+import Harness.Test.Feature qualified as Feature
+import Test.Hspec
+import Prelude
+
+--------------------------------------------------------------------------------
+
+-- * Preamble
+
+spec :: SpecWith State
+spec =
+  Feature.feature
+    Feature.Feature
+      { Feature.backends =
+          [ Feature.Backend
+              { name = "Postgres",
+                setup = postgresSetup,
+                teardown = postgresTeardown
+              }
+          ],
+        Feature.tests = tests
+      }
+
+--------------------------------------------------------------------------------
+
+-- * Tests
+
+-- | Many of these may return non-deterministic results because graphql-engine
+--   need to choose one of the available rows in a misconfigured manual relationship
+--   that has a one-to-many relationship instead of the expected one-to-one.
+--
+--   Because of that, we use 'shouldReturnOneOfYaml' and list all of the possible (valid)
+--   expected results.
+tests :: SpecWith State
+tests = do
+  it "Query by id" $ \state ->
+    shouldReturnOneOfYaml
+      ( GraphqlEngine.postGraphql
+          state
+          [graphql|
+query {
+  hasura_article(where: {id: {_eq: 1}}) {
+    id
+    author {
+      id
+    }
+  }
+}
+|]
+      )
+      [ [yaml|
+data:
+  hasura_article:
+  - id: 1
+    author:
+      id: 1
+|],
+        [yaml|
+data:
+  hasura_article:
+  - id: 1
+    author:
+      id: 3
+|]
+      ]
+
+  it "Query limit 2" $ \state ->
+    shouldReturnOneOfYaml
+      ( GraphqlEngine.postGraphql
+          state
+          [graphql|
+query {
+  hasura_article(limit: 2) {
+    id
+    author {
+      id
+    }
+  }
+}
+|]
+      )
+      [ [yaml|
+data:
+  hasura_article:
+  - id: 1
+    author:
+      id: 1
+  - id: 2
+    author:
+      id: 2
+|],
+        [yaml|
+data:
+  hasura_article:
+  - id: 1
+    author:
+      id: 3
+  - id: 2
+    author:
+      id: 2
+|]
+      ]
+
+  it "where author name" $ \state ->
+    shouldReturnOneOfYaml
+      ( GraphqlEngine.postGraphql
+          state
+          [graphql|
+query {
+  hasura_article(where: { author: { name: { _eq: "Author 1" } } }) {
+    id
+    author {
+      id
+    }
+  }
+}
+|]
+      )
+      [ [yaml|
+data:
+  hasura_article:
+  - id: 1
+    author:
+      id: 1
+|],
+        [yaml|
+data:
+  hasura_article:
+  - id: 1
+    author:
+      id: 3
+|]
+      ]
+
+  it "order by author id" $ \state ->
+    shouldReturnOneOfYaml
+      ( GraphqlEngine.postGraphql
+          state
+          [graphql|
+query {
+  hasura_article(order_by: {author: {id: asc}}) {
+    id
+    author {
+      id
+    }
+  }
+}
+|]
+      )
+      [ [yaml|
+data:
+  hasura_article:
+  - id: 1
+    author:
+      id: 1
+  - id: 2
+    author:
+      id: 2
+|],
+        [yaml|
+data:
+  hasura_article:
+  - id: 2
+    author:
+      id: 2
+  - id: 1
+    author:
+      id: 3
+|]
+      ]
+
+  it "count articles" $ \state ->
+    shouldReturnYaml
+      ( GraphqlEngine.postGraphql
+          state
+          [graphql|
+query {
+  hasura_article_aggregate {
+    aggregate {
+      count
+    }
+  }
+}
+|]
+      )
+      [yaml|
+data:
+  hasura_article_aggregate:
+    aggregate:
+      count: 2
+|]
+
+--------------------------------------------------------------------------------
+
+-- * Postgres backend
+
+----------------------
+
+-- ** Setup
+
+postgresSetup :: State -> IO ()
+postgresSetup state = do
+  -- Clear and reconfigure the metadata
+  GraphqlEngine.setSource state Postgres.defaultSourceMetadata
+  postgresSetupTables
+  postgresInsertValues
+  postgresTrackTables state
+  postgresCreateRelationships state
+
+postgresSetupTables :: IO ()
+postgresSetupTables = do
+  -- Setup tables
+  Postgres.run_
+    [sql|
+CREATE TABLE hasura.author
+(
+    id SERIAL PRIMARY KEY,
+    name TEXT,
+    created_at TIMESTAMP
+);
+|]
+
+  Postgres.run_
+    [sql|
+CREATE TABLE hasura.article (
+   id SERIAL PRIMARY KEY,
+   author_name TEXT
+);
+|]
+
+postgresInsertValues :: IO ()
+postgresInsertValues = do
+  Postgres.run_
+    [sql|
+INSERT INTO hasura.author
+    (name, created_at)
+VALUES
+    ( 'Author 1', '2017-09-21 09:39:44' ),
+    ( 'Author 2', '2017-09-21 09:50:44' ),
+    ( 'Author 1', '2017-09-21 09:55:44' );
+|]
+
+  Postgres.run_
+    [sql|
+INSERT INTO hasura.article
+    (author_name)
+VALUES
+    ( 'Author 1' ),
+    ( 'Author 2' );
+|]
+
+postgresTrackTables :: State -> IO ()
+postgresTrackTables state = do
+  GraphqlEngine.post_
+    state
+    "/v1/metadata"
+    [yaml|
+type: pg_track_table
+args:
+  source: postgres
+  table:
+    schema: hasura
+    name: author
+|]
+  GraphqlEngine.post_
+    state
+    "/v1/metadata"
+    [yaml|
+type: pg_track_table
+args:
+  source: postgres
+  table:
+    schema: hasura
+    name: article
+|]
+
+postgresCreateRelationships :: State -> IO ()
+postgresCreateRelationships state = do
+  GraphqlEngine.post_
+    state
+    "/v1/metadata"
+    [yaml|
+type: pg_create_object_relationship
+args:
+  source: postgres
+  table:
+    schema: hasura
+    name: article
+  name: author
+  using:
+    manual_configuration:
+       remote_table:
+         schema: hasura
+         name: author
+       column_mapping:
+         author_name: name
+|]
+
+----------------------
+
+-- ** Teardown
+
+postgresTeardown :: State -> IO ()
+postgresTeardown _ = do
+  Postgres.run_
+    [sql|
+DROP TABLE hasura.article;
+|]
+  Postgres.run_
+    [sql|
+DROP TABLE hasura.author;
+|]

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

@@ -20,5 +20,5 @@ response:
     AS "_3_e"      ) ) AS "root" FROM  (SELECT  *  FROM "public"."article"  WHERE
     (''true'')     ) AS "_0_root.base" LEFT OUTER JOIN LATERAL (SELECT  "_1_root.or.author.base"."id"
     AS "root.or.author.pg.id" FROM  (SELECT  *  FROM "public"."author"  WHERE (("_0_root.base"."author_id")
-    = ("id"))     ) AS "_1_root.or.author.base"      ) AS "_2_root.or.author" ON (''true'')    ORDER
+    = ("id"))    LIMIT 1 ) AS "_1_root.or.author.base"      ) AS "_2_root.or.author" ON (''true'')    ORDER
     BY "root.or.author.pg.id" DESC NULLS FIRST LIMIT 3 OFFSET 2) AS "_4_root"      '

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

@@ -20,5 +20,5 @@ response:
     AS "_3_e"      ) ) AS "root" FROM  (SELECT  *  FROM "public"."article"  WHERE
     (''true'')     ) AS "_0_root.base" LEFT OUTER JOIN LATERAL (SELECT  "_1_root.or.author.base"."id"
     AS "root.or.author.pg.id" FROM  (SELECT  *  FROM "public"."author"  WHERE (("_0_root.base"."author_id")
-    = ("id"))     ) AS "_1_root.or.author.base"      ) AS "_2_root.or.author" ON (''true'')    ORDER
+    = ("id"))    LIMIT 1 ) AS "_1_root.or.author.base"      ) AS "_2_root.or.author" ON (''true'')    ORDER
     BY "root.or.author.pg.id" DESC NULLS FIRST LIMIT 2 ) AS "_4_root"      '

server/tests-py/test_graphql_queries.py

@@ -1249,7 +1249,7 @@ class TestFallbackUnauthorizedRoleCookie:
     @classmethod
     def dir(cls):
         return 'queries/unauthorized_role'
-    
+
     def test_fallback_unauth_role_jwt_cookie_not_set(self, hge_ctx, transport):
         check_query_f(hge_ctx, self.dir() + '/cookie_header_absent_unauth_role_set.yaml', transport, add_auth=False)
 
@@ -1263,7 +1263,7 @@ class TestMissingUnauthorizedRoleAndCookie:
     @classmethod
     def dir(cls):
         return 'queries/unauthorized_role'
-    
+
     def test_error_unauth_role_not_set_jwt_cookie_not_set(self, hge_ctx, transport):
         check_query_f(hge_ctx, self.dir() + '/cookie_header_absent_unauth_role_not_set.yaml', transport, add_auth=False)
 
@@ -1297,9 +1297,13 @@ class TestGraphQLExplainCommon:
     def test_limit_orderby_column_query(self, hge_ctx):
         self.with_admin_secret("query", hge_ctx, self.dir() + '/limit_orderby_column_query.yaml')
 
+    @pytest.mark.skip_server_upgrade_test
+    # skipping due to https://github.com/hasura/graphql-engine/issues/7936
     def test_limit_orderby_relationship_query(self, hge_ctx):
         self.with_admin_secret("query", hge_ctx, self.dir() + '/limit_orderby_relationship_query.yaml')
 
+    @pytest.mark.skip_server_upgrade_test
+    # skipping due to https://github.com/hasura/graphql-engine/issues/7936
     def test_limit_offset_orderby_relationship_query(self, hge_ctx):
         self.with_admin_secret("query", hge_ctx, self.dir() + '/limit_offset_orderby_relationship_query.yaml')