Clarify vague error from within DDLCompiler.visit_create_table

[Former user]

Currently, if I do something like declare a schema with a sqlalchemy.Unicode column type with no length, and try to use it with MySQL, I get an error like so:

sqlalchemy.exc.InvalidRequestError: VARCHAR requires a length on dialect mysql

Unfortunately, the trace looks like this:

justin@dev-ops:~/py27sqla/examples/ptah_simpleauth$ ../../bin/pserve settings.ini 
Traceback (most recent call last):
  File "../../bin/pserve", line 8, in <module>
    load_entry_point('pyramid==1.3a3', 'console_scripts', 'pserve')()
  File "/home/justin/py27sqla/lib/python2.7/site-packages/pyramid-1.3a3-py2.7.egg/pyramid/scripts/pserve.py", line 32, in main
    return command.run()
  File "/home/justin/py27sqla/lib/python2.7/site-packages/pyramid-1.3a3-py2.7.egg/pyramid/scripts/pserve.py", line 275, in run
    relative_to=base, global_conf=vars)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/pyramid-1.3a3-py2.7.egg/pyramid/scripts/pserve.py", line 303, in loadapp
    return loadapp(app_spec, name=name, relative_to=relative_to, **kw)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/PasteDeploy-1.5.0-py2.7.egg/paste/deploy/loadwsgi.py", line 247, in loadapp
    return loadobj(APP, uri, name=name, **kw)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/PasteDeploy-1.5.0-py2.7.egg/paste/deploy/loadwsgi.py", line 272, in loadobj
    return context.create()
  File "/home/justin/py27sqla/lib/python2.7/site-packages/PasteDeploy-1.5.0-py2.7.egg/paste/deploy/loadwsgi.py", line 710, in create
    return self.object_type.invoke(self)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/PasteDeploy-1.5.0-py2.7.egg/paste/deploy/loadwsgi.py", line 146, in invoke
    return fix_call(context.object, context.global_conf, **context.local_conf)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/PasteDeploy-1.5.0-py2.7.egg/paste/deploy/util.py", line 56, in fix_call
    val = callable(*args, **kw)
  File "/home/justin/py27sqla/examples/ptah_simpleauth/ptah_simpleauth/app.py", line 47, in main
    Base.metadata.create_all()
  File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/schema.py", line 2515, in create_all
    tables=tables)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 2234, in _run_visitor
    conn._run_visitor(visitorcallable, element, **kwargs)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1904, in _run_visitor
    **kwargs).traverse_single(element)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/sql/visitors.py", line 86, in traverse_single
    return meth(obj, **kw)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/engine/ddl.py", line 67, in visit_metadata
    self.traverse_single(table, create_ok=True)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/sql/visitors.py", line 86, in traverse_single
    return meth(obj, **kw)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/engine/ddl.py", line 86, in visit_table
    self.connection.execute(schema.CreateTable(table))
  File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1405, in execute
    params)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1490, in _execute_ddl
    compiled = ddl.compile(dialect=dialect)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/sql/expression.py", line 1722, in compile
    return self._compiler(dialect, bind=bind, **kw)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/schema.py", line 2852, in _compiler
    return dialect.ddl_compiler(dialect, self, **kw)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 699, in __init__
    self.string = self.process(self.statement)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 718, in process
    return obj._compiler_dispatch(self, **kwargs)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/sql/visitors.py", line 59, in _compiler_dispatch
    return getter(visitor)(self, **kw)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/sql/compiler.py", line 1377, in visit_create_table
    not first_pk
  File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/dialects/mysql/base.py", line 1367, in get_column_specification
    self.dialect.type_compiler.process(column.type)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 764, in process
    return type_._compiler_dispatch(self)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/sql/visitors.py", line 59, in _compiler_dispatch
    return getter(visitor)(self, **kw)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/sql/compiler.py", line 1702, in visit_unicode
    return self.visit_VARCHAR(type_)
  File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/dialects/mysql/base.py", line 1654, in visit_VARCHAR
    self.dialect.name)
sqlalchemy.exc.InvalidRequestError: VARCHAR requires a length on dialect mysql

With the attached patch, it looks more like:

sqlalchemy.exc.CompileError: In Table ptah_nodes, Column roles: VARCHAR requires a length on dialect mysql

This allows one to address this error without stab-in-the-dark guess-and-try cycles which are pretty miserable, and should help in resolving:

https://github.com/ptahproject/ptah/issues/116

[Former user]

patch to emit clearer, more helpful error message

[Mike Bayer]

Patches like these often give me a bad taste, since they are trying to take one very specific case where there's an unideal stack trace and add particular contextual information - but there's many other places where this same trace (such as in compiling a cast()), as well as other similar ones (any number of conditions that fail on the database side, for example), can be raised, where this information is not available. Getting it in one place is not really a substitute for the developer being able to use pdb.post_mortem(), climb up the stack, and see the source of an error.

The exceptions raised directly within MySQLTypeCompiler should probably also be CompileError instances. It feels a little more appropriate for CompilerError to be caught and re-raised with more information rather than totally changing the type of exception being emitted. That way the originator of the exception can make the choice that this exception is appropriate for being "contextualized" and I'd feel like this patch is not quite so arbitrary.

Another decision to be made is the mechanics of the re-raise. This is the standard approach to a re-raise is:

except SomeException, e:
    # Py3K
    #raise NewException("message:" + str(e)) from e
    # Py2K
    raise NewException("message:" + str(e)), None, sys.exc_info()[2](2)
    # end Py2K

that at least preserves the original stack trace and in the case of Py3K provides both stack traces.

[Former user]

Fair enough, I didn't consider it a perfect patch, but I feel like there is some value to clarifying the case where "a schema declared somewhere in my code causes this traceback which is very mysterious".

I hope that we can at least agree the existing stack trace is highly frustrating and can only be solved with guesswork. It took a pretty good amount of time just to figure out where to put this patch.

It also happens to be one of the few places where a table and column name are particularly handy.

Perfect or not, I think it's tough to make an argument that sqlalchemy is better without these three lines of code.

[Mike Bayer]

will stick this in 0.7.5 for now keeping in mind it might get pushed out. An updated patch + a unit test in test/sql/test_compiler.py would expedite, though I can knock these out very quickly when I get to them.

[Former user]

I'm happy to write a test, if I can improve the patch, I'll do so - happy to work together on a patch that is more general, it took me a while to sort out this one.

[Mike Bayer]

sure, it involves emitting CompilerException from within the MySQL dialect, and it would also be nice to see if other dialects are doing something similar, then the code block in question would re-raise CompilerException only using the "reraise" format I have in my above comment. testing for exceptions uses the assert_raises_message() testing helper, most unit test modules should have a few examples of it.

[Mike Bayer]

09553dc90f4a95b314994b48068b046de1413104 contains the change to CompileException for all dialect-specific compilers, as well as the re-raise. I'm a tad concerned that changing the exception type in some of these places would not be backwards compatible for an app that is specifically catching InvalidRequestError, though this would be very unusual and I'd rather not wait til 0.8 for this change.

[Mike Bayer]

Removing milestone: 0.7.5 (automated comment)