On Tue, Aug 04, 2015 at 04:49:10PM +0200, Klaus Aehlig wrote:
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
LGTM, thanks
