Created attachment 142236 [details]
zip with input and output of test run

this issue came out of the verification process for a patch applied against 
https://bugs.documentfoundation.org/show_bug.cgi?id=117298

There is information in the comments on that issue of interest here, particularly comment 10.

Created attachment 142240 [details]
Shows the difference of datetime, date and time imported automatically from HSQLDB to Firebird

Seems that all values have been changed: datetime, date and time. Could be that it differs depending on the timezone where you live.

All datetime-values have been changed in Germany, both for daylight saving and normal time, changing it to 2 or 1 hour before the time from HSQLDB.
All date-values have been changed to 1 day before.
Nearly all time-values have been changed to 1 hour before. One very confusing change: 00:30:30 will be changed to 17:42:46 instead.

All this tested with 
Version: 6.0.4.2
Build-ID: 9b0d9b32d5dcda91d2f1a96dc04c645c450872bf
CPU-Threads: 4; BS: Linux 4.4; UI-Render: Standard; VCL: kde4; 
Gebietsschema: de-DE (de_DE.UTF-8); Calc: group

Have had a look to the attached Firebird-file:
All dates have been imported correct there - no date has been imported correct here with OpenSUSE 42.3 in Germany
Version: 6.0.4.2
Build-ID: 9b0d9b32d5dcda91d2f1a96dc04c645c450872bf
CPU-Threads: 4; BS: Linux 4.4; UI-Render: Standard; VCL: kde4; 
Gebietsschema: de-DE (de_DE.UTF-8); Calc: group

Nearly all time-values differ from the values I imported here. No idea what is going wrong, but at this time time-values should better left blank than import such "values".

Created attachment 142257 [details]
test each hour, from 00:00 (12AM) to 24:00 (12PM) included

I created a brand new hsqldb file with 6.0.4.1 LO Debian package.
I created a first table with each hour, from 00:00 to 24:00 with AM/PM format.

Then, added trace with this patch:
diff --git a/dbaccess/source/filter/hsqldb/rowinputbinary.cxx b/dbaccess/source/filter/hsqldb/rowinputbinary.cxx
index 62c37525367d..225f5b235bf5 100644
--- a/dbaccess/source/filter/hsqldb/rowinputbinary.cxx
+++ b/dbaccess/source/filter/hsqldb/rowinputbinary.cxx
@@ -16,7 +16,7 @@
  *   except in compliance with the License. You may obtain a copy of
  *   the License at http://www.apache.org/licenses/LICENSE-2.0 .
  */
-
+#include <iostream>
 #include "rowinputbinary.hxx"
 #include <com/sun/star/sdbc/DataType.hpp>
 #include <com/sun/star/io/WrongFormatException.hpp>
@@ -333,6 +333,7 @@ std::vector<Any> HsqlRowInputStream::readOneRow(const ColumnTypeVector& nColType
             {
                 sal_Int64 value = 0;
                 m_pStream->ReadInt64(value);
+std::cerr << "value = " << value << "\n";
                 css::util::Time time((value % 1000) * 1000000, value / 1000, 0, 0, true);
                 aData.push_back(makeAny(time));
             }

Here are the weird results:
-3600000
0
3600000
7200000
10800000
14400000
18000000
21600000
25200000
28800000
32400000
36000000
39600000
43200000
46800000
50400000
54000000
57600000
61200000
64800000
68400000
72000000
75600000
79200000
-3600000

1) it begins to -3600000
2) At the end, the difference isn't 3600000 between each line

I think we must understand these values to get right hours

I tried some values on table 2 since I created it by letting the default format on 24H.

(still on France with French localization and UTC+2)

Lionel/Tamas: could you give a look (and provide your thoughts:-)) to my previous comment about ms retrieved on straightforward test?

Forget difference part in my previous comment, I checked again, it stays 3600000 until the end.

This can be related:

https://bugs.documentfoundation.org/show_bug.cgi?id=63566

