Fork me on GitHub
#pathom
<
2022-11-04
>
Eric Dvorsak17:11:40

@wilkerlucio I think I found an issue with batching in pathom3: I get a java.lang.IndexOutOfBoundsException from missing-maybe-in-pending-batch?

Eric Dvorsak17:11:50

batch minimal repro:

(pco/defresolver unauthorized-toy
  [env input]
  {::pco/input  [:toy/id]
   ::pco/batch? true
   ::pco/output [{:toy/unauthorized [:toy/name :toy/id :child/id]}]}
  [{:toy/unauthorized {:toy/name "Bobby" :toy/id 1 :child/id 1}}])

(pco/defresolver toy
  [env input]
  {::pco/input  [{:toy/unauthorized [:toy/name :toy/id :child/id :child/authorized?]}]
   ::pco/batch? true
   ::pco/output [:toy/name :toy/id :child/id]}
  (mapv :toy/unauthorized input))

(pco/defresolver child-toys
  [env input]
  {::pco/input  [:child/id]
   ::pco/batch? true
   ::pco/output [{:child/toys [:toy/id]}]}
  [{:child/toys [{:toy/id 1}]}])

(pco/defresolver unauthorized-child
  [env input]
  {::pco/input  [:child/id]
   ::pco/batch? true
   ::pco/output [{:child/unauthorized [:child/name :child/id :parent/id]}]}
  [{:child/unauthorized {:child/name "Bob" :child/id 1 :parent/id 1}}])

(pco/defresolver child
  [env input]
  {::pco/input  [{:child/unauthorized [:child/name :child/id :parent/id :parent/authorized?]}]
   ::pco/batch? true
   ::pco/output [:child/name :child/id :parent/id]}
  (mapv :child/unauthorized input))

(pco/defresolver auth-child
  [env input]
  {::pco/input  [:child/id :parent/authorized?]
   ::pco/batch? true
   ::pco/output [:child/authorized?]}
  #_(Thread/sleep 1000)
  [{:child/authorized? (:parent/authorized? input)}])

(pco/defresolver auth-parent
  [env input]
  {::pco/input  [:parent/id]
   ::pco/batch? true
   ::pco/output [:parent/authorized?]}
  (Thread/sleep 1000)
  [{:parent/authorized? true}])

(def env (-> (pci/register
              [unauthorized-toy
               toy

               child-toys
               unauthorized-child
               child
               auth-child
               auth-parent])
             (p.connector/connect-env {::pvc/parser-id `env-test})))

Eric Dvorsak17:11:09

Non batch:

(pco/defresolver unauthorized-toy
  [env input]
  {::pco/input  [:toy/id]
   ::pco/output [{:toy/unauthorized [:toy/name :toy/id :child/id]}]}
  {:toy/unauthorized {:toy/name "Bobby" :toy/id 1 :child/id 1}})

(pco/defresolver toy
  [env input]
  {::pco/input  [{:toy/unauthorized [:toy/name :toy/id :child/id :child/authorized?]}]
   ::pco/output [:toy/name :toy/id :child/id]}
  (:toy/unauthorized input))

(pco/defresolver child-toys
  [env input]
  {::pco/input  [:child/id]
   ::pco/output [{:child/toys [:toy/id]}]}
  {:child/toys [{:toy/id 1}]})

(pco/defresolver unauthorized-child
  [env input]
  {::pco/input  [:child/id]
   ::pco/output [{:child/unauthorized [:child/name :child/id :parent/id]}]}
  {:child/unauthorized {:child/name "Bob" :child/id 1 :parent/id 1}})

(pco/defresolver child
  [env input]
  {::pco/input  [{:child/unauthorized [:child/name :child/id :parent/id :parent/authorized?]}]
   ::pco/output [:child/name :child/id :parent/id]}
  (:child/unauthorized input))

(pco/defresolver auth-child
  [env input]
  {::pco/input  [:child/id :parent/authorized?]
   ::pco/output [:child/authorized?]}
  #_(Thread/sleep 1000)
  {:child/authorized? (:parent/authorized? input)})

(pco/defresolver auth-parent
  [env input]
  {::pco/input  [:parent/id]
   ::pco/output [:parent/authorized?]}
  #_(Thread/sleep 1000)
  {:parent/authorized? true})

(def env (-> (pci/register
              [unauthorized-toy
               toy

               child-toys
               unauthorized-child
               child
               auth-child
               auth-parent])
             (p.connector/connect-env {::pvc/parser-id `env-test})))

