crate / crate-python

Python DB API client library for CrateDB, using HTTP.
https://cratedb.com/docs/python/
Apache License 2.0
79 stars 30 forks source link

SQLAlchemy: Add support for 'autogenerate_uuid' in column creation #580

Closed surister closed 11 months ago

surister commented 11 months ago

Solves crate/sqlalchemy-cratedb#102

Where by setting the column:

Column('x', sa.Text, primary_key=True, crate_autogenerate_uuid=True)

will generate:

x STRING DEFAULT gen_random_text_uuid() NOT NULL

Checklist

surister commented 11 months ago

Circling back to this, I'm effectively checking against

Column(..., server_default='value')

with

if default is not None:
    raise sa.exc.CompileError(
            "Can only have one default value but server_default"
            " and crate_generate_uuid are set"
               )

But It'd probably also clash with Column(..., default=lambda:str(uuid.uuid4()))

I will have a deeper look at this.

surister commented 11 months ago

Hi @surister,

thanks for your patch. It looks good to me so far.

Regarding your question about option clashes: I don't know if and how SQLAlchemy prevents to use both default and server_default, or if it doesn't do at all.

Depending on how SQLAlchemy handles that, we should not necessarily add anything on top - only if it makes sense, and doesn't overcomplicate the code.

With kind regards, Andreas.

Using default + crate_autogenerate_uuid doesn't really 'clash', it seems that CrateDB just ignores the DEFAULT directive and puts in whatever value it gets on the INSERT

surister commented 11 months ago

More importantly when using it as:

Column(..., crate_autogenerate_uuid=True, primary_key=True)

We get the following warning:

SAWarning: Column 'aaaa3.x' is marked as a member of the primary key for table 'aaaa3', but has no Python-side or server-side default generator indicated, nor does it indicate 'autoincrement=True' or 'nullable=True', and no explicit value is passed.  Primary key columns typically may not store NULL.

Because technically, there is no autoincrement_column and server_default nor default is set, so SQLAlchemy cannot assure that the value will not be Null (and nullable in this case is set to False by default)

SQLAlchemy warning source

amotl commented 11 months ago

Dear @surister,

So, the outcome for this topic could be essentially to cover that detail about func.gen_random_text_uuid() within the documentation at, for example, docs/sqlalchemy.rst.

I've just submitted crate/crate-python#585, so I am closing this. Apologies again that I was giving wrong guidance on this matter through crate/sqlalchemy-cratedb#102.

With kind regards, Andreas.