deprecate direct Text() conversion in _literal_as_text?

[malthe]

The filter() method accepts string arguments and uses them as raw SQL. This is unexpected, because everywhere else, only strings wrapped in text() are used as-is.

I consider this a bug, because it is a very questionable feature to allow raw SQL implicitly.

[Mike Bayer]

strings are accepted by most query/select methods, and in those cases are coerced to text() implicitly. The case where strings are converted to bound parameters is when they are used in expressions. This behavior should be pretty consistent across the board, and it is widely documented:

http://docs.sqlalchemy.org/en/rel_0_9/core/tutorial.html#using-text

http://docs.sqlalchemy.org/en/rel_0_9/orm/tutorial.html#using-literal-sql

this behavior has been part of SQLAlchemy for its entire lifespan.

raw SQL is something that should be handled carefully but libraries certainly accept it, our users would be pretty upset if connection.execute() for example no longer accepted a string.

[Mike Bayer]

whether or not this feature is desirable there's really no option to change this behavior at this point. specific examples of where a bound value would be preferable over text() should be separate issues.

[Mike Bayer]

let's examine ways to deprecate the implicit text() for 1.0

[Mike Bayer]

asking around to see what the appetite would be for the implicit string->text() behavior to be deprecated.

This change would really mess people up, not as much on filter()/where(), but on order_by(). it applies here too:

s = stmt.order_by("value")

it should be easy to control at least I'm pretty sure _literal_as_text() is the only point where this translation occurs.

[malthe]

I think perhaps a valid identifier might be okay, e.g. a column name. That is, something that can be resolved to a column.

[Mike Bayer]

we don't have the feature that order_by() etc. actually attempt to resolve the given string to a column label known to be in the statement. If you are selecting from table.c.foo, if you want to order by that, you say order_by(table.c.foo). The practice of doing order_by("foo") is problematic also when the ORM does query rewriting in any case and I try to discourage it for that reason.

[Mike Bayer]

let's see if we can actually do a name resolution on order by and group by. this would allow column translations to work on those too. the approach here moves the text() warning to compile time, as that's the most efficient / guaranteed to work time when we can resolve order_by/group_by names.

lots of tests needed including for the warnings, for the order_by/group_by lookups, to see if translations really work, etc. probably can go in test_compiler but should be its own suite.

[Mike Bayer]

first branch rev is at 5b0ac2748004ad091f9067.

here's scenarios we need turned into tests:

from sqlalchemy.sql import table, column, select, text, func, desc, asc


t1 = table('t1', column('a'), column('b'))

s1 = select([t1.c.b]).order_by(asc('a'), 'b')
print s1

s2 = select([t1]).alias()
print select([s2]).apply_labels().group_by('a')

s3 = select([func.foo('bar').label('fb'), t1]).order_by(desc('fb'))
print s3

s4 = select([t1]).distinct('b')
from sqlalchemy.dialects import postgresql
print s4.compile(dialect=postgresql.dialect())

from sqlalchemy import *
from sqlalchemy.orm import *
from sqlalchemy.ext.declarative import declarative_base

Base = declarative_base()

class A(Base):
    __tablename__ = 'a'

    id = Column(Integer, primary_key=True)
    bs = relationship("B")

class B(Base):
    __tablename__ = 'b'

    id = Column(Integer, primary_key=True)
    a_id = Column(Integer, ForeignKey('a.id'))

sess = Session(e)

print sess.query(A).join(B).order_by("a_id")
print sess.query(A, B, func.x().label('g')).join(B).order_by("a_id", "b_id", "g")

[malthe]

Nice!

Ideally, the warning would be "pluggable" (to allow a custom behavior that simply raises an exception), but it's definitely much better than the previous behavior.

[Mike Bayer]

that's implicit in warnings as the warnings filter can promote them to exceptions.

the issue with this warning is that it would be really helpful to put the text expression detected in the warning. However, that means the Python warnings filter will cache that unique string permanently, so an app that puts arbitrary text will eventually run out of memory.

[Mike Bayer]

going to fix that with #3178

[Mike Bayer]

• The :func:~.expression.column and :func:~.expression.table constructs are now importable from the "from sqlalchemy" namespace, just like every other Core construct.
• The implicit conversion of strings to :func:.text constructs when passed to most builder methods of :func:.select as well as :class:.Query now emits a warning with just the plain string sent. The textual conversion still proceeds normally, however. The only method that accepts a string without a warning are the "label reference" methods like order_by(), group_by(); these functions will now at compile time attempt to resolve a single string argument to a column or label expression present in the selectable; if none is located, the expression still renders, but you get the warning again. The rationale here is that the implicit conversion from string to text is more unexpected than not these days, and it is better that the user send more direction to the Core / ORM when passing a raw string as to what direction should be taken. Core/ORM tutorials have been updated to go more in depth as to how text is handled. fixes #2992

→ <<cset 7c6a45c480a8>>

[Mike Bayer]

that was a big damn deal