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

Expose libpq PQescapeIdentifier #308

Closed
dvarrazzo opened this Issue Apr 9, 2015 · 7 comments

Comments

Projects
None yet
3 participants
@dvarrazzo
Copy link
Member

dvarrazzo commented Apr 9, 2015

Allow dynamic query creation by string interpolation without resorting to adaptation (as discussed in #209), which would break switching to PQexecParams.

@dvarrazzo dvarrazzo added this to the psycopg 2.7 milestone Apr 9, 2015

@a1exsh

This comment has been minimized.

Copy link
Contributor

a1exsh commented Oct 13, 2015

How about adding psycopg2.extensions.QuotedIdentifier that would mimic the QuotedString, but use PQescapeIdentifier internallly if available and fall back to psycopg_escape_identifier_easy if not?

Since PQescapeIdentifier pretty much requires a connection, we could also add a method on connection class, but not sure if that really fits into the picture. For one, I find that suggestion to expose quote_ident/literal/nullable in #209 (comment) makes a lot of sense.

@dvarrazzo

This comment has been minimized.

Copy link
Member

dvarrazzo commented Oct 13, 2015

Proposed ideas (exposed in #358):

Add a "compose" method to the cursor to the object returning a sql statement to pass to execute:

from psycopg2.extensions import Identifier

cur.execute(
    cur.compose(
        "insert into %(table)s %(field)s values (%%s)",
        dict(table=Identifier("tbl"), field=Identifier("fld"))),
    [42])

Rough ideas are:

  • wrapper objects Identifier, Literal and maybe String and Bytea would be exposed (names to review, to avoid clashes). These objects derive from a base class e.g. Composable.
  • compose() knows how to adapt a Composable. If a normal python object is passed, it is adapted as per adapt() and passed as string, going through the regular adaptation machinery.
  • compose() takes a dict or a string as argument.
  • Composable objects expose an (e.g.) adapt() method taking a connection or cursor and returning the adapted string. The method can be called manually and is used internally by cur.compose().

All in all we have this pseudostuff:

class Composable:
    def __init__(self, wrapped):
        self.wrapped = wrapped
    def adapt(self, conn_or_cur):
        raise NotImplementedError

class Identifier(Composable):
    def adapt(self, conn_or_cur):
        pgconn = get_it_from(conn_or_cur)
        return libpq.PQescapeIdentifier(pgconn, self.wrapped)

# etc for Literal, String, Bytea

class cursor:
    def compose(sql, args):
        adapted = [] # TODO: handle args dict
        for arg in args:
            if isinstance(arg, Composable):
                adapted.append(arg.adapt())
            else:
               # TODO: prepare() etc
               adapted.append(psycopg2.extensions.adapt(arg).getquoted())

        return sql % adapted

@fogzot what do you think?

@fogzot

This comment has been minimized.

Copy link
Member

fogzot commented Oct 14, 2015

The design is very clean, I like it.
What about exposing this kind of funcionality on the connection? The cursor is there basically to manage data and I don't think it makes sense to put every new feature in it.

@a1exsh

This comment has been minimized.

Copy link
Contributor

a1exsh commented Oct 14, 2015

I'm not very interested in all the compose magic, but one thing I can see is that you still need libpq.PQescapeIdentifier hack. But why not actually make it a member function on connection object?

That could be a separate patch to just add connection.quote_ident and nothing more, then make this compose stuff work on top of it.

@dvarrazzo

This comment has been minimized.

Copy link
Member

dvarrazzo commented Oct 14, 2015

@fogzot I wouldn't put every new feature on the cursor, but I think this specific one is better on the cursor than on the connection or as a global function (which would need a connection or cursor anyway), because I assume you want to compose the query close to where you are going to use it and you already have a cursor. Composing a SQL string is not even really the connection task, which is to manage the db connection and the transaction. We just happen to need one to invoke these functions.

@a1exsh no, the above is just pseudocode to say "use the libpq function here". the hack could be used to put together a Python demo, but I don't really think we need it. I'd write the entire feature in C.

If I wanted a quick and dirty quote_ident in Python I wouldn't make it a connection method anyway, I would have it exposed in extensions taking a connection or cursor as argument, to avoid bloating the connection interface (when you will want to escape literals, bytea, ...) and to allow the function to receive a cursor too.

So, to summarize, my preferences for the interface of these quoting functions are in order:

  1. on the cursor as a compose() method, with the functions actually wrapped by objects whose interface is Composable("My string").adapt(conn_or_curs) -> str
  2. in _psycopg.so -> psycopg2.extensions as simple functions (str, conn_or_cur) -> str (note: edited, I wrote str -> str originally)
  3. (actually 3000, but MD doesn't allow me) as connection methods.
@dvarrazzo

This comment has been minimized.

Copy link
Member

dvarrazzo commented Jan 1, 2017

Feature being worked in the sql-compose branch. First docs available.

@dvarrazzo

This comment has been minimized.

Copy link
Member

dvarrazzo commented Feb 3, 2017

The module psycopg2.sql is now merged to master.

@dvarrazzo dvarrazzo closed this Feb 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment