feat(utils): @pgQuery support for scalars

[phryneas]

We had already chatted about this - this PR allows for scalars to be added as subqueries using the @pgQuery directive with the fragment argument.

If this is merged, it would be great if you could mention my employer Mayflower in the release notes, as this PR was done on company time. But if this something you don't do, thats fine, too 😃

[phryneas]

As for that failing test... I have no idea what that means. Doesn't seem to fail in every environment?

[phryneas]

Okay, figured it out - my test-SQL needed a CAST for Postgres 9. Tests are now working fine :)

[benjie]

Awesome; from a superficial glance this looks great but I'll try and review it properly next week. In the mean time, please change your CAST(${...} AS type) to ${...}::type for consistency with the rest of the project 😁

[phryneas]

sure thing 👍
should be consistent now.

[singingwolfboy]

Once #533 lands, can you rebase this pull request on top of it and simplify this test? You'll no longer need to call printSchema() yourself, you can just do expect(schema).toMatchSnapshot(). 😄

I'm not sure when that PR will land, but I wanted to give you a heads-up.

[phryneas]

will do, please ping me when it lands

[benjie]

Nice; this seems to work well! Couple minor bits of feedback to address, plus I just merged #533 so please rebase and regenerate your snapshots 👍

[benjie]

We need to test references to the graphile_utils.users table itself, so please can you change this to return a String and do something like:

Suggested change

	fragment: ${embed(queryBuilder => sql.fragment`(SELECT 101)`)}
	fragment: ${embed(queryBuilder => sql.fragment`(${queryBuilder.getTableAlias()}.name || ' ' || ${queryBuilder.getTableAlias()}.email)`)}

[benjie]

Check that these work as expected with aliases too, please.

Suggested change

	myCustomScalarWithFunctionAndArgument(test: 102)
	myCustomScalarWithFunctionAndArgument(test: 102)
	m103: myCustomScalarWithFunctionAndArgument(test: 103)
	m104: myCustomScalarWithFunctionAndArgument(test: 104)

[phryneas]

I'm guessing you want one alias for each one of them, not the same one twice?

[benjie]

These are different aliases? You might have missed the red line was a deletion... not sure why GitHub did that 🤷‍♂️

[phryneas]

yup, but they were both aliases for myCustomScalarWithFunctionAndArgument, so I wasn't sure if that was really your intention or if you wanted one alias for myCustomScalar, one for myCustomScalarWithFunctionAndArgument, etc.

ah well, seems like what I did worked out for you :D

[benjie]

This is equivalent to:

Suggested change

	sql.fragment`array(SELECT name from ${sql.fragment`graphile_utils.pets`})`
	sql.fragment`array(SELECT name from graphile_utils.pets)`

I've never used the array constructor in this way; I've always done select array_agg(name) from ... instead. It seems that this might actually be faster because it's simpler so I should definitely experiment with its usage in PostGraphile.

[benjie]

Filed: #543

[phryneas]

@benjie I think I adressed everything

[benjie]

Awesome! If you have a few more minutes it would be great if you could incorporate this into the makeExtendSchemaPlugin docs: https://github.com/graphile/graphile.github.io/edit/develop/src/pages/postgraphile/make-extend-schema-plugin.md