marijnh / Postmodern

A Common Lisp PostgreSQL programming interface
http://marijnhaverbeke.nl/postmodern
Other
400 stars 90 forks source link

New Option: Parameters can be pased in binary format #273

Closed sabracrolleton closed 3 years ago

sabracrolleton commented 3 years ago

Changelog v. 1.33.0

This version of Postmodern now provides an optional setting which will cause Postmodern to pass parameters to Postgresql in binary format IF that format is available for that datatype. Currently this means int2, int4, int8, float, double-float (except clisp) and boolean. Rational numbers continue to be passed as text.

If a query to Postgresql does not have a table column which would allow Postgresql to determine the correct datatype and you do not specify differently, Postgresql will treat the parameters passed with the query as text. The default text setting with results:

(query "select $1" 1 :single)
"1"
(query "select $1" 1.5 :single)
"1.5"
(query "select $1" T :single)
"true"
(query "select $1" nil :single)
"false"
(query "select $1" :NULL :single)
:NULL

You can specify parameter type as so:

(query "select $1::integer" 1 :single)
1

Setting cl-postgres:*use-binary-parameters* to t has the following results:

(query "select $1" 1 :single)
1
(query "select $1" 1.5 :single)
1.5
(query "select $1" T :single)
T
(query "select $1" nil :single)
NIL
(query "select $1" :NULL :single)
:NULL

The default for cl-postgres/Postmodern is to continue to pass parameters to Postgresql as text (not in binary format) in order to avoid breaking existing user code. If you want to pass parameters to Postgresql in binary format, you can either:

(setf cl-postgres:\*use-binary-parameters\* t)

or use postmodern's use-binary-parameters function:

(pomo:use-binary-parameters t)

Using binary parameters does tighten type checking when using prepared queries. You will not be able to use prepared queries with varying formats. In other words, if you have a prepared query that you pass an integer as the first parameter and a string as the second parameter the first time it is used, any subsequent uses of that prepared query during that session will also have to pass an integer as the first parameter and a string as the second parameter.

Benchmarking does indicate a slight speed and consing benefit to passing parameters as binary, but your mileage will vary depending on your use case.

In addition, this version also adds the ability to have queries returned as vectors of vectors, using a vectors keyword.

(query "select id, some_int, some_text from tests_data :where id = 1" :vectors)
or
(query (:select 'id 'some-int 'some-text :from 'test-data)
     :vectors)
#(#(1 2147483645 "text one")
  #(2 0 "text two")
  #(3 3 "text three"))

Like :array-hash, if there is no result it will return an empty array, not nil.

sabracrolleton commented 3 years ago

Finally addressing Issue #194: literal variable values are coerced into strings. This is optional only and the default is the historical passing parameters to Postgresql as strings.

slyrus commented 3 years ago

Hmm... I guess I should have voiced this earlier, now that this is merged, but my concern is that two different applications/libraries that connect to postgres might step on each other with use-binary-parameters. I suppose we can wrap each call with a let, but if we're going to do that at each call site, wouldn't it be easier to just have a query-binary, e.g., function to do the work (or maybe this already exists?). Other than that, this sounds great!

sabracrolleton commented 3 years ago

Hmm. I can revert and we can rethink this.

slyrus commented 3 years ago

Also, I think a single application/library developer may want both text and binary. Can this just be an option to the query macro? Which, come to think it, makes me think this might be a good time to make any other changes to the query macro. One thing that bothers me is that we cons up a list (the same list twice in the new code!) in query. What if we just took the args as a list as the second arg to query and then we could use proper kw args for all of the other format options, whether the query should be in binary, etc...

sabracrolleton commented 3 years ago

Will revert immediately.

slyrus commented 3 years ago

No hurry, and happy to discuss the API further.

sabracrolleton commented 3 years ago

Should we have parallel query macros? Binary and non-binary?

The following roadmap might be helpful for anyone trying to walk through the code that leads to where the low level message exchanges with Postgresql happen:

|- query (postmodern/query) - can also start from cl-postgres:initiate-connection
|----- prepare-query  (cl-postgres/public)
|---------- send-parse  (cl-postgres/protocal)
|--------------- parse-message (cl-postgres/messages)
|----- exec-prepared  (cl-postgres/public)
|---------- send-execute  (cl-postgres/protocol)
|--------------- bind-message (cl-postgres/messages)  - specifying text or binary happens here
|------------------- serialize-for-postgres (cl-postgres/sql-string)
|----- exec-query  (cl-postgres/public)
|---------- send-query  (cl-postgres/protocol)
|--------------- simple-parse-message

|- prepare (postmodern/prepare)
|----- generate-prepared
|------ ensure-prepared (postmodern/prepare) params
|-------- prepare-query (cl-postgres/public) params

|- defprepared (postmodern/prepare)
|---- generate-prepared (postmodern/prepare)
|------ ensure-prepared (postmodern/prepare) params
|-------- prepare-query (cl-postgres/public) params
|------

|- reset-prepared-statement (postmodern/prepare)
|--- prepare-query (cl-postgres/public) NIL
sabracrolleton commented 3 years ago

I like the idea of adding the option to the query macro better than parallel queries. Do you want to propose code?

sabracrolleton commented 3 years ago

After playing with it for awhile, I am struggling to come up with an easy way to just say - give me binary parameters everywhere instead of having to specify that for each query. Thoughts?

slyrus commented 3 years ago

Might this be better a per connection setting? It seems to me we have (at least) three possible "levels" for this:

  1. global
  2. per connection
  3. per query I would think that supporting reasonable defaults and the ability to override the default for each of these makes sense -- except possibly globally we might not want a binary default as that might break existing user code.
sabracrolleton commented 3 years ago

I like the per connection idea. After playing with it, per query is feeling too granular.

I definitely think that if we default to binary it will break existing user code.