Skip to content

Commit e52471d

Browse files
Auke Booijtirumaraiselvan
authored and
stevefan1999-personal
committed
server: fix an introspection query caching issue (fix hasura#4547) (hasura#4661)
Introspection queries accept variables, but we need to make sure to also touch the variables that we ignore, so that an introspection query is marked not reusable if we are not able to build a correct query plan for it. A better solution here would be to deal with such unused variables correctly, so that more introspection queries become reusable. An even better solution would be to type-safely track *how* to reuse which variables, rather than to split the reusage marking from the planning. Co-authored-by: Tirumarai Selvan <[email protected]>
1 parent 5f51967 commit e52471d

File tree

4 files changed

+217
-81
lines changed

4 files changed

+217
-81
lines changed

‎server/src-lib/Hasura/GraphQL/Resolve/InputValue.hs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ module Hasura.GraphQL.Resolve.InputValue
1010
, asPGColumnValueM
1111
, asPGColumnValue
1212

13+
, asScalarValM
14+
, asScalarVal
1315
, asEnumVal
1416
, asEnumValM
1517
, withObject
@@ -26,6 +28,8 @@ module Hasura.GraphQL.Resolve.InputValue
2628

2729
import Hasura.Prelude
2830

31+
import qualified Text.Builder as TB
32+
2933
import qualified Language.GraphQL.Draft.Syntax as G
3034

3135
import qualified Hasura.RQL.Types as RQL
@@ -120,6 +124,19 @@ asPGColumnValue v = do
120124
openInputValue :: (MonadReusability m) => AnnInpVal -> m AnnGValue
121125
openInputValue v = when (isJust $ _aivVariable v) markNotReusable $> _aivValue v
122126

127+
asScalarValM :: (MonadReusability m, MonadError QErr m) => AnnInpVal -> PGScalarType -> m (Maybe PGScalarValue)
128+
asScalarValM v tp = openInputValue v >>= \case
129+
AGScalar tp' vM ->
130+
if tp == tp'
131+
then pure vM
132+
else tyMismatch "scalar" v
133+
_ -> tyMismatch "scalar" v
134+
135+
asScalarVal :: (MonadReusability m, MonadError QErr m) => AnnInpVal -> PGScalarType -> m PGScalarValue
136+
asScalarVal v tp = asScalarValM v tp >>= \case
137+
Just val -> pure val
138+
Nothing -> throw500 $ "unexpected null for ty " <> TB.run (toSQL tp)
139+
123140
-- | Note: only handles “synthetic” enums (see 'EnumValuesInfo'). Enum table references are handled
124141
-- by 'asPGColumnType' and its variants.
125142
asEnumVal :: (MonadReusability m, MonadError QErr m) => AnnInpVal -> m (G.NamedType, G.EnumValue)

‎server/src-lib/Hasura/GraphQL/Resolve/Introspect.hs

Lines changed: 141 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import qualified Data.HashMap.Strict as Map
1111
import qualified Data.HashSet as Set
1212
import qualified Data.Text as T
1313
import qualified Language.GraphQL.Draft.Syntax as G
14+
import qualified Hasura.SQL.Value as S
15+
import qualified Hasura.SQL.Types as S
1416

1517
import Hasura.GraphQL.Context
1618
import Hasura.GraphQL.Resolve.Context
@@ -56,38 +58,40 @@ retJT = pure . J.toJSON
5658

5759
-- 4.5.2.1
5860
scalarR
59-
:: (Monad m)
61+
:: (MonadReusability m, MonadError QErr m)
6062
=> ScalarTyInfo
6163
-> Field
6264
-> m J.Object
63-
scalarR (ScalarTyInfo descM name _ _) fld =
65+
scalarR (ScalarTyInfo descM name _ _) fld = do
66+
dummyReadIncludeDeprecated fld
6467
withSubFields (_fSelSet fld) $ \subFld ->
65-
case _fName subFld of
66-
"__typename" -> retJT "__Type"
67-
"kind" -> retJ TKSCALAR
68-
"description" -> retJ $ fmap G.unDescription descM
69-
"name" -> retJ name
70-
_ -> return J.Null
68+
case _fName subFld of
69+
"__typename" -> retJT "__Type"
70+
"kind" -> retJ TKSCALAR
71+
"description" -> retJ $ fmap G.unDescription descM
72+
"name" -> retJ name
73+
_ -> return J.Null
7174

7275
-- 4.5.2.2
7376
objectTypeR
7477
:: ( MonadReader r m, Has TypeMap r
75-
, MonadError QErr m)
78+
, MonadError QErr m, MonadReusability m)
7679
=> ObjTyInfo
7780
-> Field
7881
-> m J.Object
79-
objectTypeR objectType fld =
82+
objectTypeR objectType fld = do
83+
dummyReadIncludeDeprecated fld
8084
withSubFields (_fSelSet fld) $ \subFld ->
81-
case _fName subFld of
82-
"__typename" -> retJT "__Type"
83-
"kind" -> retJ TKOBJECT
84-
"name" -> retJ $ namedTyToTxt n
85-
"description" -> retJ $ fmap G.unDescription descM
86-
"interfaces" -> fmap J.toJSON $ mapM (`ifaceR` subFld) $ Set.toList iFaces
87-
"fields" -> fmap J.toJSON $ mapM (`fieldR` subFld) $
88-
sortOn _fiName $
89-
filter notBuiltinFld $ Map.elems flds
90-
_ -> return J.Null
85+
case _fName subFld of
86+
"__typename" -> retJT "__Type"
87+
"kind" -> retJ TKOBJECT
88+
"name" -> retJ $ namedTyToTxt n
89+
"description" -> retJ $ fmap G.unDescription descM
90+
"interfaces" -> fmap J.toJSON $ mapM (`ifaceR` subFld) $ Set.toList iFaces
91+
"fields" -> fmap J.toJSON $ mapM (`fieldR` subFld) $
92+
sortOn _fiName $
93+
filter notBuiltinFld $ Map.elems flds
94+
_ -> return J.Null
9195
where
9296
descM = _otiDesc objectType
9397
n = _otiName objectType
@@ -108,23 +112,24 @@ getImplTypes aot = do
108112

109113
-- 4.5.2.3
110114
unionR
111-
:: (MonadReader t m, MonadError QErr m, Has TypeMap t)
115+
:: (MonadReader t m, MonadError QErr m, Has TypeMap t, MonadReusability m)
112116
=> UnionTyInfo -> Field -> m J.Object
113-
unionR u@(UnionTyInfo descM n _) fld =
117+
unionR u@(UnionTyInfo descM n _) fld = do
118+
dummyReadIncludeDeprecated fld
114119
withSubFields (_fSelSet fld) $ \subFld ->
115-
case _fName subFld of
116-
"__typename" -> retJT "__Field"
117-
"kind" -> retJ TKUNION
118-
"name" -> retJ $ namedTyToTxt n
119-
"description" -> retJ $ fmap G.unDescription descM
120-
"possibleTypes" -> fmap J.toJSON $
121-
mapM (`objectTypeR` subFld) =<< getImplTypes (AOTUnion u)
122-
_ -> return J.Null
120+
case _fName subFld of
121+
"__typename" -> retJT "__Field"
122+
"kind" -> retJ TKUNION
123+
"name" -> retJ $ namedTyToTxt n
124+
"description" -> retJ $ fmap G.unDescription descM
125+
"possibleTypes" -> fmap J.toJSON $
126+
mapM (`objectTypeR` subFld) =<< getImplTypes (AOTUnion u)
127+
_ -> return J.Null
123128

124129
-- 4.5.2.4
125130
ifaceR
126131
:: ( MonadReader r m, Has TypeMap r
127-
, MonadError QErr m)
132+
, MonadError QErr m, MonadReusability m)
128133
=> G.NamedType
129134
-> Field
130135
-> m J.Object
@@ -136,63 +141,115 @@ ifaceR n fld = do
136141

