]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
Add max_identifier_length parameter; warn for Oracle
authorMike Bayer <mike_mp@zzzcomputing.com>
Wed, 2 Oct 2019 15:20:58 +0000 (11:20 -0400)
committerMike Bayer <mike_mp@zzzcomputing.com>
Wed, 2 Oct 2019 18:55:11 +0000 (14:55 -0400)
Added new :func:`.create_engine` parameter
:paramref:`.create_engine.max_identifier_length`. This overrides the
dialect-coded "max identifier length" in order to accommodate for databases
that have recently changed this length and the SQLAlchemy dialect has
not yet been adjusted to detect for that version.  This parameter interacts
with the existing :paramref:`.create_engine.label_length` parameter in that
it establishes the maximum (and default) value for anonymously generated
labels.

The Oracle dialect now emits a warning if Oracle version 12.2 or greater is
used, and the :paramref:`.create_engine.max_identifier_length` parameter is
not set.   The version in this specific case defaults to that of the
"compatibility" version set in the Oracle server configuration, not the
actual server version.   In version 1.4, the default max_identifier_length
for 12.2 or greater will move to 128 characters.  In order to maintain
forwards compatibility, applications should set
:paramref:`.create_engine.max_identifier_length` to 30 in order to maintain
the same length behavior, or to 128 in order to test the upcoming behavior.
This length determines among other things how generated constraint names
are truncated for statements like ``CREATE CONSTRAINT`` and ``DROP
CONSTRAINT``, which means a the new length may produce a name-mismatch
against a name that was generated with the old length, impacting database
migrations.

Fixes: #4857
Change-Id: Ib62efb00c6180c375869029b57353d90385d7950
(cherry picked from commit 88761b8b0b0cfa67cdd6a4913e3a0ea5cba93cbb)

doc/build/changelog/unreleased_13/4857.rst [new file with mode: 0644]
lib/sqlalchemy/dialects/oracle/base.py
lib/sqlalchemy/engine/__init__.py
lib/sqlalchemy/engine/default.py
lib/sqlalchemy/testing/warnings.py
test/dialect/oracle/test_dialect.py
test/sql/test_labels.py

diff --git a/doc/build/changelog/unreleased_13/4857.rst b/doc/build/changelog/unreleased_13/4857.rst
new file mode 100644 (file)
index 0000000..ea2165c
--- /dev/null
@@ -0,0 +1,41 @@
+.. change::
+    :tags: usecase, engine
+    :tickets: 4857
+
+    Added new :func:`.create_engine` parameter
+    :paramref:`.create_engine.max_identifier_length`. This overrides the
+    dialect-coded "max identifier length" in order to accommodate for databases
+    that have recently changed this length and the SQLAlchemy dialect has
+    not yet been adjusted to detect for that version.  This parameter interacts
+    with the existing :paramref:`.create_engine.label_length` parameter in that
+    it establishes the maximum (and default) value for anonymously generated
+    labels.   Additionally, post-connection detection of max identifier lengths
+    has been added to the dialect system.  This feature is first being used
+    by the Oracle dialect.
+
+    .. seealso::
+
+        :ref:`oracle_max_identifier_lengths` - in the Oracle dialect documentation
+
+.. change::
+    :tags: usecase, oracle
+    :tickets: 4857
+
+    The Oracle dialect now emits a warning if Oracle version 12.2 or greater is
+    used, and the :paramref:`.create_engine.max_identifier_length` parameter is
+    not set.   The version in this specific case defaults to that of the
+    "compatibility" version set in the Oracle server configuration, not the
+    actual server version.   In version 1.4, the default max_identifier_length
+    for 12.2 or greater will move to 128 characters.  In order to maintain
+    forwards compatibility, applications should set
+    :paramref:`.create_engine.max_identifier_length` to 30 in order to maintain
+    the same length behavior, or to 128 in order to test the upcoming behavior.
+    This length determines among other things how generated constraint names
+    are truncated for statements like ``CREATE CONSTRAINT`` and ``DROP
+    CONSTRAINT``, which means a the new length may produce a name-mismatch
+    against a name that was generated with the old length, impacting database
+    migrations.
+
+    .. seealso::
+
+        :ref:`oracle_max_identifier_lengths` - in the Oracle dialect documentation
\ No newline at end of file
index 9f0b23e12e568003de6d2118661d8dfe111a5fdf..5c781e3a6f8cdfef63af94ad2f7f58f5ac2bbb94 100644 (file)
@@ -67,6 +67,110 @@ against data dictionary data received from Oracle, so unless identifier names
 have been truly created as case sensitive (i.e. using quoted names), all
 lowercase names should be used on the SQLAlchemy side.
 
