> I'm not that familiar with this part of the code, so my (only)
> concern here is: are we sure that the `fields` argument passed to
> the function never contains duplicates?  If we are, LGTM, if not,
> using `take count` later could return additional fields from the
> filter.

That's a bit of an edge case: in no case such queries make sense,
so it is not specified what should happen in this case.
But let's play it safe: it is always OK to return each requested
field only once, so the following interdiff should solve the concern.

commit 48f7e5d2908da23e6efb8238d33d66b7dbfc5700
Author: Klaus Aehlig <[email protected]>
Date:   Tue Aug 4 16:46:27 2015 +0200

    Interdiff [PATCH stable-2.12 3/3] In queries collect all needed data

diff --git a/src/Ganeti/Query/Query.hs b/src/Ganeti/Query/Query.hs
index bdbfa08..4d8fc83 100644
--- a/src/Ganeti/Query/Query.hs
+++ b/src/Ganeti/Query/Query.hs
@@ -219,7 +219,7 @@ genericQuery fieldsMap collector nameFn configFn getFn cfg
   runResultT $ do
   cfilter <- toError $ compileFilter fieldsMap qfilter
   let allfields = ordNub $ fields ++ filterArguments qfilter
-      count = length fields
+      count = length $ ordNub fields
       selected = getSelectedFields fieldsMap allfields
       (fdefs, fgetters, _) = unzip3 selected
       live' = live && needsLiveData fgetters

-- 
Klaus Aehlig
Google Germany GmbH, Dienerstr. 12, 80331 Muenchen
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschaeftsfuehrer: Graham Law, Christine Elizabeth Flores

Reply via email to