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

6 files changed, 277 insertions, 208 deletions

diff --git a/lib/sqlalchemy/dialects/mssql/base.py b/lib/sqlalchemy/dialects/mssql/base.py
index 4dd8e7261..54f0043c4 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 4d308d26b..e52b6d8bb 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 3218b9fa3..221f982a8 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 6d8037bae..a30cfa7e4 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 da384bdab..7e9199bfa 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 166e592b6..b41a77622 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