+.. _oracle_max_identifier_lengths:
+
+Max Identifier Lengths
+----------------------
+
+Oracle has changed the default max identifier length as of Oracle Server
+version 12.2.   Prior to this version, the length was 30, and for 12.2 and
+greater it is now 128.   This change impacts SQLAlchemy in the area of
+generated SQL label names as well as the generation of constraint names,
+particularly in the case where the constraint naming convention feature
+described at :ref:`constraint_naming_conventions` is being used.
+
+To assist with this change and others, Oracle includes the concept of a
+"compatibility" version, which is a version number that is independent of the
+actual server version in order to assist with migration of Oracle databases,
+and may be configured within the Oracle server itself. This compatibility
+version is retrieved using the query  ``SELECT value FROM v$parameter WHERE
+name = 'compatible';``.   The SQLAlchemy Oracle dialect as of version 1.3.9
+will use this query upon first connect in order to determine the effective
+compatibility version of the server, which determines what the maximum allowed
+identifier length is for the server.
+
+For the duration of the SQLAlchemy 1.3 series, the default max identifier
+length will remain at 30, even if compatibility version 12.2 or greater is in
+use.  When the newer version is detected, a warning will be emitted upon first
+connect, which refers the user to make use of the
+:paramref:`.create_engine.max_identifier_length` parameter in order to assure
+forwards compatibility with SQLAlchemy 1.4, which will be changing this value
+to 128 when compatibility version 12.2 or greater is detected.
+
+Using :paramref:`.create_engine.max_identifier_length`, the effective identifier
+length used by the SQLAlchemy dialect will be used as given, overriding the
+current default value of 30, so that when Oracle 12.2 or greater is used, the
+newer identifier length may be taken advantage of::
+
+    engine = create_engine(
+        "oracle+cx_oracle://scott:tiger@oracle122",
+        max_identifier_length=128)
+
+The maximum identifier length comes into play both when generating anonymized
+SQL labels in SELECT statements, but more crucially when generating constraint
+names from a naming convention.  It is this area that has created the need for
+SQLAlchemy to change this default conservatively.   For example, the following
+naming convention produces two very different constraint names based on the
+identifier length::
+
+    from sqlalchemy import Column
+    from sqlalchemy import Index
+    from sqlalchemy import Integer
+    from sqlalchemy import MetaData
+    from sqlalchemy import Table
+    from sqlalchemy.dialects import oracle
+    from sqlalchemy.schema import CreateIndex
+
+    m = MetaData(naming_convention={"ix": "ix_%(column_0N_name)s"})
+
+    t = Table(
+        "t",
+        m,
+        Column("some_column_name_1", Integer),
+        Column("some_column_name_2", Integer),
+        Column("some_column_name_3", Integer),
+    )
+
+    ix = Index(
+        None,
+        t.c.some_column_name_1,
+        t.c.some_column_name_2,
+        t.c.some_column_name_3,
+    )
+
+    oracle_dialect = oracle.dialect(max_identifier_length=30)
+    print(CreateIndex(ix).compile(dialect=oracle_dialect))
+
+With an identifier length of 30, the above CREATE INDEX looks like::
+
+    CREATE INDEX ix_some_column_name_1s_70cd ON t
+    (some_column_name_1, some_column_name_2, some_column_name_3)
+
+However with length=128, it becomes::
+
+    CREATE INDEX ix_some_column_name_1some_column_name_2some_column_name_3 ON t
+    (some_column_name_1, some_column_name_2, some_column_name_3)
+
+The implication here is that by upgrading SQLAlchemy to version 1.4 on
+an existing Oracle 12.2 or greater database, the generation of constraint
+names will change, which can impact the behavior of database migrations.
+A key example is a migration that wishes to "DROP CONSTRAINT" on a name that
+was previously generated with the shorter length.  This migration will fail
+when the identifier length is changed without the name of the index or
+constraint first being adjusted.
+
+Therefore, applications are strongly advised to make use of
+:paramref:`.create_engine.max_identifier_length` in order to maintain control
+of the generation of truncated names, and to fully review and test all database
+migrations in a staging environment when changing this value to ensure that the
+impact of this change has been mitigated.
+
+
+.. versionadded:: 1.3.9 Added the
+   :paramref:`.create_engine.max_identifier_length` parameter; the Oracle
+   dialect now detects compatibility version 12.2 or greater and warns
+   about upcoming max identitifier length changes in SQLAlchemy 1.4.
+
 
 LIMIT/OFFSET Support
 --------------------
