include options, to be default in 0.8, for literal_binds=True for --sql mode

Issue #255 resolved
Wichert Akkerman
created an issue

I have a very simple migration:

def upgrade():
    stmt = sa.text('ALTER TYPE article_type ADD VALUE IF NOT EXISTS :value')
    for article_type in ['foo']:
        op.execute(stmt.bindparams(value=article_type))

When running that with alembic upgrade head --sql the bind parameter is not substituted in the generated SQL:

-- Running upgrade  -> 4bd730372bac

ALTER TYPE article_type ADD VALUE IF NOT EXISTS %(value)s;

Comments (12)

  1. Wichert Akkerman reporter

    For what it's worth the reason I tried this approach is that

    1. the op.execute documentation mentioned a text() construct would work, and
    2. the alembic.ddl.impl.text documentation mentioned using text() should be preferred over plain strings

    Rambling on a bit: reading documentation I could not see the relation between alembic.ddl.impl.text() and sqlalchemy.text(), especially because the example for alembic.ddl.impl.text uses sqlalchemy.text. I was also wondering why the example there uses a connection connection object, when that isn't something that is generally available/used in an alembic migration context.migrations. Perhaps that documentation was copied over from SQLAlchemy? That would also explain why it says the bindparams argument has been deprecated since 0.9.0 when alembic is only up to version 0.7.2.

  2. Michael Bayer repo owner

    so the code is

    .. automodule:: alembic.ddl.impl
        :members:
        :undoc-members:
    

    and sphinx says:

    In an automodule directive with the members option set, only module members whose module attribute is equal to the module name as given to automodule will be documented. This is to prevent documentation of imported classes or functions. Set the imported-members option if you want to prevent this behavior and document all available members. Note that attributes from imported modules will not be documented, because attribute documentation is discovered by parsing the source file of the current module.

    and:

    >>> from alembic.ddl import impl
    impl.>>> impl.text
    <function text at 0x102e43ed8>
    >>> impl.text.__module__
    'sqlalchemy.expression'
    >>> 
    

    so, I don't know wtf this is about.

  3. Michael Bayer repo owner

    and it is not generating on a local build. which means....read the docs is on a stale version of sphinx? That seems very hard to believe, I just used some very recent features the other day on it. perhaps it installs sphinx into the build environment....that might be it

  4. Michael Bayer repo owner

    ok doing a wipe + rebuild there, for this one, text() works on SQLA, and I'm still trying to remember if the issue of alembic setting literal_binds for sql mode exists somewhere, because at the moment it's not doing it at all.

  5. Michael Bayer repo owner

    I'm pretty sure that the alembic docs so far have recommended that you don't use actual bound parameters...e.g. if you were using insert() constructs, we tell you to use bulk_insert(), as well as op.inline_literal().

    I think we probably should have literal_bindparam turned on for --sql, and I think when alembic was first developed we didn't really have literal_bindparam going on at that time. So at the moment I'm not comfortable flipping it on unconditionally in the 0.7 series, it'll have to be an option that we'd probably default to True in 0.8. the flag needs to have its effect right here: https://bitbucket.org/zzzeek/alembic/src/082a6c63668a601c0833c99b4ca23fe9459a3a40/alembic/ddl/impl.py?at=master#cl-99

    as for the readthedocs build, their stuff is not working and I am very annoyed so I'm going to bug them now.

  6. Michael Bayer repo owner
    • Added a new option :paramref:.EnvironmentContext.configure.literal_binds, which will pass the literal_binds flag into the compilation of SQL constructs when using "offline" mode. This has the effect that SQL objects like inserts, updates, deletes as well as textual statements sent using text() will be compiled such that the dialect will attempt to render literal values "inline" automatically. Only a subset of types is typically supported; the :meth:.Operations.inline_literal construct remains as the construct used to force a specific literal representation of a value. The :paramref:.EnvironmentContext.configure.literal_binds flag is added to the "offline" section of the env.py files generated in new environments. fixes #255
    • enhance the op_fixture as well as MigrationContext._stdout_connection() so that it uses the real DefaultImpl and MigrationContext fully in tests.

    → <<cset 0e1c0989894f>>

  7. Michael Bayer repo owner

    OK so, this is just a runtime env.py flag, so it's not that big a deal to add it into the template for new migration environments. You'll need to add it to your own env.py.

    here's a migration:

    def upgrade():
        op.execute(sa.text("update table set foo=:bar").bindparams(bar='bat'))
    

    when i run without the changeset:

    INFO  [alembic.migration] Context impl SQLiteImpl.
    INFO  [alembic.migration] Generating static SQL
    INFO  [alembic.migration] Will assume non-transactional DDL.
    CREATE TABLE alembic_version (
        version_num VARCHAR(32) NOT NULL
    );
    
    INFO  [alembic.migration] Running upgrade  -> 2037f8370292, rev1
    -- Running upgrade  -> 2037f8370292
    
    update table set foo=?;
    
    INSERT INTO alembic_version (version_num) VALUES ('2037f8370292');
    

    when I run with it:

    INFO  [alembic.migration] Context impl SQLiteImpl.
    INFO  [alembic.migration] Generating static SQL
    INFO  [alembic.migration] Will assume non-transactional DDL.
    CREATE TABLE alembic_version (
        version_num VARCHAR(32) NOT NULL
    );
    
    INFO  [alembic.migration] Running upgrade  -> 2037f8370292, rev1
    -- Running upgrade  -> 2037f8370292
    
    update table set foo='bat';
    
    INSERT INTO alembic_version (version_num) VALUES ('2037f8370292');
    

    this requires that you add literal_binds=True to your offline environment:

    def run_migrations_offline():
        url = config.get_main_option("sqlalchemy.url")
        context.configure(
            url=url, target_metadata=target_metadata, literal_binds=True)
    
        with context.begin_transaction():
            context.run_migrations()
    

    this flag is added to the env.py files for new environments as well.

  8. Log in to comment