Support raw-string queries

[tomchristie]

No description provided.

[mungojam]

Is there a way that this could be combined with the SQL generators in SQLAlchemy drivers in order to add support for any databases that SQLALCHEMY supports? like oracle or odbc ones

[tomchristie]

@mungojam The problem isn't the SQL generation, it's the drivers. The standard database drivers are all based on a thread-concurrency model, rather than the more co-operative concurrency (async).

The interface layer (DBAPI) between SQLAlchemy and the database drivers is itself also inherently a thread-concurrency interface.

The main point of the "databases" package is to wrap up some of the newer async drivers (asyncpg, aiomysql, etc.) together with SQLAlchemy's query generation and type coercion all together in a nice interface.

[sylvoslee]

When is it achieved?

[gvbgduh]

Yes, I'm missing this bit as well or even probably some details.
I like the whole idea and how drivers are encapsulated behind, but sometimes it would be nice to have ability to utilise some power of (in my case asyncpg) the driver or have an ability to call the particular drivers methods like copy_to_table, cursor, etc. or even to communicate to the driver directly using the already established pooling.

It's not fully impossible though, as example, it seems to be ok to pass a raw query as from sqlalchemy import text, like

data = [(uuid_1, value_1), ...]

query_str = f"""
    INSERT INTO some_table (uuid, value)
    (SELECT r.uuid, r. value
    FROM unnest(ARRAY{data}:: some_table[]) as r)
    RETURNING *
"""

results = await database.fetch_all(text(query_str))

but it feels a bit hacky in this case, instead of ... FROM unnest($1:: some_table[]) as r)... form. text object has some limitations as well.

I also would be happy to contribute, especially with some hints to align it with whole concept.

UPDATE: Apparently there's a way to adjust the text object, considering the example above:

query_str = f"""
    INSERT INTO some_table (uuid, value)
    (SELECT r.uuid, r. value
    FROM unnest(:array_data:: some_table[]) as r)
    RETURNING *
"""
query = text(query_str).bindparams(array_data=data)
results = await database.fetch_all(query)

UPDATE 2: Oh, apparently it was already answered in encode/starlette#329.

[tomchristie]

sometimes it would be nice to have ability to utilise some power of (in my case asyncpg) the driver or have an ability to call the particular drivers methods like copy_to_table, cursor, etc. or even to communicate to the driver directly using the already established pooling.

I'm not necessarily against us providing an interface onto the raw driver for cases where you want to drop down into a non-cross-database-compatible interface. Happy to consider any API suggestions there. Alternatively if there are more types of operation that we should be providing but currently aren't, then we should provide a cross-database-compatible interface onto those. Not familiar with copy_to_table to know if that applies.

[gvbgduh]

if there are more types of operation that we should be providing but currently aren't, then we should provide a cross-database-compatible interface onto those.

It definitely sounds reasonable and I also think it's a better practice. Raw calls to the driver shouldn't be considered as a recommended approach, but can take a place for say 0.1-0.5% of cases when a developer is ready to be responsible for the action.

While it'll take a bit of time for me to come up with a thorough suggestion about cross-db interfaces, I'd like to draft a PR for raw calls to discuss how it might look.

[gvbgduh]

I drafted some approaches, happy for any comments, suggestions, complains.

There's also a subtle issue with using sqlalchemy.text. The text object doesn't provide the context, so result_columns are not defined. At least for postgres it breaks mapping, but data is still in _row, but I haven't investigated other backends. Probably not a big issue considering the circumstances.

[tomchristie]

The text object doesn't provide the context, so result_columns are not defined. At least for postgres it breaks mapping, but data is still in _row, but I haven't investigated other backends. Probably not a big issue considering the circumstances.

Yeah, I think we'll want to make sure integer indexing into the mapping still works for text, but document column name indexing as being unsupported in that case.

(Also we might want to think about if we really want the records to be a Mapping interface, or an unpack-able Sequence.)

ie. a, b = {0: "Some value", 1: "Another value"} --> a == 0, b == 1 if we're using a mapping interface.

[gvbgduh]

The problem only applies to asyncpg. aiomysql and aiosqlite return list/tuple of tuples (or tuple) where asyncpg returns the Records that our transformed to local Records with broken mapping. The raw record allows it to access by an int index or by the str key, but the local one's __getitem__ relies on this mapping. I've made an attempt to make the behaviour compatible with other drivers, but not very happy with implementation. Will continue thinking.
Also, I'm not sure why travis is failing, but it's probably something with formatting.

[tomchristie]

Yeah - run scripts/lint to fix things up. https://travis-ci.org/encode/databases/jobs/501366204

but not very happy with implementation

I think what we may want here will end up being a couple of different things...

1. Support plaintext queries in all the API interfaces like so:

def execute(query: typing.Union[Query,str], **kwargs: typing.Any):
    if isinstance(query, str):
        query = sqlalchemy.text(query)
        query.bindparams(**kwargs)
    elif kwargs:
        query = query.values(**kwargs)

2. Add support for explcit connection management, similar to our existing transaction support, so...

Support for this: connection = await database.connection() ... connection.release()
And this: async with database.connection() as conn:

Inside either of those block you are garunteed to be holding onto a single connection from the pool throughout. I'd been thinking we might want this in any case. Our existing style acquires and releases the connection as needed, and only strictly holds onto it if you're inside a transaction. This works great because we're in autocommit mode, so there's no particular need for each query to strictly be on the same connection, unless the developer explicitly requests a transaction around a block of queries. However I could imagine that there might be cases where a developer doesn't want a transaction, but does want to strictly acquire-and-hold the connection over an entire endpoint.

3. Add support for connection.raw_connection to access the underlying driver's connection interface.

We'd probably address those each independantly, in turn.

[gvbgduh]

That sounds like a brilliant plan.

So, for p. 3 I'd like to suggest the following PR.

there might be cases where a developer doesn't want a transaction, but does want to strictly acquire-and-hold the connection over an entire endpoint.

Yeah, I think it's quite possible, at least ... CURSOR WITH HOLD seems to be the case.
I would be happy to come up with solution here (p2) and probably after it for p.1.

UPDATE: Thanks for the review, fix the comments first.

[gvbgduh]

Might worth considering to expose connection_counter, but importance is arguable.