137142
ifaceR'
138143
:: ( MonadReader r m, Has TypeMap r
139-
, MonadError QErr m)
144+
, MonadError QErr m, MonadReusability m)
140145
=> IFaceTyInfo
141146
-> Field
142147
-> m J.Object
143-
ifaceR' i@(IFaceTyInfo descM n flds) fld =
148+
ifaceR' i@(IFaceTyInfo descM n flds) fld = do
149+
dummyReadIncludeDeprecated fld
144150
withSubFields (_fSelSet fld) $ \subFld ->
145-
case _fName subFld of
146-
"__typename" -> retJT "__Type"
147-
"kind" -> retJ TKINTERFACE
148-
"name" -> retJ $ namedTyToTxt n
149-
"description" -> retJ $ fmap G.unDescription descM
150-
"fields" -> fmap J.toJSON $ mapM (`fieldR` subFld) $
151-
sortOn _fiName $
152-
filter notBuiltinFld $ Map.elems flds
153-
"possibleTypes" -> fmap J.toJSON $ mapM (`objectTypeR` subFld)
154-
=<< getImplTypes (AOTIFace i)
155-
_ -> return J.Null
151+
case _fName subFld of
152+
"__typename" -> retJT "__Type"
153+
"kind" -> retJ TKINTERFACE
154+
"name" -> retJ $ namedTyToTxt n
155+
"description" -> retJ $ fmap G.unDescription descM
156+
"fields" -> fmap J.toJSON $ mapM (`fieldR` subFld) $
157+
sortOn _fiName $
158+
filter notBuiltinFld $ Map.elems flds
159+
"possibleTypes" -> fmap J.toJSON $ mapM (`objectTypeR` subFld)
160+
=<< getImplTypes (AOTIFace i)
161+
_ -> return J.Null
156162

157163
-- 4.5.2.5
158164
enumTypeR
159-
:: ( Monad m )
165+
:: ( Monad m, MonadReusability m, MonadError QErr m )
160166
=> EnumTyInfo
161167
-> Field
162168
-> m J.Object
163-
enumTypeR (EnumTyInfo descM n vals _) fld =
169+
enumTypeR (EnumTyInfo descM n vals _) fld = do
170+
dummyReadIncludeDeprecated fld
164171
withSubFields (_fSelSet fld) $ \subFld ->
165-
case _fName subFld of
166-
"__typename" -> retJT "__Type"
167-
"kind" -> retJ TKENUM
168-
"name" -> retJ $ namedTyToTxt n
169-
"description" -> retJ $ fmap G.unDescription descM
170-
"enumValues" -> fmap J.toJSON $ mapM (enumValueR subFld) $
171-
sortOn _eviVal $ Map.elems (normalizeEnumValues vals)
172-
_ -> return J.Null
172+
case _fName subFld of
173+
"__typename" -> retJT "__Type"
174+
"kind" -> retJ TKENUM
175+
"name" -> retJ $ namedTyToTxt n
176+
"description" -> retJ $ fmap G.unDescription descM
177+
"enumValues" -> do
178+
includeDeprecated <- readIncludeDeprecated subFld
179+
fmap J.toJSON $
180+
mapM (enumValueR subFld) $
181+
filter (\val -> includeDeprecated || not (_eviIsDeprecated val)) $
182+
sortOn _eviVal $
183+
Map.elems (normalizeEnumValues vals)
184+
_ -> return J.Null
185+
186+
readIncludeDeprecated
187+
:: ( Monad m, MonadReusability m, MonadError QErr m )
188+
=> Field
189+
-> m Bool
190+
readIncludeDeprecated subFld = do
191+
let argM = Map.lookup "includeDeprecated" (_fArguments subFld)
192+
case argM of
193+
Nothing -> pure False
194+
Just arg -> asScalarVal arg S.PGBoolean >>= \case
195+
S.PGValBoolean b -> pure b
196+
_ -> throw500 "unexpected non-Boolean argument for includeDeprecated"
197+
198+
{- Note [Reusability of introspection queries with variables]
199+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
200+
Introspection queries can have variables, too, in particular to influence one of
201+
two arguments: the @name@ argument of the @__type@ field, and the
202+
@includeDeprecated@ argument of the @fields@ and @enumValues@ fields. The
203+
current code does not cache all introspection queries with variables correctly.
204+
As a workaround to this, whenever a variable is passed to an @includeDeprecated@
205+
argument, we mark the query as unreusable. This is the purpose of
206+
'dummyReadIncludeDeprecated'.
207+
208+
Now @fields@ and @enumValues@ are intended to be used when introspecting,
209+
respectively [object and interface types] and enum types. However, it does not
210+
suffice to only call 'dummyReadIncludeDeprecated' for such types, since @fields@
211+
and @enumValues@ are valid GraphQL fields regardless of what type we are looking
212+
at. So precisely because @__Type@ is _thought of_ as a union, but _not
213+
actually_ a union, we need to call 'dummyReadIncludeDeprecated' in all cases.
214+
215+
See also issue #4547.
216+
-}
217+
218+
dummyReadIncludeDeprecated
219+
:: ( Monad m, MonadReusability m, MonadError QErr m )
220+
=> Field
221+
-> m ()
222+
dummyReadIncludeDeprecated fld =
223+
void $ forM (toList (_fSelSet fld)) $ \subFld ->
224+
case _fName subFld of
225+
"fields" -> readIncludeDeprecated subFld
226+
"enumValues" -> readIncludeDeprecated subFld
227+
_ -> return False
228+
173229

