Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mock exec plan 2 #71

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Mock exec plan 2 #71

wants to merge 8 commits into from

Conversation

tirumaraiselvan
Copy link
Owner

mock exec plan 2

=> G.OperationType
-> Seq.Seq VQ.Field
-> m GQLReqParsed
fieldsToRequest = undefined
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fill from RemoteJoins. Make sure variables are "unresolved". Discuss.

-- pure $ mergeResponses fieldResps
pure $ head (toList fieldResps)
where
mergeResponses = undefined
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fill from RemoteJoins

EP.RPQuery queryPlan -> do
(tx, genSql) <- EQ.queryOpFromPlan usrVars queryVars queryPlan
let queryOp = ExOpQuery tx (Just genSql)
pure $ pure $ GQFieldResolvedHasura queryOp
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

think about caching here

GQFieldPartialHasura (gCtx, field) -> do
(queryTx, plan, genSql) <-
getQueryOp gCtx sqlGenCtx userInfo (Seq.singleton field)
traverse_ (addPlanToCache . EP.RPQuery) plan
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

think about caching here

forM fieldPlans $ \case
GQFieldPartialHasura (gCtx, field) -> do
(lqOp, plan) <- getSubsOp pgExecCtx gCtx sqlGenCtx userInfo field
traverse_ (addPlanToCache . EP.RPSubs) plan
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

think about caching here

, HTTP.responseTimeout = HTTP.responseTimeoutMicro (timeout * 1000000)
}

liftIO $ logGraphqlQuery logger $ QueryLog q Nothing reqId
-- liftIO $ logGraphqlQuery logger $ QueryLog q Nothing reqId
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move logging out of here to the call-site of execRemoteGQ?

@@ -26,13 +27,17 @@ runGQ
-> m (HttpResponse EncJSON)
runGQ reqId userInfo reqHdrs req = do
E.ExecutionCtx _ sqlGenCtx pgExecCtx planCache sc scVer _ enableAL <- ask
execPlan <- E.getResolvedExecPlan pgExecCtx planCache
fieldPlans <- E.getResolvedExecPlan pgExecCtx planCache
userInfo sqlGenCtx enableAL sc scVer req
Copy link
Owner Author

@tirumaraiselvan tirumaraiselvan Oct 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

log everything here?

Copy link
Owner Author

@tirumaraiselvan tirumaraiselvan Oct 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but what about logging in runHasuraGQ : https://github.com/tirumaraiselvan/graphql-engine/pull/71/files#diff-ee138319d8341444234c6af198a5ea0bR57?

Maybe log the request here and its "sql map" inside runHasuraGQ?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or we can log at the "begin" and log at the "end"

(mapM generateFieldPlan selSet)
VQ.RSubscription field ->
(GQExecPlanPartial G.OperationTypeMutation) <$>
(mapM generateFieldPlan (Seq.singleton field))
Copy link
Owner Author

@tirumaraiselvan tirumaraiselvan Oct 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't like this sequencing of a singleton but dunno what to do without complicating types.

enableAL sc scVer reqUnparsed = do
planM <- liftIO $ EP.getPlan scVer (userRole userInfo)
opNameM queryStr planCache
-> m (Seq.Seq GQFieldResolvedPlan)
Copy link
Owner Author

@tirumaraiselvan tirumaraiselvan Oct 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In RJ, this will change to GQFieldResolvedPlan -> QExecPlan (https://github.com/hasura/graphql-engine/blob/821c1c3525e36f462bedee96908646e43225c442/server/src-lib/Hasura/GraphQL/Execute.hs#L180)

where QExecPlan is type QExecPlan = ([QExecPlanUnresolved], GQFieldResolvedPlan) (https://github.com/hasura/graphql-engine/blob/821c1c3525e36f462bedee96908646e43225c442/server/src-lib/Hasura/GraphQL/Execute.hs#L95)

Might be a good idea to rename QExecPlan -> GQFieldPlan and QExecPlanUnresolved -> GQRemoteRelUnresolvedPlan

case opType of
G.OperationTypeQuery ->
forM fieldPlans $ \case
GQFieldPartialHasura (gCtx, field) -> do
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GV.RQuery selSet ->
runInTx $ encJFromJValue <$> traverse (explainField userInfo gCtx sqlGenCtx) (toList selSet)
GV.RMutation _ ->
E.GQExecPlanPartial opType fieldPlans <-
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think Explain is that ugly anymore because of the new GQExecPlanPartial structure!

GQFieldPartialHasura (gCtx, field) -> do
(queryTx, plan, genSql) <-
getQueryOp gCtx sqlGenCtx userInfo (Seq.singleton field)
-- traverse_ (addPlanToCache . EP.RPQuery) plan
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how to cache?

forM fieldPlans $ \case
GQFieldPartialHasura (gCtx, field) -> do
(lqOp, plan) <- getSubsOp pgExecCtx gCtx sqlGenCtx userInfo field
-- traverse_ (addPlanToCache . EP.RPSubs) plan
Copy link
Owner Author

@tirumaraiselvan tirumaraiselvan Oct 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how to cache? Might not be a problem here because subscriptions are always single field.

tirumaraiselvan pushed a commit that referenced this pull request Apr 21, 2021
* ci: add hlint escape hatch

Co-authored-by: Antoine Leblanc <[email protected]>
GITHUB_PR_NUMBER: 6164
GITHUB_PR_URL: hasura#6164

* Applied changes to new workflow files.

* Add missing label trigger for lint worklow

Co-authored-by: Antoine Leblanc <[email protected]>
GitOrigin-RevId: 3e22c30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant