ref: Add new helper functions journalValueAndFilterPostings(With)?.

Combining valuation with filtration is subtle and error-prone (see e.g. #1625).
We have to do in in both MultiBalanceReport and PostingsReport, where it
is done in slightly different ways. This refactors this functionality
into separate functions which are called in both reports, for uniform
behaviour.

hledger-lib/Hledger/Data/Journal.hs

@@ -521,19 +521,26 @@ filterJournalPostings q j@Journal{jtxns=ts} = j{jtxns=map (filterTransactionPost
 filterJournalRelatedPostings :: Query -> Journal -> Journal
 filterJournalRelatedPostings q j@Journal{jtxns=ts} = j{jtxns=map (filterTransactionRelatedPostings q) ts}
 
--- | Within each posting's amount, keep only the parts matching the query.
+-- | Within each posting's amount, keep only the parts matching the query, and
+-- remove any postings with all amounts removed.
 -- This can leave unbalanced transactions.
 filterJournalAmounts :: Query -> Journal -> Journal
 filterJournalAmounts q j@Journal{jtxns=ts} = j{jtxns=map (filterTransactionAmounts q) ts}
 
--- | Filter out all parts of this transaction's amounts which do not match the query.
+-- | Filter out all parts of this transaction's amounts which do not match the
+-- query, and remove any postings with all amounts removed.
 -- This can leave the transaction unbalanced.
 filterTransactionAmounts :: Query -> Transaction -> Transaction
-filterTransactionAmounts q t@Transaction{tpostings=ps} = t{tpostings=map (filterPostingAmount q) ps}
+filterTransactionAmounts q t@Transaction{tpostings=ps} = t{tpostings=mapMaybe (filterPostingAmount q) ps}
 
--- | Filter out all parts of this posting's amount which do not match the query.
+-- | Filter out all parts of this posting's amount which do not match the query, and remove the posting
-filterPostingAmount :: Query -> Posting -> Posting
+-- if this removes all amounts.
-filterPostingAmount q p@Posting{pamount=as} = p{pamount=filterMixedAmount (q `matchesAmount`) as}
+filterPostingAmount :: Query -> Posting -> Maybe Posting
+filterPostingAmount q p@Posting{pamount=as}
+  | null newamt = Nothing
+  | otherwise   = Just p{pamount=Mixed newamt}
+  where
+    Mixed newamt = filterMixedAmount (q `matchesAmount`) as
 
 filterTransactionPostings :: Query -> Transaction -> Transaction
 filterTransactionPostings q t@Transaction{tpostings=ps} = t{tpostings=filter (q `matchesPosting`) ps}

hledger-lib/Hledger/Reports/MultiBalanceReport.hs

@@ -251,20 +251,17 @@ getPostingsByColumn rspec j priceoracle reportspan =
 -- | Gather postings matching the query within the report period.
 getPostings :: ReportSpec -> Journal -> PriceOracle -> [Posting]
 getPostings rspec@ReportSpec{_rsQuery=query, _rsReportOpts=ropts} j priceoracle =
-    journalPostings .
+    journalPostings $ journalValueAndFilterPostingsWith rspec' j priceoracle
-    valueJournal .
-    filterJournalAmounts symq $      -- remove amount parts excluded by cur:
-    filterJournalPostings reportq j  -- remove postings not matched by (adjusted) query
   where
-    symq = dbg3 "symq" . filterQuery queryIsSym $ dbg3 "requested q" query
+    rspec' = rspec{_rsQuery=depthless, _rsReportOpts = ropts'}
+    ropts' = if isJust (valuationAfterSum ropts)
+        then ropts{value_=Nothing, cost_=NoCost}  -- If we're valuing after the sum, don't do it now
+        else ropts
+
     -- The user's query with no depth limit, and expanded to the report span
     -- if there is one (otherwise any date queries are left as-is, which
     -- handles the hledger-ui+future txns case above).
-    reportq = dbg3 "reportq" $ depthless query
+    depthless = dbg3 "depthless" $ filterQuery (not . queryIsDepth) query
-    depthless = dbg3 "depthless" . filterQuery (not . queryIsDepth)
-    valueJournal j' | isJust (valuationAfterSum ropts) = j'
-                    | otherwise = journalApplyValuationFromOptsWith rspec j' priceoracle
-
 
 -- | Given a set of postings, eg for a single report column, gather
 -- the accounts that have postings and calculate the change amount for

hledger-lib/Hledger/Reports/PostingsReport.hs

@@ -116,26 +116,18 @@ matchedPostingsBeforeAndDuring rspec@ReportSpec{_rsReportOpts=ropts,_rsQuery=q}
     dbg5 "beforeps, duringps" $ span (beforestartq `matchesPosting`) beforeandduringps
   where
     beforestartq = dbg3 "beforestartq" $ dateqtype $ DateSpan Nothing $ spanStart reportspan
-    beforeandduringps =
+    beforeandduringps = sortOn (if date2_ ropts then postingDate2 else postingDate)  -- sort postings by date or date2
-      dbg5 "ps4" $ sortOn sortdate $                                          -- sort postings by date or date2
+      . (if invert_ ropts then map negatePostingAmount else id)                      -- with --invert, invert amounts
-      dbg5 "ps3" $ (if invert_ ropts then map negatePostingAmount else id) $  -- with --invert, invert amounts
+      . journalPostings $ journalValueAndFilterPostings rspec{_rsQuery=beforeandduringq} j
-                   journalPostings $
-                   journalApplyValuationFromOpts rspec $                      -- convert to cost and apply valuation
-      dbg5 "ps2" $ filterJournalAmounts symq $                                -- remove amount parts which the query's cur: terms would exclude
-      dbg5 "ps1" $ filterJournal beforeandduringq j                           -- filter postings by the query, with no start date or depth limit
 
+    -- filter postings by the query, with no start date or depth limit
     beforeandduringq = dbg4 "beforeandduringq" $ And [depthless $ dateless q, beforeendq]
       where
         depthless  = filterQuery (not . queryIsDepth)
         dateless   = filterQuery (not . queryIsDateOrDate2)
         beforeendq = dateqtype $ DateSpan Nothing $ spanEnd reportspan
 
-    sortdate = if date2_ ropts then postingDate2 else postingDate
+    dateqtype = if queryIsDate2 dateq || (queryIsDate dateq && date2_ ropts) then Date2 else Date
-    filterJournal = if related_ ropts then filterJournalRelatedPostings else filterJournalPostings  -- with -r, replace each posting with its sibling postings
-    symq = dbg4 "symq" $ filterQuery queryIsSym q
-    dateqtype
-      | queryIsDate2 dateq || (queryIsDate dateq && date2_ ropts) = Date2
-      | otherwise = Date
       where
         dateq = dbg4 "dateq" $ filterQuery queryIsDateOrDate2 $ dbg4 "q" q  -- XXX confused by multiple date:/date2: ?
 

hledger-lib/Hledger/Reports/ReportOptions.hs

@@ -39,6 +39,8 @@ module Hledger.Reports.ReportOptions (
   reportOptsToggleStatus,
   simplifyStatuses,
   whichDateFromOpts,
+  journalValueAndFilterPostings,
+  journalValueAndFilterPostingsWith,
   journalApplyValuationFromOpts,
   journalApplyValuationFromOptsWith,
   mixedAmountApplyValuationAfterSumFromOptsWith,
@@ -59,7 +61,7 @@ module Hledger.Reports.ReportOptions (
 )
 where
 
-import Control.Applicative (Const(..), (<|>))
+import Control.Applicative (Const(..), (<|>), liftA2)
 import Control.Monad ((<=<), join)
 import Data.Either (fromRight)
 import Data.Either.Extra (eitherToMaybe)
@@ -498,6 +500,31 @@ flat_ = not . tree_
 -- depthFromOpts :: ReportOpts -> Int
 -- depthFromOpts opts = min (fromMaybe 99999 $ depth_ opts) (queryDepth $ queryFromOpts nulldate opts)
 
+-- | Convert a 'Journal''s amounts to cost and/or to value (see
+-- 'journalApplyValuationFromOpts'), and filter by the 'ReportSpec' 'Query'.
+--
+-- We make sure to first filter by amt: and cur: terms, then value the
+-- 'Journal', then filter by the remaining terms.
+journalValueAndFilterPostings :: ReportSpec -> Journal -> Journal
+journalValueAndFilterPostings rspec j = journalValueAndFilterPostingsWith rspec j priceoracle
+  where priceoracle = journalPriceOracle (infer_prices_ $ _rsReportOpts rspec) j
+
+-- | Like 'journalValueAndFilterPostings', but takes a 'PriceOracle' as an argument.
+journalValueAndFilterPostingsWith :: ReportSpec -> Journal -> PriceOracle -> Journal
+journalValueAndFilterPostingsWith rspec@ReportSpec{_rsQuery=q, _rsReportOpts=ropts} j =
+    -- Filter by the remainder of the query
+      filterJournal reportq
+    -- Apply valuation and costing
+    . journalApplyValuationFromOptsWith rspec
+    -- Filter by amount and currency, so it matches pre-valuation/costing
+      (if queryIsNull amtsymq then j else filterJournalAmounts amtsymq j)
+  where
+    -- with -r, replace each posting with its sibling postings
+    filterJournal = if related_ ropts then filterJournalRelatedPostings else filterJournalPostings
+    amtsymq = dbg3 "amtsymq" $ filterQuery queryIsAmtOrSym q
+    reportq = dbg3 "reportq" $ filterQuery (not . queryIsAmtOrSym) q
+    queryIsAmtOrSym = liftA2 (||) queryIsAmt queryIsSym
+
 -- | Convert this journal's postings' amounts to cost and/or to value, if specified
 -- by options (-B/--cost/-V/-X/--value etc.). Strip prices if not needed. This
 -- should be the main stop for performing costing and valuation. The exception is

hledger/hledger.m4.md

@@ -1244,6 +1244,22 @@ $ hledger print -X A
 
 ```
 
+## Interaction of valuation and queries
+
+When matching postings based on queries in the presence of valuation, the
+following happens.
+
+1. The query is separated into two parts:
+    1. the currency (`cur:`) or amount (`amt:`).
+    2. all other parts.
+2. The postings are matched to the currency and amount queries based on pre-valued amounts.
+3. Valuation is applied to the postings.
+4. The postings are matched to the other parts of the query based on post-valued amounts.
+
+See:
+[1625](https://github.com/simonmichael/hledger/issues/1625)
+
+
 ## Effect of valuation on reports
 
 Here is a reference for how valuation is supposed to affect each part of hledger's reports (and a glossary).