From: Federico Caselli Date: Wed, 16 Aug 2023 20:18:10 +0000 (+0200) Subject: Change Sequence and Identity oracle only kwargs. X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7bc66ca3640dcccbed77d7eb670e9d195d7d71a3;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Change Sequence and Identity oracle only kwargs. Deprecate Oracle only parameters :paramref:`_schema.Sequence.order`, paramref:`_schema.Identity.order` and :paramref:`_schema.Identity.on_null`. They should be configured using the dialect kwargs ``oracle_order`` and oracle_on_null``. Fixes: #10247 Change-Id: I124a16c9a482745e6f15669008968284fc435998 --- diff --git a/doc/build/changelog/unreleased_20/10247.rst b/doc/build/changelog/unreleased_20/10247.rst new file mode 100644 index 0000000000..1024693cab --- /dev/null +++ b/doc/build/changelog/unreleased_20/10247.rst @@ -0,0 +1,8 @@ +.. change:: + :tags: schema + :tickets: 10247 + + Deprecate Oracle only parameters :paramref:`_schema.Sequence.order`, + :paramref:`_schema.Identity.order` and :paramref:`_schema.Identity.on_null`. + They should be configured using the dialect kwargs ``oracle_order`` and + ``oracle_on_null``. diff --git a/lib/sqlalchemy/dialects/oracle/base.py b/lib/sqlalchemy/dialects/oracle/base.py index d993ef2692..58ccf17dad 100644 --- a/lib/sqlalchemy/dialects/oracle/base.py +++ b/lib/sqlalchemy/dialects/oracle/base.py @@ -25,7 +25,7 @@ available, which are the use of IDENTITY columns (Oracle 12 and above only) or the association of a SEQUENCE with the column. Specifying GENERATED AS IDENTITY (Oracle 12 and above) -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Starting from version 12 Oracle can make use of identity columns using the :class:`_sql.Identity` to specify the autoincrementing behavior:: @@ -50,9 +50,14 @@ The :class:`_schema.Identity` object support many options to control the incrementing value, etc. In addition to the standard options, Oracle supports setting :paramref:`_schema.Identity.always` to ``None`` to use the default -generated mode, rendering GENERATED AS IDENTITY in the DDL. It also supports -setting :paramref:`_schema.Identity.on_null` to ``True`` to specify ON NULL -in conjunction with a 'BY DEFAULT' identity column. +generated mode, rendering GENERATED AS IDENTITY in the DDL. +Oracle also supports two custom options specified using dialect kwargs: + +* ``oracle_on_null``: when set to ``True`` renders ``ON NULL`` in conjunction + with a 'BY DEFAULT' identity column. +* ``oracle_order``: when ``True``, renders the ORDER keyword, indicating the + identity is definitively ordered. May be necessary to provide deterministic + ordering using Oracle RAC. Using a SEQUENCE (all Oracle versions) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -77,6 +82,13 @@ This step is also required when using table reflection, i.e. autoload_with=engin autoload_with=engine ) +In addition to the standard options, Oracle supports the following custom +option specified using dialect kwargs: + +* ``oracle_order``: when ``True``, renders the ORDER keyword, indicating the + sequence is definitively ordered. May be necessary to provide deterministic + ordering using Oracle RAC. + .. versionchanged:: 1.4 Added :class:`_schema.Identity` construct in a :class:`_schema.Column` to specify the option of an autoincrementing column. @@ -1318,8 +1330,9 @@ class OracleDDLCompiler(compiler.DDLCompiler): text = text.replace("NO MINVALUE", "NOMINVALUE") text = text.replace("NO MAXVALUE", "NOMAXVALUE") text = text.replace("NO CYCLE", "NOCYCLE") - if identity_options.order is not None: - text += " ORDER" if identity_options.order else " NOORDER" + options = identity_options.dialect_options["oracle"] + if options.get("order") is not None: + text += " ORDER" if options["order"] else " NOORDER" return text.strip() def visit_computed_column(self, generated, **kw): @@ -1341,7 +1354,7 @@ class OracleDDLCompiler(compiler.DDLCompiler): else: kind = "ALWAYS" if identity.always else "BY DEFAULT" text = "GENERATED %s" % kind - if identity.on_null: + if identity.dialect_options["oracle"].get("on_null"): text += " ON NULL" text += " AS IDENTITY" options = self.get_identity_options(identity) @@ -1437,6 +1450,8 @@ class OracleDialect(default.DefaultDialect): {"resolve_synonyms": False, "on_commit": None, "compress": False}, ), (sa_schema.Index, {"bitmap": False, "compress": False}), + (sa_schema.Sequence, {"order": None}), + (sa_schema.Identity, {"order": None, "on_null": None}), ] @util.deprecated_params( @@ -2398,7 +2413,7 @@ class OracleDialect(default.DefaultDialect): parts = [p.strip() for p in identity_options.split(",")] identity = { "always": parts[0] == "ALWAYS", - "on_null": default_on_null == "YES", + "oracle_on_null": default_on_null == "YES", } for part in parts[1:]: @@ -2418,7 +2433,7 @@ class OracleDialect(default.DefaultDialect): elif "CACHE_SIZE" in option: identity["cache"] = int(value) elif "ORDER_FLAG" in option: - identity["order"] = value == "Y" + identity["oracle_order"] = value == "Y" return identity @reflection.cache diff --git a/lib/sqlalchemy/sql/schema.py b/lib/sqlalchemy/sql/schema.py index 79239fc5cd..525e8f4cf5 100644 --- a/lib/sqlalchemy/sql/schema.py +++ b/lib/sqlalchemy/sql/schema.py @@ -3608,7 +3608,7 @@ class CallableColumnDefault(ColumnDefault): ) -class IdentityOptions: +class IdentityOptions(DialectKWArgs): """Defines options for a named database sequence or an identity column. .. versionadded:: 1.3.18 @@ -3630,6 +3630,7 @@ class IdentityOptions: cycle: Optional[bool] = None, cache: Optional[int] = None, order: Optional[bool] = None, + **dialect_kw: Any, ) -> None: """Construct a :class:`.IdentityOptions` object. @@ -3649,6 +3650,8 @@ class IdentityOptions: :param order: optional boolean value; if ``True``, renders the ORDER keyword. + .. deprecated:: 2.0.21 Use ``oracle_order`` instead. + """ self.start = start self.increment = increment @@ -3658,12 +3661,44 @@ class IdentityOptions: self.nomaxvalue = nomaxvalue self.cycle = cycle self.cache = cache - self.order = order + if order is not None: + if "oracle_order" in dialect_kw: + raise exc.ArgumentError( + "Cannot specify both 'order' and 'oracle_order'. " + "Plese use only 'oracle_order'." + ) + dialect_kw["oracle_order"] = order + self._validate_dialect_kwargs(dialect_kw) @property def _increment_is_negative(self) -> bool: return self.increment is not None and self.increment < 0 + @property + def order(self) -> Optional[bool]: + """Alias of the ``dialect_kwargs`` ``'oracle_order'``. + + .. deprecated:: 2.0.21 The 'order' attribute is deprecated. + """ + value: Optional[bool] = self.dialect_kwargs.get("oracle_order") + return value + + def _as_dict(self) -> Dict[str, Any]: + return { + k: v + for k, v in { + "start": self.start, + "increment": self.increment, + "minvalue": self.minvalue, + "maxvalue": self.maxvalue, + "nominvalue": self.nominvalue, + "nomaxvalue": self.nomaxvalue, + "cycle": self.cycle, + "cache": self.cache, + }.items() + if v != None + } + class Sequence(HasSchemaAttr, IdentityOptions, DefaultGenerator): """Represents a named database sequence. @@ -3705,6 +3740,13 @@ class Sequence(HasSchemaAttr, IdentityOptions, DefaultGenerator): column: Optional[Column[Any]] data_type: Optional[TypeEngine[int]] + @util.deprecated_params( + order=( + "2.1", + "This parameter is supported only by Oracle, " + "use ``oracle_order`` instead.", + ) + ) def __init__( self, name: str, @@ -3724,6 +3766,7 @@ class Sequence(HasSchemaAttr, IdentityOptions, DefaultGenerator): metadata: Optional[MetaData] = None, quote_schema: Optional[bool] = None, for_update: bool = False, + **dialect_kw: Any, ) -> None: """Construct a :class:`.Sequence` object. @@ -3868,6 +3911,7 @@ class Sequence(HasSchemaAttr, IdentityOptions, DefaultGenerator): cycle=cycle, cache=cache, order=order, + **dialect_kw, ) self.column = None self.name = quoted_name(name, quote) @@ -3905,20 +3949,13 @@ class Sequence(HasSchemaAttr, IdentityOptions, DefaultGenerator): def _copy(self) -> Sequence: return Sequence( name=self.name, - start=self.start, - increment=self.increment, - minvalue=self.minvalue, - maxvalue=self.maxvalue, - nominvalue=self.nominvalue, - nomaxvalue=self.nomaxvalue, - cycle=self.cycle, schema=self.schema, - cache=self.cache, - order=self.order, data_type=self.data_type, optional=self.optional, metadata=self.metadata, for_update=self.for_update, + **self._as_dict(), + **self.dialect_kwargs, ) def _set_table(self, column: Column[Any], table: Table) -> None: @@ -5997,9 +6034,21 @@ class Identity(IdentityOptions, FetchedValue, SchemaItem): is_identity = True + @util.deprecated_params( + order=( + "2.1", + "This parameter is supported only by Oracle, " + "use ``oracle_order`` instead.", + ), + on_null=( + "2.1", + "This parameter is supported only by Oracle, " + "use ``oracle_on_null`` instead.", + ), + ) def __init__( self, - always: bool = False, + always: Optional[bool] = False, on_null: Optional[bool] = None, start: Optional[int] = None, increment: Optional[int] = None, @@ -6010,6 +6059,7 @@ class Identity(IdentityOptions, FetchedValue, SchemaItem): cycle: Optional[bool] = None, cache: Optional[int] = None, order: Optional[bool] = None, + **dialect_kw: Any, ) -> None: """Construct a GENERATED { ALWAYS | BY DEFAULT } AS IDENTITY DDL construct to accompany a :class:`_schema.Column`. @@ -6056,6 +6106,15 @@ class Identity(IdentityOptions, FetchedValue, SchemaItem): ORDER keyword. """ + self.dialect_options + if on_null is not None: + if "oracle_on_null" in dialect_kw: + raise exc.ArgumentError( + "Cannot specify both 'on_null' and 'oracle_on_null'. " + "Plese use only 'oracle_on_null'." + ) + dialect_kw["oracle_on_null"] = on_null + IdentityOptions.__init__( self, start=start, @@ -6067,11 +6126,20 @@ class Identity(IdentityOptions, FetchedValue, SchemaItem): cycle=cycle, cache=cache, order=order, + **dialect_kw, ) self.always = always - self.on_null = on_null self.column = None + @property + def on_null(self) -> Optional[bool]: + """Alias of the ``dialect_kwargs`` ``'oracle_on_null'``. + + .. deprecated:: 2.0.21 The 'on_null' attribute is deprecated. + """ + value: Optional[bool] = self.dialect_kwargs.get("oracle_on_null") + return value + def _set_parent(self, parent: SchemaEventTarget, **kw: Any) -> None: assert isinstance(parent, Column) if not isinstance( @@ -6106,18 +6174,13 @@ class Identity(IdentityOptions, FetchedValue, SchemaItem): return self._copy(**kw) def _copy(self, **kw: Any) -> Identity: - i = Identity( - always=self.always, - on_null=self.on_null, - start=self.start, - increment=self.increment, - minvalue=self.minvalue, - maxvalue=self.maxvalue, - nominvalue=self.nominvalue, - nomaxvalue=self.nomaxvalue, - cycle=self.cycle, - cache=self.cache, - order=self.order, - ) + i = Identity(**self._as_dict(), **self.dialect_kwargs) return self._schema_item_copy(i) + + def _as_dict(self) -> Dict[str, Any]: + return { + # always=None means something different than always=False + "always": self.always, + **super()._as_dict(), + } diff --git a/test/dialect/oracle/test_compiler.py b/test/dialect/oracle/test_compiler.py index c7a6858d4c..42e43c8838 100644 --- a/test/dialect/oracle/test_compiler.py +++ b/test/dialect/oracle/test_compiler.py @@ -38,6 +38,7 @@ from sqlalchemy.sql.selectable import LABEL_STYLE_TABLENAME_PLUS_COL from sqlalchemy.testing import assert_raises_message from sqlalchemy.testing import AssertsCompiledSQL from sqlalchemy.testing import eq_ +from sqlalchemy.testing import expect_deprecated from sqlalchemy.testing import fixtures from sqlalchemy.testing.assertions import eq_ignore_whitespace from sqlalchemy.testing.schema import Column @@ -1573,7 +1574,6 @@ class CompileTest(fixtures.TestBase, AssertsCompiledSQL): nominvalue=True, nomaxvalue=True, cycle=False, - order=False, ), ), ) @@ -1581,9 +1581,39 @@ class CompileTest(fixtures.TestBase, AssertsCompiledSQL): schema.CreateTable(t), "CREATE TABLE t (y INTEGER GENERATED ALWAYS AS IDENTITY " "(INCREMENT BY 7 START WITH 4 NOMINVALUE NOMAXVALUE " - "NOCYCLE NOORDER))", + "NOCYCLE))", ) + def test_column_identity_dialect_args(self): + m = MetaData() + t = Table( + "t", + m, + Column("y", Integer, Identity(cycle=True, oracle_order=False)), + Column("x", Integer, Identity(nomaxvalue=True, oracle_order=True)), + ) + self.assert_compile( + schema.CreateTable(t), + "CREATE TABLE t (" + "y INTEGER GENERATED BY DEFAULT AS IDENTITY (CYCLE NOORDER), " + "x INTEGER GENERATED BY DEFAULT AS IDENTITY (NOMAXVALUE ORDER)" + ")", + ) + + def test_deprecated_options(self): + with expect_deprecated( + ".+use ``oracle_on_null`` instead", + ".+use ``oracle_order`` instead", + ): + idx = Identity(order=False, on_null=True) + eq_(idx.dialect_options["oracle"]["order"], False) + eq_(idx.dialect_options["oracle"]["on_null"], True) + + def test_deprecated_attrs(self): + idx = Identity(oracle_order=True, oracle_on_null=True) + eq_(idx.order, True) + eq_(idx.on_null, True) + def test_column_identity_no_generated(self): m = MetaData() t = Table("t", m, Column("y", Integer, Identity(always=None))) @@ -1601,7 +1631,9 @@ class CompileTest(fixtures.TestBase, AssertsCompiledSQL): def test_column_identity_on_null(self, always, on_null, text): m = MetaData() t = Table( - "t", m, Column("y", Integer, Identity(always, on_null=on_null)) + "t", + m, + Column("y", Integer, Identity(always, oracle_on_null=on_null)), ) self.assert_compile( schema.CreateTable(t), @@ -1656,6 +1688,25 @@ class SequenceTest(fixtures.TestBase, AssertsCompiledSQL): dialect=oracle.OracleDialect(), ) + def test_compile_dialect_args(self): + self.assert_compile( + ddl.CreateSequence(Sequence("my_seq", oracle_order=False)), + "CREATE SEQUENCE my_seq NOORDER", + dialect=oracle.OracleDialect(), + ) + self.assert_compile( + ddl.CreateSequence( + Sequence("my_seq", nominvalue=True, oracle_order=True) + ), + "CREATE SEQUENCE my_seq NOMINVALUE ORDER", + dialect=oracle.OracleDialect(), + ) + + def test_deprecated_options(self): + with expect_deprecated(".+use ``oracle_order`` instead"): + seq = Sequence("foo", order=False) + eq_(seq.dialect_options["oracle"]["order"], False) + class RegexpTest(fixtures.TestBase, testing.AssertsCompiledSQL): __dialect__ = "oracle" diff --git a/test/dialect/oracle/test_reflection.py b/test/dialect/oracle/test_reflection.py index 00d8363720..519459c503 100644 --- a/test/dialect/oracle/test_reflection.py +++ b/test/dialect/oracle/test_reflection.py @@ -1314,8 +1314,14 @@ class IdentityReflectionTest(fixtures.TablesTest): @classmethod def define_tables(cls, metadata): - Table("t1", metadata, Column("id1", Integer, Identity(on_null=True))) - Table("t2", metadata, Column("id2", Integer, Identity(order=True))) + Table( + "t1", + metadata, + Column("id1", Integer, Identity(oracle_on_null=True)), + ) + Table( + "t2", metadata, Column("id2", Integer, Identity(oracle_order=True)) + ) def test_reflect_identity(self): insp = inspect(testing.db) @@ -1323,23 +1329,23 @@ class IdentityReflectionTest(fixtures.TablesTest): "always": False, "start": 1, "increment": 1, - "on_null": False, + "oracle_on_null": False, "maxvalue": 10**28 - 1, "minvalue": 1, "cycle": False, "cache": 20, - "order": False, + "oracle_order": False, } for col in insp.get_columns("t1") + insp.get_columns("t2"): if col["name"] == "id1": is_true("identity" in col) exp = common.copy() - exp["on_null"] = True + exp["oracle_on_null"] = True eq_(col["identity"], exp) if col["name"] == "id2": is_true("identity" in col) exp = common.copy() - exp["order"] = True + exp["oracle_order"] = True eq_(col["identity"], exp) diff --git a/test/sql/test_identity_column.py b/test/sql/test_identity_column.py index 2603a9e101..2eef36829d 100644 --- a/test/sql/test_identity_column.py +++ b/test/sql/test_identity_column.py @@ -157,26 +157,6 @@ class IdentityDDL(_IdentityDDLFixture, fixtures.TestBase): # this uses the connection dialect __requires__ = ("identity_columns_standard",) - def test_on_null(self): - t = Table( - "foo_table", - MetaData(), - Column( - "foo", - Integer(), - Identity(always=False, on_null=True, start=42, cycle=True), - ), - ) - text = " ON NULL" if testing.against("oracle") else "" - self.assert_compile( - CreateTable(t), - ( - "CREATE TABLE foo_table (foo INTEGER GENERATED BY DEFAULT" - + text - + " AS IDENTITY (START WITH 42 CYCLE))" - ), - ) - class DefaultDialectIdentityDDL(_IdentityDDLFixture, fixtures.TestBase): # this uses the default dialect @@ -195,7 +175,7 @@ class NotSupportingIdentityDDL(testing.AssertsCompiledSQL, fixtures.TestBase): t = Table( "foo_table", MetaData(), - Column("foo", Integer(), Identity("always", start=3)), + Column("foo", Integer(), Identity(always=True, start=3)), ) t_exp = Table( "foo_table", @@ -224,7 +204,7 @@ class NotSupportingIdentityDDL(testing.AssertsCompiledSQL, fixtures.TestBase): Column( "foo", Integer(), - Identity("always", start=3), + Identity(always=True, start=3), primary_key=True, autoincrement=autoincrement, ), @@ -261,7 +241,7 @@ class IdentityTest(fixtures.TestBase): assert_raises_message(ArgumentError, text, fn, server_onupdate="42") def test_to_metadata(self): - identity1 = Identity("by default", cycle=True, start=123) + identity1 = Identity(always=False, cycle=True, start=123) m = MetaData() t = Table( "t", m, Column("x", Integer), Column("y", Integer, identity1) diff --git a/test/sql/test_sequences.py b/test/sql/test_sequences.py index 0781ff294c..1078546418 100644 --- a/test/sql/test_sequences.py +++ b/test/sql/test_sequences.py @@ -74,12 +74,6 @@ class SequenceDDLTest(fixtures.TestBase, testing.AssertsCompiledSQL): Sequence("foo_seq", minvalue=42, increment=-2), "INCREMENT BY -2 MINVALUE 42", ), - ( - # remove this when the `order` parameter is removed - # issue #10207 - ensure ORDER does not render - Sequence("foo_seq", order=True), - "", - ), ( Sequence("foo_seq", minvalue=-42, increment=-2), "INCREMENT BY -2 MINVALUE -42",