Binding/Prepared Statements

[james-scott-meta]

As far as I can tell, join-monster does not use bind variables in oracle or otherwise. Is there any way within join-monster to allow bind variables (in oracle) or some other way of making sure that the database doesn't need to recreate the query plan every time?

(I'm stuck with Oracle, but open question on the same issue with PostgreSQL as well.)

[acarl005]

Not yet. This is something we'd like to add.

[theobat]

isn't this supported by using knex (or some other query builder) out of the box ?

[bradzacher]

whilst pretty much any DB layer supports it, yes, join-monster doesn't support generating it yet.

[theobat]

wouldn't it be better to use some query builder instead of raw strings to support this ? Like using a composable sql query builder api to do a better sql query building than strings or template litterals ? Or instead of building it as a hardcoded query building capacity within knex, (which at the moment is done by pure text composition if I remember correctly) ?

I don't know if this is already possible, perhaps doing something like:

sqlExpr: (table) =>  knex.raw(`COALESCE(:table:.key, 'ok')`, {table})

is already enough ?

I mean, as far as I understand, join monster is doing query composition, why bother with the building side

[bradzacher]

that's the reason this issue is left open.
because right now join monster works by generating a base sql statement + joins.
In the case of wheres based on query arguments, the where function can only return a (Promised) string.

This means that there is currently no way to use truely parameterized queries with JM (as you can't return a string and a map of parameter keys to parameter values).

That being said, there's no reason you can't pre-sanitize your values (and you certainly should). Knex's raw is one way to do it.

At my company, we run sqlstring over each query argument before returning it from a where function.

Whilst these methods works fine (as long as there are no issues with the library's security!), you lose out on a big part of DB level optimisation. By hard inserting the query arguments, it means (as far as the DB is concerned) you are sending a completely new SQL request every time.
Which means that the DB can't cache/pre-optimise a query execution plan.
Using true parameterized queries allows you to better take advantage of the DB's optimisation.

[theobat]

I see, but my point is join monster should not be responsible for how the sql is parsed and passed to the db-engine, it should only expect an interface. I mean, instead of the sql ast, there should probably be an ast type (the type being a parameter, like knex for instance). The reason why I say this is because, composing knex objects together to build a query (even if those objects are knex.raw) does yield Prepared Statements in the db when executed. The fact that the above example is not yielding parametrized query is simply because we go through the custom join monster ast (which enforces string types as you said)...

In essence, I think join monster should manipulate sql query part objects, not parsing any real sql per say, just composing. Then the end user would be expected to use an interface/library that is composable.
In terms of interface:

• a function compose(QueryObject a, QueryObject b) returning QueryObject made of a and b
• a function execute(QueryObject) actually sending the query object to the database and or, transforming it into text
  That should be enough, don't know if this is clear though. Another way to go would be to expect actual sql capabilities in the interface like where, orderBy etc..

[bradzacher]

JM does generate its own AST before passing this to a builder to generate the SQL as per your configured implementation.

Right now the supported implementations are (at least..) MySQL, Postgres, Oracle, SQLite, MariaDB.
However, there's no reason that someone couldn't PR a builder which generates what you want.
I'm no expert on the codebase, but I believe something like that is technically feasible.

However, I doubt that form would be useful beyond whatever library you design it for (each library would probably have its own flavours of "query part objects" with no common standard).

JM ultimately is designed to be library agnostic. By generating a raw SQL statement by default, it means that it's easily interoperable with pretty much any DB library in node.

Parameterized queries are something difficult because not only does their syntax and nuances differ per SQL flavour, some libraries within the same SQL flavour handle them differently (i.e. within mysql some libraries support only ? ambiguous binding where parameter order really matters, some also support :named binding).

[theobat]

JM does generate its own AST before passing this to a builder to generate the SQL as per your >configured implementation.

Yeah I know I was just making a case for the argument: "it shouldn't, let the query-builder type of libraries do this".

Right now the supported implementations are (at least..) MySQL, PostgreSQL, Oracle.
However, there's no reason that someone couldn't PR a builder which generates what you want.

Again, what I'm saying is: a lot of work for things that have already been done. If, the expected metadata was returning a query object type (defined as a configuration at the beginning), this would not be necessary, it would be expected from the user in his metadata creation in the schemas

Though I doubt it would be useful beyond whatever library you design it for (each library would >probably have its own flavours of "query part objects" with no common standard).

Except if it's a parameter itself, that's why I was talking about interfaces.

This would defacto solve the issue with prepared statements and hugely simplify the extensibility of the lib. But in the end this is a huge amount of work though, with little practical benefits in the short term.

@acarl005 perhaps you have any reflections on this ?

[s97712]

Has plan to do this? Here is my solution #360 , but may need add more test

[melounek]

with little practical benefits in the short term

Am I missing something? Because from what I know, some queries are very slow without bindings.
Like was already said by @james-scott-meta:

making sure that the database doesn't need to recreate the query plan every time

So from this point of view I would put this issue pretty big priority and try to merge something like @s97712 is suggesting asap.