Eric Dvorsak17:11:25

Query:

[{[:child/id 1] [:child/name {:child/toys [:toy/id :toy/name]}]}]

Eric Dvorsak17:11:39

What I noticed is that when running batch resolvers, the toy resolver is ran in parallel with the unauthorized-toy resolvers so even though the planner found the right path with the right inputs for toy through unauthorized-toy, it tries to resolve it before it and end up with missing attributes. I suspect that the bug is somewhere in the planner rather than in missing-maybe-in-pending-batch? where the exception is caused by a path in batch-pending* that is shorter than the one in the env

Eric Dvorsak18:11:20

Seems like it might be a runner rather than planner issue since the snapshot plans in pathom viz are the same

wilkerlucio18:11:29

thanks, can you please open an issue for it?

Eric Dvorsak19:11:38

I thought that the planner was only running once in pathom3 but it's actually done on every subquery?

wilkerlucio19:11:25

yeah, thats correct, but a bit more complicated than that

wilkerlucio19:11:01

pathom does verify (at parent planning) that a sub query is possible, so it fails early if it detects an impossible path at the subquery

wilkerlucio19:11:33

but then later it plans again, because the plan depends on what data actually came (in contrast to what data it expected to have)

Eric Dvorsak19:11:59

but it's using the plan-cache* so not replanning from scratch right?

wilkerlucio19:11:36

correct, in case it matches the expectation it will possibly use from cache

Eric Dvorsak19:11:22

do you have an hunch on how this bug is happening? trying to get a grasp of the runner code to find it myself

wilkerlucio19:11:56

afk at the moment, I hope I’ll be able to have a look by tomorrow

Eric Dvorsak19:11:05

ok I'll add my findings to the issue if I get something

👍 1
Eric Dvorsak12:11:18

@wilkerlucio so I think it might actually be the planner

Eric Dvorsak12:11:27

I haven't figured out yet how the planning works exactly but it definitely seems odd that one sub-plan is (2 unauthorized-toy) -> (1 toy) directly, while another is doing (unauthorized-child)->(child)->(auth-parent)->(auth-child) where (auth-child) result should have been the input of (1 toy)

Eric Dvorsak12:11:03

it might just be working without batches if the planning goes depth first

Eric Dvorsak08:11:56

@wilkerlucio I suspect the problem is that comput-run-graph is only generating a run plan for one level of the AST, so it misses nested inputs. It tries to resolve them on the next level but at this point the resolver that is missing these nested inputs is already planned in the previous level and decoupled from the subgraph that resolves them

Eric Dvorsak08:11:27

I suspect compute-nested-requirements might lead the planner to expect outputs without considering required inputs

Eric Dvorsak13:11:20

@wilkerlucio I was able to successfully complete the query by passing the ::pcp/id-counter to the parser env directly and removing it from pcp/base-env as I suspected nodes were getting overwritten

Eric Dvorsak13:11:06

also I had to set pcp/optimize-graph? to false

wilkerlucio13:11:25

hello Eric, sorry, I wasn't able to sit at all at my computer this weekend, so I didn't had a chance to check, and glad you are making such investigation on it

wilkerlucio13:11:24

but one thing, about optimize graph, I highly discourage disabling it, its a crucial part of the planner to stay performant, if its the case it works after removing, it may be a signal that some optimization might be broken, and if so, the best path is to figure what is broken there

Eric Dvorsak13:11:39

yes I removed it to see if it was working without

Eric Dvorsak13:11:45

but I still think there is something wrong with the plan, I would expect a graph where toy/name is resolved by unauthorized/toy AND child/authorized?

Eric Dvorsak13:11:49

what I haven't figured out yet is why the planner seems to find that path when planning toy/name, but doesn't add it to the plan and still comes up with just toy/unauthorized -> toy

Eric Dvorsak17:11:32

more or less back to zero. only certitude so far is that by tweaking missing-maybe-in-pending-batch? so that it doesn't try to subvec the path if pending path is shorter it sometimes solve the query (non-deterministic)

Eric Dvorsak17:11:21

@wilkerlucio so my latest theory is that shape-reachable? is computing a whole new plan and determines that all the inputs of toy can be reached through unauthorized/toy (which is true if the planner was providing the plan for the nested ones) then the issue happens when the runner gets the result of unauthorized/toy and tries to run toy before a new round of planning that actually produces a plan that reaches the whole shape

Eric Dvorsak12:11:48

