Apply amt:/cur: queries after valuation, not before (to fix boolean queries)

[simonmichael]

This is a draft for discussion. It reverses #1625 (matching amt:/cur: before valuation) to fix #2371 (some boolean queries not working).

Here's the current description:

Until now, at least since #1625 (2021), cur: and amt: matched the
pre-cost/value-conversion commodity symbols and amounts.

To do that, we were separating the cur:/amt: parts from the rest of
the report query. But with boolean queries #1989 (2023), it's not
possible to do that reliably without changing the query's meaning (?).

So we no longer do that, which means that cur: and amt: now match the
post valuation amounts. This is a breaking change for reports:

• register and all balance reports (using journalValueAndFilterPostingsWith)
  and aregister, hledger-ui & hledger-web register views (using accountTransactionsReport)
  no longer pre-filter by amt:/cur:. amt:/cur: with these reports now
  match the post-valuation amounts and commodity symbols, not the
  pre-valuation ones.

• print, codes, descriptions, notes, hledger-web journal view (using entriesReport)
  now apply all queries after valuation, not before it
  (for consistency with the above).

• Old and new amt:/cur: tests
• fixed boolean query test

This fixes boolean queries, or at least some of them. There are other cases where reports transform the report query (eg with filterQuery), which have not been fully audited. A common case is removing the depth: limit from the query. It's not yet clear if these cases can also break boolean queries - more auditing, testing and producing repros will help.

"Early amt:/cur:" (before valuation) has been the status quo for years. So this needs more thought and ideas about whether and how to preserve compatibility. As it stands, this would break some existing reports, and might make those reports hard or impossible to produce at all (testing needed).

Some options to consider ?

1. Try to match with amt:/cur: both before and after valuation.
   (We do this with acct:, trying to match both before and after account name aliasing.)
   Even if advisable, I'm not sure how feasible it would be to make this robust.
   Eg, where A amounts have been converted to value in B:

   • reg -V cur:A matches the B amounts (because they were formerly A)
   • reg -V cur:B also matches the B amounts
   • reg -V expr:'(cur:A AND cash) OR checking' is still problematic; cur:A is part of a more complex query, can't extract and prefilter with it

2. Keep the old behaviour for simple queries, but disable it when a problematic boolean query is detected (one with amt:/cur: in subexpressions).
   Inconsistent behaviour would probably be bad.
   Eg:

   • reg -V expr:'cur:A AND cash' cur:A is in a top level AND, can extract it and prefilter with it (and exclude it from the postfilter so it doesn't exclude the B result)
   • reg -V expr:'(cur:A AND cash) OR checking' cur:A is in a subquery, can't extract/prefilter with it; must leave it in the postfilter (it will exclude the B result)

3. Disallow amt:/cur: in boolean queries or subqueries.
   (We already disallow date: in OR subexpressions.)
   This might be over-restrictive.
   Eg:

   • reg -V expr:'cur:A AND cash' "Error: amt: and cur: are not supported in boolean queries"
   • reg -V expr:'(cur:A AND cash) OR checking' "Error: amt: and cur: must be part of a top-level AND query"
   • reg -V cur:A cash works as before

4. Think harder about

   • more safe/limited/reasonable ways to extract the amt:/cur: queries for pre-filtering, preserving the old behaviour without altering boolean queries.
   • why those reports were doing that pre-filtering, and if it's really needed. Reports often make a variant of the user's query, eg removing a depth limit, or changing the period in order to calculate starting balances, while preserving the rest of the query as-is.
   • how to ease the transition, if behaviour is changed

[simonmichael]

I forgot about this and haven't been testing it further. But I probably wouldn't have noticed any problem. I think that people will notice only if it's shipped in a release, probably after a few months' delay.

To do:

• Add a few more tests demonstrating the changed behaviour of amt: and cur:
• Post these examples widely (mail list, chat, mastodon..) and ask for comment.

[simonmichael]

Or we could:

5. Change cur: and amt: to work after valuation, so they can be used safely in boolean queries. But also add ocur: and oamt: (origcur: and origamt: ?) which work the old way, ie before valuation, and are not allowed in boolean queries. This would give an escape hatch when someone finds the new behaviour breaking their setup.

[simonmichael]

Abandoned in favour of #2387, which also fixes #2371.