replace "parens" flags with self_grouping()

[sqlalchemy-bot]

Issue created by Michael Bayer (zzzeek)

---

ClauseElements should know if they need to be parenthesized when lumped into something else. additionally, they should be lumped inside of other objects using a group container so that no "parens" flag needs to be set on the target object.

Heres the start of the idea which I dont have time to complete right now:

Index: lib/sqlalchemy/sql.py
===================================================================
--- lib/sqlalchemy/sql.py   (revision 2611)
+++ lib/sqlalchemy/sql.py   (working copy)
@@ -1060,7 +1060,10 @@
         child items from a different context (such as schema-level
         collections instead of clause-level)."""
         return [       
+    
+    def self_group(self):
+        return self
+
     def supports_execution(self):
         """Return True if this clause element represents a complete
         executable statement.
@@ -1175,8 +1178,7 @@
         return self._negate()

     def _negate(self):
-        self.parens=True
-        return _BooleanExpression(_TextClause("NOT"), self, None)
+        return _BooleanExpression(_TextClause("NOT"), self.self_group(), None)

 class _CompareMixin(object):
     """Defines comparison operations for ``ClauseElement`` instances.
@@ -1384,6 +1386,7 @@

         return True

+        
 class ColumnElement(Selectable, _CompareMixin):
     """Represent an element that is useable within the 
     "column clause" portion of a ``SELECT`` statement. 
@@ -1864,6 +1867,12 @@
         clauses = [clause.copy_container() for clause in self.clauses](]
-)
         return ClauseList(parens=self.parens, *clauses)

+    def self_group(self):
+        if self.parens:
+            return _Grouping(self)
+        else:
+            return self
+
     def append(self, clause):
         if _is_literal(clause):
             clause = _TextClause(unicode(clause))
@@ -1912,12 +1921,14 @@
         return _CompoundClause(self.operator, *clauses)

     def append(self, clause):
-        if isinstance(clause, _CompoundClause):
-            clause.parens = True
-        ClauseList.append(self, clause)
+        ClauseList.append(self, clause.self_group())

+    def self_group(self):
+        return _Grouping(self)
+
     def get_children(self, **kwargs):
         return self.clauses
+
     def accept_visitor(self, visitor):
         visitor.visit_compound(self)

@@ -2064,15 +2075,15 @@
     """

     def __init__(self, left, right, operator, type=None):
-        self.left = left
-        self.right = right
+        self.left = left.self_grouping()
+        self.right = right.self_grouping()
         self.operator = operator
         self.type = sqltypes.to_instance(type)
         self.parens = False
-        if isinstance(self.left, _BinaryClause) or hasattr(self.left, '_selectable'):
-            self.left.parens = True
-        if isinstance(self.right, _BinaryClause) or hasattr(self.right, '_selectable'):
-            self.right.parens = True
+#        if isinstance(self.left, _BinaryClause) or hasattr(self.left, '_selectable'):
+#            self.left.parens = True
+#        if isinstance(self.right, _BinaryClause) or hasattr(self.right, '_selectable'):
+#            self.right.parens = True

     def copy_container(self):
         return self.__class__(self.left.copy_container(), self.right.copy_container(), self.operator)
@@ -2358,6 +2369,14 @@

     engine = property(lambda s: s.selectable.engine)

+class _Grouping(ClauseElement):
+    def __init__(self, elem):
+        self.elem = elem
+    def accept_visitor(self, visitor):
+        visitor.visit_grouping(self)
+    def get_children(self, **kwargs):
+        return self.elem,
+
 class _Label(ColumnElement):
     """represent a label, as typically applied to any column-level element
     using the ``AS`` sql keyword.
@@ -2375,7 +2394,7 @@
         self.obj = obj
         self.case_sensitive = getattr(obj, "case_sensitive", True)
         self.type = sqltypes.to_instance(type or getattr(obj, 'type', None))
-        obj.parens=True
+        #obj.parens=True

     key = property(lambda s: s.name)
     _label = property(lambda s: s.name)
Index: lib/sqlalchemy/ansisql.py
===================================================================
--- lib/sqlalchemy/ansisql.py   (revision 2611)
+++ lib/sqlalchemy/ansisql.py   (working copy)
@@ -245,7 +245,10 @@
         """

         return ""
-
+    
+    def visit_grouping(self, grouping):
+        self.strings[grouping](grouping) = "(" + self.strings(grouping.elem) + ")"
+        
     def visit_label(self, label):
         labelname = self._truncated_identifier("colident", label.name)

basically objects know how to "group" themselves, and container objects know to call the "group" method on the object, which could return just "self" or a _Group instance that compiles in the parenthesis.

the "parens" flag would still exist on a few key constructs in order to "force" them to have parens or not from the outside API or for elements created specifically (like the argument list for func), but no ClauseElement should be modifying other ClauseElements passed to it, in favor of using the grouping method.

high priority for now since i want the SQL language to be a lot cleaner.

[sqlalchemy-bot]

Michael Bayer (zzzeek) wrote:

---

Removing milestone: 0.4.0 (automated comment)

[sqlalchemy-bot]

Michael Bayer (zzzeek) wrote:

---

this was implemented in changeset:2620

[sqlalchemy-bot]

Changes by Michael Bayer (zzzeek): removed "0.4.0" milestone

[sqlalchemy-bot]

Changes by Michael Bayer (zzzeek): set state to "resolved"