chore: Optimize postgres - use of rowCallback approach

[Ivansete-status]

Description

This represents a quite important time reduction in Postgres queries. However, still slower than SQLite.
The main issue happened because the we were processing the obtained rows multiple times, and there were an overhead due to changing the rows objects from one type to another.
In this PR we aim to reduce the type conversion overhead by providing a rowCallback(result: ptr PGresult) to the dbconn object.

( Thanks to @NagyZoltanPeter for the help to achieve that. )

Changes

• Use of rowCalback to retrieve rows in a more direct way.
• Change logic on how to retrieve the first free connection - waku/common/databases/db_postgres/pgasyncpool.nim

Issue

#1842

[github-actions]

This PR may contain changes to database schema of one of the drivers.

If you are introducing any changes to the schema, make sure the upgrade from the latest release to this change passes without any errors/issues.

Please make sure the label release-notes is added to make sure upgrade instructions properly highlight this change.

[github-actions]

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2171

Built from 3145510

[NagyZoltanPeter]

Read through and totally awesome work! Thank you!!!

[NagyZoltanPeter]

@Ivansete-status: One concern I thought about, but not connected directly to this PR.
The query where clause processing (aka string replacing arguments) maybe harmful. I did not look through the whole data flow, but as this is fully exposed to the interface level, it might be possible that one can achieve SQL injection attack against the db.
Maybe it is worth to track down later.
Also maybe to consider to switch to prepared statements and execution if possible, that would mean some little perf gain on DB side either.

[Ivansete-status]

Also maybe to consider to switch to prepared statements and execution if possible, that would mean some little perf gain on DB side either.

Thanks for the comment!
In fact, today I was planning to do exactly that and use prepared statements in the select queries. The thing is that when we have concurrent queries, each connection stays in "busy" state (db.pqisBusy() == 1) for a lot of time, once the query has been sent (db.pqsendQuery(...).)

I agree to review the "SQL injection" issue in the future.

[NagyZoltanPeter]

Also maybe to consider to switch to prepared statements and execution if possible, that would mean some little perf gain on DB side either.

Thanks for the comment!
In fact, today I was planning to do exactly that and use prepared statements in the select queries. The thing is that when we have concurrent queries, each connection stays in "busy" state (db.pqisBusy() == 1) for a lot of time, once the query has been sent (db.pqsendQuery(...).)

I agree to review the "SQL injection" issue in the future.

Indeed, prepared statement secure that one as well.

[SionoiS]

LGTM

Thanks!

[SionoiS]

We don't log anything anymore? Or was it not used.

[Ivansete-status]

Good point! It was not used. In fact, all the exposed procs return Result so it is pretty likely that we won't need many longs in this file.