Repository navigation
Automatic posting matches multiple unrelated currencies when using query cur:X #1582
Description
- Minimum reproducible example:
= cur:USD
(b:USD) *1
2017-12-14
revenue:job -10 EUR
revenue:job -10 USD
assets
Output:
hledger -f test.hledger print --auto
2017-12-14 ; modified:
revenue:job -10 EUR
revenue:job -10 USD
(b:USD) -10 USD ; generated-posting: = cur:USD
assets
(b:USD) 10 EUR ; generated-posting: = cur:USD
(b:USD) 10 USD ; generated-posting: = cur:USD
Notice this is creating an automatic posting for EUR, and also two automatic postings for USD. The expected behavior is a single automatic posting for only USD.
- The output of hledger --version
1.21
Activity
bcashlol commentedon Jun 25, 2021
Note this works as expected:
Output:
But this does not work as expected:
Output:
Notice there are now two automatic postings on the inferred amounts, instead of just one as before.
If we do not use inferred amounts, it works as expected:
Output:
simonmichael commentedon Jun 25, 2021
Thanks for the report @bcashlol. Just looking at your first example, hledger is working as expected. Consider:
Your auto posting rule also matches the inferred balancing postings, you'll need to restrict it by account as well, eg:
bcashlol commentedon Jun 25, 2021
@simonmichael Actually I already tried to address your example in my second comment. It is not working as expected, since it gives different results if you manually write out the postings versus let hledger infer the exact same postings.
Consider that these two examples I gave give two different outputs, even though using your logic, they should be the same:
The above gives a correct output.
The above equivalent transaction gives an incorrect output (with an extra automatic posting for EUR).
Notice that in one of the above examples where we do restrict by account both EUR and USD are matched, but in the other only USD is matched. Regardless of whether we restrict by account, EUR should not be matched. Also notice in other examples where we do not match by account, EUR is matched in some cases when it should not be.
simonmichael commentedon Jun 25, 2021
Trying the examples in your latest comment, again I don't see a problem. Maybe I'm missing it. Can you show an actual repro (input, command, output with problem highlighted) ? Or if it's easier, come to chat.
Incidentally
&is not part of hledger query syntax, and I'm surprised it doesn't seem to cause trouble.bcashlol commentedon Jun 25, 2021
Yes, the same example from my 2nd comment:
Input:
Output:
This auto generated posting is wrong:
The query is to match USD. This auto posting matched EUR!
Also note that if you change the input file by manually writing out the assets postings instead of letting them be inferred, then you get a different (and correct) output. #1582 (comment) See "If we do not use inferred amounts, it works as expected:"
simonmichael commentedon Jun 25, 2021
That did it. You're right, it's a bug [since the beginning it seems]. Good catch, thanks!
Xitian9 commentedon Jun 28, 2021
I'm willing to bet this is due to an confusion I ran across when changing the internal representation of
MixedAmount. I have a strong tendency to view the content of aPostingas anAmount, as it will always have just a single entry when read from the journal. However, it is actually aMixedAmount, and will sometimes have multiple entries when it's an auto posting. It's obvious to state, but easily forgotten when coding. My guess is some piece of code is checking whether a commodity is present in aMixedAmount, and then assuming all amounts in there have that commodity.There is actually a fair amount of code which manually checks whether there is a single
Amountin aMixedAmountand only works if there is. If we wanted to encode this in the type system, we could havepamount :: Either Amount MixedAmount, but this is probably more effort than it's worth.Xitian9 commentedon Jun 28, 2021
Yes, I'm betting the problem is here:
https://github.com/simonmichael/hledger/blob/eae4922d122dbc25a09a243a2f8e8b332c34dcd7/hledger-lib/Hledger/Query.hs#L560-L563
simonmichael commentedon Jun 28, 2021
You could be right, but it's not a regression, I think ? At least it didn't seem so to me, I tried some old versions.
Xitian9 commentedon Jun 28, 2021
No, I don't think it's a regression. It's just a point that I frequently encountered and had to deal with: it wouldn't surprise me if it got missed earlier.
lib: Make sure automatic postings generated from postings with more t…
lib: Make sure automatic postings generated from postings with more t…
lib: Make sure automatic postings generated from postings with more t…
simonmichael commentedon Jul 1, 2021
Fixed by @Xitian9.