@wilkerlucio been wrong so many times now I'm not sure what it's worth but I'm now suspecting the fact that :child/unauthorized is present twice in the graph (one for [:child/id 1] and one within :toy/unauthorized at some point during the graph run

kendall.buchanan20:11:28

What is the proper way to pass a param (e.g. pco/params) to an intermediate resolver?

dvingo14:11:33

one technique is to collect all query params in all top level children of the root query and make them available in the environment, for example: https://github.com/holyjak/minimalist-fulcro-template/blob/7ca46fbbb53ac6d18ff54fd80f89b5109d502fe7/src/com/example/server/pathom.clj#L68

kendall.buchanan16:11:40

Great idea, thank you.

👍 1
kendall.buchanan13:11:44

@wilkerlucio While I appreciated @U051V5LLP’s answer above, I’m a little surprised by the behavior. I’m finding, in practice, if I don’t have a query that immediately resolves to its query, the param is lost. In other words, the param is only good for one hop. Why is that the case?

wilkerlucio13:11:39

hello @U0HJA5ZQT, its a bit a tricky issue, originally most of the time params were used to things like pagination of a collection, where the param goes straight to the attribute that provides the collection, what I'm guessing in your case is that you may wanna use a param at some dependency of the attribute in which you place the param, is that a correct assessment?

kendall.buchanan14:11:31

It’s simpler than that: I to provide the param at the beginning of an HTTP request, and know it’ll be available two, three resolvers in.

kendall.buchanan14:11:02

Lemme mock it up real quick…

kendall.buchanan14:11:25

; Ident
{:user/username ""}