@@ -348,6 +452,7 @@ columns for non-unique indexes, all but the last column for unique indexes).
 from itertools import groupby
 import re
 
+from ... import exc
 from ... import schema as sa_schema
 from ... import sql
 from ... import types as sqltypes
@@ -1086,6 +1191,11 @@ class OracleDialect(default.DefaultDialect):
     supports_unicode_binds = False
     max_identifier_length = 30
 
+    # this should be set to
+    # "SELECT value FROM v$parameter WHERE name = 'compatible'"
+    # upon connect.
+    _compat_server_version_info = None
+
     supports_simple_order_by_label = False
     cte_follows_insert = True
 
@@ -1138,6 +1248,13 @@ class OracleDialect(default.DefaultDialect):
 
     def initialize(self, connection):
         super(OracleDialect, self).initialize(connection)
+
+        _compat_server_version_info = self._get_compat_server_version_info(
+            connection
+        )
+        if _compat_server_version_info is not None:
+            self._compat_server_version_info = _compat_server_version_info
+
         self.implicit_returning = self.__dict__.get(
             "implicit_returning", self.server_version_info > (10,)
         )
@@ -1147,6 +1264,21 @@ class OracleDialect(default.DefaultDialect):
             self.colspecs.pop(sqltypes.Interval)
             self.use_ansi = False
 
+    def _get_compat_server_version_info(self, connection):
+        try:
+            return connection.execute(
+                "SELECT value FROM v$parameter WHERE name = 'compatible'"
+            ).scalar()
+        except exc.DBAPIError as err:
+            util.warn("Could not determine compatibility version: %s" % err)
+
+    @property
+    def _effective_compat_server_version_info(self):
+        if self._compat_server_version_info is not None:
+            return self._compat_server_version_info
+        else:
+            return self.server_version_info
+
     @property
     def _is_oracle_8(self):
         return self.server_version_info and self.server_version_info < (9,)
@@ -1167,6 +1299,27 @@ class OracleDialect(default.DefaultDialect):
         # Oracle does not support RELEASE SAVEPOINT
         pass
 
