graphile-engine icon indicating copy to clipboard operation
graphile-engine copied to clipboard

Allow sql`foo = bar` instead of sql.fragment`foo = bar`

Open rattrayalex opened this issue 4 years ago • 4 comments
trafficstars

Feature description

At first I was going to suggest documenting that sql.fragment is an alias for sql.query, since the former is used in the graphile.org docs for extending the schema, whereas only the latter is documented in the pg-sql2 readme. Turns out they're the same.

For people like myself who may be interested in heavily customizing the schema at the JS layer, convenient query building is important.

I might suggest allowing people to directly use build.pgSql`foo = bar` rather than requiring .fragment or .query, which is more verbose.

Additionally, if possible, it'd certainly be nice if it could just be build.sql instead of build.pgSql. I assume that's not done because technically it could be used with other db's, but for people just using postgres it'd be great for this to be convenient.

Among other reasons, syntax highlighters recognize sql`foo = bar` but would not recognize pgSql, and the mandatory rename is boilerplate.

I also happen think it'd be nice to allow shorthand for sql.join() like sql(['foo = bar', 'qux is not null'], ' and ') but I imagine this would not be to your taste and I'm not sure how frequently it's really needed anyway. (You can check for the .raw property on the first argument to your template tag function to accomplish this).

Supporting development

I [tick all that apply]:

  • [ ] am interested in building this feature myself
  • [ ] am interested in collaborating on building this feature
  • [ ] am willing to help testing this feature before it's released
  • [ ] am willing to write a test-driven test suite for this feature (before it exists)
  • [x] am a Graphile sponsor ❤️
  • [ ] have an active support or consultancy contract with Graphile

rattrayalex avatar Apr 14 '21 01:04 rattrayalex

Yeah, this is one of the first things I did in V5 🤣

build has a naming convention to avoid naming conflicts; everything added to it that comes from graphile-build-pg has to have pg in the name. I might change this to namespaces (build.pg.sql) in V5; haven’t decided yet (because it complicates the hooks for expanding Build).

benjie avatar Apr 14 '21 08:04 benjie

Bah, I guess I get that… I guess I'm curious, what non-postgres usecases have proven most valuable so far? I always get a bit leery when a deep-db tool adds support for more DB's, assuming that eg Hasura's MySQL support will just distract from their ability to make Postgres use awesome.

rattrayalex avatar Apr 14 '21 18:04 rattrayalex

I've tagged it as "consider for v5" but really it's already implemented so :shrug:

Bah, I guess I get that… I guess I'm curious, what non-postgres usecases have proven most valuable so far? I always get a bit leery when a deep-db tool adds support for more DB's, assuming that eg Hasura's MySQL support will just distract from their ability to make Postgres use awesome.

Well in terms of financial value, the plugins I added to one project for integrating Stripe were pretty valuable...

Keep in mind PostGraphile is designed to be extended. It's a fundamental design goal. So even if we don't add other features in core, we need to give people space to integrate with other services without introducing naming conflicts on patch/minor updates. Anyone that extends the Build object (whether related to PostgreSQL or otherwise) should do so using their own initials or other unique text: https://www.graphile.org/graphile-build/hooks/#namespaces

benjie avatar Apr 15 '21 09:04 benjie

Fair enough! Certainly would want someone to be able to build a mysql plugin if they wanted to without too much confusion.

I guess in that case I might cast my vote for build.pg.sql over build.pgSql in the next version.

(Though I do personally think given extant usage it'd be fair for the pg plugin to be first-class, with build.sql and context.client referring to postgres, and a future non-pg plugin using build.mysql.sql and so on)

rattrayalex avatar Apr 15 '21 12:04 rattrayalex

pg-sql2 does indeed have sql`...` and is usable in PostGraphile V5. Also we went ahead and added sql directly to Build :man_shrugging:

benjie avatar Sep 27 '23 16:09 benjie