WIP: Fix ignoring kwargs and ensure deterministic output (#439)

[luliangce]

Changes

Fixes #439.

This PR fixes an issue where Index kwargs (such as postgresql_using, mysql_length, etc.) were being ignored during code generation.

Key changes:

• Modified render_index in src/sqlacodegen/generators.py to include kwargs in the generated Index definition.
• Enforced sorting of kwargs keys using sorted() to ensure deterministic output (avoiding random reordering of arguments in generated code).
• Added a new test case test_index_with_kwargs in tests/test_generator_declarative.py.
• Updated existing GeoAlchemy2 tests in tests/test_generator_declarative.py. These tests previously passed because the generator ignored the postgresql_using='gist' argument present in the metadata.

# https://github.com/geoalchemy/geoalchemy2/blob/0c311122e5228a885e262d8f79b037cb74c0949b/geoalchemy2/admin/dialects/postgresql.py#L38C1-L57C15
def create_spatial_index(bind, table, col):
    """Create spatial index on the given column."""
    if col.type.use_N_D_index:
        postgresql_ops = {col.name: "gist_geometry_ops_nd"}
    else:
        postgresql_ops = {}
    if _check_spatial_type(col.type, Raster):
        col_func = func.ST_ConvexHull(col)
    else:
        col_func = col
    idx = Index(
        _spatial_idx_name(table.name, col.name),
        col_func,
        postgresql_using="gist", 
        postgresql_ops=postgresql_ops,
        _column_flag=True,
    )
    if bind is not None:
        idx.create(bind=bind)
    return idx

Checklist

If this is a user-facing code change, like a bugfix or a new feature, please ensure that
you've fulfilled the following conditions (where applicable):

• You've added tests (in tests/) which would fail without your patch
• You've added a new changelog entry (in CHANGES.rst).

If this is a trivial change, like a typo fix or a code reformatting, then you can ignore
these instructions.

Updating the changelog

If there are no entries after the last release, use **UNRELEASED** as the version.
If, say, your patch fixes issue #123, the entry should look like this:

- Fix big bad boo-boo in task groups
  (`#123 <https://github.com/agronholm/sqlacodegen/issues/123>`_; PR by @yourgithubaccount)

If there's no issue linked, just link to your pull request instead by updating the
changelog after you've created the PR.

[coveralls]

coverage: 97.367% (+0.007%) from 97.36%
when pulling 63693b5 on luliangce:master
into 615d79c on agronholm:master.

[sheinbergon]

@luliangce very nice, very nice 👏

Please add an entry to CHANGES.rst

[luliangce]

@luliangce very nice, very nice 👏

Please add an entry to CHANGES.rst

Should be good now, please take another look

[sheinbergon]

@agronholm wdyt?

[agronholm]

I'm fine with this, but I do wonder about the claim about randomness. Is this something that was actually observed to happen?

[sheinbergon]

I'm fine with this, but I do wonder about the claim about randomness. Is this something that was actually observed to happen?

@luliangce can you elaborate more about random re-ordering?

[luliangce]

@sheinbergon @agronholm Sorry — as I continued looking into how SQLAlchemy retrieves index kwargs during reflection and what kinds of parameters it can actually obtain, I ran into a few special cases.

Some index options like the ones below may not be simple data types, and the current approach would generate code that doesn’t work. I’ll need a few days to experiment with this and try to find a proper solution.

https://github.com/sqlalchemy/sqlalchemy/blob/259636fabb81289fb01fb3322638544a06cf3222/lib/sqlalchemy/dialects/sqlite/base.py#L2956-L2959
https://docs.sqlalchemy.org/en/20/dialects/postgresql.html#partial-indexes
https://docs.sqlalchemy.org/en/20/dialects/sqlite.html#partial-indexes
https://docs.sqlalchemy.org/en/20/dialects/mssql.html#filtered-indexes

[sheinbergon]

@sheinbergon @agronholm Sorry — as I continued looking into how SQLAlchemy retrieves index kwargs during reflection and what kinds of parameters it can actually obtain, I ran into a few special cases.

Some index options like the ones below may not be simple data types, and the current approach would generate code that doesn’t work. I’ll need a few days to experiment with this and try to find a proper solution.

https://github.com/sqlalchemy/sqlalchemy/blob/259636fabb81289fb01fb3322638544a06cf3222/lib/sqlalchemy/dialects/sqlite/base.py#L2956-L2959 https://docs.sqlalchemy.org/en/20/dialects/postgresql.html#partial-indexes https://docs.sqlalchemy.org/en/20/dialects/sqlite.html#partial-indexes https://docs.sqlalchemy.org/en/20/dialects/mssql.html#filtered-indexes

Great. We'll be waiting for your revised PR. Bear in mind, this work doesn't need to support every use case to begin with. You can always support common cases and just open new issues to be taken care of at a later time

[sheinbergon]

Hello again @luliangce

Were you able to make any progress with your work?