Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non prepared execution mode #41

Closed
char101 opened this issue Mar 1, 2020 · 9 comments
Closed

Non prepared execution mode #41

char101 opened this issue Mar 1, 2020 · 9 comments

Comments

@char101
Copy link

char101 commented Mar 1, 2020

Hi,

First I want to say that this is a great library, simple but powerful.

Anyway, I just found a case where executing a query with bind parameters results in much more slow execution time (5s) vs running it with hardcoded values (30ms). I believe this is caused due to the use of prepared statement, where the server created an execution plan without looking at the types of the bind parameters, thus some indexes are not used. To fix this we could explicitly cast the bind parameter, e.g. ${value}::INT, but it is a bit of an annoyance to have to cast every bind parameter. So if possible I think it would be great to have a non-prepared execution mode as an alternative.

For comparison, the same query runs fast with python/psycopg2 because although it supports bind parameter in the client (via %s format) it sends the query to the server in non-prepared mode, that is it directly replaces the bind parameter placeholder with its values.

@Minigugus
Copy link
Contributor

By doing ${value}::INT, postgres translate it to $1::INT, so it seems that the problem is not really adding a non-prepared mode but imposing types for parameters in queries.

Moreover, postgres already detects parameter types to encode them, maybe it could automatically add casts ? e.g. ${1337} -> $1::int instead of just $1 ?

@porsager
Copy link
Owner

porsager commented Mar 1, 2020

Hi @char101 ..

First off, thanks..

That's a really interesting case. Which PostgreSQL version are you using?

Postgres.js will pass along the types it has inferred over the wire. Maybe you could try to run an explain with dynamic values and one with static values?

@porsager
Copy link
Owner

porsager commented Mar 1, 2020

@Minigugus the inferred types are already passed alongside the parameters in the protocol, and I think it'd be a mess to start rewriting queries by adding explicit types, it's best to leave that up to the only one who knows what it should be for sure - the user 🙂

@char101 by the way, I can't read if you tested with explicit types (::int) and got good results?

@porsager
Copy link
Owner

porsager commented Mar 1, 2020

Uh btw @char101 . Could you try your query with the latest on master as well? (just npm install porsager/postgres)

@char101
Copy link
Author

char101 commented Mar 2, 2020

By doing ${value}::INT, postgres translate it to $1::INT, so it seems that the problem is not really adding a non-prepared mode but imposing types for parameters in queries.

Sure if that can be done automatically and accurately.

Moreover, postgres already detects parameter types to encode them, maybe it could automatically add casts ? e.g. ${1337} -> $1::int instead of just $1 ?

What if the user supply an incorrect type, for example, a numeric string to an integer column?

@char101
Copy link
Author

char101 commented Mar 2, 2020

Uh btw @char101 . Could you try your query with the latest on master as well? (just npm install porsager/postgres)

I have changed it to the master branch and still experience the long execution time without type casts.

Also I am using express and with the master branch version, I am getting this error: Do not know how to serialize a BigInt when sending the query result as json. This does not happen before when using the released version. From what I saw, the result of a SELECT COUNT(n) query is a BigInt in the master branch version.

I am using PostgreSQL 12.2 and node 13.7.0.

Postgres.js will pass along the types it has inferred over the wire. Maybe you could try to run an explain with dynamic values and one with static values?

A prepared statement is supposed to be run multiple times, so the planner might not look at the type of the bind variable since it can change later. Also I think the bind variable is only used in the execution phase, not in the preparing phase.

This are the query plans

As you can see the plan is similar when using hardcoded values and bind parameter with type casts, while the execution time of the query with bind parameter without type casts is 160x slower.

@char101
Copy link
Author

char101 commented Mar 2, 2020

My solution currently is to add type casts to all the bind variables. TBH it isn't really that much of an effort 😀 since they are only either INT or TEXT.

@char101
Copy link
Author

char101 commented Mar 2, 2020

In the parameterized without type cast query plan the query planner choose to cast the integer values to numeric rather than int. This might be a possible cause for the slow execution since it means that existing index will not be used and numeric operations are also slower compared to integer operations.

Also adding type cast to ::NUMERIC to the bind parameter in the query rather than ::INT results in the same slow execution.

This is actually consistent with the type of the bind variables sent by the library:

[
  { type: 1700, value: '1' },
  { type: 1700, value: '1' },
  { type: 1700, value: '1' },
  { type: 1700, value: '1' },
  { type: 0, value: 'evaluator' }
]

1700 is NUMERIC rather than INTEGER (23).

The execution time is now normal if I change the 1700 in https://github.com/porsager/postgres/blob/master/lib/types.js#L99 to 23.

@porsager
Copy link
Owner

porsager commented Mar 8, 2020

This is great debugging @char101 :)

In the next major version I think I'll switch to unknown types instead for javascript Numbers, since Postgres itself is better at casting to the correct type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants