hasura · abooij · Apr 29, 2020 · Apr 30, 2020 · Apr 30, 2020 · May 4, 2020
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,7 +8,7 @@ A scheduled trigger can be used to execute custom business logic based on time.
 
 A cron trigger will be useful when something needs to be done periodically. For example, you can create a cron trigger to  generate an end-of-day sales report every weekday at 9pm.
 
-You can also schedule one-off events based on a timestamp. For example, a new scheduled event can be created for 2 weeks from when a user signs up to send them an email about their experience. 
+You can also schedule one-off events based on a timestamp. For example, a new scheduled event can be created for 2 weeks from when a user signs up to send them an email about their experience.
 
 <Add docs links>
 
@@ -51,6 +51,7 @@ Read more about the session argument for computed fields in the [docs](https://h
 (Add entries here in the order of: server, console, cli, docs, others)
 - server: compile with GHC 8.10.1, closing a space leak with subscriptions. (close #4517) (#3388)
 
+- server: fix an issue with query plan caching where valid queries would intermittently fail (fix #3773)
 - server: avoid loss of precision when passing values in scientific notation (fix #4733)
 - server: fix mishandling of GeoJSON inputs in subscriptions (fix #3239)
 - console: avoid count queries for large tables (#4692)

@@ -210,6 +210,7 @@ deriving instance (Hashable (f TxtEncodedPGVal)) => Hashable (ValidatedVariables
 deriving instance (J.ToJSON (f TxtEncodedPGVal)) => J.ToJSON (ValidatedVariables f)
 
 type ValidatedQueryVariables = ValidatedVariables (Map.HashMap G.Variable)
+-- | See the explanation for 'CohortVariables'.
 type ValidatedSyntheticVariables = ValidatedVariables []
 
 -- | Checks if the provided arguments are valid values for their corresponding types.

@@ -54,8 +54,7 @@ import           Hasura.Server.Utils               (mkClientHeadersForward, mkSe
 import           Hasura.Server.Version             (HasVersion)
 import           Hasura.Session
 import           Hasura.SQL.Types
-import           Hasura.SQL.Value                  (PGScalarValue (..), pgScalarValueToJson,
-                                                    toTxtValue)
+import           Hasura.SQL.Value                  (PGScalarValue(..), toTxtValue)
 
 newtype ActionContext
   = ActionContext {_acName :: ActionName}
@@ -539,7 +538,7 @@ callWebhook manager outputType outputFields reqHeaders confHeaders
 annInpValueToJson :: AnnInpVal -> J.Value
 annInpValueToJson annInpValue =
   case _aivValue annInpValue of
-    AGScalar _ pgColumnValueM -> maybe J.Null pgScalarValueToJson pgColumnValueM
+    AGScalar _ pgColumnValueM -> maybe J.Null J.toJSON pgColumnValueM
     AGEnum _ enumValue        -> case enumValue of
       AGESynthetic enumValueM   -> J.toJSON enumValueM
       AGEReference _ enumValueM -> J.toJSON enumValueM

@@ -79,26 +79,35 @@ openOpaqueValue :: (MonadReusability m) => OpaqueValue a -> m a
 openOpaqueValue (OpaqueValue v isVariable) = when isVariable markNotReusable $> v
 
 asPGColumnTypeAndValueM
-  :: (MonadReusability m, MonadError QErr m)
+  :: forall m . (MonadReusability m, MonadError QErr m)
   => AnnInpVal
   -> m (PGColumnType, WithScalarType (Maybe (OpaqueValue PGScalarValue)))
 asPGColumnTypeAndValueM v = do
-  (columnType, scalarValueM) <- case _aivValue v of
-    AGScalar colTy val -> pure (PGColumnScalar colTy, WithScalarType colTy val)
-    AGEnum _ (AGEReference reference maybeValue) -> do
-      let maybeScalarValue = PGValText . RQL.getEnumValue <$> maybeValue
-      pure (PGColumnEnumReference reference, WithScalarType PGText maybeScalarValue)
-    _ -> tyMismatch "pgvalue" v
+  (columnType, scalarValueM) <- toPG (_aivValue v)
 
   for_ (_aivVariable v) $ \variableName -> if
     -- If the value is a nullable variable, then the caller might make a different decision based on
     -- whether the result is 'Nothing' or 'Just', which would change the generated query, so we have
     -- to unconditionally mark the query non-reusable.
     | G.isNullable (_aivType v) -> markNotReusable
-    | otherwise                 -> recordVariableUse variableName columnType
-
+    | otherwise                 -> case _aivDefault v of
+        Nothing -> do
+          recordVariableUse variableName columnType ReusableNoDefault
+        Just dv -> do
+          (_ , WithScalarType _ defValueM) <- toPG dv
+          let defVal = case defValueM of
+                Nothing  -> ReusableNoDefault
+                Just val -> ReusableDefault val
+          recordVariableUse variableName columnType defVal
   let isVariable = isJust $ _aivVariable v
   pure (columnType, fmap (flip OpaqueValue isVariable) <$> scalarValueM)
+  where
+  toPG :: AnnGValue -> m (PGColumnType, WithScalarType (Maybe PGScalarValue))
+  toPG (AGScalar colTy val) = pure (PGColumnScalar colTy, WithScalarType colTy val)
+  toPG (AGEnum _ (AGEReference reference maybeValue)) =
+    let maybeScalarValue = PGValText . RQL.getEnumValue <$> maybeValue
+    in  pure (PGColumnEnumReference reference, WithScalarType PGText maybeScalarValue)
+  toPG _ = tyMismatch "pgvalue" v
 
 asPGColumnTypeAndAnnValueM
   :: (MonadReusability m, MonadError QErr m) => AnnInpVal -> m (PGColumnType, Maybe OpaquePGValue)

@@ -30,6 +30,7 @@ import           Hasura.GraphQL.Validate.Context
 import           Hasura.GraphQL.Validate.Field
 import           Hasura.GraphQL.Validate.InputValue
 import           Hasura.GraphQL.Validate.Types
+import           Hasura.SQL.Types
 import           Hasura.RQL.Types
 
 data QueryParts
@@ -96,7 +97,13 @@ validateVariables varDefsL inpVals = withPathK "variableValues" $ do
       let inpValM = Map.lookup var inpVals
       annInpValM <- withPathK (G.unName $ G.unVariable var) $
                     mapM (validateInputValue jsonParser ty) inpValM
-      let varValM = annInpValM <|> annDefM
+      let varValM = case annInpValM of
+            Nothing -> case annDefM of
+              Nothing -> Nothing
+              Just dv -> Just $ dv { _aivDefault = Just $ _aivValue dv }
+            Just iv -> case annDefM of
+              Nothing -> Just iv
+              Just dv -> Just $ iv { _aivDefault = Just $ _aivValue dv }
       onNothing varValM $ throwVE $
         "expecting a value for non-nullable variable: " <>
         showVars [var] <>
@@ -121,11 +128,23 @@ validateVariablesForReuse (ReusableVariableTypes varTypes) varValsM =
 
     flip Map.traverseWithKey varTypes $ \varName varType ->
       withPathK (G.unName $ G.unVariable varName) $ do
-        varVal <- onNothing (Map.lookup varName varVals) $
+        let
+          (ReusableVariableInfo varPGColType defValR) = varType
+          defValM = case defValR of
+            -- No default value specified for this argument
+            ReusableNoDefault -> Nothing
+            -- We have a default value: so construct a WithScalarType,
+            -- following the logic of parsePGScalarValue
+            ReusableDefault x -> Just $
+              case varPGColType of
+                PGColumnScalar st -> WithScalarType st x
+                _                 -> WithScalarType PGText x
+          specValM = Map.lookup varName varVals
+        specVal <- traverse (parsePGScalarValue varPGColType) specValM
+        onNothing (specVal <|> defValM) $
           throwVE "expected a value for non-nullable variable"
           -- TODO: we don't have the graphql type
           -- <> " of type: " <> T.pack (show varType)
-        parsePGScalarValue varType varVal
     where
       varVals = fromMaybe Map.empty varValsM
 

@@ -312,10 +312,14 @@ withParsed expectedTy valParser val fn = do
   case unP parsedVal of
     Nothing              ->
       if G.isNullable expectedTy
-      then AnnInpVal expectedTy Nothing <$> fn Nothing
+      then do
+        v <- fn Nothing
+        return $ AnnInpVal expectedTy Nothing v Nothing
       else throwVE $ "null value found for non-nullable type: "
            <> G.showGT expectedTy
-    Just (Right v)       -> AnnInpVal expectedTy Nothing <$> fn (Just v)
+    Just (Right v)       -> do
+      v' <- fn (Just v)
+      return $ AnnInpVal expectedTy Nothing v' Nothing
     Just (Left (var, v)) -> do
       let varTxt = G.unName $ G.unVariable var
       unless (isTypeAllowed expectedTy $ _aivType v) $

@@ -69,6 +69,8 @@ module Hasura.GraphQL.Validate.Types
 
   , ReusableVariableTypes(..)
   , ReusableVariableValues
+  , ReusableDefaultValue(..)
+  , ReusableVariableInfo(..)
 
   , QueryReusability(..)
   , _Reusable
@@ -711,6 +713,7 @@ data AnnInpVal
   { _aivType     :: !G.GType
   , _aivVariable :: !(Maybe G.Variable)
   , _aivValue    :: !AnnGValue
+  , _aivDefault  :: !(Maybe AnnGValue)
   } deriving (Show, Eq)
 
 type AnnGObject = OMap.InsOrdHashMap G.Name AnnInpVal
@@ -781,10 +784,21 @@ stripTypenames = map filterExecDef
           in Just $ G.SelectionField  f{G._fSelectionSet = newSelset}
       _                  -> Just s
 
+data ReusableDefaultValue a
+  = ReusableNoDefault
+  | ReusableDefault a
+  deriving (Show, Eq)
+$(J.deriveToJSON J.defaultOptions{J.constructorTagModifier = drop 8} ''ReusableDefaultValue)
+
+data ReusableVariableInfo
+  = ReusableVariableInfo RQL.PGColumnType (ReusableDefaultValue PGScalarValue)
+  deriving (Show, Eq)
+$(J.deriveToJSON J.defaultOptions{J.constructorTagModifier = drop 8} ''ReusableVariableInfo)
+
 -- | Used by 'Hasura.GraphQL.Validate.validateVariablesForReuse' to parse new sets of variables for
 -- reusable query plans; see also 'QueryReusability'.
 newtype ReusableVariableTypes
-  = ReusableVariableTypes { unReusableVarTypes :: Map.HashMap G.Variable RQL.PGColumnType }
+  = ReusableVariableTypes { unReusableVarTypes :: Map.HashMap G.Variable ReusableVariableInfo }
   deriving (Show, Eq, Semigroup, Monoid, J.ToJSON)
 type ReusableVariableValues = Map.HashMap G.Variable (WithScalarType PGScalarValue)
 
@@ -819,22 +833,23 @@ instance Monoid QueryReusability where
   mempty = Reusable mempty
 
 class (Monad m) => MonadReusability m where
-  recordVariableUse :: G.Variable -> RQL.PGColumnType -> m ()
+  recordVariableUse :: G.Variable -> RQL.PGColumnType -> ReusableDefaultValue PGScalarValue -> m ()
   markNotReusable :: m ()
 
 instance (MonadReusability m) => MonadReusability (ReaderT r m) where
-  recordVariableUse a b = lift $ recordVariableUse a b
+  recordVariableUse a b c = lift $ recordVariableUse a b c
   markNotReusable = lift markNotReusable
 instance (MonadReusability m) => MonadReusability (StateT s m) where
-  recordVariableUse a b = lift $ recordVariableUse a b
+  recordVariableUse a b c = lift $ recordVariableUse a b c
   markNotReusable = lift markNotReusable
 
 newtype ReusabilityT m a = ReusabilityT { unReusabilityT :: StateT QueryReusability m a }
   deriving (Functor, Applicative, Monad, MonadError e, MonadReader r, MonadIO)
 
 instance (Monad m) => MonadReusability (ReusabilityT m) where
-  recordVariableUse varName varType = ReusabilityT $
-    modify' (<> Reusable (ReusableVariableTypes $ Map.singleton varName varType))
+  recordVariableUse varName varType defaultVal = ReusabilityT $
+    modify' (<> Reusable (ReusableVariableTypes $
+                          Map.singleton varName (ReusableVariableInfo varType defaultVal)))
   markNotReusable = ReusabilityT $ put NotReusable
 
 runReusabilityT :: ReusabilityT m a -> m (a, QueryReusability)

@@ -334,26 +334,33 @@ data PGScalarType
   = PGSmallInt
   | PGInteger
   | PGBigInt
+
   | PGSerial
   | PGBigSerial
+
   | PGFloat
   | PGDouble
   | PGNumeric
   | PGMoney
+
   | PGBoolean
   | PGChar
   | PGVarchar
   | PGText
   | PGCitext
+
   | PGDate
   | PGTimeStamp
   | PGTimeStampTZ
   | PGTimeTZ
+
   | PGJSON
   | PGJSONB
+
   | PGGeometry
   | PGGeography
   | PGRaster
+
   | PGUUID
   | PGUnknown !T.Text
   deriving (Show, Eq, Lift, Generic, Data)

@@ -1,7 +1,6 @@
 module Hasura.SQL.Value
   ( PGScalarValue(..)
   , pgColValueToInt
-  , pgScalarValueToJson
   , withConstructorFn
   , parsePGValue
 
@@ -61,56 +60,63 @@ data PGScalarValue
   = PGValInteger !Int32
   | PGValSmallInt !Int16
   | PGValBigInt !Int64
+
   | PGValFloat !Float
   | PGValDouble !Double
   | PGValNumeric !Scientific
   | PGValMoney !Scientific
+
   | PGValBoolean !Bool
   | PGValChar !Char
   | PGValVarchar !T.Text
   | PGValText !T.Text
   | PGValCitext !T.Text
+
   | PGValDate !Day
   | PGValTimeStamp !LocalTime
   | PGValTimeStampTZ !UTCTime
   | PGValTimeTZ !ZonedTimeOfDay
+
   | PGNull !PGScalarType
+
   | PGValJSON !Q.JSON
   | PGValJSONB !Q.JSONB
+
   | PGValGeo !GeometryWithCRS
   | PGValRaster !RasterWKB
+
   | PGValUUID !UUID.UUID
   | PGValUnknown !T.Text
   deriving (Show, Eq)
 
-pgScalarValueToJson :: PGScalarValue -> Value
-pgScalarValueToJson = \case
-  PGValInteger i  -> toJSON i
-  PGValSmallInt i -> toJSON i
-  PGValBigInt i   -> toJSON i
-  PGValFloat f    -> toJSON f
-  PGValDouble d   -> toJSON d
-  PGValNumeric sc -> toJSON sc
-  PGValMoney m    -> toJSON m
-  PGValBoolean b  -> toJSON b
-  PGValChar t     -> toJSON t
-  PGValVarchar t  -> toJSON t
-  PGValText t     -> toJSON t
-  PGValCitext t   -> toJSON t
-  PGValDate d     -> toJSON d
-  PGValTimeStamp u ->
-    toJSON $ formatTime defaultTimeLocale "%FT%T%QZ" u
-  PGValTimeStampTZ u ->
-    toJSON $ formatTime defaultTimeLocale "%FT%T%QZ" u
-  PGValTimeTZ (ZonedTimeOfDay tod tz) ->
-    toJSON (show tod ++ timeZoneOffsetString tz)
-  PGNull _ -> Null
-  PGValJSON (Q.JSON j)    -> j
-  PGValJSONB (Q.JSONB j)  -> j
-  PGValGeo o    -> toJSON o
-  PGValRaster r -> toJSON r
-  PGValUUID u -> toJSON u
-  PGValUnknown t -> toJSON t
+instance ToJSON PGScalarValue where
+  toJSON = \case
+    PGValInteger i  -> toJSON i
+    PGValSmallInt i -> toJSON i
+    PGValBigInt i   -> toJSON i
+    PGValFloat f    -> toJSON f
+    PGValDouble d   -> toJSON d
+    PGValNumeric sc -> toJSON sc
+    PGValMoney m    -> toJSON m
+    PGValBoolean b  -> toJSON b
+    PGValChar t     -> toJSON t
+    PGValVarchar t  -> toJSON t
+    PGValText t     -> toJSON t
+    PGValCitext t   -> toJSON t
+    PGValDate d     -> toJSON d
+    PGValTimeStamp u ->
+      toJSON $ formatTime defaultTimeLocale "%FT%T%QZ" u
+    PGValTimeStampTZ u ->
+      toJSON $ formatTime defaultTimeLocale "%FT%T%QZ" u
+    PGValTimeTZ (ZonedTimeOfDay tod tz) ->
+      toJSON (show tod ++ timeZoneOffsetString tz)
+    PGNull _ -> Null
+    PGValJSON (Q.JSON j)    -> j
+    PGValJSONB (Q.JSONB j)  -> j
+    PGValGeo o    -> toJSON o
+    PGValRaster r -> toJSON r
+    PGValUUID u -> toJSON u
+    PGValUnknown t -> toJSON t
 
 pgColValueToInt :: PGScalarValue -> Maybe Int
 pgColValueToInt (PGValInteger i)  = Just $ fromIntegral i

@@ -0,0 +1,29 @@
+- description: Test that default values are properly cached (#3773)
+  url: /v1/graphql
+  status: 200
+  response:
+    data:
+      users:
+      - name: Alyssa
+  query:
+    query: |
+      query ($id:Int!=1) {
+        users (where: {id: {_eq: $id}}) {
+          name
+        }
+      }
+
+- description: Test that default values are properly cached (#3773)
+  url: /v1/graphql
+  status: 200
+  response:
+    data:
+      users:
+      - name: Alyssa
+  query:
+    query: |
+      query ($id:Int!=1) {
+        users (where: {id: {_eq: $id}}) {
+          name
+        }
+      }
@@ -606,3 +606,6 @@ def dir(cls):
 
     def test_include_directive(self, hge_ctx, transport):
         check_query_f(hge_ctx, self.dir() + '/include_directive.yaml', transport)
+
+    def test_default_values(self, hge_ctx, transport):
+        check_query_f(hge_ctx, self.dir() + '/default_values.yaml', transport)