174230
-- 4.5.2.6
175231
inputObjR
176232
:: ( MonadReader r m, Has TypeMap r
177-
, MonadError QErr m)
233+
, MonadError QErr m, MonadReusability m)
178234
=> InpObjTyInfo
179235
-> Field
180236
-> m J.Object
181-
inputObjR (InpObjTyInfo descM nt flds _) fld =
237+
inputObjR (InpObjTyInfo descM nt flds _) fld = do
238+
dummyReadIncludeDeprecated fld
182239
withSubFields (_fSelSet fld) $ \subFld ->
183-
case _fName subFld of
184-
"__typename" -> retJT "__Type"
185-
"kind" -> retJ TKINPUT_OBJECT
186-
"name" -> retJ $ namedTyToTxt nt
187-
"description" -> retJ $ fmap G.unDescription descM
188-
"inputFields" -> fmap J.toJSON $ mapM (inputValueR subFld) $
189-
sortOn _iviName $ Map.elems flds
190-
_ -> return J.Null
240+
case _fName subFld of
241+
"__typename" -> retJT "__Type"
242+
"kind" -> retJ TKINPUT_OBJECT
243+
"name" -> retJ $ namedTyToTxt nt
244+
"description" -> retJ $ fmap G.unDescription descM
245+
"inputFields" -> fmap J.toJSON $ mapM (inputValueR subFld) $
246+
sortOn _iviName $ Map.elems flds
247+
_ -> return J.Null
191248

192249
-- 4.5.2.7
193250
listTypeR
194251
:: ( MonadReader r m, Has TypeMap r
195-
, MonadError QErr m)
252+
, MonadError QErr m, MonadReusability m)
196253
=> G.ListType -> Field -> m J.Object
197254
listTypeR (G.ListType ty) fld =
198255
withSubFields (_fSelSet fld) $ \subFld ->
@@ -205,7 +262,7 @@ listTypeR (G.ListType ty) fld =
205262
-- 4.5.2.8
206263
nonNullR
207264
:: ( MonadReader r m, Has TypeMap r
208-
, MonadError QErr m)
265+
, MonadError QErr m, MonadReusability m)
209266
=> G.GType -> Field -> m J.Object
210267
nonNullR gTyp fld =
211268
withSubFields (_fSelSet fld) $ \subFld ->
@@ -220,7 +277,7 @@ nonNullR gTyp fld =
220277

221278
namedTypeR
222279
:: ( MonadReader r m, Has TypeMap r
223-
, MonadError QErr m)
280+
, MonadError QErr m, MonadReusability m)
224281
=> G.NamedType
225282
-> Field
226283
-> m J.Object
@@ -230,22 +287,25 @@ namedTypeR nt fld = do
230287

