sdementen / piecash

Pythonic interface to GnuCash SQL documents
Other
290 stars 73 forks source link

Date check fails with GC 3 book (using the GnuCash 2.x date formatting in queries) #98

Closed alensiljak closed 6 years ago

alensiljak commented 6 years ago

Queries with transaction dates fail to retrieve any records. This happens with piecash 0.19 as well as py3-gnucash3 branch. The compiled SQL for sqlite is below.

SELECT splits.tx_guid, splits.value_num, splits.value_denom, splits.quantity_num, splits.quantity_denom, splits.guid, splits.account_guid, splits.memo, splits.action, splits.reconcile_state, splits.reconcile_date, splits.lot_guid 
FROM splits JOIN transactions ON transactions.guid = splits.tx_guid 
WHERE '9bf7ca7befcf635874bf72dc847f20d7' = splits.account_guid 
  AND transactions.post_date >= '20170701105900' 
  AND transactions.post_date <= '20180630105900'

I was under impression that the py3-gc3 branch would address this. Am I missing something? Thanks!

I guess it is a separate issue that the time is set to 10:59:00?

Oh, also, pip contains v0.19 whereas in code it is still 0.18.

alensiljak commented 6 years ago

I guess the problem is the storage_format?

class _DateAsDateTime(types.TypeDecorator):
    """Used to customise the DateTime type for sqlite (ie without the separators as in gnucash
    """
    impl = types.TypeEngine

    def __init__(self, neutral_time=True, *args, **kwargs):
        super(_DateAsDateTime, self).__init__(*args, **kwargs)
        self.neutral_time = neutral_time

    def load_dialect_impl(self, dialect):
        if dialect.name == "sqlite":
            return sqlite.DATETIME(
                storage_format="%(year)04d%(month)02d%(day)02d%(hour)02d%(minute)02d%(second)02d",
                regexp=r"(\d{4})-?(\d{2})-?(\d{2}) ?(\d{2}):?(\d{2}):?(\d{2})",
            )
        else:
            return types.DateTime()
alensiljak commented 6 years ago

The updated ISO datetime value for storage_format: %(year)04d-%(month)02d-%(day)02d %(hour)02d:%(minute)02d:%(second)02d

alensiljak commented 6 years ago

Much better now. Can continue with the tax reports. :) Would you like a merge request? It's a simple-enough fix.

ghost commented 6 years ago

@MisterY I think (to be confirmed) in gnucash 3, you will have a mix of date format in the sqlite:

Now, for doing queries based on date/datetime, this is indeed a biggest issue when using the SQL as it is difficult to handle both forward directly (even in pure SQL, this could be tricky). So either, all the dates are converted to the new format or all the dates are still in the old format. Otherwise, working directly in SA/SQL is difficult. Alternative is load all objects and filter in python land.

alensiljak commented 6 years ago

Just checked the table transactions

select post_date from transactions
where length(post_date) < 19
order by post_date

The conversion seems to have updated all the dates to the new format in the database. (at least in the transactions table, from what I see)

The problem here seems to be that the engine still compiles the dates in the SQL query to the old format yyyymmddhhmmss. Once I've added the dashes and spaces to match the ISO format, the queries started working again.

I can confirm that I'm now getting the data for this tax year only.

The only update required would be to apply the new mask as GC3 seems to update all the dates during the migration process.

sdementen commented 6 years ago

What is the migration process? Is it GC3 that automatically convert the dates when you open a book? Or is it a specific operation in the GC3 UI? I think if you change from XML to SQLite format, GC3 will indeed save all dates in the new format in the new SQLite file. However, for existing books from GC2, I had the impression that by default GC3 won't change the date formats on an existing book.

alensiljak commented 6 years ago

Yes, on the first opening it takes a bit longer and it does all the conversions, like field length, the dates, etc. I don't see any old date formats but your hint might have a role there. For a few weeks I was back to using XML format due to SQLite issues with GC3 so that may have converted the dates.

More importantly, shouldn't the default storage_format use the new date format for GC3 instead of trying to be compatible with the previous version? The field length supports the new date format, it is the default now, and it takes one SQL statement to update any old values to the new format. The issue is that, without this change, the date queries are non-functional for any new data.

sdementen commented 6 years ago

Normally piecash3 will save in the new format yet parse the two date formats. If not, it is indeed a bug. And if in the query, it also uses the old format, it is also a bug (and I guess both would be related)

alensiljak commented 6 years ago

Yes, the query is what I meant in the original report. The date comparison comes out like AND transactions.post_date >= '20170701105900' in the compiled statement, which results in no records found in the end. I guess it is only a simple text comparison of the values in the field.

alensiljak commented 6 years ago

This would be the patch

 piecash/sa_extra.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/piecash/sa_extra.py b/piecash/sa_extra.py
index aff5547..dde25cb 100644
--- a/piecash/sa_extra.py
+++ b/piecash/sa_extra.py
@@ -130,7 +130,7 @@ class _DateAsDateTime(types.TypeDecorator):
     def load_dialect_impl(self, dialect):
         if dialect.name == "sqlite":
             return sqlite.DATETIME(
-                storage_format="%(year)04d%(month)02d%(day)02d%(hour)02d%(minute)02d%(second)02d",
+                storage_format="%(year)04d-%(month)02d-%(day)02d %(hour)02d:%(minute)02d:%(second)02d",
                 regexp=r"(\d{4})-?(\d{2})-?(\d{2}) ?(\d{2}):?(\d{2}):?(\d{2})",
             )
         else:
sdementen commented 6 years ago

Yes perfect! I though I had done that but I missed it. Could you do a PR with your patch?

sdementen commented 6 years ago

fixed