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

2 files changed, 202 insertions, 146 deletions

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