(In reply to Tamas Bunth from comment #8)
> This can be related:
> 
> https://bugs.documentfoundation.org/show_bug.cgi?id=63566

It is related to datetime and date, but not to the very confusing time-values in the attached databases.

(In reply to Tamas Bunth from comment #8)
> This can be related:
> 
> https://bugs.documentfoundation.org/show_bug.cgi?id=63566

Part of it is.

So - today I can migrate that table using the current Base import functionality (hsqldb:sdbc) source data to (firebird:sdbc) target data and date, time and timestamp all transfer without any APPARENT change in values. Truth is the values changed.

As the other issue points out a feature of HSQLdb is to store time values as the UTC+0 time and accounts (changes) the time representation supplied from the user based on the users local time zone, later when it supplies a time representation for the value it again accounts for the users local time zone - users in different time zones see the 'time' in their time zone, the value stored never changed.

Firebird doesn't know how to do that at the moment. Users in different time zones reading the value will see identical time representations but they are one their own to deal with the fact the time they are seeing may be from the perspective of a different time zone from their time zone. (note; from scanning their bug/ticket tracker it looks like that (support time zones) is in the works and possibly in one of the minor 3.x releases, but I didn't drill down to see if that means going to a common storage tz (UTC+0).

You can see it all at work today on your desktop. 

It took the example hsql file attached here and moved the table into a firebird odb using the Base import routine. When that is finished it looks like the data moved without change. But then I change the time zone for my OS and voila the HSQLdb and firebird ODB files show they actually have different data values in the table.  (will attach a couple of scree shots showing that)

IDK what the right answer is here - the only way to not loose the old data is to move the UTC+0 value into the firebird table, but then the display will change for all users not in the UTC+0 time zone. 

But that only accounts for some of the change that is happening - for the rest it is something else.

Created attachment 142263 [details]
draw file with screen shots

screen shots of the tables hsqldb / firebird
before and after moving data 
first via the migration assistant
then via drag drop
finally, the before and after from the drag and drop but after changing my OS timezone from New York to Reykjavik

If you look at the hsqldb code we use, inin

  workdir/UnpackedTarball/hsqldb/

I see a comment in src/org/hsqldb/HsqlDateTime.java
 
   HSQLDB uses the client and server's default timezone for all DATETIME
   operations. It stores the DATETIME values in .log and .script files using
   the default locale of the server. The same values are stored as binary
   UTC timestamps in .data files. If the database is trasported from one
   timezone to another, then the DATETIME values in cached tables will be
   handled as UTC but those in other tables will be treated as local. So
   a timestamp representing 12 noon stored in Tokyo timezone will be treated
   as 9 pm in London when stored in a cached table but the same value stored
   in a memory table will be treated as 12 noon.
 
and then if I look for the code which loads Time values, I find in
 
  src/org/hsqldb/rowio/RowInputBinary.java

that the code looks like

    protected Time readTime() throws IOException, HsqlException {
        return new Time(HsqlDateTime.getNormalisedTime(readLong()));
    }

where Time is java.sql.Time, which has a constructor like:

    /**
     * Constructs a <code>Time</code> object using a milliseconds time value.
     *
     * @param time milliseconds since January 1, 1970, 00:00:00 GMT;
     *             a negative number is milliseconds before
     *               January 1, 1970, 00:00:00 GMT
     */
    public Time(long time)

and getNormalisedTime looks like

    public static long getNormalisedTime(long t) {
        synchronized (tempCalDefault) {
            setTimeInMillis(tempCalDefault, t);
            resetToTime(tempCalDefault);
            return getTimeInMillis(tempCalDefault);
        }
    }

which, if I inline all the methods it calls, looks like:

    private static Calendar tempCalDefault = new GregorianCalendar();
    public static long getNormalisedTime(long t) {
        synchronized (tempCalDefault) {
            tempCalDefault.setTimeInMillis(t);
	         tempCalDefault.set(Calendar.YEAR, 1970);
	         tempCalDefault.set(Calendar.MONTH, 0);
 	         tempCalDefault.set(Calendar.DATE, 1);
 	         tempCalDefault.set(Calendar.MILLISECOND, 0)
            return tempCalDefault.getTimeInMillis();
        }
    }

So the answer is that the time value is UTC milliseconds, which needs to be normalised to a 0-24 hours range.

Thank you Noel for the code pointer. I don't know if the patch for time part is ok but date and datetime are even more complicated to deal.
datetime part of table of Drew's file show a difference of 1 hour or 2 hours depending on the row!
I don't know how to normalize the retrieved value here.
Adding dateTime.IsUTC = true; or dateTime.IsUTC = false; change nothing on the results.
The only thing I found is no need to use Boost (ptime/millisec) since we can use DateTime::CreateFromUnixTime to get the same result.

I think we should include some warning message in dialog box which proposes the conversion of FB. Indeed, the FB migration is not safe at all for the moment.

BTW, I think we can add DATE in the bugtitle in addition to DATETIME and TIME since DATE may also be wrong.

Let's increase the importance because it's not a corner case and it corrupts  data silently.

Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=91e6174a088b8c722e8c6d3482c1ae9a6818a7c5

tdf#117732: Mig FB, fix reading of time

It will be available in 6.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.

Drew/Robert: once the patch is on a daily build, could you provide some feedback?
Indeed, a part of the patch (requested by a dev) works but is still a mystery to me.
As indicated in the patch, it concerns TIME fields only, not DATE or DATETIME.

Tried a simple conversion with a DATE field, in the same locale (en-US), and (without the patch) 1/2/2019 was changed to 1/31/2019.

(In reply to Eike Rathke from comment #16)
> Tried a simple conversion with a DATE field, in the same locale (en-US), and
> (without the patch) 1/2/2019 was changed to 1/31/2019.

Yes it was expected. As indicated in my previous comment, the patch concerned only DATE and DATETIME.
Let's remove the target so it'll be clearer :-)
About the 2 other types, I gave a try but am stuck, see comment 13.

Comment 14 says that Julien Nabet committed a patch to master.  New
line 344 or 343 of dbaccess/source/filters/rowinputbinary.cxx reads

    nHours = (valueInSecs / 3600) + 1;

What, I wonder, is the purpose of "+ 1"?  Running this way, I observe
of tst_t:

( ) In first row (id=0), 15:10:10 is converted to 16:10:10

( ) In second row (id=1), 23:30:30 is converted to 24:30:30, and the
    migration quite reasonably stops with message:

        firebird_sdbc_error:
        *value exceeds the range for a valid time
        caused by
        'isc_dsql_execute'

Julien Nabet wrote in comment 13:

> I don't know how to normalize the retrieved value here.
> Adding dateTime.IsUTC = true; or dateTime.IsUTC = false; change
> nothing on the results.

I am getting somewhat plausible results by running LO with envvar
TZ=UTC.  For reference, with TZ=UTC, LibreOffice version 6.0.5.2
(currently in debian-buster) displays the table in the original hsqldb
file as:

    id  tst_dt                  tst_d           tst_t
    --  -------------------     ----------      --------
    0   2010-01-01 15:10:00     2010-01-01      15:10:10
    1   2010-02-01 23:30:00     2010-02-02      23:30:30
    2   2010-03-01 05:00:00     2010-03-01      05:00:00
    3   2010-04-02 03:30:00     2010-04-01      04:30:00
    4   2010-06-01 02:15:00     2010-05-31      03:15:10
    5   2010-06-30 04:00:00     2010-06-30      05:00:00
    6   2010-07-01 02:22:00     2010-06-30      03:22:22

I take this as the best indication of what is "really" in the table.

(In reply to Terrence Enger from comment #19)
>...
I must recognize I didn't use TZ=UTC, just my locale (France = UTC+1 or +2 depending on the period of the year). I remembered having test every hour and it was working. Now if it works for only my locale, feel free to patch this part since I'm quite lost about what to do here.

Drew Jensen wrote in comment 10:

> As the other issue points out a feature of HSQLdb is to store time
> values as the UTC+0 time and accounts (changes) the time
> representation supplied from the user based on the users local time
> zone, later when it supplies a time representation for the value it
> again accounts for the users local time zone - users in different
> time zones see the 'time' in their time zone, the value stored never
> changed.
>
> Firebird doesn't know how to do that at the moment. Users in
> different time zones reading the value will see identical time
> representations but they are one their own to deal with the fact the
> time they are seeing may be from the perspective of a different time
> zone from their time zone. (note; from scanning their bug/ticket
> tracker

The planning document <https://www.firebirdsql.org/en/planning-board/>
references CORE-694 <http://tracker.firebirdsql.org/browse/CORE-694>
and CORE-909 <http://tracker.firebirdsql.org/browse/CORE-909>.

> it looks like that (support time zones) is in the works and
> possibly in one of the minor 3.x releases, but I didn't drill down
> to see if that means going to a common storage tz (UTC+0).

The first README for the time zone feature, "Time Zone support (FB
4.0)",
<https://github.com/FirebirdSQL/firebird/blob/work/time-zone-support/doc/sql.extensions/README.time_zone.md>
is quite new (May, 2018), so it might be right.

The first paragraph says:

    Time zone support consists of TIME WITH TIME ZONE and TIMESTAMP
    WITH TIME ZONE data types, expressions and statements to work with
    time zones and conversion between data types without/with time
    zones.

and under the heading "SET TIME ZONE BIND statement":

    Old clients may not understand the new data types, so it's
    possible to define the bind to LEGACY and the expressions will be
    returned as TIME WITHOUT TIME ZONE and TIMESTAMP WITHOUT TIME
    ZONE, with appropriate conversion.

    The bind configuration is also applicable to input parameters.

The planning document <https://www.firebirdsql.org/en/roadmap/>
classifies time zone support as an optional feature of FB 4.0: it may
be postponed if its development does not fit the release schedule.
"Beta Release (stable enough)" of FB 4.0 was 2018-05-01, but time zone
support is still "in progress".  I see no planned date for release for
use in production.

Whether LibreOffice will move to Firebird 4.0, and when, and whether
time zone support will make it into FB 4.0, are questions far above my
pay grade.


> IDK what the right answer is here - the only way to not loose the
> old data is to move the UTC+0 value into the firebird table,

Agreed.  Especially surprising would be DATE changed by a migration
executed west of GMT.

I think it is of the utmost importance that we not *surprise* users.

> but then the display will change for all users not in the UTC+0 time
> zone.

One can imagine translating times and timestamps between GMT in the
table and local time in the UI.  This would, IIUC, duplicate the
visible behaviour of embedded HSQL for TIME and TIMESTAMP and improve
on the behaviour for DATE fields.

This is what future FB will do after we execute the statement "SET
TIME ZONE BIND LEGACY".  How many places in LibreOffice code would
have to change to do this ourselves, I wonder?

Of course, any user who issues direct SQL gets whatever she gets.  But
a view may be defined using direct SQL, so this is another opportunity
to issue a warning in advance of an unpleasant surprise.

Does anyone think that the correct way would be to use the current OS TZ informtion when the migration assistant to change the data so what the user sees in the Base UI is the same before and after migration?

I bet if that were asked to users a bunch would say yes, and some absolutely.

So that is actually the issue, isn't it. 

It is not a big deal to change the values, with a four step process, add column, move data with offset fixup, drop the old column, rename the new one to match the old. (of course there is any optional steps of dropping and recreating things like views and relations that might of used those old columns..but) - but big enough that I can easily bet that some of the users who would of voted yes above are not going to be happy about that as a solution - even with clear examples of how to do it. (which I would be willing to put together)

Let me to take the role for a minute to advocate for making the fixup final in the migration assistant it would be along this line: Firebird today doesn't understand this timezone fixup stuff, it just takes what you give it as a value and gives you back that same value. If the migration assistant leaves UTC values in the tables and the users continue to add local time values going forward now there is a real mess if ever Firebird comes out with an upgrade that starts recognizing TZ, cause now the datastore is mixed.

Let me clear up my English deficiency in the comment.

When I said it isn't a big deal to change the values I meant that as something done in the Firebird Base environment, if and, after the migration assistant moves the unchanged (stored) values from the HSQLdb tables into the Firebird tables. This would be something left to the users to decide to do or not, after the migration assistant finished and by them. It would be addressed from the project by giving details on how to do that.

Finally, as to why just use the current OS TZ information, because it is likely a good bet that the setting is not going to differ for many of the users from when they are running the migration assistant and they used the Base file being migrated. The documentation supplied about the migration assistant would need to make that clear that the data is being altered and why IMO.

I have created bug 119625 for the error message of comment 18.

(In reply to Drew Jensen from comment #22)
> Does anyone think that the correct way would be to use the current OS TZ
> informtion when the migration assistant to change the data so what the user
> sees in the Base UI is the same before and after migration?

I think it would make sense letting the user decide that. I would suggest a pop-up dialog right after the user pressed "yes, I want to migrate". This dialog would appear only if the database contains TIME, DATE, or DATETIME data types, and it would ask the user if he wants to interpret his data as (UTC+0) or he would prefer the recalculation of data using OS timezone information.

> It is not a big deal to change the values, with a four step process, add
> column, move data with offset fixup, drop the old column, rename the new one
> to match the old. (of course there is any optional steps of dropping and
> recreating things like views and relations that might of used those old
> columns..but) - but big enough that I can easily bet that some of the users
> who would of voted yes above are not going to be happy about that as a
> solution - even with clear examples of how to do it. (which I would be
> willing to put together)

I agree. A built-in solution is better (like the suggestion above) than a wiki page describing what to do.

I would suggest to create a new bug report marked as feature request with the above changes and close this bug as "won't fix" or "moved"

(In reply to Tamas Bunth from comment #25)
> (In reply to Drew Jensen from comment #22)
> > Does anyone think that the correct way would be to use the current OS TZ
> > informtion when the migration assistant to change the data so what the user
> > sees in the Base UI is the same before and after migration?
> 
> I think it would make sense letting the user decide that. I would suggest a
> pop-up dialog right after the user pressed "yes, I want to migrate". This
> dialog would appear only if the database contains TIME, DATE, or DATETIME
> data types, and it would ask the user if he wants to interpret his data as
> (UTC+0) or he would prefer the recalculation of data using OS timezone
> information.

Yes, asking the user is likely the best solution. Explaining it clearly, succinctly, will probably be the hard part. 


> 
> > It is not a big deal to change the values, with a four step process, add
> > column, move data with offset fixup, drop the old column, rename the new one
> > to match the old. (of course there is any optional steps of dropping and
> > recreating things like views and relations that might of used those old
> > columns..but) - but big enough that I can easily bet that some of the users
> > who would of voted yes above are not going to be happy about that as a
> > solution - even with clear examples of how to do it. (which I would be
> > willing to put together)
> 
> I agree. A built-in solution is better (like the suggestion above) than a
> wiki page describing what to do.
> 
> I would suggest to create a new bug report marked as feature request with
> the above changes and close this bug as "won't fix" or "moved"

Alright as an new RFE entry then.

Opened the RFE here: Migration Assistant: User dialog to set treatment of date and time values during migration
https://bugs.documentfoundation.org/show_bug.cgi?id=119675

*** Bug 124051 has been marked as a duplicate of this bug. ***

I had big problems with migrating dates in my BASE database until I deduced that all dates that had been entered with a value that would have been in UK Daylight Saving Time were out by 1 day after migration.

I got around this by creating and populating a table with three fields :-
Key field YEAR (as INTEGER) with values from 1916 to 2019
START_DATE (as DATE) containing the date of the start of DST for the year.
END_DATE (as DATE) containing the date of the end of DST for the year.

(Note that this table was complicated by the fact that the UK kept DST all year during part of WW2 and from 1968 to 1970, so some end dates were 31st December and some start dates were 1st January).

Then I wrote a macro to adjust all such dates in the remaining database tables by 1 day in the opposite direction to the direction changed by the migration.

This worked perfectly, and I have now successfully migrated to Firebird, which is definitely much faster than HSQLDB with JAVA.

I suspect that many users would have the skills to do something similar, but most users wouldn't, so a method of automatically performing this adjustment needs to be developed and incorporated into the migration.

Here another solution, tested with the database https://bugs.documentfoundation.org/attachment.cgi?id=142236

Add a varchar-field for each date, time or datetimefield:
Dat_dt VARCHAR(19)
Dat_d VARCHAR(10)
Dat_t VARCHAR(8)

Start in Tools → SQL
UPDATE "tst_data" SET "Dat_dt" = LEFT(CAST( "tst_dt" AS VARCHAR ( 30 ) ), 19 ), "Dat_d" = CAST( "tst_d" AS VARCHAR ( 10 ) ), "Dat_t" = CAST( "tst_t" AS VARCHAR ( 8 ) );

So the dates, times and datetimes will be saved in the HSQLDB as strings, which wouldn't be changed by migration.

After the migration do start Tools → SQL
UPDATE "tst_data" SET "tst_dt" = "Dat_dt", "tst_d" = "Dat_d", "tst_t" = "Dat_t";

All fields will be updated to the right date, time or datetime.