From 9d5f1c0f532749391b51bf3008771a92eb3c2f05 Mon Sep 17 00:00:00 2001 From: Federico Caselli Date: Wed, 4 Sep 2024 20:10:45 +0200 Subject: [PATCH] Float and Numeric aren't set as autoincrement The :class:`.Float` and :class:`.Numeric` types are no longer automatically considered as auto-incrementing columns when the :paramref:`_schema.Column.autoincrement` parameter is left at its default of ``"auto"`` on a :class:`_schema.Column` that is part of the primary key. When the parameter is set to ``True``, a :class:`.Numeric` type will be accepted as an auto-incrementing datatype for primary key columns, but only if its scale is explicitly given as zero; otherwise, an error is raised. This is a change from 2.0 where all numeric types including floats were automatically considered as "autoincrement" for primary key columns. Fixes: #11811 Change-Id: Icdfe084d425166199d6647335c5b53ea5b4b416e --- doc/build/changelog/unreleased_21/11811.rst | 13 +++++ lib/sqlalchemy/sql/schema.py | 23 ++++++--- test/sql/test_defaults.py | 12 +++++ test/sql/test_metadata.py | 53 +++++++++++++++++++++ 4 files changed, 94 insertions(+), 7 deletions(-) create mode 100644 doc/build/changelog/unreleased_21/11811.rst diff --git a/doc/build/changelog/unreleased_21/11811.rst b/doc/build/changelog/unreleased_21/11811.rst new file mode 100644 index 0000000000..34d0683dd9 --- /dev/null +++ b/doc/build/changelog/unreleased_21/11811.rst @@ -0,0 +1,13 @@ +.. change:: + :tags: bug, schema + :tickets: 11811 + + The :class:`.Float` and :class:`.Numeric` types are no longer automatically + considered as auto-incrementing columns when the + :paramref:`_schema.Column.autoincrement` parameter is left at its default + of ``"auto"`` on a :class:`_schema.Column` that is part of the primary key. + When the parameter is set to ``True``, a :class:`.Numeric` type will be + accepted as an auto-incrementing datatype for primary key columns, but only + if its scale is explicitly given as zero; otherwise, an error is raised. + This is a change from 2.0 where all numeric types including floats were + automatically considered as "autoincrement" for primary key columns. diff --git a/lib/sqlalchemy/sql/schema.py b/lib/sqlalchemy/sql/schema.py index 21c44d8170..fd376c9ee3 100644 --- a/lib/sqlalchemy/sql/schema.py +++ b/lib/sqlalchemy/sql/schema.py @@ -5089,12 +5089,20 @@ class PrimaryKeyConstraint(ColumnCollectionConstraint): @util.ro_memoized_property def _autoincrement_column(self) -> Optional[Column[int]]: def _validate_autoinc(col: Column[Any], autoinc_true: bool) -> bool: - if col.type._type_affinity is None or not issubclass( - col.type._type_affinity, - ( - type_api.INTEGERTYPE._type_affinity, - type_api.NUMERICTYPE._type_affinity, - ), + if col.type._type_affinity is not None and issubclass( + col.type._type_affinity, type_api.NUMERICTYPE._type_affinity + ): + scale = col.type.scale # type: ignore[attr-defined] + if scale != 0 and autoinc_true: + raise exc.ArgumentError( + f"Column type {col.type} with non-zero scale " + f"{scale} on column '{col}' is not " + f"compatible with autoincrement=True" + ) + elif not autoinc_true: + return False + elif col.type._type_affinity is None or not issubclass( + col.type._type_affinity, type_api.INTEGERTYPE._type_affinity ): if autoinc_true: raise exc.ArgumentError( @@ -5104,7 +5112,8 @@ class PrimaryKeyConstraint(ColumnCollectionConstraint): else: return False elif ( - not isinstance(col.default, (type(None), Sequence)) + col.default is not None + and not isinstance(col.default, Sequence) and not autoinc_true ): return False diff --git a/test/sql/test_defaults.py b/test/sql/test_defaults.py index bcfdfcdb9c..5ebc86608b 100644 --- a/test/sql/test_defaults.py +++ b/test/sql/test_defaults.py @@ -1232,6 +1232,18 @@ class AutoIncrementTest(fixtures.TestBase): 1, ) + @testing.combinations( + sa.Float, sa.DOUBLE_PRECISION, sa.Numeric, sa.Numeric(asdecimal=False) + ) + def test_autoincrement_not_float(self, type_): + t = Table( + "table", sa.MetaData(), Column("col", type_, primary_key=True) + ) + + eq_(t.autoincrement_column, None) + eq_(t.primary_key._autoincrement_column, None) + eq_(t.c.col.autoincrement, "auto") + class SpecialTypePKTest(fixtures.TestBase): """test process_result_value in conjunction with primary key columns. diff --git a/test/sql/test_metadata.py b/test/sql/test_metadata.py index 1b068c02f7..c9c6c55c02 100644 --- a/test/sql/test_metadata.py +++ b/test/sql/test_metadata.py @@ -16,6 +16,7 @@ from sqlalchemy import desc from sqlalchemy import Enum from sqlalchemy import event from sqlalchemy import exc +from sqlalchemy import Float from sqlalchemy import ForeignKey from sqlalchemy import ForeignKeyConstraint from sqlalchemy import func @@ -2134,6 +2135,58 @@ class PKAutoIncrementTest(fixtures.TestBase): lambda: pk._autoincrement_column, ) + def test_float_illegal_autoinc(self): + """test that Float is not acceptable if autoincrement=True""" + t = Table("t", MetaData(), Column("a", Float, autoincrement=True)) + pk = PrimaryKeyConstraint(t.c.a) + t.append_constraint(pk) + + with expect_raises_message( + exc.ArgumentError, "Column type FLOAT with non-zero scale " + ): + pk._autoincrement_column, + + def test_numeric_nonzero_scale_illegal_autoinc(self): + """test that Numeric() with non-zero scale is not acceptable if + autoincrement=True""" + t = Table( + "t", MetaData(), Column("a", Numeric(10, 5), autoincrement=True) + ) + pk = PrimaryKeyConstraint(t.c.a) + t.append_constraint(pk) + + with expect_raises_message( + exc.ArgumentError, + r"Column type NUMERIC\(10, 5\) with non-zero scale 5", + ): + pk._autoincrement_column, + + def test_numeric_zero_scale_autoinc_not_auto(self): + """test that Numeric() is not automatically assigned to + autoincrement""" + t = Table( + "t", MetaData(), Column("a", Numeric(10, 0), primary_key=True) + ) + + is_(t.autoincrement_column, None) + + def test_integer_autoinc_is_auto(self): + """test that Integer() is automatically assigned to autoincrement""" + t = Table("t", MetaData(), Column("a", Integer, primary_key=True)) + + is_(t.autoincrement_column, t.c.a) + + def test_numeric_zero_scale_autoinc_explicit_ok(self): + """test that Numeric() with zero scale is acceptable if + autoincrement=True""" + t = Table( + "t", + MetaData(), + Column("a", Numeric(10, 0), autoincrement=True, primary_key=True), + ) + + is_(t.autoincrement_column, t.c.a) + def test_single_integer_default(self): t = Table( "t", -- 2.47.3