Specifying an Array(String) column type results in a KeyError during Alembic Migrations

[geudrik]

Environment details

• Programming language: Python 3.8
• OS: Docker (Debian Slim base)
• Language runtime version: Py3.8
• Package version: master@a0ebf758eda351c0a20103f9e8c2243f002b2e6e

Steps to reproduce

1. Define a table with a column with an array type permissions: List[String] = Column(ARRAY(String))
2. Run an Alembic automigration

I added two prints in my local version to produce dict and two arrays - see the NOTE THIS LINE below

root@8da01ba789c0:/app# alembic revision --autogenerate -m "test"
INFO  [alembic.runtime.migration] Context impl SpannerImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
{'BOOL': <class 'sqlalchemy.sql.sqltypes.Boolean'>, 'BYTES': <class 'sqlalchemy.sql.sqltypes.LargeBinary'>, 'DATE': <class 'sqlalchemy.sql.sqltypes.DATE'>, 'DATETIME': <class 'sqlalchemy.sql.sqltypes.DATETIME'>, 'FLOAT64': <class 'sqlalchemy.sql.sqltypes.Float'>, 'INT64': <class 'sqlalchemy.sql.sqltypes.BIGINT'>, 'NUMERIC': NUMERIC(precision=38, scale=9), 'STRING': <class 'sqlalchemy.sql.sqltypes.String'>, 'TIME': <class 'sqlalchemy.sql.sqltypes.TIME'>, 'TIMESTAMP': <class 'sqlalchemy.sql.sqltypes.TIMESTAMP'>}
['time', 'TIMESTAMP', 'YES']
['permissions', 'ARRAY<STRING(MAX)>', 'YES']   <-- NOTE THIS LINE
Traceback (most recent call last):
  File "/usr/local/bin/alembic", line 8, in <module>
    sys.exit(main())
  File "/usr/local/lib/python3.8/site-packages/alembic/config.py", line 588, in main
    CommandLine(prog=prog).main(argv=argv)
  File "/usr/local/lib/python3.8/site-packages/alembic/config.py", line 582, in main
    self.run_cmd(cfg, options)
  File "/usr/local/lib/python3.8/site-packages/alembic/config.py", line 559, in run_cmd
    fn(
  File "/usr/local/lib/python3.8/site-packages/alembic/command.py", line 227, in revision
    script_directory.run_env()
  File "/usr/local/lib/python3.8/site-packages/alembic/script/base.py", line 563, in run_env
    util.load_python_file(self.dir, "env.py")
  File "/usr/local/lib/python3.8/site-packages/alembic/util/pyfiles.py", line 92, in load_python_file
    module = load_module_py(module_id, path)
  File "/usr/local/lib/python3.8/site-packages/alembic/util/pyfiles.py", line 108, in load_module_py
    spec.loader.exec_module(module)  # type: ignore
  File "<frozen importlib._bootstrap_external>", line 783, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "alembic/env.py", line 72, in <module>
    run_migrations_online()
  File "alembic/env.py", line 66, in run_migrations_online
    context.run_migrations()
  File "<string>", line 8, in run_migrations
  File "/usr/local/lib/python3.8/site-packages/alembic/runtime/environment.py", line 851, in run_migrations
    self.get_context().run_migrations(**kw)
  File "/usr/local/lib/python3.8/site-packages/alembic/runtime/migration.py", line 600, in run_migrations
    for step in self._migrations_fn(heads, self):
  File "/usr/local/lib/python3.8/site-packages/alembic/command.py", line 203, in retrieve_migrations
    revision_context.run_autogenerate(rev, context)
  File "/usr/local/lib/python3.8/site-packages/alembic/autogenerate/api.py", line 520, in run_autogenerate
    self._run_environment(rev, migration_context, True)
  File "/usr/local/lib/python3.8/site-packages/alembic/autogenerate/api.py", line 567, in _run_environment
    compare._populate_migration_script(
  File "/usr/local/lib/python3.8/site-packages/alembic/autogenerate/compare.py", line 53, in _populate_migration_script
    _produce_net_changes(autogen_context, upgrade_ops)
  File "/usr/local/lib/python3.8/site-packages/alembic/autogenerate/compare.py", line 87, in _produce_net_changes
    comparators.dispatch("schema", autogen_context.dialect.name)(
  File "/usr/local/lib/python3.8/site-packages/alembic/util/langhelpers.py", line 266, in go
    fn(*arg, **kw)
  File "/usr/local/lib/python3.8/site-packages/alembic/autogenerate/compare.py", line 126, in _autogen_for_tables
    _compare_tables(
  File "/usr/local/lib/python3.8/site-packages/alembic/autogenerate/compare.py", line 212, in _compare_tables
    sqla_compat._reflect_table(inspector, t, None)
  File "/usr/local/lib/python3.8/site-packages/alembic/util/sqla_compat.py", line 226, in _reflect_table
    return inspector.reflecttable(table, None)
  File "/usr/local/lib/python3.8/site-packages/sqlalchemy/engine/reflection.py", line 664, in reflecttable
    for col_d in self.get_columns(
  File "/usr/local/lib/python3.8/site-packages/sqlalchemy/engine/reflection.py", line 390, in get_columns
    col_defs = self.dialect.get_columns(
  File "/usr/local/lib/python3.8/site-packages/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py", line 104, in wrapper
    return function(self, connection, *args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py", line 490, in get_columns
    type_ = _type_map[col[1]]
KeyError: 'ARRAY<STRING(MAX)>'

print() were added on line 476 and 489 for debugging sake

[geudrik]

Oh, I also tried via the latest commit hash, same issue.

[IlyaFaer]

Interesting, the original SQLAlchemy reflection tests suite doesn't contain any single case for arrays?!
Okay. I've reproduced the error and already have a solution. Just adding some tests and etc.

[geudrik]

Why test when you can use production as test? 😄 Appreciate the quick fix!

[geudrik]

I think there are some additional bugs here / this successive issue should block #119

Error parsing Spanner DDL statement: ALTER TABLE ttp_mitre_enterprise ALTER COLUMN parent_id DROP NOT NULL

This is the alembic error I get using this branch trying to run a migration. In this case, the alembic migration is attempting to execute the following alembic code - altering a column and dropping the NOT NULL constraint

op.alter_column('ttp_mitre_enterprise', 'parent_id',
               existing_type=sa.String(length=2621440),
               nullable=True)

[larkee]

The error mentioned in #118 (comment) is an issue of differing syntax. FWICT, the generated query is using the PostGres syntax:

ALTER TABLE table_id ALTER COLUMN column_id DROP NOT NULL

However, the Spanner way of doing this would be:

ALTER TABLE table_id ALTER COLUMN column_id column_type [null_constaint]

e.g. to remove NOT NULL

ALTER TABLE Venue ALTER COLUMN LastUpdateTime TIMESTAMP

e.g. to add NOT NULL

ALTER TABLE Venue ALTER COLUMN LastUpdateTime TIMESTAMP NOT NULL

@IlyaFaer Would you be able to look into adding this support?

[IlyaFaer]

@larkee, yeah, I'll take a look, of course. Though it seems like a second issue.

[larkee]

@larkee, yeah, I'll take a look, of course. Though it seems like a second issue.

Good point. I'll create another issue 👍