metosin / porsas Goto Github PK

The default implementation stores all unique SQL sentences on a HashMap, which is a potential memoery leak. The default should be a fast bounded cache.

Handlers for functions query-one and query in async context are equals therefore both functions returns only first row of result set.

This is mostly a heads up that next.jdbc row builders have a couple of extra expectations compared to when you first created porsas. Illustrated by the updated record-based builder in the test suite here: https://github.com/seancorfield/next-jdbc/blob/develop/test/next/jdbc/result_set_test.clj#L380-L406

The two main points:

• There was an implicit assumption that all builders would support :cols and :rsmeta to return the vector of column names and the ResultSetMetaData but that has been made explicit and all the built-in builders now explicitly support clojure.lang.ILookup for those keys,
• A new, low-level method has been added to RowBuilder -- with-column-value -- that is assumed to just do the "assoc" or "conj" equivalent, and is now used from most implementations of with-column; it is explicitly used by the new next.jdbc.result-set/builder-adapter and the existing as-*-adapter functions have been rewritten in terms of that.

the underlaying client already supports it, so, why not.

Using nvd-clojure:

; clojure -Sdeps '{:aliases {:nvd {:replace-paths [], :replace-deps #:nvd-clojure{nvd-clojure #:mvn{:version "RELEASE"}}, :exec-fn nvd.task/check :jvm-opts ["-Dclojure.main.report=stderr"]}}}' -X:nvd :classpath '"'"$(lein with-profile -base,-system,-user,-provided,-dev classpath)"'"' 2>/dev/null

We get

Checking dependencies for stdin  ...
  using nvd-clojure:  and dependency-check: 6.5.3
+----------------------------------+-------------------------------------------------------------------------------------------------------------------------------------------+
| dependency                       | status                                                                                                                                    |
+----------------------------------+-------------------------------------------------------------------------------------------------------------------------------------------+
| netty-transport-4.1.69.Final.jar | CVE-2021-43797                                                                                                                            |
| vertx-pg-client-4.2.1.jar        | CVE-2016-7048, CVE-2016-0766, CVE-2015-0244, CVE-2015-3166, CVE-2019-1021... |
+----------------------------------+-------------------------------------------------------------------------------------------------------------------------------------------+

37 vulnerabilities detected. Severity: HIGH

Examining the dep tree:

; lein with-profile -base,-system,-user,-provided,-dev deps :tree

It can be seen these come via vertx-pg-client

 [io.vertx/vertx-pg-client "4.2.1"]
   [io.vertx/vertx-core "4.2.1"]
     [com.fasterxml.jackson.core/jackson-core "2.11.4"]
     [io.netty/netty-buffer "4.1.69.Final"]
     [io.netty/netty-codec-http2 "4.1.69.Final"]
     [io.netty/netty-codec-http "4.1.69.Final"]
     [io.netty/netty-common "4.1.69.Final"]
     [io.netty/netty-handler-proxy "4.1.69.Final"]
       [io.netty/netty-codec-socks "4.1.69.Final"]
     [io.netty/netty-handler "4.1.69.Final"]
       [io.netty/netty-codec "4.1.69.Final"]
     [io.netty/netty-resolver-dns "4.1.69.Final"]
       [io.netty/netty-codec-dns "4.1.69.Final"]
     [io.netty/netty-resolver "4.1.69.Final"]
     [io.netty/netty-transport "4.1.69.Final"]
   [io.vertx/vertx-sql-client "4.2.1"]
 [org.postgresql/postgresql "42.3.1"]
   [org.checkerframework/checker-qual "3.5.0" :scope "runtime"]

I'll submit a PR to bump the dependency.

I see that setting the pipelining limit of PgPools is supported but I don't see it ever being used.

According to the vertx docs one has an option to create a pooled client as opposed to a pool and query using that to opt into pipelining. Querying using a pool as done here will not be pipelined.

Is this something you think porsas could support and perhaps accept a PR for? If not, could you please share the reasons why not?

At least against PostgreSQL.

Currently, only query-one and query are supported, here's a nice intro of all things possible: https://www.julienviet.com/reactive-pg-client/guide/java/

JDBC & Async stuff could be in separate modules?

Should porsas support also things like lazy streaming values, updating values etc? Maybe not, as there is already next.jdbc....

Currently, A CompiledQueries record is used to host different functions like :query and :query-one. It should be a protocol so we would get static analysis on the arities etc.