231288
namedTypeR'
232289
:: ( MonadReader r m, Has TypeMap r
233-
, MonadError QErr m)
290+
, MonadError QErr m, MonadReusability m)
234291
=> Field
235292
-> TypeInfo
236293
-> m J.Object
237-
namedTypeR' fld = \case
238-
TIScalar colTy -> scalarR colTy fld
239-
TIObj objTyInfo -> objectTypeR objTyInfo fld
240-
TIEnum enumTypeInfo -> enumTypeR enumTypeInfo fld
241-
TIInpObj inpObjTyInfo -> inputObjR inpObjTyInfo fld
242-
TIIFace iFaceTyInfo -> ifaceR' iFaceTyInfo fld
243-
TIUnion unionTyInfo -> unionR unionTyInfo fld
294+
namedTypeR' fld tyInfo = do
295+
-- Now fetch the required type information from the corresponding
296+
-- information generator
297+
case tyInfo of
298+
TIScalar colTy -> scalarR colTy fld
299+
TIObj objTyInfo -> objectTypeR objTyInfo fld
300+
TIEnum enumTypeInfo -> enumTypeR enumTypeInfo fld
301+
TIInpObj inpObjTyInfo -> inputObjR inpObjTyInfo fld
302+
TIIFace iFaceTyInfo -> ifaceR' iFaceTyInfo fld
303+
TIUnion unionTyInfo -> unionR unionTyInfo fld
244304

245305
-- 4.5.3
246306
fieldR
247307
:: ( MonadReader r m, Has TypeMap r
248-
, MonadError QErr m)
308+
, MonadError QErr m, MonadReusability m)
249309
=> ObjFldInfo -> Field -> m J.Object
250310
fieldR (ObjFldInfo descM n params ty _) fld =
251311
withSubFields (_fSelSet fld) $ \subFld ->
@@ -262,7 +322,7 @@ fieldR (ObjFldInfo descM n params ty _) fld =
262322
-- 4.5.4
263323
inputValueR
264324
:: ( MonadReader r m, Has TypeMap r
265-
, MonadError QErr m)
325+
, MonadError QErr m, MonadReusability m)
266326
=> Field -> InpValInfo -> m J.Object
267327
inputValueR fld (InpValInfo descM n defM ty) =
268328
withSubFields (_fSelSet fld) $ \subFld ->
@@ -291,7 +351,7 @@ enumValueR fld (EnumValInfo descM enumVal isDeprecated) =
291351
-- 4.5.6
292352
directiveR
293353
:: ( MonadReader r m, Has TypeMap r
294-
, MonadError QErr m)
354+
, MonadError QErr m, MonadReusability m)
295355
=> Field -> DirectiveInfo -> m J.Object
296356
directiveR fld (DirectiveInfo descM n args locs) =
297357
withSubFields (_fSelSet fld) $ \subFld ->
@@ -311,7 +371,7 @@ showDirLoc = \case
311371

312372
gtypeR
313373
:: ( MonadReader r m, Has TypeMap r
314-
, MonadError QErr m)
374+
, MonadError QErr m, MonadReusability m)
315375
=> G.GType -> Field -> m J.Object
316376
gtypeR ty fld =
317377
case ty of
@@ -322,7 +382,7 @@ gtypeR ty fld =
322382

323383
schemaR
324384
:: ( MonadReader r m, Has TypeMap r
325-
, MonadError QErr m)
385+
, MonadError QErr m, MonadReusability m)
326386
=> Field -> m J.Object
327387
schemaR fld =
328388
withSubFields (_fSelSet fld) $ \subFld -> do
@@ -348,7 +408,7 @@ typeR fld = do
348408
args = _fArguments fld
349409

350410
typeR'
351-
:: (MonadReader r m, Has TypeMap r, MonadError QErr m)
411+
:: (MonadReader r m, Has TypeMap r, MonadError QErr m, MonadReusability m)
352412
=> G.NamedType -> Field -> m J.Value
353413
typeR' n fld = do
354414
tyMap <- asks getter

0 commit comments

Comments
 (0)