diff options
author | Mike Bayer <mike_mp@zzzcomputing.com> | 2014-09-01 20:19:54 -0400 |
---|---|---|
committer | Mike Bayer <mike_mp@zzzcomputing.com> | 2014-09-01 20:19:54 -0400 |
commit | 7c6a45c480a865ac9580eb33fcca2dae5b19dd11 (patch) | |
tree | 870c078707cde0af769a940b1fc1a15ce7966691 /lib/sqlalchemy | |
parent | 382f82538b5484b1c384c71fbf84438312cbe34f (diff) | |
download | sqlalchemy-7c6a45c480a865ac9580eb33fcca2dae5b19dd11.tar.gz |
- 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
Diffstat (limited to 'lib/sqlalchemy')
-rw-r--r-- | lib/sqlalchemy/__init__.py | 2 | ||||
-rw-r--r-- | lib/sqlalchemy/orm/mapper.py | 3 | ||||
-rw-r--r-- | lib/sqlalchemy/orm/query.py | 12 | ||||
-rw-r--r-- | lib/sqlalchemy/sql/compiler.py | 28 | ||||
-rw-r--r-- | lib/sqlalchemy/sql/elements.py | 110 | ||||
-rw-r--r-- | lib/sqlalchemy/sql/expression.py | 3 | ||||
-rw-r--r-- | lib/sqlalchemy/sql/schema.py | 2 | ||||
-rw-r--r-- | lib/sqlalchemy/sql/selectable.py | 61 | ||||
-rw-r--r-- | lib/sqlalchemy/sql/sqltypes.py | 4 | ||||
-rw-r--r-- | lib/sqlalchemy/testing/__init__.py | 2 | ||||
-rw-r--r-- | lib/sqlalchemy/util/langhelpers.py | 8 |
11 files changed, 180 insertions, 55 deletions
diff --git a/lib/sqlalchemy/__init__.py b/lib/sqlalchemy/__init__.py index 2ab717996..853566172 100644 --- a/lib/sqlalchemy/__init__.py +++ b/lib/sqlalchemy/__init__.py @@ -15,6 +15,7 @@ from .sql import ( case, cast, collate, + column, delete, desc, distinct, @@ -39,6 +40,7 @@ from .sql import ( over, select, subquery, + table, text, true, tuple_, diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index cd735c865..a59a38a5b 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -1456,8 +1456,7 @@ class Mapper(InspectionAttr): "Flushing object %s with " "incompatible polymorphic identity %r; the " "object may not refresh and/or load correctly", - state_str(state), - dict_[polymorphic_key] + (state_str(state), dict_[polymorphic_key]) ) self._set_polymorphic_identity = _set_polymorphic_identity diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py index 090284d9f..ba557ef79 100644 --- a/lib/sqlalchemy/orm/query.py +++ b/lib/sqlalchemy/orm/query.py @@ -218,7 +218,7 @@ class Query(object): def _adapt_col_list(self, cols): return [ self._adapt_clause( - expression._literal_as_text(o), + expression._literal_as_label_reference(o), True, True) for o in cols ] @@ -1282,7 +1282,7 @@ class Query(object): """ for criterion in list(criterion): - criterion = expression._literal_as_text(criterion) + criterion = expression._expression_literal_as_text(criterion) criterion = self._adapt_clause(criterion, True, True) @@ -1381,8 +1381,7 @@ class Query(object): """ - if isinstance(criterion, util.string_types): - criterion = sql.text(criterion) + criterion = expression._expression_literal_as_text(criterion) if criterion is not None and \ not isinstance(criterion, sql.ClauseElement): @@ -2359,8 +2358,7 @@ class Query(object): ORM tutorial """ - if isinstance(statement, util.string_types): - statement = sql.text(statement) + statement = expression._expression_literal_as_text(statement) if not isinstance(statement, (expression.TextClause, @@ -2606,7 +2604,7 @@ class Query(object): # .with_only_columns() after we have a core select() so that # we get just "SELECT 1" without any entities. return sql.exists(self.add_columns('1').with_labels(). - statement.with_only_columns(['1'])) + statement.with_only_columns([1])) def count(self): """Return a count of rows this Query would return. diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py index fac4980b0..e4597dcd8 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -494,6 +494,22 @@ class SQLCompiler(Compiled): def visit_grouping(self, grouping, asfrom=False, **kwargs): return "(" + grouping.element._compiler_dispatch(self, **kwargs) + ")" + def visit_label_reference(self, element, **kwargs): + selectable = self.stack[-1]['selectable'] + try: + col = selectable._inner_column_dict[element.text] + except KeyError: + # treat it like text() + util.warn_limited( + "Can't resolve label reference %r; converting to text()", + util.ellipses_string(element.text)) + return self.process( + elements.TextClause._create_text(element.text) + ) + else: + kwargs['render_label_as_label'] = col + return self.process(col, **kwargs) + def visit_label(self, label, add_to_result_map=None, within_label_clause=False, @@ -761,7 +777,8 @@ class SQLCompiler(Compiled): { 'correlate_froms': entry['correlate_froms'], 'iswrapper': toplevel, - 'asfrom_froms': entry['asfrom_froms'] + 'asfrom_froms': entry['asfrom_froms'], + 'selectable': cs }) keyword = self.compound_keywords.get(cs.keyword) @@ -1480,7 +1497,8 @@ class SQLCompiler(Compiled): new_entry = { 'asfrom_froms': new_correlate_froms, 'iswrapper': iswrapper, - 'correlate_froms': all_correlate_froms + 'correlate_froms': all_correlate_froms, + 'selectable': select, } self.stack.append(new_entry) @@ -1791,7 +1809,8 @@ class SQLCompiler(Compiled): self.stack.append( {'correlate_froms': set([update_stmt.table]), "iswrapper": False, - "asfrom_froms": set([update_stmt.table])}) + "asfrom_froms": set([update_stmt.table]), + "selectable": update_stmt}) self.isupdate = True @@ -2247,7 +2266,8 @@ class SQLCompiler(Compiled): def visit_delete(self, delete_stmt, **kw): self.stack.append({'correlate_froms': set([delete_stmt.table]), "iswrapper": False, - "asfrom_froms": set([delete_stmt.table])}) + "asfrom_froms": set([delete_stmt.table]), + "selectable": delete_stmt}) self.isdelete = True text = "DELETE " diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py index 8cae83169..0ea05fa0e 100644 --- a/lib/sqlalchemy/sql/elements.py +++ b/lib/sqlalchemy/sql/elements.py @@ -19,7 +19,8 @@ from .visitors import Visitable, cloned_traverse, traverse from .annotation import Annotated import itertools from .base import Executable, PARSE_AUTOCOMMIT, Immutable, NO_ARG -from .base import _generative, Generative +from .base import _generative +import numbers import re import operator @@ -624,7 +625,7 @@ class ColumnElement(operators.ColumnOperators, ClauseElement): __visit_name__ = 'column' primary_key = False foreign_keys = [] - _label = None + _label = _columns_clause_label = None _key_label = key = None _alt_names = () @@ -1180,6 +1181,10 @@ class TextClause(Executable, ClauseElement): _hide_froms = [] + # help in those cases where text() is + # interpreted in a column expression situation + key = _label = _columns_clause_label = None + def __init__( self, text, @@ -1694,13 +1699,16 @@ class ClauseList(ClauseElement): self.operator = kwargs.pop('operator', operators.comma_op) self.group = kwargs.pop('group', True) self.group_contents = kwargs.pop('group_contents', True) + text_converter = kwargs.pop( + '_literal_as_text', + _expression_literal_as_text) if self.group_contents: self.clauses = [ - _literal_as_text(clause).self_group(against=self.operator) + text_converter(clause).self_group(against=self.operator) for clause in clauses] else: self.clauses = [ - _literal_as_text(clause) + text_converter(clause) for clause in clauses] def __iter__(self): @@ -1767,7 +1775,7 @@ class BooleanClauseList(ClauseList, ColumnElement): clauses = util.coerce_generator_arg(clauses) for clause in clauses: - clause = _literal_as_text(clause) + clause = _expression_literal_as_text(clause) if isinstance(clause, continue_on): continue @@ -2280,6 +2288,13 @@ class Extract(ColumnElement): return self.expr._from_objects +class _label_reference(ColumnElement): + __visit_name__ = 'label_reference' + + def __init__(self, text): + self.text = text + + class UnaryExpression(ColumnElement): """Define a 'unary' expression. @@ -2343,7 +2358,8 @@ class UnaryExpression(ColumnElement): """ return UnaryExpression( - _literal_as_text(column), modifier=operators.nullsfirst_op) + _literal_as_label_reference(column), + modifier=operators.nullsfirst_op) @classmethod def _create_nullslast(cls, column): @@ -2383,7 +2399,8 @@ class UnaryExpression(ColumnElement): """ return UnaryExpression( - _literal_as_text(column), modifier=operators.nullslast_op) + _literal_as_label_reference(column), + modifier=operators.nullslast_op) @classmethod def _create_desc(cls, column): @@ -2421,7 +2438,7 @@ class UnaryExpression(ColumnElement): """ return UnaryExpression( - _literal_as_text(column), modifier=operators.desc_op) + _literal_as_label_reference(column), modifier=operators.desc_op) @classmethod def _create_asc(cls, column): @@ -2458,7 +2475,7 @@ class UnaryExpression(ColumnElement): """ return UnaryExpression( - _literal_as_text(column), modifier=operators.asc_op) + _literal_as_label_reference(column), modifier=operators.asc_op) @classmethod def _create_distinct(cls, expr): @@ -2742,9 +2759,13 @@ class Over(ColumnElement): """ self.func = func if order_by is not None: - self.order_by = ClauseList(*util.to_list(order_by)) + self.order_by = ClauseList( + *util.to_list(order_by), + _literal_as_text=_literal_as_label_reference) if partition_by is not None: - self.partition_by = ClauseList(*util.to_list(partition_by)) + self.partition_by = ClauseList( + *util.to_list(partition_by), + _literal_as_text=_literal_as_label_reference) @util.memoized_property def type(self): @@ -2804,7 +2825,8 @@ class Label(ColumnElement): self.name = _anonymous_label( '%%(%d %s)s' % (id(self), getattr(element, 'name', 'anon')) ) - self.key = self._label = self._key_label = self.name + self.key = self._label = self._key_label = \ + self._columns_clause_label = self.name self._element = element self._type = type_ self._proxies = [element] @@ -2869,7 +2891,7 @@ class ColumnClause(Immutable, ColumnElement): :class:`.Column` class, is typically invoked using the :func:`.column` function, as in:: - from sqlalchemy.sql import column + from sqlalchemy import column id, name = column("id"), column("name") stmt = select([id, name]).select_from("user") @@ -2909,7 +2931,7 @@ class ColumnClause(Immutable, ColumnElement): :class:`.Column` class. The :func:`.column` function can be invoked with just a name alone, as in:: - from sqlalchemy.sql import column + from sqlalchemy import column id, name = column("id"), column("name") stmt = select([id, name]).select_from("user") @@ -2941,7 +2963,7 @@ class ColumnClause(Immutable, ColumnElement): (which is the lightweight analogue to :class:`.Table`) to produce a working table construct with minimal boilerplate:: - from sqlalchemy.sql import table, column + from sqlalchemy import table, column, select user = table("user", column("id"), @@ -2957,6 +2979,10 @@ class ColumnClause(Immutable, ColumnElement): :class:`.schema.MetaData`, DDL, or events, unlike its :class:`.Table` counterpart. + .. versionchanged:: 1.0.0 :func:`.expression.column` can now + be imported from the plain ``sqlalchemy`` namespace like any + other SQL element. + :param text: the text of the element. :param type: :class:`.types.TypeEngine` object which can associate @@ -3035,6 +3061,13 @@ class ColumnClause(Immutable, ColumnElement): def _label(self): return self._gen_label(self.name) + @_memoized_property + def _columns_clause_label(self): + if self.table is None: + return None + else: + return self._label + def _gen_label(self, name): t = self.table @@ -3438,12 +3471,29 @@ def _clause_element_as_expr(element): return element -def _literal_as_text(element): +def _literal_as_label_reference(element): + if isinstance(element, util.string_types): + return _label_reference(element) + else: + return _literal_as_text(element) + + +def _expression_literal_as_text(element): + return _literal_as_text(element, warn=True) + + +def _literal_as_text(element, warn=False): if isinstance(element, Visitable): return element elif hasattr(element, '__clause_element__'): return element.__clause_element__() elif isinstance(element, util.string_types): + if warn: + util.warn_limited( + "Textual SQL expression %(expr)r should be " + "explicitly declared as text(%(expr)r)", + {"expr": util.ellipses_string(element)}) + return TextClause(util.text_type(element)) elif isinstance(element, (util.NoneType, bool)): return _const_expr(element) @@ -3498,6 +3548,8 @@ def _literal_as_binds(element, name=None, type_=None): else: return element +_guess_straight_column = re.compile(r'^\w\S*$', re.I) + def _interpret_as_column_or_from(element): if isinstance(element, Visitable): @@ -3512,7 +3564,31 @@ def _interpret_as_column_or_from(element): elif hasattr(insp, "selectable"): return insp.selectable - return ColumnClause(str(element), is_literal=True) + # be forgiving as this is an extremely common + # and known expression + if element == "*": + guess_is_literal = True + elif isinstance(element, (numbers.Number)): + return ColumnClause(str(element), is_literal=True) + else: + element = str(element) + # give into temptation, as this fact we are guessing about + # is not one we've previously ever needed our users tell us; + # but let them know we are not happy about it + guess_is_literal = not _guess_straight_column.match(element) + util.warn_limited( + "Textual column expression %(column)r should be " + "explicitly declared with text(%(column)r), " + "or use %(literal_column)s(%(column)r) " + "for more specificity", + { + "column": util.ellipses_string(element), + "literal_column": "literal_column" + if guess_is_literal else "column" + }) + return ColumnClause( + element, + is_literal=guess_is_literal) def _const_expr(element): diff --git a/lib/sqlalchemy/sql/expression.py b/lib/sqlalchemy/sql/expression.py index fd57f9be8..d96f048b9 100644 --- a/lib/sqlalchemy/sql/expression.py +++ b/lib/sqlalchemy/sql/expression.py @@ -106,7 +106,8 @@ from .elements import _literal_as_text, _clause_element_as_expr,\ _is_column, _labeled, _only_column_elements, _string_or_unprintable, \ _truncated_label, _clone, _cloned_difference, _cloned_intersection,\ _column_as_key, _literal_as_binds, _select_iterables, \ - _corresponding_column_or_error + _corresponding_column_or_error, _literal_as_label_reference, \ + _expression_literal_as_text from .selectable import _interpret_as_from diff --git a/lib/sqlalchemy/sql/schema.py b/lib/sqlalchemy/sql/schema.py index 8225a3533..d9fd37f92 100644 --- a/lib/sqlalchemy/sql/schema.py +++ b/lib/sqlalchemy/sql/schema.py @@ -2433,7 +2433,7 @@ class CheckConstraint(Constraint): super(CheckConstraint, self).\ __init__(name, deferrable, initially, _create_rule, info=info) - self.sqltext = _literal_as_text(sqltext) + self.sqltext = _literal_as_text(sqltext, warn=False) if table is not None: self._set_parent_with_dispatch(table) elif _autoattach: diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py index 4808a3935..cf2c213d2 100644 --- a/lib/sqlalchemy/sql/selectable.py +++ b/lib/sqlalchemy/sql/selectable.py @@ -15,8 +15,8 @@ from .elements import ClauseElement, TextClause, ClauseList, \ from .elements import _clone, \ _literal_as_text, _interpret_as_column_or_from, _expand_cloned,\ _select_iterables, _anonymous_label, _clause_element_as_expr,\ - _cloned_intersection, _cloned_difference, True_, _only_column_elements,\ - TRUE + _cloned_intersection, _cloned_difference, True_, \ + _literal_as_label_reference from .base import Immutable, Executable, _generative, \ ColumnCollection, ColumnSet, _from_objects, Generative from . import type_api @@ -36,6 +36,12 @@ def _interpret_as_from(element): insp = inspection.inspect(element, raiseerr=False) if insp is None: if isinstance(element, util.string_types): + util.warn_limited( + "Textual SQL FROM expression %(expr)r should be " + "explicitly declared as text(%(expr)r), " + "or use table(%(expr)r) for more specificity", + {"expr": util.ellipses_string(element)}) + return TextClause(util.text_type(element)) elif hasattr(insp, "selectable"): return insp.selectable @@ -1177,7 +1183,7 @@ class TableClause(Immutable, FromClause): collection of columns, which are typically produced by the :func:`.expression.column` function:: - from sqlalchemy.sql import table, column + from sqlalchemy import table, column user = table("user", column("id"), @@ -1218,11 +1224,9 @@ class TableClause(Immutable, FromClause): :class:`~.schema.Table` object. It may be used to construct lightweight table constructs. - Note that the :func:`.expression.table` function is not part of - the ``sqlalchemy`` namespace. It must be imported from the - ``sql`` package:: - - from sqlalchemy.sql import table, column + .. versionchanged:: 1.0.0 :func:`.expression.table` can now + be imported from the plain ``sqlalchemy`` namespace like any + other SQL element. :param name: Name of the table. @@ -1626,9 +1630,13 @@ class GenerativeSelect(SelectBase): self._bind = bind if order_by is not None: - self._order_by_clause = ClauseList(*util.to_list(order_by)) + self._order_by_clause = ClauseList( + *util.to_list(order_by), + _literal_as_text=_literal_as_label_reference) if group_by is not None: - self._group_by_clause = ClauseList(*util.to_list(group_by)) + self._group_by_clause = ClauseList( + *util.to_list(group_by), + _literal_as_text=_literal_as_label_reference) @property def for_update(self): @@ -1784,7 +1792,8 @@ class GenerativeSelect(SelectBase): else: if getattr(self, '_order_by_clause', None) is not None: clauses = list(self._order_by_clause) + list(clauses) - self._order_by_clause = ClauseList(*clauses) + self._order_by_clause = ClauseList( + *clauses, _literal_as_text=_literal_as_label_reference) def append_group_by(self, *clauses): """Append the given GROUP BY criterion applied to this selectable. @@ -1801,7 +1810,12 @@ class GenerativeSelect(SelectBase): else: if getattr(self, '_group_by_clause', None) is not None: clauses = list(self._group_by_clause) + list(clauses) - self._group_by_clause = ClauseList(*clauses) + self._group_by_clause = ClauseList( + *clauses, _literal_as_text=_literal_as_label_reference) + + @property + def _inner_column_dict(self): + raise NotImplementedError() def _copy_internals(self, clone=_clone, **kw): if self._limit_clause is not None: @@ -1869,6 +1883,12 @@ class CompoundSelect(GenerativeSelect): GenerativeSelect.__init__(self, **kwargs) + @property + def _inner_column_dict(self): + return dict( + (c.key, c) for c in self.c + ) + @classmethod def _create_union(cls, *selects, **kwargs): """Return a ``UNION`` of multiple selectables. @@ -2092,7 +2112,7 @@ class HasPrefixes(object): def _setup_prefixes(self, prefixes, dialect=None): self._prefixes = self._prefixes + tuple( - [(_literal_as_text(p), dialect) for p in prefixes]) + [(_literal_as_text(p, warn=False), dialect) for p in prefixes]) class Select(HasPrefixes, GenerativeSelect): @@ -2477,6 +2497,15 @@ class Select(HasPrefixes, GenerativeSelect): """ return _select_iterables(self._raw_columns) + @_memoized_property + def _inner_column_dict(self): + d = dict( + (c._label or c.key, c) + for c in _select_iterables(self._raw_columns)) + d.update((c.key, c) for c in _select_iterables(self.froms)) + + return d + def is_derived_from(self, fromclause): if self in fromclause._cloned_set: return True @@ -2706,7 +2735,7 @@ class Select(HasPrefixes, GenerativeSelect): """ if expr: - expr = [_literal_as_text(e) for e in expr] + expr = [_literal_as_label_reference(e) for e in expr] if isinstance(self._distinct, list): self._distinct = self._distinct + expr else: @@ -2945,9 +2974,9 @@ class Select(HasPrefixes, GenerativeSelect): names = set() def name_for_col(c): - if c._label is None: + if c._columns_clause_label is None: return (None, c) - name = c._label + name = c._columns_clause_label if name in names: name = c.anon_label else: diff --git a/lib/sqlalchemy/sql/sqltypes.py b/lib/sqlalchemy/sql/sqltypes.py index 4de4885c6..2729bc83e 100644 --- a/lib/sqlalchemy/sql/sqltypes.py +++ b/lib/sqlalchemy/sql/sqltypes.py @@ -183,7 +183,7 @@ class String(Concatenable, TypeEngine): util.warn_limited( "Unicode type received non-unicode " "bind param value %r.", - util.ellipses_string(value)) + (util.ellipses_string(value),)) return value return process else: @@ -199,7 +199,7 @@ class String(Concatenable, TypeEngine): util.warn_limited( "Unicode type received non-unicode bind " "param value %r.", - util.ellipses_string(value)) + (util.ellipses_string(value),)) return value return process else: diff --git a/lib/sqlalchemy/testing/__init__.py b/lib/sqlalchemy/testing/__init__.py index e53fb28b1..1f37b4b45 100644 --- a/lib/sqlalchemy/testing/__init__.py +++ b/lib/sqlalchemy/testing/__init__.py @@ -21,7 +21,7 @@ def against(*queries): from .assertions import emits_warning, emits_warning_on, uses_deprecated, \ eq_, ne_, is_, is_not_, startswith_, assert_raises, \ assert_raises_message, AssertsCompiledSQL, ComparesTables, \ - AssertsExecutionResults, expect_deprecated + AssertsExecutionResults, expect_deprecated, expect_warnings from .util import run_as_contextmanager, rowset, fail, provide_metadata, adict diff --git a/lib/sqlalchemy/util/langhelpers.py b/lib/sqlalchemy/util/langhelpers.py index c260e0931..76f85f605 100644 --- a/lib/sqlalchemy/util/langhelpers.py +++ b/lib/sqlalchemy/util/langhelpers.py @@ -1206,8 +1206,8 @@ class _hash_limit_string(compat.text_type): """ - def __new__(cls, value, args, num): - interpolated = value % args + \ + def __new__(cls, value, num, args): + interpolated = (value % args) + \ (" (this warning may be suppressed after %d occurrences)" % num) self = super(_hash_limit_string, cls).__new__(cls, interpolated) self._hash = hash("%s_%d" % (value, hash(interpolated) % num)) @@ -1230,13 +1230,13 @@ def warn(msg): warnings.warn(msg, exc.SAWarning, stacklevel=2) -def warn_limited(msg, *args): +def warn_limited(msg, args): """Issue a warning with a paramterized string, limiting the number of registrations. """ if args: - msg = _hash_limit_string(msg, args, 10) + msg = _hash_limit_string(msg, 10, args) warnings.warn(msg, exc.SAWarning, stacklevel=2) |