ledger / ledger

Double-entry accounting system with a command-line reporting interface
https://www.ledger-cli.org
Other
5.35k stars 502 forks source link

The result of "balance assignments" in a transaction depends on the order of imported files #1831

Open ennob opened 4 years ago

ennob commented 4 years ago

Using "balance assignments" should be the same as calculating the correct amount yourself and writing the amount explicitly. For example, the following:

2019/01/01 * Payee
    Some:Account
    Some:Other:Account     = 20 USD

should always result in an amount being transferred from Some:Other:Account to Some:Account such that the renaming amount is $20.

The problem happens when you use 'import', and either Some:Account or Some:Other:Account is in a file that has not been imported yet. In that case the amount in the unknown account is assumed to be $0, and the transaction transfers the wrong amount.

What should happen is that all the files should be imported first and sorted by date of transaction. All transactions should then be processed in order. As it is now the result of the above can vary wildly depending on the order of import statements (which could be non-obvious when using wildcards).

tbm commented 4 years ago

Balance assignments and checks are done on the file order because it's done at the time of parsing and not later. This behaviour surprises some users but it's the way it works. I was going to say that this is documented in the manual but I can't find it, so maybe this needs to be documented.

I think your issue is mostly issue #1659 i.e. that the order of !include can change.

which could be non-obvious when using wildcards

I don't think it has to be non-obvious. sorting includes with wildcards seems like the right thing to do, see issue #1659

ennob commented 4 years ago

Yes, I know that it is done at the time of parsing. My argument is that that is not the right way of doing it. It is not a matter of documenting stupid behavior, it is a matter of changing it to work correctly.

If this tool is to be used in anything other than hobby use cases, it cannot be the case that renaming a file could result in a completely different amount being transferred by an unchanged transaction. Without any kind of error or warning message.

jwiegley commented 4 years ago

I welcome anyone to revise this part of the code. Balance assignments came to Ledger very late in the design, so none of the core logic was built with them in mind. It's file order because that's what makes the most sense in the code.

ennob commented 4 years ago

I might look into a fix for this. I haven't gone through the code yet so I'm not making any promises :)

If it turns out to require major refactoring, how would people feel about a temporary fix in the form of a warning message? The warning could for example be emitted if balance assignments are used on accounts that are unknown at the time of assignment.

jwiegley commented 4 years ago

At the very least that should be an error if --strict is being used.

hugopeixoto commented 4 years ago

I just ran into this problem while using this feature. It took me a while to figure out what the problem was. Here's a simplified reproduction:

# sample.txt
2018-01-01 Initial assignment
  Assets:Cash             10000.00 EUR
  Equity
# sample2.txt
2019-01-01 Dummy expense
  Assets:Cash              -100.00 EUR
  Expenses

2020-01-01 Reassignment
  Assets:Cash           = 10000.00 EUR
  Equity

If I parse them as a single file, I get a balance of 10k:

$ cat sample.txt sample2.txt | ledger -f - balance Assets:Cash
        10000.00 EUR  Assets:Cash

If I parse them separately, I get 20k:

$ ledger -f sample.txt -f sample2.txt balance Assets:Cash
        20000.00 EUR  Assets:Cash

I had to add the 100 eur expense to avoid the "null amount per transaction" error in the single file scenario. It isn't triggered in the separate files scenario, which is also a sign that something's off.

I don't think it's only related to the include order. Shouldn't both uses output 10000.00 EUR?