snowflakedb / snowflake-sqlalchemy

Snowflake SQLAlchemy
https://pypi.python.org/pypi/snowflake-sqlalchemy/
Apache License 2.0
234 stars 152 forks source link

Add ability to set ORDER / NOORDER sequence on columns with IDENTITY #493

Closed DanielTatarkin closed 6 months ago

DanielTatarkin commented 7 months ago

Please answer these questions before submitting your pull requests. Thanks!

  1. What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

    Fixes #436

  2. Fill out the following pre-review checklist:

    • [x] I am adding a new automated test(s) to verify correctness of my new code
    • [ ] I am adding new logging messages
    • [ ] I am adding new credentials
    • [ ] I am adding a new dependency
  3. Please describe how your code solves the related issue.

    CREATE TABLE DDL supports optional ORDER | NOORDER value for AUTOINCREMENT | IDENTITY sequences on a column. Before this change, using a column with a sequence like Column(NUMBER, Identity(start=1, increment=1, order=True) would default to session parameter NOORDER_SEQUENCE_AS_DEFAULT TRUE by default which would always set NOORDER instead of what is being set in the column definition.

    For us, this lead to discrepancies between ORM defined models and the Snowflake state seen by Alembic.

github-actions[bot] commented 7 months ago

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

DanielTatarkin commented 7 months ago

I have read the CLA Document and I hereby sign the CLA

DanielTatarkin commented 7 months ago

Semi-related issue #494

DanielTatarkin commented 6 months ago

Hi,

Thank you for your contribution 💪

Pls provide test cases proving correctness of proposed solution.

Example test case could look like this

def test_table_create_order():
    metadata= MetaData()
    table = Table("test_table", m, Column("pk", Integer, Identity(start=1, increment=1, order=True)))
    ddl_schema = schema.CreateTable(table)
    res = _schema.compile(dialect=SnowflakeDialect())
    expected = "..."
    assert res.string == expected, res.string

@sfc-gh-mraba thank you for the example, I added a unit test to tests/test_sequence.py (test_table_with_identity), let me know if the location is appropriate.