; Query
['(:checklists {:my-param true})]

(pco/defresolver resolver-1
  [_ {username :user/username}]
  {:group/ids (fetch-group-ids username)})

(pco/defresolver resolver-2
  [env {group-ids :group/ids}]
  (let [{:keys [my-param]} (pco/params env)]
    {:checklists (fetch-checklists group-ids my-param)}))

kendall.buchanan14:11:16

AFAICT this doesn’t work. It only works if the ident leads directly to :checklists

wilkerlucio14:11:59

I see, in this case you are in different entities, so by design this is not a way params are designed to flow, the params is an extra dimension for the attribute in which you use it, if you wanna flow it down its up to you, because if I try to do that at Pathom level, there will be ambiguities that I can't reconcile generically, because the user might not want those things down there, mixed with other (or even conflicting) params, makes sense?

kendall.buchanan14:11:27

i.e. I can choose to pass the param on?

wilkerlucio14:11:34

just one thing, I was giving a bit more look at your example, and it does work in that case, but I think what you want is to have that down somehow, but I think the example may not represent what you trying to convey

wilkerlucio14:11:40

(ns com.wsscode.pathom3.demos.params-flow
  (:require
    [com.wsscode.pathom3.connect.indexes :as pci]
    [com.wsscode.pathom3.connect.operation :as pco]
    [com.wsscode.pathom3.interface.eql :as p.eql]))

(defn fetch-group-ids [username]
  )

(pco/defresolver resolver-1
  [_ {username :user/username}]
  {:group/ids (fetch-group-ids username)})

(defn fetch-checklists [group-ids my-param]
  (println "P" my-param))

(pco/defresolver resolver-2
  [env {group-ids :group/ids}]
  {::pco/output [:checklists]}
  (let [{:keys [my-param]} (pco/params env)]
    {:checklists (fetch-checklists group-ids my-param)}))

(def env
  (pci/register
    [resolver-1
     resolver-2]))

(comment
  (p.eql/process env
    {:user/username ""}
    ['(:checklists {:my-param true})]))

wilkerlucio14:11:47

(p.eql/process env
    {:user/username ""}
    ['(:checklists {:my-param true})])
P true
=> {:checklists nil}

wilkerlucio14:11:38

you can use env to pass down, like we said before, I'm not sure if we can make it propagate down as a param, but it might be possible, I have to spend a bit more time to verify if forwarding params down is possible

kendall.buchanan14:11:30

Mmmm. My real world example I’m testing with has resolvers four or five depths in.

kendall.buchanan14:11:52

I’ll have to look carefully for where the param drops off, and what’s different about that from the example I gave.

kendall.buchanan14:11:28

But do you see any conceptual problems with the example above? I understand the notion of conflicting params… but is that not what namespacing is about?

wilkerlucio14:11:33

in this example, we send params to :checklists, and read them at the resolver that provides :checklists, when there is this match, you should always have the param avaialble

wilkerlucio14:11:07

not really, because that's a difference in context, its not about the param itself, but where its applicable

wilkerlucio14:11:17

one thing that I still trying to figure is that if you want to forward it laterally (meaning its the same entity, but a different resolver in the execution graph) or if its vertical (flowing down to a different entity)

wilkerlucio14:11:44

a more concrete example will be useful to understand your context

kendall.buchanan14:11:04

Thank you for your help… lemme dig deeper, cause I’m not sure why the example I shared above worked, and what I’m working on does not. It’s a matter of tracing through more resolvers.

kendall.buchanan14:11:30

Ah! Figured it out.

kendall.buchanan14:11:56

I had another resolver with the same output it was resolving to, and my tests were going in different directions.

kendall.buchanan14:11:31

But yeah… the reason I originally kicked off this question WAS because of what you’re talking about: needing the param for an entity that did not match the provided query entity. 👈

wilkerlucio14:11:36

to give some reasoning on my design decisions, I like to keep things open so users have the flexibility to explore different ways, in Pathom case I think we still have so much to learn about how to model/query things, when regarding params, the same entity might want to use the same param with different values for different attributes, this means merging laterally would cause a conflict in such scenario

wilkerlucio14:11:24

similar idea goes when flowing down, how can I know that flowing is intentional? what if the user wants the param in a level ,but flowing down might configure something unintentionally? if we flow down, we do for every attribute? and how many levels it would flow down?

kendall.buchanan14:11:41

I suppose if the params had context around them…

kendall.buchanan14:11:11

['(:checklists {:my-param true})] → There’s context around what that param was provided for.

kendall.buchanan14:11:52

I suppose if there was a way a resolver could access that somehow, knowing that the param was not intended for it.

wilkerlucio14:11:19

one way is to make a plugin to add this context to env, maybe something like: {[PATH attribute] params}

wilkerlucio14:11:44

one other thing you can use is the query meta

wilkerlucio14:11:52

its a way to add information for a entity section

wilkerlucio14:11:41

so instead of [{(:checklists {:my-param true}) [:sub-query]}], you can try: [{:checklists ^{:my-param true} [:sub-query]}]

wilkerlucio14:11:12

that's a different dimension, where you add data to a query fragment, instead of the attribute

kendall.buchanan14:11:44

And how is it accessed? (meta env)?

wilkerlucio14:11:55

let me check that

wilkerlucio14:11:13

(ns com.wsscode.pathom3.demos.params-flow
  (:require
    [com.wsscode.pathom3.connect.indexes :as pci]
    [com.wsscode.pathom3.connect.operation :as pco]
    [com.wsscode.pathom3.interface.eql :as p.eql]))

(pco/defresolver resolver-1
  [env _]
  {:foo
   (let [{:keys [my-param]} (-> env
                                :com.wsscode.pathom3.connect.planner/graph
                                :com.wsscode.pathom3.connect.planner/source-ast
                                :query
                                meta)]
    (str "bar - " my-param))})

(pco/defresolver resolver-2 []
  {:checklists [{:id 1}]})

(def env
  (pci/register
    [resolver-1
     resolver-2]))

(comment
  (p.eql/process env
    ['{:checklists ^{:my-param true} [:foo]}]))

wilkerlucio14:11:07

note that, if you are sending this query over the wire, make sure its encoding/decoding meta, otherwise it wont work, an alternative is to send the AST instead of the query, which exposes the meta out (via boundary interface you can send {:pathom/ast AST})

kendall.buchanan14:11:16

Okay, so {:my-param true} has no context in relation to :checklists—it’s just along for the ride, wherever it might be needed.

kendall.buchanan14:11:34

This is all very helpful, thank you.

🙏 1
wilkerlucio14:11:39

it is in the context of :checklists subquery, and will only be available there (attributes in that vector)

wilkerlucio14:11:48

but it gets detached from the :checklists attribute

kendall.buchanan14:11:29

Okay, yeah, better.

kendall.buchanan14:11:43

K, thanks @wilkerlucio, plenty of options to work with here.

🙂 1
kendall.buchanan20:11:47

I have the following [{:org.benefit/groups [:group/id :group/name]}]

kendall.buchanan20:11:37

But an intermediate resolver (`:groups/children`) can take a param (`:public-mode? true`). What’s the right EQL syntax to pass it to the intermediate resolver?

kendall.buchanan20:11:15

This appears to work, but it also returns additional results in the response map (which are not wanted or needed):

kendall.buchanan20:11:17

[(:group/children {:public-mode? true})
 {:org.benefit/groups [:group/id :group/name]}]