commit fbc263a9ca2f8166f35c377fa00b52792272eb31
Author: Michael Hanselmann <hansmi@google.com>
Date:   Tue Mar 1 18:25:04 2011 +0100

    query: Fix bug when names are specified
    
    If the client/caller would specify names through the use of a filter,
    the result would be sorted. This is a regression over earlier Ganeti
    versions and verified in QA. This patch adds an optional parameter to
    control the sorting and provides unittests.
    
    Signed-off-by: Michael Hanselmann <hansmi@google.com>
    Reviewed-by: René Nussbaumer <rn@google.com>

diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index 2b3f2e0..2e59a0b 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -464,6 +464,9 @@ class _QueryBase:
     self.requested_data = self.query.RequestedData()
     self.names = self.query.RequestedNames()
 
+    # Sort only if no names were requested
+    self.sort_by_name = not self.names
+
     self.do_locking = None
     self.wanted = None
 
@@ -530,13 +533,15 @@ class _QueryBase:
     """Collect data and execute query.
 
     """
-    return query.GetQueryResponse(self.query, self._GetQueryData(lu))
+    return query.GetQueryResponse(self.query, self._GetQueryData(lu),
+                                  sort_by_name=self.sort_by_name)
 
   def OldStyleQuery(self, lu):
     """Collect data and execute query.
 
     """
-    return self.query.OldStyleQuery(self._GetQueryData(lu))
+    return self.query.OldStyleQuery(self._GetQueryData(lu),
+                                    sort_by_name=self.sort_by_name)
 
 
 def _GetWantedNodes(lu, nodes):
diff --git a/lib/query.py b/lib/query.py
index 40a03be..6497c60 100644
--- a/lib/query.py
+++ b/lib/query.py
@@ -629,13 +629,18 @@ class Query:
     """
     return GetAllFields(self._fields)
 
-  def Query(self, ctx):
+  def Query(self, ctx, sort_by_name=True):
     """Execute a query.
 
     @param ctx: Data container passed to field retrieval functions, must
       support iteration using C{__iter__}
+    @type sort_by_name: boolean
+    @param sort_by_name: Whether to sort by name or keep the input data's
+      ordering
 
     """
+    sort = (self._name_fn and sort_by_name)
+
     result = []
 
     for idx, item in enumerate(ctx):
@@ -648,15 +653,16 @@ class Query:
       if __debug__:
         _VerifyResultRow(self._fields, row)
 
-      if self._name_fn:
+      if sort:
         (status, name) = _ProcessResult(self._name_fn(ctx, item))
         assert status == constants.RS_NORMAL
         # TODO: Are there cases where we wouldn't want to use NiceSort?
-        sortname = utils.NiceSortKey(name)
+        result.append((utils.NiceSortKey(name), idx, row))
       else:
-        sortname = None
+        result.append(row)
 
-      result.append((sortname, idx, row))
+    if not sort:
+      return result
 
     # TODO: Would "heapq" be more efficient than sorting?
 
@@ -667,7 +673,7 @@ class Query:
 
     return map(operator.itemgetter(2), result)
 
-  def OldStyleQuery(self, ctx):
+  def OldStyleQuery(self, ctx, sort_by_name=True):
     """Query with "old" query result format.
 
     See L{Query.Query} for arguments.
@@ -681,7 +687,7 @@ class Query:
                                  errors.ECODE_INVAL)
 
     return [[value for (_, value) in row]
-            for row in self.Query(ctx)]
+            for row in self.Query(ctx, sort_by_name=sort_by_name)]
 
 
 def _ProcessResult(value):
@@ -776,14 +782,17 @@ def _PrepareFieldList(fields, aliases):
   return result
 
 
-def GetQueryResponse(query, ctx):
+def GetQueryResponse(query, ctx, sort_by_name=True):
   """Prepares the response for a query.
 
   @type query: L{Query}
   @param ctx: Data container, see L{Query.Query}
+  @type sort_by_name: boolean
+  @param sort_by_name: Whether to sort by name or keep the input data's
+    ordering
 
   """
-  return objects.QueryResponse(data=query.Query(ctx),
+  return objects.QueryResponse(data=query.Query(ctx, sort_by_name=sort_by_name),
                                fields=query.GetFields()).ToDict()
 
 
diff --git a/test/ganeti.query_unittest.py b/test/ganeti.query_unittest.py
index cbbe490..38e67f4 100755
--- a/test/ganeti.query_unittest.py
+++ b/test/ganeti.query_unittest.py
@@ -1118,6 +1118,99 @@ class TestQueryFilter(unittest.TestCase):
       ["node1", "node44"],
       ])
 
+    # Name field, but no sorting, result must be in incoming order
+    q = query.Query(fielddefs, ["pnode", "snode"], namefield="pnode")
+    self.assertFalse(q.RequestedData())
+    self.assertEqual(q.Query(data, sort_by_name=False),
+      [[(constants.RS_NORMAL, "node1"), (constants.RS_NORMAL, "node44")],
+       [(constants.RS_NORMAL, "node30"), (constants.RS_NORMAL, "node90")],
+       [(constants.RS_NORMAL, "node25"), (constants.RS_NORMAL, "node1")],
+       [(constants.RS_NORMAL, "node20"), (constants.RS_NORMAL, "node1")]])
+    self.assertEqual(q.OldStyleQuery(data, sort_by_name=False), [
+      ["node1", "node44"],
+      ["node30", "node90"],
+      ["node25", "node1"],
+      ["node20", "node1"],
+      ])
+    self.assertEqual(q.Query(reversed(data), sort_by_name=False),
+      [[(constants.RS_NORMAL, "node20"), (constants.RS_NORMAL, "node1")],
+       [(constants.RS_NORMAL, "node25"), (constants.RS_NORMAL, "node1")],
+       [(constants.RS_NORMAL, "node30"), (constants.RS_NORMAL, "node90")],
+       [(constants.RS_NORMAL, "node1"), (constants.RS_NORMAL, "node44")]])
+    self.assertEqual(q.OldStyleQuery(reversed(data), sort_by_name=False), [
+      ["node20", "node1"],
+      ["node25", "node1"],
+      ["node30", "node90"],
+      ["node1", "node44"],
+      ])
+
+  def testEqualNamesOrder(self):
+    fielddefs = query._PrepareFieldList([
+      (query._MakeField("pnode", "PNode", constants.QFT_TEXT, "Primary"),
+       None, 0, lambda ctx, item: item["pnode"]),
+      (query._MakeField("num", "Num", constants.QFT_NUMBER, "Num"),
+       None, 0, lambda ctx, item: item["num"]),
+      ], [])
+
+    data = [
+      { "pnode": "node1", "num": 100, },
+      { "pnode": "node1", "num": 25, },
+      { "pnode": "node2", "num": 90, },
+      { "pnode": "node2", "num": 30, },
+      ]
+
+    q = query.Query(fielddefs, ["pnode", "num"], namefield="pnode",
+                    filter_=["|", ["=", "pnode", "node1"],
+                                  ["=", "pnode", "node2"],
+                                  ["=", "pnode", "node1"]])
+    self.assertEqual(q.RequestedNames(), ["node1", "node2"],
+                     msg="Did not return unique names")
+    self.assertFalse(q.RequestedData())
+    self.assertEqual(q.Query(data),
+      [[(constants.RS_NORMAL, "node1"), (constants.RS_NORMAL, 100)],
+       [(constants.RS_NORMAL, "node1"), (constants.RS_NORMAL, 25)],
+       [(constants.RS_NORMAL, "node2"), (constants.RS_NORMAL, 90)],
+       [(constants.RS_NORMAL, "node2"), (constants.RS_NORMAL, 30)]])
+    self.assertEqual(q.Query(data, sort_by_name=False),
+      [[(constants.RS_NORMAL, "node1"), (constants.RS_NORMAL, 100)],
+       [(constants.RS_NORMAL, "node1"), (constants.RS_NORMAL, 25)],
+       [(constants.RS_NORMAL, "node2"), (constants.RS_NORMAL, 90)],
+       [(constants.RS_NORMAL, "node2"), (constants.RS_NORMAL, 30)]])
+
+    data = [
+      { "pnode": "nodeX", "num": 50, },
+      { "pnode": "nodeY", "num": 40, },
+      { "pnode": "nodeX", "num": 30, },
+      { "pnode": "nodeX", "num": 20, },
+      { "pnode": "nodeM", "num": 10, },
+      ]
+
+    q = query.Query(fielddefs, ["pnode", "num"], namefield="pnode",
+                    filter_=["|", ["=", "pnode", "nodeX"],
+                                  ["=", "pnode", "nodeY"],
+                                  ["=", "pnode", "nodeY"],
+                                  ["=", "pnode", "nodeY"],
+                                  ["=", "pnode", "nodeM"]])
+    self.assertEqual(q.RequestedNames(), ["nodeX", "nodeY", "nodeM"],
+                     msg="Did not return unique names")
+    self.assertFalse(q.RequestedData())
+
+    # First sorted by name, then input order
+    self.assertEqual(q.Query(data, sort_by_name=True),
+      [[(constants.RS_NORMAL, "nodeM"), (constants.RS_NORMAL, 10)],
+       [(constants.RS_NORMAL, "nodeX"), (constants.RS_NORMAL, 50)],
+       [(constants.RS_NORMAL, "nodeX"), (constants.RS_NORMAL, 30)],
+       [(constants.RS_NORMAL, "nodeX"), (constants.RS_NORMAL, 20)],
+       [(constants.RS_NORMAL, "nodeY"), (constants.RS_NORMAL, 40)]])
+
+    # Input order
+    self.assertEqual(q.Query(data, sort_by_name=False),
+      [[(constants.RS_NORMAL, "nodeX"), (constants.RS_NORMAL, 50)],
+       [(constants.RS_NORMAL, "nodeY"), (constants.RS_NORMAL, 40)],
+       [(constants.RS_NORMAL, "nodeX"), (constants.RS_NORMAL, 30)],
+       [(constants.RS_NORMAL, "nodeX"), (constants.RS_NORMAL, 20)],
+       [(constants.RS_NORMAL, "nodeM"), (constants.RS_NORMAL, 10)]])
+
   def testFilter(self):
     (DK_A, DK_B) = range(1000, 1002)
 
