Check for nested `sql` query after 'IN' keyword.

[HwangTaehyun]

Hi @blakeembrey!

First, thanks for the helpful open source!

I use this for Prisma raw query very well, but I think it would be helpful if the error is thrown when using the array value after 'IN' keyword in the raw query by mistake.

It is not easy to find what is error in that case. Could you review this PR?

I would like to do this PR as part of hacktoberfest. Would you add the label "HACKTOBERFEST-ACCEPTED" to this PR?

Thanks!

[blakeembrey]

Also, as a follow up comment, there's a couple of other issues with going down this path:

1. Building a parser isn't something I'd want to maintain, i.e. what about IN( or IN (? Could regex it but it's a slippery slope, someone else will end up with a similar request to validate other parts.
2. It is actually valid to use an array value after IN when trying to use it with tuples, e.g. x IN ((1, 2), (3, 4)). I believe the way tuples are represented in JS is as arrays, so restricting this introduces problems for other people.

[HwangTaehyun]

Thanks for the review @blakeembrey. I understand. Then, what do you think of using the js variable like the array type after 'IN' keyword? Mostly, many developers have confused about when to use nested SQL or only js variables.

I would like to use

const ids = [1, 3, 5, 10, 20]
const result =
  await prisma.$queryRaw`SELECT * FROM User WHERE id IN (${ids})`

also with another nested query like

const ids = [1, 3, 5, 10, 20]
const result =
  await prisma.$queryRaw`SELECT * FROM User WHERE id IN (${Prisma.join(ids)})`

That being said, in the following case, I think it should use the nested query.

const nested = sql`SELECT id FROM authors WHERE name = ${"Blake"}`;
const query = sql`SELECT * FROM books WHERE author_id IN (${nested})`;

So, it would be helpful if the sql-template-tag supports using the js variable like the array type after 'IN' keyword.

[blakeembrey]

This library doesn't really change anything about the underlying SQL engine, so it's entirely valid to have all the constructs you mention above. For example, I do have usages in postgres that are more like WHERE (x, y) IN (${...}).

[HwangTaehyun]

Oh, I use that code for the Prisma, and does not recognize ${ids} when not to use Prisma.join. That being said, when I use the expression ${ids} instead of ${Prisma.join(ids)} in the test code, Sql recognizes this as length 1 array [[1,3,5]] instead of [1,3,5]. Is it okay?

[blakeembrey]

Is it okay?

Yes, but it’s likely not what you intended. The latter of [[1,3,5]] is useful when you’re trying to use IN with tuples. Most likely, you should be always using join like IN (${join(…)}) and it’s a bug as you point out, but I don’t really want to turn this module into a ton of SQL validation.

[blakeembrey]

One possibility that could be a good trade-off is putting this behind a development environment flag, so any performance impact of adding this feature to aid debugging can be limited to development environments. That would eliminate my concern around performance for adding this or additional checks. The other concern would be whether every SQL driver acts the same around treatment of arrays. I’m not sure I’m prepared to prescribe that yet, I only really have personal experience with postgres but this module works with anything (i.e. SQLite, MySQL, CockroachDB, Planetscale, MSSQL, etc).

[HwangTaehyun]

That makes more sense! Then, could I add this feature to work only in development? (by checking process.env.NODE_ENV is 'development'.) Checking in on development is very helpful!

And, I think my environment is MySQL, so this case is not working for me.

[HwangTaehyun]

I want to know your perspective on the above that! @blakeembrey

[blakeembrey]

Can you try asking Prisma first with their thoughts? I think it'd be ideal to avoid doing any parsing in this library still.

[HwangTaehyun]

Yes, I will do!

[blakeembrey]

Closing as it won't be supported in this library directly, sorry! I'd like to keep things as simple and small as possible. I still believe it makes sense to validate somewhere upstream such as in an actual query language that might be able to apply a regex to the entire SQL instead of just parts of it.