From: Mike Bayer Date: Wed, 2 Oct 2019 15:20:58 +0000 (-0400) Subject: Add max_identifier_length parameter; warn for Oracle X-Git-Tag: rel_1_3_9~6 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5188afe8703b7c8e03086d3898a4f44038316a57;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Add max_identifier_length parameter; warn for Oracle 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) --- diff --git a/doc/build/changelog/unreleased_13/4857.rst b/doc/build/changelog/unreleased_13/4857.rst new file mode 100644 index 0000000000..ea2165c302 --- /dev/null +++ b/doc/build/changelog/unreleased_13/4857.rst @@ -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 diff --git a/lib/sqlalchemy/dialects/oracle/base.py b/lib/sqlalchemy/dialects/oracle/base.py index 9f0b23e12e..5c781e3a6f 100644 --- a/lib/sqlalchemy/dialects/oracle/base.py +++ b/lib/sqlalchemy/dialects/oracle/base.py @@ -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( diff --git a/lib/sqlalchemy/engine/__init__.py b/lib/sqlalchemy/engine/__init__.py index d008d52125..a6eec7719f 100644 --- a/lib/sqlalchemy/engine/__init__.py +++ b/lib/sqlalchemy/engine/__init__.py @@ -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 diff --git a/lib/sqlalchemy/engine/default.py b/lib/sqlalchemy/engine/default.py index b56755d62c..83f60bf108 100644 --- a/lib/sqlalchemy/engine/default.py +++ b/lib/sqlalchemy/engine/default.py @@ -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 diff --git a/lib/sqlalchemy/testing/warnings.py b/lib/sqlalchemy/testing/warnings.py index 2692450adb..f508c2e47c 100644 --- a/lib/sqlalchemy/testing/warnings.py +++ b/lib/sqlalchemy/testing/warnings.py @@ -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( diff --git a/test/dialect/oracle/test_dialect.py b/test/dialect/oracle/test_dialect.py index 1401d40d03..f11bab1aef 100644 --- a/test/dialect/oracle/test_dialect.py +++ b/test/dialect/oracle/test_dialect.py @@ -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): diff --git a/test/sql/test_labels.py b/test/sql/test_labels.py index 901f941bea..80cf68740d 100644 --- a/test/sql/test_labels.py +++ b/test/sql/test_labels.py @@ -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(),