Unable to pass FieldArg as parameter for resource .get()

[hanahsa]

Summary

Passing FieldArg as one of the parameters of resouce.get({}) results in an error.

Steps to reproduce

return comments.get({
  post_id: $parentStep.get('id'),
  user_id: fieldArgs.get('user_id'),
});

Expected results

The comment to be returned.

Actual results

Error occurred during optimize; whilst processing PgSelect{1}<comments>[20] in dependents-first mode an error occurred: Error: Expected __TrackedValue➊[18] (1th dependency of PgSelect{1}<comments>[20]; step with id 2) to be a PgClassExpressionStep

Additional context

I could work around this by instead doing

const commentsSelect = comments.find({ post_id: $parentStep.get('id') });
const userIdPlaceholder = commentsSelect.placeholder(fieldArgs.get('user_id'));
commentsSelect.where(sql`user_id = ${userIdPlaceholder}`);
return commentsSelect.single();

but this feels clunky.

In grafast-vbeta.6 and postgraphile-vbeta.21 I had this workaround working

return comments.get({
  post_id: $parentStep.get('id'),
  user_id: (fieldArgs.getRaw('user_id') as __TrakcedValueStep).optimize(),
});

but this is resulting in the same error in the latest versions.

Possible Solution

[benjie]

This is where the error is thrown:

https://github.com/graphile/crystal/blob/15ec6f6483a7472e349ee659771e52ac8e731314/grafast/dataplan-pg/src/steps/pgSelect.ts#L2639-L2641

[benjie]

I think if you made this change it would be fine?

 return comments.get({
   post_id: $parentStep.get('id'),
-  user_id: fieldArgs.get('user_id'),
+  user_id: fieldArgs.getRaw('user_id'),
 });

[benjie]

I think the issue is that the optimize method is not being suitably discerning about the identifierMatches - if they're incompatible with the optimize approach then we shouldn't even try and inline. I think the check itself is also misguided, clearly I was solving a "just make it work in these two situation and throw an error if it's outside those" style of problem at the time, but now's the time to think about it more carefully and handle those edge cases (mostly; I think we should be checking: is the parent step allowed to depend on the given steps - if so, then it should be fine to inline, if not then we definitely shouldn't).

[benjie]

I was unable to reproduce this; please can you create a minimal reproduction against benjie/ouch-my-finger and link to it here.

[hanahsa]

created https://github.com/benjie/ouch-my-finger/pull/15. As commented there though, not sure if its 100% minimal

[benjie]

Thanks!

[benjie]

Issue reproduced; the SQL file didn't need the CREATE UNIQUE INDEX lines (and in fact, running it with them didn't work because they caused duplicates versus the PK indexes that already existed), but that wasn't much bother to remove. Thanks :raised_hands: