From 53beef47ea1de3cefb0f6aa6ae155482cc637d8f Mon Sep 17 00:00:00 2001 From: Anon Ray Date: Mon, 24 May 2021 21:26:32 +0530 Subject: [PATCH] server, console, docs: fix `untrack_function` metadata query bug Co-authored-by: Sameer Kolhar <6604943+kolharsam@users.noreply.github.com> GitOrigin-RevId: a68a6f8b5c5243372332bfec7d9cdcd3011c001f --- .../Services/RemoteSchema/Actions.js | 2 + console/src/metadata/queryUtils.ts | 3 +- .../metadata-api/custom-functions.rst | 8 ++-- .../src-lib/Hasura/RQL/DDL/Schema/Function.hs | 37 ++++++++++++++++--- 4 files changed, 40 insertions(+), 10 deletions(-) diff --git a/console/src/components/Services/RemoteSchema/Actions.js b/console/src/components/Services/RemoteSchema/Actions.js index 7308ad3f288..c7cf1c9ce07 100644 --- a/console/src/components/Services/RemoteSchema/Actions.js +++ b/console/src/components/Services/RemoteSchema/Actions.js @@ -76,10 +76,12 @@ const makeRequest = ( ) => { return (dispatch, getState) => { const source = getState().tables.currentDataSource; + const { resourceVersion } = getState().metadata; const upQuery = { type: 'bulk', source, args: upQueries, + resource_version: resourceVersion, }; const downQuery = { diff --git a/console/src/metadata/queryUtils.ts b/console/src/metadata/queryUtils.ts index fbf9e2439e7..a06e70e0ecb 100644 --- a/console/src/metadata/queryUtils.ts +++ b/console/src/metadata/queryUtils.ts @@ -601,7 +601,8 @@ export const getUntrackFunctionQuery = ( name: string, schema: string, source: string -) => getMetadataQuery('untrack_function', source, { name, schema }); +) => + getMetadataQuery('untrack_function', source, { function: { name, schema } }); export const getRenameRelationshipQuery = ( table: QualifiedTable, diff --git a/docs/graphql/core/api-reference/metadata-api/custom-functions.rst b/docs/graphql/core/api-reference/metadata-api/custom-functions.rst index fadac50ab41..7a528bebc98 100644 --- a/docs/graphql/core/api-reference/metadata-api/custom-functions.rst +++ b/docs/graphql/core/api-reference/metadata-api/custom-functions.rst @@ -140,8 +140,10 @@ Remove an SQL function ``search_articles``: { "type": "pg_untrack_function", "args": { - "schema": "public", - "name": "search_articles", + "function": { + "schema": "public", + "name": "search_articles" + }, "source": "default" } } @@ -156,7 +158,7 @@ Args syntax - Required - Schema - Description - * - table + * - function - true - :ref:`FunctionName ` - Name of the SQL function diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Function.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Function.hs index cf47ff135ef..b2f7478d8f1 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Function.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Function.hs @@ -104,12 +104,37 @@ instance (Backend b) => ToJSON (UnTrackFunction b) where toJSON = genericToJSON hasuraJSON instance (Backend b) => FromJSON (UnTrackFunction b) where - parseJSON v = withSource <|> withoutSource - where - withoutSource = UnTrackFunction <$> parseJSON v <*> pure defaultSource - withSource = flip (withObject "UnTrackFunction") v \o -> - UnTrackFunction <$> o .: "function" - <*> o .:? "source" .!= defaultSource + -- Following was the previous implementation, which while seems to be correct, + -- has an unexpected behaviour. In the case when @source@ key is present but + -- @function@ key is absent, it would silently coerce it into a @default@ + -- source. The culprint being the _alternative_ operator, which silently fails + -- the first parse. This note exists so that we don't try to simplify using + -- the _alternative_ pattern here. + -- Previous implementation :- + -- Consider the following JSON - + -- { + -- "source": "custom_source", + -- "schema": "public", + -- "name": "my_function" + -- } + -- it silently fails parsing the source here because @function@ key is not + -- present, and proceeds to parse using @withoutSource@ as default source. Now + -- this is surprising for the user, because they mention @source@ key + -- explicitly. A better behaviour is to explicitly look for @function@ key if + -- a @source@ key is present. + -- >> + -- parseJSON v = withSource <|> withoutSource + -- where + -- withoutSource = UnTrackFunction <$> parseJSON v <*> pure defaultSource + -- withSource = flip (withObject "UnTrackFunction") v \o -> do + -- UnTrackFunction <$> o .: "function" + -- <*> o .:? "source" .!= defaultSource + parseJSON v = flip (withObject "UnTrackFunction") v $ \o -> do + source <- o .:? "source" + case source of + Just src -> flip UnTrackFunction src <$> o .: "function" + Nothing -> UnTrackFunction <$> parseJSON v <*> pure defaultSource + askFunctionInfo :: forall b m