Add support for RETURNING in SQLAlchemy dialect

[chaudum]

Summary of the changes / Why this is an improvement

Example:

import random

from sqlalchemy import create_engine, inspect, func, Column, String, DateTime
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import sessionmaker
from sqlalchemy.schema import FetchedValue

engine = create_engine(
    "crate://",
    connect_args={
        "username": "crate",
    },
)
Session = sessionmaker(bind=engine)
Base = declarative_base(bind=engine)

class Log(Base):
    __tablename__ = "logs"
    __table_args__ = {"schema": "doc"}
    __mapper_args__ = {"exclude_properties": ["id"]}

    id = Column("_id", String, server_default=FetchedValue(), primary_key=True)
    ts = Column(DateTime, server_default=func.current_timestamp())
    level = Column(String)
    message = Column(String)

if __name__ == "__main__":
    o = Log(level="info", message="Hello World")
    session = Session()
    session.add(o)
    session.commit()

    print(f"{o.id=}, {o.ts=}, {o.level=}, {o.message=}")

[mfussenegger]

Will this still work with older CrateDB versions which don't have RETURNING ?

[chaudum]

Yes, as long as you don't add a column with server_default=FetchedValue() which requires RETURNING`, e.g:

class LogOld(Base):
    __tablename__ = "logs_old"
    __table_args__ = {"schema": "doc"}

    ts = Column(DateTime, primary_key=True)
    level = Column(String, primary_key=True)
    message = Column(String, primary_key=True)

With the schema:

create table logs_old (ts timestamp with time zone, level text, message text)

[chaudum]

If implicit_returning is False, it would never add the RETURNING clause to the INSERT statement.

I guess what you mean is that we should not change behaviour for old versions. ( it will be a different exception).

[mfussenegger]

Just wanted to make sure that it doesn't add a RETURNING to other statements that previously worked

Could you add a changes entry?

[chaudum]

Just wanted to make sure that it doesn't add a RETURNING to other statements that previously worked

I will test a bit more.

Could you add a changes entry?

Sure

[chaudum]

I found an issue when running one of our test suites, where an INSERT statement would add a RETURNING clause. However cannot reproduce it when the model is used outside of the test suite.

[chaudum]

Ok, so as soon as one column of the PK has a server side generated value, the SQLAlchemy ORM will append a RETURNING (in order to get the full PK for the model instance).

Example that works at the moment, but would fail with this change:

from sqlalchemy import create_engine, Column, String, DateTime, text
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import sessionmaker


engine = create_engine(
    "crate://",
    connect_args={
        "username": "crate",
        "servers": "localhost:4200",
    },
)

Session = sessionmaker(bind=engine)
Base = declarative_base(bind=Session)


class Event(Base):
    __tablename__ = "event"
    __table_args__ = {"schema": "doc"}

    ts = Column("ts", DateTime, primary_key=True, server_default=text("current_timestamp"))
    event = Column("event", String, primary_key=True)


if __name__ == "__main__":
    session = Session()
    o = Event(event="status")
    session.add(o)
    session.commit()
    print(o)

CREATE TABLE doc.event (
  ts TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP,
  event TEXT
)

[chaudum]

I am not sure if this is a show stopper, because having the primary key in the model but not in the database is an awful workaround for not being able to use the FetchedValue() for _id. What do you think?

[mfussenegger]

Can users override the implicit_returning value? If so, I think a (breaking?) changes entry that explains the impact of the change and how to work around it if necessary should be good enough.

[chaudum]

Yes, you can override it when creating the engine: create_engine('crate://',..., implicit_returning=False).

[chaudum]

Not sure why Python 3.5 keeps failing with ##[error] raise SystemError('Failed to start Crate instance in time.')