fix default argument of None for DefaultDialect.reflecttable->exclude_columns, should be ()

[Former user]

This code in reflection.py:reflecttable():

    if pk_cons:
        pk_cols = [table.c[pk](table.c[pk) 
                    for pk in pk_cons['constrained_columns']('constrained_columns') 
                    if pk in table.c and pk not in exclude_columns
                ] + [for pk in table.primary_key if pk.key in exclude_columns](pk)

Should check if exclude_columns is None. All the previous references to exclude_columns do, and it turns out it is necessary:

>>> conn.engine.reflecttable(est)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<string>", line 1, in <lambda>
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/util/deprecations.py", line 100, in warned
    return fn(*args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/base.py", line 2533, in reflecttable
    self.dialect.reflecttable(conn, table, include_columns)
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/default.py", line 260, in reflecttable
    return insp.reflecttable(table, include_columns, exclude_columns)
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/reflection.py", line 419, in reflecttable
    if pk in table.c and pk not in exclude_columns
TypeError: argument of type 'NoneType' is not iterable
>>>

This was in version 0.7.8, but the same problem exists in github master.

[Mike Bayer]

exclude_columns has not been considered public API, even though it seems to have found its way to reflection.reflecttable, it's not present in the usual Table() API. Are you using it directly or is there some codepath that it becomes None? (I'm not seeing one). It's assumed to always be non-None right now (the boolean checks are for emptiness).

[Former user]

Are you using it directly

No. This is all I have to do to reproduce the bug:

$ python
Python 2.7.3 (default, Jan  2 2013, 13:56:14) 
[4.7.2](GCC) on linux2
Type "help", "copyright", "credits" or "license" for more information.
Debug helper loaded.  Type "print D.__doc__" for help.
>>> import sqlalchemy
>>> conn = sqlalchemy.create_engine("firebird://ro:ro@10.7.0.3/lubenet/mobile.gdb").connect()>>> metatable = sqlalchemy.MetaData(conn)
>>> est = sqlalchemy.Table("estline", metatable)
>>> conn.engine.reflecttable(est)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<string>", line 1, in <lambda>
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/util/deprecations.py", line 100, in warned
    return fn(*args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/base.py", line 2533, in reflecttable
    self.dialect.reflecttable(conn, table, include_columns)
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/default.py", line 260, in reflecttable
    return insp.reflecttable(table, include_columns, exclude_columns)
  File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/reflection.py", line 419, in reflecttable
    if pk in table.c and pk not in exclude_columns
TypeError: argument of type 'NoneType' is not iterable
>>> sqlalchemy.__version__
'0.7.8'
>>>

or is there some codepath that it becomes None?

Yes, there is. It is the one you see in the backtrace. I'll spell it out for you, using the line numbers from rel_0_7 in github:

• I call sqlalchemy/engine/base.py:2516 Engine.reflecttable()
• On line 2532 it calls sqlalchemy/engine/default.py:258 DefaultDialect.reflecttable(..., exclude_columns=None)
• On line 260 it calls sqlalchemy/engine/reflection.py:258 Inspector.reflecttable(), passing None for exclude_columns.
• On line 419, it does not modify pk not in exclude_columns, and execute pk not in exclude_columns.
• Thus TypeError is raised.

[Mike Bayer]

OK engine.reflecttable() was removed in 0.8 and as you can see in 0.7 it's marked deprecated. So while the signature of DefaultDialect.reflecttable should be fixed, I'm not seeing any current public API issues. As for 0.7 I can check in a fix but I don't know that more 0.7 releases are planned at this time.

[Mike Bayer]

5f0b864ad02409cf668fa7db in 0.9 makes both include/exclude arguments mandatory, the Dialect.reflecttable method is not even needed at this point, though I'm leaving it in as an alternative for 3rd party dialects in rare circumstances.

[Mike Bayer]

Removing milestone: 0.9.0 (automated comment)