+    def _check_max_identifier_length(self, connection):
+        if self._effective_compat_server_version_info >= (12, 2):
+            util.warn(
+                "Oracle compatibility version %r is known to have a maximum "
+                "identifier length of 128, rather than the historical default "
+                "of 30. SQLAlchemy 1.4 will use 128 for this "
+                "database; please set max_identifier_length=128 "
+                "in create_engine() in order to "
+                "test the application with this new length, or set to 30 in "
+                "order to assure that 30 continues to be used.  "
+                "In particular, pay close attention to the behavior of "
+                "database migrations as dynamically generated names may "
+                "change. See the section 'Max Identifier Lengths' in the "
+                "SQLAlchemy Oracle dialect documentation for background."
+                % ((self.server_version_info,))
+            )
+            return 128
+        else:
+            # use the default
+            return None
+
     def _check_unicode_returns(self, connection):
         additional_tests = [
             expression.cast(
index d008d52125af89f09c0bc53933365be25fcc3f82..a6eec7719f675f22e50e45536bfba9cc5abc38da 100644 (file)
@@ -316,7 +316,15 @@ def create_engine(*args, **kwargs):
         the size of dynamically generated column labels to that many
         characters. If less than 6, labels are generated as
         "_(counter)". If ``None``, the value of
-        ``dialect.max_identifier_length`` is used instead.
+        ``dialect.max_identifier_length``, which may be affected via the
+        :paramref:`.create_engine.max_identifier_length` parameter,
+        is used instead.   The value of :paramref:`.create_engine.label_length`
+        may not be larger than that of
+        :paramref:`.create_engine.max_identfier_length`.
+
+        .. seealso::
+
+            :paramref:`.create_engine.max_identifier_length`
 
     :param listeners: A list of one or more
         :class:`~sqlalchemy.interfaces.PoolListener` objects which will
@@ -327,6 +335,21 @@ def create_engine(*args, **kwargs):
         "sqlalchemy.engine" logger. Defaults to a hexstring of the
         object's id.
 
+    :param max_identifier_length: integer; override the max_identifier_length
+        determined by the dialect.  if ``None`` or zero, has no effect.  This
+        is the database's configured maximum number of characters that may be
+        used in a SQL identifier such as a table name, column name, or label
+        name. All dialects determine this value automatically, however in the
+        case of a new database version for which this value has changed but
+        SQLAlchemy's dialect has not been adjusted, the value may be passed
+        here.
+
+        .. versionadded:: 1.3.9
+
+        .. seealso::
+
+            :paramref:`.create_engine.label_length`
+
     :param max_overflow=10: the number of connections to allow in
         connection pool "overflow", that is connections that can be
         opened above and beyond the pool_size setting, which defaults
index b56755d62c5eb988f7eb2822ff4068409eb0a736..83f60bf1089f3cffc76224fb91be62a0ee9c2b87 100644 (file)
@@ -111,6 +111,7 @@ class DefaultDialect(interfaces.Dialect):
     # length at which to truncate
     # any identifier.
     max_identifier_length = 9999
+    _user_defined_max_identifier_length = None
 
     # length at which to truncate
     # the name of an index.
@@ -206,6 +207,7 @@ class DefaultDialect(interfaces.Dialect):
         case_sensitive=True,
         supports_native_boolean=None,
         empty_in_strategy="static",
+        max_identifier_length=None,
         label_length=None,
         **kwargs
     ):
@@ -250,11 +252,10 @@ class DefaultDialect(interfaces.Dialect):
                 "'dynamic', or 'dynamic_warn'"
             )
 
-        if label_length and label_length > self.max_identifier_length:
-            raise exc.ArgumentError(
-                "Label length of %d is greater than this dialect's"
-                " maximum identifier length of %d"
-                % (label_length, self.max_identifier_length)
+        self._user_defined_max_identifier_length = max_identifier_length
+        if self._user_defined_max_identifier_length:
+            self.max_identifier_length = (
+                self._user_defined_max_identifier_length
             )
         self.label_length = label_length
 
@@ -314,6 +315,21 @@ class DefaultDialect(interfaces.Dialect):
         ):
             self._description_decoder = self.description_encoding = None
 
+        if not self._user_defined_max_identifier_length:
+            max_ident_length = self._check_max_identifier_length(connection)
+            if max_ident_length:
+                self.max_identifier_length = max_ident_length
+
+        if (
+            self.label_length
+            and self.label_length > self.max_identifier_length
+        ):
+            raise exc.ArgumentError(
+                "Label length of %d is greater than this dialect's"
+                " maximum identifier length of %d"
+                % (self.label_length, self.max_identifier_length)
+            )
+
     def on_connect(self):
         """return a callable which sets up a newly created DBAPI connection.
 
@@ -328,6 +344,18 @@ class DefaultDialect(interfaces.Dialect):
         """
         return None
 
