check does not work with ExecuteSQLOp present

[BayerSe]

Hi,

we're using ExecuteSQLOp to add triggers to some columns.

With these added, alembic check nowadays raises a not implemented error, it seems as if because ExecuteSQLOp does not implement .to_diff_tuple().

op = ops.ExecuteSQLOp(sqltext='CREATE OR REPLACE TRIGGER ...')
op.to_diff_tuple()  # NotImplementedError

This used to work in version 1.6.5 and I think this error was introduced in 1.7.0 when .to_diff_tuple() was added to the base class MigrateOperation, see here.

Here's the stacktrace of alembic check:

INFO  [alembic.runtime.migration] Context impl OracleImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.autogenerate.compare] Detected added table 'his_test'
Traceback (most recent call last):
  File "/home/user/PycharmProjects/project/.venv/bin/alembic", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/config.py", line 630, in main
    CommandLine(prog=prog).main(argv=argv)
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/config.py", line 624, in main
    self.run_cmd(cfg, options)
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/config.py", line 601, in run_cmd
    fn(
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/command.py", line 294, in check
    diffs = migration_script.upgrade_ops.as_diffs()
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/operations/ops.py", line 2556, in as_diffs
    return list(OpContainer._ops_as_diffs(self))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/operations/ops.py", line 2566, in _ops_as_diffs
    yield op.to_diff_tuple()
          ^^^^^^^^^^^^^^^^^^
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/operations/ops.py", line 88, in to_diff_tuple
    raise NotImplementedError
NotImplementedError

I have no idea how this worked in 1.6.5 as already there, ExecuteSQLOp and some other ops did not have .to_diff_tuple(), but with .to_diff_tuple() in the base class check does not work as of today.

Thanks
Sebastian

[zzzeek]

so...you are using the process_revision_directives to add in ExecuteSQLOp to the model-side revision stream and... if that's included within the alembic check stream then...I guess you don't care what's in the tuple, I would assume it's only there if something else was already in the diff stream, is that accurate ?

[zzzeek]

yes. see it does not normally make sense that ExecuteSQLOp would be in a diff stream unless as you are doing you have a hook adding it in. the "diff" then wants to show "what changed?" so here it would just be some code like "exec" and then a SQL statement. ill try to look at 1.6.5 to see what this would have done then.

[zzzeek]

well how were you doingalembic check in 1.6.5 ?

[BayerSe]

Oh! You're completely right, I overlooked that check only came later. I was not responsible for this part of the code, so my knowledge on that is a bit fuzzy.

Basically I just realized that a colleague wrote a script that checked whether --autogenerate generated a file and if that is the case, we raised. Somewhen later, this custom script was replaced with check and until now, no one realized that this broke something.

I guess it'd be easiest if I go back to the old script since what we're doing above seems to be an edge case.

[zzzeek]

we should add a method to ExecuteSQLOp to return something here, that's fine

[BayerSe]

That would be great, thanks!

[zzzeek]

interested in...a pull request ? :)

[BayerSe]

Possibly - let me come back to you.

[BayerSe]

I gave it a shot: #1336