tlocke / pg8000

A Pure-Python PostgreSQL Driver
BSD 3-Clause "New" or "Revised" License
515 stars 46 forks source link

Add support for valid SQL statements with parameters #159

Closed bjorkegeek closed 1 month ago

bjorkegeek commented 1 month ago

The pg8000 colon syntax for parameters is neat and all... but using it means that the strings passed to Connection.run are not syntactically valid PostgreSQL, which breaks other tooling. I prefer to use the $n placeholders which are considered valid PostgreSQL.

I'm suggesting adding this calling convention con.run("SELECT 'Hello '||$1;", 'world') to Connection.run

For the moment I'm monkey patching pg8000 in my project to support this, using the following function:

import pg8000.native
import functools

def monkey_patch_pg8000_run():

    original_run = pg8000.native.Connection.run

    @functools.wraps(original_run)
    def patched_run(self, sql, *params, stream=None, types=None, **kwparams):
        if params:
            if kwparams:
                raise ValueError("Cannot mix positional and keyword arguments")
            if types is not None:
                oids = [types.get(i + 1) for i in range(len(params))]
            else:
                oids = ()
            self._context = self.execute_unnamed(sql, params, oids=oids, stream=stream)
            return self._context.rows
        else:
            return original_run(self, sql, stream=stream, types=types, **kwparams)

    pg8000.native.Connection.run = patched_run

If the project maintainer agrees to this I can submit a pull request.

tlocke commented 1 month ago

Hi @bjorkegeek , thanks for your idea. When you talk about 'tooling' could you give an example of what you mean? Also, I've put a version on the branch https://github.com/tlocke/pg8000/tree/159-add-support-for-valid-sql-statements-with-parameters that lets you use $n style placeholders as well as the :x style ones. The parameters are still keyword parameters, but since Python preserves their order, we can refer to them by position. Anyway, let me know what you think.

tlocke commented 1 month ago

I've added this at https://github.com/tlocke/pg8000/commit/46c00021ade1d19466b07ed30392386c5f0a6b8e, but let me know if you have any thoughts on it.

bjorkegeek commented 1 month ago

Thanks for considering my feature request. I'm reluctant to use this in my code for two reasons:

  1. Using keyword arguments where the keywords are discarded, but the order is relevant feels un-pythonic and makes the code hard to read.
  2. pg8000 will parse the SQL for no reason (or is there some reason I have missed?)

I have sent you a third variant for you to consider as a PR. https://github.com/tlocke/pg8000/pull/166

tlocke commented 1 month ago

For me the current solution is easy to explain to people. So I can say, 'you specify the parameters as keyword arguments, and then refer to them either by name or position', that's basically all you need to know. It's also immediately obvious how a mixing of styles works. I haven't been able to come up with another method that is as easy to comprehend.