From: Mike Bayer Date: Wed, 25 Sep 2019 21:42:51 +0000 (-0400) Subject: Unify generation between Core and ORM query X-Git-Tag: rel_1_4_0b1~716 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cb9215504c0131facc8ed1b22746d3dc53e628b9;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Unify generation between Core and ORM query generation is to be enhanced to include caching functionality, so ensure that Query and all generative in Core (e.g. select, DML etc) are using the same generations system. Additionally, deprecate Select.append methods and state Select methods independently of their append versions. Mutability of expression objects is a special case only when generating new objects during a visit. Fixes: #4637 Change-Id: I3dfac00d5e0f710c833b236f7a0913e1ca24dde4 --- diff --git a/lib/sqlalchemy/dialects/mssql/base.py b/lib/sqlalchemy/dialects/mssql/base.py index 4dd8e72616..54f0043c4b 100644 --- a/lib/sqlalchemy/dialects/mssql/base.py +++ b/lib/sqlalchemy/dialects/mssql/base.py @@ -1688,13 +1688,13 @@ class MSSQLCompiler(compiler.SQLCompiler): [c for c in select.c if c.key != "mssql_rn"] ) if offset_clause is not None: - limitselect.append_whereclause(mssql_rn > offset_clause) + limitselect = limitselect.where(mssql_rn > offset_clause) if limit_clause is not None: - limitselect.append_whereclause( + limitselect = limitselect.where( mssql_rn <= (limit_clause + offset_clause) ) else: - limitselect.append_whereclause(mssql_rn <= (limit_clause)) + limitselect = limitselect.where(mssql_rn <= (limit_clause)) return self.process(limitselect, **kwargs) else: return compiler.SQLCompiler.visit_select(self, select, **kwargs) diff --git a/lib/sqlalchemy/orm/base.py b/lib/sqlalchemy/orm/base.py index 4d308d26b3..e52b6d8bb5 100644 --- a/lib/sqlalchemy/orm/base.py +++ b/lib/sqlalchemy/orm/base.py @@ -207,12 +207,10 @@ _SET_DEFERRED_EXPIRED = util.symbol("SET_DEFERRED_EXPIRED") _DEFER_FOR_STATE = util.symbol("DEFER_FOR_STATE") -def _generative(*assertions): - """Mark a method as generative, e.g. method-chained.""" - +def _assertions(*assertions): @util.decorator def generate(fn, *args, **kw): - self = args[0]._clone() + self = args[0] for assertion in assertions: assertion(self, fn.__name__) fn(self, *args[1:], **kw) diff --git a/lib/sqlalchemy/orm/dynamic.py b/lib/sqlalchemy/orm/dynamic.py index 3218b9fa3b..221f982a8c 100644 --- a/lib/sqlalchemy/orm/dynamic.py +++ b/lib/sqlalchemy/orm/dynamic.py @@ -310,7 +310,7 @@ class AppenderMixin(object): ).added_items ) else: - return iter(self._clone(sess)) + return iter(self._generate(sess)) def __getitem__(self, index): sess = self.session @@ -320,7 +320,7 @@ class AppenderMixin(object): attributes.PASSIVE_NO_INITIALIZE, ).indexed(index) else: - return self._clone(sess).__getitem__(index) + return self._generate(sess).__getitem__(index) def count(self): sess = self.session @@ -332,9 +332,9 @@ class AppenderMixin(object): ).added_items ) else: - return self._clone(sess).count() + return self._generate(sess).count() - def _clone(self, sess=None): + def _generate(self, sess=None): # note we're returning an entirely new Query class instance # here without any assignment capabilities; the class of this # query is determined by the session. diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py index 6d8037bae5..a30cfa7e47 100644 --- a/lib/sqlalchemy/orm/query.py +++ b/lib/sqlalchemy/orm/query.py @@ -27,8 +27,8 @@ from . import interfaces from . import loading from . import persistence from . import properties +from .base import _assertions from .base import _entity_descriptor -from .base import _generative from .base import _is_aliased_class from .base import _is_mapped_class from .base import _orm_columns @@ -52,7 +52,9 @@ from ..sql import expression from ..sql import roles from ..sql import util as sql_util from ..sql import visitors +from ..sql.base import _generative from ..sql.base import ColumnCollection +from ..sql.base import Generative from ..sql.selectable import ForUpdateArg @@ -64,7 +66,7 @@ _path_registry = PathRegistry.root @inspection._self_inspects @log.class_logger -class Query(object): +class Query(Generative): """ORM-level SQL construction object. :class:`.Query` is the source of all SELECT statements generated by the @@ -312,11 +314,11 @@ class Query(object): for o in cols ] - @_generative() + @_generative def _set_lazyload_from(self, state): self.lazy_loaded_from = state - @_generative() + @_generative def _adapt_all_clauses(self): self._orm_only_adapt = False @@ -526,10 +528,7 @@ class Query(object): return self def _clone(self): - cls = self.__class__ - q = cls.__new__(cls) - q.__dict__ = self.__dict__.copy() - return q + return self._generate() @property def statement(self): @@ -678,7 +677,7 @@ class Query(object): def __clause_element__(self): return self.enable_eagerloads(False).with_labels().statement - @_generative() + @_generative def only_return_tuples(self, value): """When set to True, the query results will always be a tuple, specifically for single element queries. The default is False. @@ -688,7 +687,7 @@ class Query(object): """ self._only_return_tuples = value - @_generative() + @_generative def enable_eagerloads(self, value): """Control whether or not eager joins and subqueries are rendered. @@ -715,7 +714,7 @@ class Query(object): "proceed with query.yield_per()." % message ) - @_generative() + @_generative def with_labels(self): """Apply column labels to the return value of Query.statement. @@ -741,7 +740,7 @@ class Query(object): """ self._with_labels = True - @_generative() + @_generative def enable_assertions(self, value): """Control whether assertions are generated. @@ -774,7 +773,7 @@ class Query(object): """ return self._criterion - @_generative() + @_generative def _with_current_path(self, path): """indicate that this query applies to objects loaded within a certain path. @@ -786,7 +785,8 @@ class Query(object): """ self._current_path = path - @_generative(_no_clauseelement_condition) + @_generative + @_assertions(_no_clauseelement_condition) def with_polymorphic( self, cls_or_mappers, selectable=None, polymorphic_on=None ): @@ -820,7 +820,7 @@ class Query(object): polymorphic_on=polymorphic_on, ) - @_generative() + @_generative def yield_per(self, count): r"""Yield only ``count`` rows at a time. @@ -1085,7 +1085,7 @@ class Query(object): return db_load_fn(self, primary_key_identity) - @_generative() + @_generative def correlate(self, *args): """Return a :class:`.Query` construct which will correlate the given FROM clauses to that of an enclosing :class:`.Query` or @@ -1116,7 +1116,7 @@ class Query(object): ) ) - @_generative() + @_generative def autoflush(self, setting): """Return a Query with a specific 'autoflush' setting. @@ -1128,7 +1128,7 @@ class Query(object): """ self._autoflush = setting - @_generative() + @_generative def populate_existing(self): """Return a :class:`.Query` that will expire and refresh all instances as they are loaded, or reused from the current :class:`.Session`. @@ -1142,7 +1142,7 @@ class Query(object): """ self._populate_existing = True - @_generative() + @_generative def _with_invoke_all_eagers(self, value): """Set the 'invoke all eagers' flag which causes joined- and subquery loaders to traverse into already-loaded related objects @@ -1207,7 +1207,7 @@ class Query(object): return self.filter(with_parent(instance, property, entity_zero.entity)) - @_generative() + @_generative def add_entity(self, entity, alias=None): """add a mapped entity to the list of result columns to be returned.""" @@ -1219,7 +1219,7 @@ class Query(object): m = _MapperEntity(self, entity) self._set_entity_selectables([m]) - @_generative() + @_generative def with_session(self, session): """Return a :class:`.Query` that will use the given :class:`.Session`. @@ -1423,11 +1423,11 @@ class Query(object): q._set_entities(entities) return q - @_generative() + @_generative def _set_enable_single_crit(self, val): self._enable_single_crit = val - @_generative() + @_generative def _from_selectable(self, fromclause): for attr in ( "_statement", @@ -1477,7 +1477,7 @@ class Query(object): except StopIteration: return None - @_generative() + @_generative def with_entities(self, *entities): r"""Return a new :class:`.Query` replacing the SELECT list with the given entities. @@ -1503,7 +1503,7 @@ class Query(object): """ self._set_entities(entities) - @_generative() + @_generative def add_columns(self, *column): """Add one or more column expressions to the list of result columns to be returned.""" @@ -1550,7 +1550,7 @@ class Query(object): def _conditional_options(self, *args): return self._options(True, *args) - @_generative() + @_generative def _options(self, conditional, *args): # most MapperOptions write to the '_attributes' dictionary, # so copy that as well @@ -1585,7 +1585,7 @@ class Query(object): """ return fn(self) - @_generative() + @_generative def with_hint(self, selectable, text, dialect_name="*"): """Add an indexing or other executional context hint for the given entity or selectable to @@ -1640,7 +1640,7 @@ class Query(object): """ return self._execution_options - @_generative() + @_generative def execution_options(self, **kwargs): """ Set non-SQL options which take effect during execution. @@ -1658,7 +1658,7 @@ class Query(object): """ self._execution_options = self._execution_options.union(kwargs) - @_generative() + @_generative @util.deprecated( "0.9", "The :meth:`.Query.with_lockmode` method is deprecated and will " @@ -1691,7 +1691,7 @@ class Query(object): """ self._for_update_arg = LockmodeArg.parse_legacy_query(mode) - @_generative() + @_generative def with_for_update( self, read=False, @@ -1735,7 +1735,7 @@ class Query(object): key_share=key_share, ) - @_generative() + @_generative def params(self, *args, **kwargs): r"""add values for bind parameters which may have been specified in filter(). @@ -1756,7 +1756,8 @@ class Query(object): self._params = self._params.copy() self._params.update(kwargs) - @_generative(_no_statement_condition, _no_limit_offset) + @_generative + @_assertions(_no_statement_condition, _no_limit_offset) def filter(self, *criterion): r"""apply the given filtering criterion to a copy of this :class:`.Query`, using SQL expressions. @@ -1821,7 +1822,8 @@ class Query(object): ] return self.filter(*clauses) - @_generative(_no_statement_condition, _no_limit_offset) + @_generative + @_assertions(_no_statement_condition, _no_limit_offset) def order_by(self, *criterion): """apply one or more ORDER BY criterion to the query and return the newly resulting ``Query`` @@ -1849,7 +1851,8 @@ class Query(object): else: self._order_by = self._order_by + criterion - @_generative(_no_statement_condition, _no_limit_offset) + @_generative + @_assertions(_no_statement_condition, _no_limit_offset) def group_by(self, *criterion): """apply one or more GROUP BY criterion to the query and return the newly resulting :class:`.Query` @@ -1876,7 +1879,8 @@ class Query(object): else: self._group_by = self._group_by + criterion - @_generative(_no_statement_condition, _no_limit_offset) + @_generative + @_assertions(_no_statement_condition, _no_limit_offset) def having(self, criterion): r"""apply a HAVING criterion to the query and return the newly resulting :class:`.Query`. @@ -2299,7 +2303,8 @@ class Query(object): jp = prev self._joinpath = jp - @_generative(_no_statement_condition, _no_limit_offset) + @_generative + @_assertions(_no_statement_condition, _no_limit_offset) def _join(self, keys, outerjoin, full, create_aliases, from_joinpoint): """consumes arguments from join() or outerjoin(), places them into a consistent format with which to form the actual JOIN constructs. @@ -2858,7 +2863,8 @@ class Query(object): self._joinpoint = self._joinpath self._filter_aliases = () - @_generative(_no_statement_condition) + @_generative + @_assertions(_no_statement_condition) def reset_joinpoint(self): """Return a new :class:`.Query`, where the "join point" has been reset back to the base FROM entities of the query. @@ -2871,7 +2877,8 @@ class Query(object): """ self._reset_joinpoint() - @_generative(_no_clauseelement_condition) + @_generative + @_assertions(_no_clauseelement_condition) def select_from(self, *from_obj): r"""Set the FROM clause of this :class:`.Query` explicitly. @@ -2919,7 +2926,8 @@ class Query(object): self._set_select_from(from_obj, False) - @_generative(_no_clauseelement_condition) + @_generative + @_assertions(_no_clauseelement_condition) def select_entity_from(self, from_obj): r"""Set the FROM clause of this :class:`.Query` to a core selectable, applying it as a replacement FROM clause @@ -3052,7 +3060,8 @@ class Query(object): else: return list(self[item : item + 1])[0] - @_generative(_no_statement_condition) + @_generative + @_assertions(_no_statement_condition) def slice(self, start, stop): """Computes the "slice" of the :class:`.Query` represented by the given indices and returns the resulting :class:`.Query`. @@ -3098,7 +3107,8 @@ class Query(object): if isinstance(self._offset, int) and self._offset == 0: self._offset = None - @_generative(_no_statement_condition) + @_generative + @_assertions(_no_statement_condition) def limit(self, limit): """Apply a ``LIMIT`` to the query and return the newly resulting ``Query``. @@ -3106,7 +3116,8 @@ class Query(object): """ self._limit = limit - @_generative(_no_statement_condition) + @_generative + @_assertions(_no_statement_condition) def offset(self, offset): """Apply an ``OFFSET`` to the query and return the newly resulting ``Query``. @@ -3114,7 +3125,8 @@ class Query(object): """ self._offset = offset - @_generative(_no_statement_condition) + @_generative + @_assertions(_no_statement_condition) def distinct(self, *expr): r"""Apply a ``DISTINCT`` to the query and return the newly resulting ``Query``. @@ -3146,7 +3158,7 @@ class Query(object): else: self._distinct = expr - @_generative() + @_generative def prefix_with(self, *prefixes): r"""Apply the prefixes to the query and return the newly resulting ``Query``. @@ -3177,7 +3189,7 @@ class Query(object): else: self._prefixes = prefixes - @_generative() + @_generative def suffix_with(self, *suffixes): r"""Apply the suffix to the query and return the newly resulting ``Query``. @@ -3215,7 +3227,8 @@ class Query(object): """ return list(self) - @_generative(_no_clauseelement_condition) + @_generative + @_assertions(_no_clauseelement_condition) def from_statement(self, statement): """Execute the given SELECT statement and return results. @@ -4013,14 +4026,14 @@ class Query(object): from_clause, eager_join, eager_join.stop_on ) - statement.append_from(from_clause) + statement.select_from.non_generative(statement, from_clause) if context.order_by: - statement.append_order_by( - *context.adapter.copy_and_process(context.order_by) + statement.order_by.non_generative( + statement, *context.adapter.copy_and_process(context.order_by) ) - statement.append_order_by(*context.eager_order_by) + statement.order_by.non_generative(statement, *context.eager_order_by) return statement def _simple_statement(self, context): @@ -4054,7 +4067,9 @@ class Query(object): statement = statement.correlate(*self._correlate) if context.eager_order_by: - statement.append_order_by(*context.eager_order_by) + statement.order_by.non_generative( + statement, *context.eager_order_by + ) return statement def _adjust_for_single_inheritance(self, context): diff --git a/lib/sqlalchemy/sql/base.py b/lib/sqlalchemy/sql/base.py index da384bdabc..7e9199bfa8 100644 --- a/lib/sqlalchemy/sql/base.py +++ b/lib/sqlalchemy/sql/base.py @@ -43,13 +43,18 @@ def _from_objects(*elements): return itertools.chain(*[element._from_objects for element in elements]) -@util.decorator -def _generative(fn, *args, **kw): - """Mark a method as generative.""" +def _generative(fn): + @util.decorator + def _generative(fn, *args, **kw): + """Mark a method as generative.""" - self = args[0]._generate() - fn(self, *args[1:], **kw) - return self + self = args[0]._generate() + fn(self, *args[1:], **kw) + return self + + decorated = _generative(fn) + decorated.non_generative = fn + return decorated def _clone(element, **kw): diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py index 166e592b62..b41a77622b 100644 --- a/lib/sqlalchemy/sql/selectable.py +++ b/lib/sqlalchemy/sql/selectable.py @@ -2393,7 +2393,53 @@ class SelectStatementGrouping(GroupedElement, SelectBase): return self.element._from_objects -class GenerativeSelect(SelectBase): +class DeprecatedSelectBaseGenerations(object): + @util.deprecated( + "1.4", + "The :meth:`.GenerativeSelect.append_order_by` method is deprecated " + "and will be removed in a future release. Use the generative method " + ":meth:`.GenerativeSelect.order_by`.", + ) + def append_order_by(self, *clauses): + """Append the given ORDER BY criterion applied to this selectable. + + The criterion will be appended to any pre-existing ORDER BY criterion. + + This is an **in-place** mutation method; the + :meth:`~.GenerativeSelect.order_by` method is preferred, as it + provides standard :term:`method chaining`. + + .. seealso:: + + :meth:`.GenerativeSelect.order_by` + + """ + self.order_by.non_generative(self, *clauses) + + @util.deprecated( + "1.4", + "The :meth:`.GenerativeSelect.append_group_by` method is deprecated " + "and will be removed in a future release. Use the generative method " + ":meth:`.GenerativeSelect.group_by`.", + ) + def append_group_by(self, *clauses): + """Append the given GROUP BY criterion applied to this selectable. + + The criterion will be appended to any pre-existing GROUP BY criterion. + + This is an **in-place** mutation method; the + :meth:`~.GenerativeSelect.group_by` method is preferred, as it + provides standard :term:`method chaining`. + + .. seealso:: + + :meth:`.GenerativeSelect.group_by` + + """ + self.group_by.non_generative(self, *clauses) + + +class GenerativeSelect(DeprecatedSelectBaseGenerations, SelectBase): """Base class for SELECT statements where additional elements can be added. @@ -2676,7 +2722,14 @@ class GenerativeSelect(SelectBase): """ - self.append_order_by(*clauses) + if len(clauses) == 1 and clauses[0] is None: + self._order_by_clause = ClauseList() + 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, _literal_as_text_role=roles.OrderByRole + ) @_generative def group_by(self, *clauses): @@ -2697,45 +2750,6 @@ class GenerativeSelect(SelectBase): """ - self.append_group_by(*clauses) - - def append_order_by(self, *clauses): - """Append the given ORDER BY criterion applied to this selectable. - - The criterion will be appended to any pre-existing ORDER BY criterion. - - This is an **in-place** mutation method; the - :meth:`~.GenerativeSelect.order_by` method is preferred, as it - provides standard :term:`method chaining`. - - .. seealso:: - - :meth:`.GenerativeSelect.order_by` - - """ - if len(clauses) == 1 and clauses[0] is None: - self._order_by_clause = ClauseList() - 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, _literal_as_text_role=roles.OrderByRole - ) - - def append_group_by(self, *clauses): - """Append the given GROUP BY criterion applied to this selectable. - - The criterion will be appended to any pre-existing GROUP BY criterion. - - This is an **in-place** mutation method; the - :meth:`~.GenerativeSelect.group_by` method is preferred, as it - provides standard :term:`method chaining`. - - .. seealso:: - - :meth:`.GenerativeSelect.group_by` - - """ if len(clauses) == 1 and clauses[0] is None: self._group_by_clause = ClauseList() else: @@ -3052,7 +3066,127 @@ class CompoundSelect(GenerativeSelect): bind = property(bind, _set_bind) -class Select(HasPrefixes, HasSuffixes, GenerativeSelect): +class DeprecatedSelectGenerations(object): + @util.deprecated( + "1.4", + "The :meth:`.Select.append_correlation` method is deprecated " + "and will be removed in a future release. Use the generative " + "method :meth:`.Select.correlate`.", + ) + def append_correlation(self, fromclause): + """append the given correlation expression to this select() + construct. + + This is an **in-place** mutation method; the + :meth:`~.Select.correlate` method is preferred, as it provides + standard :term:`method chaining`. + + """ + + self.correlate.non_generative(self, fromclause) + + @util.deprecated( + "1.4", + "The :meth:`.Select.append_column` method is deprecated " + "and will be removed in a future release. Use the generative " + "method :meth:`.Select.column`.", + ) + def append_column(self, column): + """append the given column expression to the columns clause of this + select() construct. + + E.g.:: + + my_select.append_column(some_table.c.new_column) + + This is an **in-place** mutation method; the + :meth:`~.Select.column` method is preferred, as it provides standard + :term:`method chaining`. + + See the documentation for :meth:`.Select.with_only_columns` + for guidelines on adding /replacing the columns of a + :class:`.Select` object. + + """ + self.column.non_generative(self, column) + + @util.deprecated( + "1.4", + "The :meth:`.Select.append_prefix` method is deprecated " + "and will be removed in a future release. Use the generative " + "method :meth:`.Select.prefix_with`.", + ) + def append_prefix(self, clause): + """append the given columns clause prefix expression to this select() + construct. + + This is an **in-place** mutation method; the + :meth:`~.Select.prefix_with` method is preferred, as it provides + standard :term:`method chaining`. + + """ + self.prefix_with.non_generative(self, clause) + + @util.deprecated( + "1.4", + "The :meth:`.Select.append_whereclause` method is deprecated " + "and will be removed in a future release. Use the generative " + "method :meth:`.Select.where`.", + ) + def append_whereclause(self, whereclause): + """append the given expression to this select() construct's WHERE + criterion. + + The expression will be joined to existing WHERE criterion via AND. + + This is an **in-place** mutation method; the + :meth:`~.Select.where` method is preferred, as it provides standard + :term:`method chaining`. + + """ + self.where.non_generative(self, whereclause) + + @util.deprecated( + "1.4", + "The :meth:`.Select.append_having` method is deprecated " + "and will be removed in a future release. Use the generative " + "method :meth:`.Select.having`.", + ) + def append_having(self, having): + """append the given expression to this select() construct's HAVING + criterion. + + The expression will be joined to existing HAVING criterion via AND. + + This is an **in-place** mutation method; the + :meth:`~.Select.having` method is preferred, as it provides standard + :term:`method chaining`. + + """ + + self.having.non_generative(self, having) + + @util.deprecated( + "1.4", + "The :meth:`.Select.append_from` method is deprecated " + "and will be removed in a future release. Use the generative " + "method :meth:`.Select.select_from`.", + ) + def append_from(self, fromclause): + """append the given FromClause expression to this select() construct's + FROM clause. + + This is an **in-place** mutation method; the + :meth:`~.Select.select_from` method is preferred, as it provides + standard :term:`method chaining`. + + """ + self.select_from.non_generative(self, fromclause) + + +class Select( + HasPrefixes, HasSuffixes, DeprecatedSelectGenerations, GenerativeSelect +): """Represents a ``SELECT`` statement. """ @@ -3711,7 +3845,13 @@ class Select(HasPrefixes, HasSuffixes, GenerativeSelect): :class:`.Select` object. """ - self.append_column(column) + self._reset_memoizations() + column = coercions.expect(roles.ColumnsClauseRole, column) + + if isinstance(column, ScalarSelect): + column = column.self_group(against=operators.comma_op) + + self._raw_columns = self._raw_columns + [column] @util.dependencies("sqlalchemy.sql.util") def reduce_columns(self, sqlutil, only_synonyms=True): @@ -3828,7 +3968,8 @@ class Select(HasPrefixes, HasSuffixes, GenerativeSelect): """ - self.append_whereclause(whereclause) + self._reset_memoizations() + self._whereclause = and_(True_._ifnone(self._whereclause), whereclause) @_generative def having(self, having): @@ -3836,7 +3977,8 @@ class Select(HasPrefixes, HasSuffixes, GenerativeSelect): its HAVING clause, joined to the existing clause via AND, if any. """ - self.append_having(having) + self._reset_memoizations() + self._having = and_(True_._ifnone(self._having), having) @_generative def distinct(self, *expr): @@ -3889,7 +4031,9 @@ class Select(HasPrefixes, HasSuffixes, GenerativeSelect): select([func.count('*')]).select_from(table1) """ - self.append_from(fromclause) + self._reset_memoizations() + fromclause = coercions.expect(roles.FromClauseRole, fromclause) + self._from_obj = self._from_obj.union([fromclause]) @_generative def correlate(self, *fromclauses): @@ -3932,6 +4076,7 @@ class Select(HasPrefixes, HasSuffixes, GenerativeSelect): :ref:`correlated_subqueries` """ + self._auto_correlate = False if fromclauses and fromclauses[0] is None: self._correlate = () @@ -3975,100 +4120,6 @@ class Select(HasPrefixes, HasSuffixes, GenerativeSelect): coercions.expect(roles.FromClauseRole, f) for f in fromclauses ) - def append_correlation(self, fromclause): - """append the given correlation expression to this select() - construct. - - This is an **in-place** mutation method; the - :meth:`~.Select.correlate` method is preferred, as it provides - standard :term:`method chaining`. - - """ - - self._auto_correlate = False - self._correlate = set(self._correlate).union( - coercions.expect(roles.FromClauseRole, f) for f in fromclause - ) - - def append_column(self, column): - """append the given column expression to the columns clause of this - select() construct. - - E.g.:: - - my_select.append_column(some_table.c.new_column) - - This is an **in-place** mutation method; the - :meth:`~.Select.column` method is preferred, as it provides standard - :term:`method chaining`. - - See the documentation for :meth:`.Select.with_only_columns` - for guidelines on adding /replacing the columns of a - :class:`.Select` object. - - """ - self._reset_memoizations() - column = coercions.expect(roles.ColumnsClauseRole, column) - - if isinstance(column, ScalarSelect): - column = column.self_group(against=operators.comma_op) - - self._raw_columns = self._raw_columns + [column] - - def append_prefix(self, clause): - """append the given columns clause prefix expression to this select() - construct. - - This is an **in-place** mutation method; the - :meth:`~.Select.prefix_with` method is preferred, as it provides - standard :term:`method chaining`. - - """ - clause = coercions.expect(roles.WhereHavingRole, clause) - self._prefixes = self._prefixes + (clause,) - - def append_whereclause(self, whereclause): - """append the given expression to this select() construct's WHERE - criterion. - - The expression will be joined to existing WHERE criterion via AND. - - This is an **in-place** mutation method; the - :meth:`~.Select.where` method is preferred, as it provides standard - :term:`method chaining`. - - """ - - self._reset_memoizations() - self._whereclause = and_(True_._ifnone(self._whereclause), whereclause) - - def append_having(self, having): - """append the given expression to this select() construct's HAVING - criterion. - - The expression will be joined to existing HAVING criterion via AND. - - This is an **in-place** mutation method; the - :meth:`~.Select.having` method is preferred, as it provides standard - :term:`method chaining`. - - """ - self._reset_memoizations() - self._having = and_(True_._ifnone(self._having), having) - - def append_from(self, fromclause): - """append the given FromClause expression to this select() construct's - FROM clause. - - This is an **in-place** mutation method; the - :meth:`~.Select.select_from` method is preferred, as it provides - standard :term:`method chaining`. - - """ - self._reset_memoizations() - fromclause = coercions.expect(roles.FromClauseRole, fromclause) - self._from_obj = self._from_obj.union([fromclause]) - @_memoized_property def selected_columns(self): """A :class:`.ColumnCollection` representing the columns that diff --git a/test/ext/test_baked.py b/test/ext/test_baked.py index 00c6a78b4d..6750b18910 100644 --- a/test/ext/test_baked.py +++ b/test/ext/test_baked.py @@ -1379,7 +1379,7 @@ class CustomIntegrationTest(testing.AssertsCompiledSQL, BakedTest): class CachingQuery(Query): cache = {} - @_generative() + @_generative def set_cache_key(self, key): self._cache_key = key diff --git a/test/sql/test_compiler.py b/test/sql/test_compiler.py index c789a879ad..7eda0207a2 100644 --- a/test/sql/test_compiler.py +++ b/test/sql/test_compiler.py @@ -792,7 +792,7 @@ class SelectTest(fixtures.TestBase, AssertsCompiledSQL): s = select( [], exists([1], table2.c.otherid == table1.c.myid), from_obj=table1 ) - s.append_column(table1) + s.column.non_generative(s, table1) self.assert_compile( s, "SELECT mytable.myid, mytable.name, " diff --git a/test/sql/test_deprecations.py b/test/sql/test_deprecations.py index 09deb12944..a75de3f113 100644 --- a/test/sql/test_deprecations.py +++ b/test/sql/test_deprecations.py @@ -809,7 +809,10 @@ class SelectableTest(fixtures.TestBase, AssertsCompiledSQL): "JOIN (SELECT 1 AS a, 2 AS b) AS joinfrom " "ON basefrom.a = joinfrom.a", ) - replaced.append_column(joinfrom.c.b) + + with testing.expect_deprecated(r"The Select.append_column\(\)"): + replaced.append_column(joinfrom.c.b) + self.assert_compile( replaced, "SELECT basefrom.a, joinfrom.b FROM (SELECT 1 AS a) AS basefrom " @@ -1023,3 +1026,101 @@ class TextualSelectTest(fixtures.TestBase, AssertsCompiledSQL): "The SelectBase.c and SelectBase.columns" ): eq_(t.c.c.type._type_affinity, String) + + +class DeprecatedAppendMethTest(fixtures.TestBase, AssertsCompiledSQL): + __dialect__ = "default" + + def _expect_deprecated(self, clsname, methname, newmeth): + return testing.expect_deprecated( + r"The %s.append_%s\(\) method is deprecated " + r"and will be removed in a future release. Use the generative " + r"method %s.%s\(\)." % (clsname, methname, clsname, newmeth) + ) + + def test_append_whereclause(self): + t = table("t", column("q")) + stmt = select([t]) + + with self._expect_deprecated("Select", "whereclause", "where"): + stmt.append_whereclause(t.c.q == 5) + + self.assert_compile(stmt, "SELECT t.q FROM t WHERE t.q = :q_1") + + def test_append_having(self): + t = table("t", column("q")) + stmt = select([t]).group_by(t.c.q) + + with self._expect_deprecated("Select", "having", "having"): + stmt.append_having(t.c.q == 5) + + self.assert_compile( + stmt, "SELECT t.q FROM t GROUP BY t.q HAVING t.q = :q_1" + ) + + def test_append_order_by(self): + t = table("t", column("q"), column("x")) + stmt = select([t]).where(t.c.q == 5) + + with self._expect_deprecated( + "GenerativeSelect", "order_by", "order_by" + ): + stmt.append_order_by(t.c.x) + + self.assert_compile( + stmt, "SELECT t.q, t.x FROM t WHERE t.q = :q_1 ORDER BY t.x" + ) + + def test_append_group_by(self): + t = table("t", column("q")) + stmt = select([t]) + + with self._expect_deprecated( + "GenerativeSelect", "group_by", "group_by" + ): + stmt.append_group_by(t.c.q) + + stmt = stmt.having(t.c.q == 5) + + self.assert_compile( + stmt, "SELECT t.q FROM t GROUP BY t.q HAVING t.q = :q_1" + ) + + def test_append_correlation(self): + t1 = table("t1", column("q")) + t2 = table("t2", column("q"), column("p")) + + inner = select([t2.c.p]).where(t2.c.q == t1.c.q) + + with self._expect_deprecated("Select", "correlation", "correlate"): + inner.append_correlation(t1) + stmt = select([t1]).where(t1.c.q == inner.scalar_subquery()) + + self.assert_compile( + stmt, + "SELECT t1.q FROM t1 WHERE t1.q = " + "(SELECT t2.p FROM t2 WHERE t2.q = t1.q)", + ) + + def test_append_column(self): + t1 = table("t1", column("q"), column("p")) + stmt = select([t1.c.q]) + with self._expect_deprecated("Select", "column", "column"): + stmt.append_column(t1.c.p) + self.assert_compile(stmt, "SELECT t1.q, t1.p FROM t1") + + def test_append_prefix(self): + t1 = table("t1", column("q"), column("p")) + stmt = select([t1.c.q]) + with self._expect_deprecated("Select", "prefix", "prefix_with"): + stmt.append_prefix("FOO BAR") + self.assert_compile(stmt, "SELECT FOO BAR t1.q FROM t1") + + def test_append_from(self): + t1 = table("t1", column("q")) + t2 = table("t2", column("q")) + + stmt = select([t1]) + with self._expect_deprecated("Select", "from", "select_from"): + stmt.append_from(t1.join(t2, t1.c.q == t2.c.q)) + self.assert_compile(stmt, "SELECT t1.q FROM t1 JOIN t2 ON t1.q = t2.q") diff --git a/test/sql/test_generative.py b/test/sql/test_generative.py index 262333dc55..8d347a522a 100644 --- a/test/sql/test_generative.py +++ b/test/sql/test_generative.py @@ -527,7 +527,7 @@ class ClauseTest(fixtures.TestBase, AssertsCompiledSQL): class Vis(CloningVisitor): def visit_select(self, select): - select.append_whereclause(t1.c.col2 == 7) + select.where.non_generative(select, t1.c.col2 == 7) s3 = Vis().traverse(s2) assert str(s3) == s3_assert @@ -537,7 +537,7 @@ class ClauseTest(fixtures.TestBase, AssertsCompiledSQL): class Vis(ClauseVisitor): def visit_select(self, select): - select.append_whereclause(t1.c.col2 == 7) + select.where.non_generative(select, t1.c.col2 == 7) Vis().traverse(s2) assert str(s2) == s3_assert @@ -546,7 +546,7 @@ class ClauseTest(fixtures.TestBase, AssertsCompiledSQL): class Vis(CloningVisitor): def visit_select(self, select): - select.append_whereclause(t1.c.col3 == 9) + select.where.non_generative(select, t1.c.col3 == 9) s4 = Vis().traverse(s3) print(str(s3)) @@ -719,7 +719,7 @@ class ClauseTest(fixtures.TestBase, AssertsCompiledSQL): class Vis(CloningVisitor): def visit_select(self, select): - select.append_whereclause(t1.c.col2 == 7) + select.where.non_generative(select, t1.c.col2 == 7) self.assert_compile( select([t2]).where( diff --git a/test/sql/test_selectable.py b/test/sql/test_selectable.py index c54f27c23b..76cece5e40 100644 --- a/test/sql/test_selectable.py +++ b/test/sql/test_selectable.py @@ -345,8 +345,8 @@ class SelectableTest( sel = select([literal_column("1").label("a")]) eq_(list(sel.selected_columns.keys()), ["a"]) cloned = visitors.ReplacingCloningVisitor().traverse(sel) - cloned.append_column(literal_column("2").label("b")) - cloned.append_column(func.foo()) + cloned.column.non_generative(cloned, literal_column("2").label("b")) + cloned.column.non_generative(cloned, func.foo()) eq_(list(cloned.selected_columns.keys()), ["a", "b", "foo()"]) def test_clone_col_list_changes_then_proxy(self): @@ -354,7 +354,7 @@ class SelectableTest( stmt = select([t.c.q]).subquery() def add_column(stmt): - stmt.append_column(t.c.p) + stmt.column.non_generative(stmt, t.c.p) stmt2 = visitors.cloned_traverse(stmt, {}, {"select": add_column}) eq_(list(stmt.c.keys()), ["q"]) @@ -365,7 +365,7 @@ class SelectableTest( stmt = select([t.c.q]).subquery() def add_column(stmt): - stmt.append_column(t.c.p) + stmt.column.non_generative(stmt, t.c.p) stmt2 = visitors.cloned_traverse(stmt, {}, {"select": add_column}) eq_(list(stmt.c.keys()), ["q"]) @@ -395,7 +395,7 @@ class SelectableTest( "JOIN (SELECT 1 AS a, 2 AS b) AS joinfrom " "ON basefrom.a = joinfrom.a", ) - replaced.append_column(joinfrom.c.b) + replaced.column.non_generative(replaced, joinfrom.c.b) self.assert_compile( replaced, "SELECT basefrom.a, joinfrom.b FROM (SELECT 1 AS a) AS basefrom " @@ -948,7 +948,7 @@ class SelectableTest( s = select([t]) with testing.expect_deprecated("The SelectBase.c"): - s.append_whereclause(s.c.x > 5) + s.where.non_generative(s, s.c.x > 5) assert_raises_message( exc.InvalidRequestError, r"select\(\) construct refers to itself as a FROM", diff --git a/test/sql/test_text.py b/test/sql/test_text.py index 9483d10b02..bec56f2f7e 100644 --- a/test/sql/test_text.py +++ b/test/sql/test_text.py @@ -77,12 +77,12 @@ class SelectCompositionTest(fixtures.TestBase, AssertsCompiledSQL): def test_select_composition_two(self): s = select() - s.append_column(column("column1")) - s.append_column(column("column2")) - s.append_whereclause(text("column1=12")) - s.append_whereclause(text("column2=19")) + s = s.column(column("column1")) + s = s.column(column("column2")) + s = s.where(text("column1=12")) + s = s.where(text("column2=19")) s = s.order_by("column1") - s.append_from(text("table1")) + s = s.select_from(text("table1")) self.assert_compile( s, "SELECT column1, column2 FROM table1 WHERE "