+    def _check_max_identifier_length(self, connection):
+        """Perform a connection / server version specific check to determine
+        the max_identifier_length.
+
+        If the dialect's class level max_identifier_length should be used,
+        can return None.
+
+        .. versionadded:: 1.3.9
+
+        """
+        return None
+
     def _check_unicode_returns(self, connection, additional_tests=None):
         if util.py2k and not self.supports_unicode_statements:
             cast_to = util.binary_type
index 2692450adb7b71781adabf48202001f5f4920617..f508c2e47c6b1ab95996cc3699e3eb71fdffe6ae 100644 (file)
@@ -22,6 +22,13 @@ def setup_filters():
     warnings.filterwarnings("error", category=sa_exc.SADeprecationWarning)
     warnings.filterwarnings("error", category=sa_exc.SAWarning)
 
+    warnings.filterwarnings(
+        "ignore",
+        category=sa_exc.SAWarning,
+        message=r"Oracle compatibility version .* is known to have a "
+        "maximum identifier",
+    )
+
     # some selected deprecations...
     warnings.filterwarnings("error", category=DeprecationWarning)
     warnings.filterwarnings(
index 1401d40d033c401e46cfb9b147bb239ff6fb4c9b..f11bab1aefcb63ba73ad7732f1ec7a8ebcee972f 100644 (file)
@@ -164,6 +164,8 @@ class CompatFlagsTest(fixtures.TestBase, AssertsCompiledSQL):
         dialect._check_unicode_description = Mock()
         dialect._get_default_schema_name = Mock()
         dialect._detect_decimal_char = Mock()
+        dialect.__check_max_identifier_length = Mock()
+        dialect._get_compat_server_version_info = Mock()
         return dialect
 
     def test_ora8_flags(self):
index 901f941bea39dd54063f5631b3c7ce631c0e6d8a..80cf68740d3e1542de25aeb3ae4c7c65baa813fd 100644 (file)
@@ -4,19 +4,21 @@ from sqlalchemy import Integer
 from sqlalchemy import MetaData
 from sqlalchemy import or_
 from sqlalchemy import select
+from sqlalchemy import testing
 from sqlalchemy.engine import default
 from sqlalchemy.sql import column
 from sqlalchemy.sql import table
 from sqlalchemy.sql.elements import _truncated_label
 from sqlalchemy.testing import assert_raises
+from sqlalchemy.testing import assert_raises_message
 from sqlalchemy.testing import AssertsCompiledSQL
 from sqlalchemy.testing import engines
 from sqlalchemy.testing import eq_
 from sqlalchemy.testing import fixtures
+from sqlalchemy.testing import mock
 from sqlalchemy.testing.schema import Column
 from sqlalchemy.testing.schema import Table
 
-
 IDENT_LENGTH = 29
 
 
@@ -37,7 +39,9 @@ class MaxIdentTest(fixtures.TestBase, AssertsCompiledSQL):
 
     def _length_fixture(self, length=IDENT_LENGTH, positional=False):
         dialect = default.DefaultDialect()
-        dialect.max_identifier_length = length
+        dialect.max_identifier_length = (
+            dialect._user_defined_max_identifier_length
+        ) = length
         if positional:
             dialect.paramstyle = "format"
             dialect.positional = True
@@ -45,9 +49,126 @@ class MaxIdentTest(fixtures.TestBase, AssertsCompiledSQL):
 
     def _engine_fixture(self, length=IDENT_LENGTH):
         eng = engines.testing_engine()
-        eng.dialect.max_identifier_length = length
+        eng.dialect.max_identifier_length = (
+            eng.dialect._user_defined_max_identifier_length
+        ) = length
         return eng
 
+    def test_label_length_raise_too_large(self):
+        max_ident_length = testing.db.dialect.max_identifier_length
+        eng = engines.testing_engine(
+            options={"label_length": max_ident_length + 10}
+        )
+        assert_raises_message(
+            exceptions.ArgumentError,
+            "Label length of %d is greater than this dialect's maximum "
+            "identifier length of %d"
+            % (max_ident_length + 10, max_ident_length),
+            eng.connect,
+        )
+
+    def test_label_length_custom_maxlen(self):
+        max_ident_length = testing.db.dialect.max_identifier_length
+        eng = engines.testing_engine(
+            options={
+                "label_length": max_ident_length + 10,
+                "max_identifier_length": max_ident_length + 20,
+            }
+        )
+        with eng.connect() as conn:
+            eq_(conn.dialect.max_identifier_length, max_ident_length + 20)
+
+    def test_label_length_custom_maxlen_dialect_only(self):
+        dialect = default.DefaultDialect(max_identifier_length=47)
+        eq_(dialect.max_identifier_length, 47)
+
+    def test_label_length_custom_maxlen_user_set_manually(self):
+        eng = engines.testing_engine()
+        eng.dialect.max_identifier_length = 47
+
+        # assume the dialect has no on-connect change
+        with mock.patch.object(
+            eng.dialect,
+            "_check_max_identifier_length",
+            side_effect=lambda conn: None,
+        ):
+            with eng.connect():
+                pass
+
+        # it was maintained
+        eq_(eng.dialect.max_identifier_length, 47)
+
+    def test_label_length_too_large_custom_maxlen(self):
+        max_ident_length = testing.db.dialect.max_identifier_length
+        eng = engines.testing_engine(
+            options={
+                "label_length": max_ident_length - 10,
+                "max_identifier_length": max_ident_length - 20,
+            }
+        )
+        assert_raises_message(
+            exceptions.ArgumentError,
+            "Label length of %d is greater than this dialect's maximum "
+            "identifier length of %d"
+            % (max_ident_length - 10, max_ident_length - 20),
+            eng.connect,
+        )
+
+    def test_custom_max_identifier_length(self):
+        max_ident_length = testing.db.dialect.max_identifier_length
+        eng = engines.testing_engine(
+            options={"max_identifier_length": max_ident_length + 20}
+        )
+        with eng.connect() as conn:
+            eq_(conn.dialect.max_identifier_length, max_ident_length + 20)
+
+    def test_max_identifier_length_onconnect(self):
+        eng = engines.testing_engine()
+
+        def _check_max_identifer_length(conn):
+            return 47
+
+        with mock.patch.object(
+            eng.dialect,
+            "_check_max_identifier_length",
+            side_effect=_check_max_identifer_length,
+        ) as mock_:
+            with eng.connect():
+                eq_(eng.dialect.max_identifier_length, 47)
+        eq_(mock_.mock_calls, [mock.call(mock.ANY)])
+
+    def test_max_identifier_length_onconnect_returns_none(self):
+        eng = engines.testing_engine()
+
+        max_ident_length = eng.dialect.max_identifier_length
+
+        def _check_max_identifer_length(conn):
+            return None
+
+        with mock.patch.object(
+            eng.dialect,
+            "_check_max_identifier_length",
+            side_effect=_check_max_identifer_length,
+        ) as mock_:
+            with eng.connect():
+                eq_(eng.dialect.max_identifier_length, max_ident_length)
+        eq_(mock_.mock_calls, [mock.call(mock.ANY)])
+
+    def test_custom_max_identifier_length_onconnect(self):
+        eng = engines.testing_engine(options={"max_identifier_length": 49})
+
+        def _check_max_identifer_length(conn):
+            return 47
+
+        with mock.patch.object(
+            eng.dialect,
+            "_check_max_identifier_length",
+            side_effect=_check_max_identifer_length,
+        ) as mock_:
+            with eng.connect():
+                eq_(eng.dialect.max_identifier_length, 49)
+        eq_(mock_.mock_calls, [])  # was not called
+
     def test_table_alias_1(self):
         self.assert_compile(
             self.table2.alias().select(),