Skip to content
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

Double "%" in --sql output #562

Closed
RazerM opened this issue May 15, 2019 · 7 comments

Comments

@RazerM
Copy link
Contributor

commented May 15, 2019

Seems like someone had this before but didn't follow up with details: #439

alembic: 1.0.10
sqlalchemy: 1.2.19
psycopg2: 2.8.2

Revision:

"""Test revision

Revision ID: 3e64c96e6cc5
Revises: ae08c6f2822a
Create Date: 2019-05-15 11:30:29.061244

"""

# revision identifiers, used by Alembic.
revision = '3e64c96e6cc5'
down_revision = 'ae08c6f2822a'
branch_labels = None
depends_on = None

from alembic import op


def upgrade():
    op.execute('''
        CREATE FUNCTION
          _tmp_raise_exception()
        RETURNS void AS
        $body$
        BEGIN
          RAISE EXCEPTION 'Here is a number: %', 15;
        END;
        $body$
        LANGUAGE 'plpgsql'
    ''')


def downgrade():
    op.execute('DROP FUNCTION _tmp_raise_exception()')

--sql output (note the %%):

$ alembic upgrade --sql ae08c6f2822a:3e64c96e6cc5
INFO  [alembic.runtime.migration] Context impl PostgresqlImpl.
INFO  [alembic.runtime.migration] Generating static SQL
INFO  [alembic.runtime.migration] Will assume transactional DDL.
BEGIN;

INFO  [alembic.runtime.migration] Running upgrade ae08c6f2822a -> 3e64c96e6cc5, Test revision
-- Running upgrade ae08c6f2822a -> 3e64c96e6cc5

CREATE FUNCTION
          _tmp_raise_exception()
        RETURNS void AS
        $body$
        BEGIN
          RAISE EXCEPTION 'Here is a number: %%', 15;
        END;
        $body$
        LANGUAGE 'plpgsql';

UPDATE alembic_version SET version_num='3e64c96e6cc5' WHERE alembic_version.version_num = 'ae08c6f2822a';

COMMIT;

This generated SQL is invalid.

@zzzeek

This comment has been minimized.

Copy link
Member

commented May 15, 2019

some drivers need to double the percent signs in order to comply with the DBAPI. can you go into env.py and apply:

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

    context.get_context().dialect.identifier_preparer._double_percents = False
    with context.begin_transaction():
        context.run_migrations()





@zzzeek

This comment has been minimized.

Copy link
Member

commented May 15, 2019

Here's the patch for the issue. Can you or @mikeywaites provide a PR ? needs a changelog file and docstrings for the new parameter.

diff --git a/alembic/runtime/environment.py b/alembic/runtime/environment.py
index c4d6586..b918269 100644
--- a/alembic/runtime/environment.py
+++ b/alembic/runtime/environment.py
@@ -289,6 +289,7 @@ class EnvironmentContext(util.ModuleClsProxy):
         connection=None,
         url=None,
         dialect_name=None,
+        dialect_opts=None,
         transactional_ddl=None,
         transaction_per_migration=False,
         output_buffer=None,
@@ -355,6 +356,11 @@ class EnvironmentContext(util.ModuleClsProxy):
          "postgresql", "mssql", etc.
          The type of dialect to be used will be derived from this if
          ``connection`` and ``url`` are not passed.
+        :param dialect_opts: dictionary of options to be passed to dialect
+         constructor.
+
+         .. versionadded:: 1.0.11
+
         :param transactional_ddl: Force the usage of "transactional"
          DDL on or off;
          this otherwise defaults to whether or not the dialect in
@@ -812,6 +818,7 @@ class EnvironmentContext(util.ModuleClsProxy):
             url=url,
             dialect_name=dialect_name,
             environment_context=self,
+            dialect_opts=dialect_opts,
             opts=opts,
         )
 
diff --git a/alembic/runtime/migration.py b/alembic/runtime/migration.py
index a4ad740..75c58f4 100644
--- a/alembic/runtime/migration.py
+++ b/alembic/runtime/migration.py
@@ -145,6 +145,7 @@ class MigrationContext(object):
         dialect_name=None,
         dialect=None,
         environment_context=None,
+        dialect_opts=None,
         opts=None,
     ):
         """Create a new :class:`.MigrationContext`.
@@ -169,6 +170,8 @@ class MigrationContext(object):
         """
         if opts is None:
             opts = {}
+        if dialect_opts is None:
+            dialect_opts = {}
 
         if connection:
             if not isinstance(connection, Connection):
@@ -180,10 +183,10 @@ class MigrationContext(object):
             dialect = connection.dialect
         elif url:
             url = sqla_url.make_url(url)
-            dialect = url.get_dialect()()
+            dialect = url.get_dialect()(**dialect_opts)
         elif dialect_name:
             url = sqla_url.make_url("%s://" % dialect_name)
-            dialect = url.get_dialect()()
+            dialect = url.get_dialect()(**dialect_opts)
         elif not dialect:
             raise Exception("Connection, url, or dialect_name is required.")
 
diff --git a/alembic/templates/generic/env.py b/alembic/templates/generic/env.py
index 15cb472..4e27908 100644
--- a/alembic/templates/generic/env.py
+++ b/alembic/templates/generic/env.py
@@ -40,7 +40,8 @@ def run_migrations_offline():
     """
     url = config.get_main_option("sqlalchemy.url")
     context.configure(
-        url=url, target_metadata=target_metadata, literal_binds=True
+        url=url, target_metadata=target_metadata, literal_binds=True,
+        paramstyle="named"
     )
 
     with context.begin_transaction():
diff --git a/alembic/templates/multidb/env.py b/alembic/templates/multidb/env.py
index 607efaa..fcb677f 100644
--- a/alembic/templates/multidb/env.py
+++ b/alembic/templates/multidb/env.py
@@ -73,6 +73,7 @@ def run_migrations_offline():
                 output_buffer=buffer,
                 target_metadata=target_metadata.get(name),
                 literal_binds=True,
+                dialect_opts={"paramstyle": "named"}
             )
             with context.begin_transaction():
                 context.run_migrations(engine_name=name)
diff --git a/alembic/templates/pylons/env.py b/alembic/templates/pylons/env.py
index f8abf44..6ca3fc1 100644
--- a/alembic/templates/pylons/env.py
+++ b/alembic/templates/pylons/env.py
@@ -53,6 +53,7 @@ def run_migrations_offline():
         url=meta.engine.url,
         target_metadata=target_metadata,
         literal_binds=True,
+        dialect_opts={"paramstyle": "named"}
     )
     with context.begin_transaction():
         context.run_migrations()

@zzzeek zzzeek added this to the fasttrack milestone May 15, 2019

@zzzeek

This comment has been minimized.

Copy link
Member

commented May 15, 2019

oh and a test in tests/test_offline_environment.py !

@RazerM

This comment has been minimized.

Copy link
Contributor Author

commented May 15, 2019

Thanks for the env.py workaround. I might be able to provide a PR tonight.

@zzzeek

This comment has been minimized.

Copy link
Member

commented Jul 20, 2019

a real test for this will be hard. test_offline_environment isn't really set up for this kind of test.

@zzzeek

This comment has been minimized.

Copy link
Member

commented Jul 20, 2019

also i think my patch above is wrong for the generic env.py

@sqla-tester

This comment has been minimized.

Copy link
Collaborator

commented Jul 20, 2019

Mike Bayer has proposed a fix for this issue in the master branch:

Add dialect_options to environment; set paramstyle=named for offline https://gerrit.sqlalchemy.org/1370

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.