Icinga / icinga-notifications-web

Icinga Notifications Web — Manage incidents and who gets notified about them how and when
GNU General Public License v2.0
9 stars 0 forks source link

Schema: browser_session: let the code insert authenticated_at #195

Closed Al2Klimov closed 1 month ago

Al2Klimov commented 1 month ago

The browser_session.authenticated_at DEFAULT has to go, as it's not portable to MySQL (https://github.com/Icinga/icinga-notifications/pull/203/files#r1624022883).

ncosta-ic commented 1 month ago

The browser_session.authenticated_at DEFAULT has to go, as it's not portable to MySQL (https://github.com/Icinga/icinga-notifications/pull/203/files#r1624022883).

How is it not portable? As you can see, it can be ported: https://onecompiler.com/mariadb/42ftjgf9c And this right here would be the PostgreSQL implementation: https://onecompiler.com/postgresql/42ftjrbzd

For the MariaDB variant, the timestamp creation was taken from Stackoverflow as this snippet properly calculates the UTC unix timestamp with millisecond precision, without daytime-saving derivations.

Al2Klimov commented 1 month ago

As you can see, it can be ported: https://onecompiler.com/mariadb/42ftjgf9c

Since which MariaDB version does this work?

ncosta-ic commented 1 month ago

As you can see, it can be ported: https://onecompiler.com/mariadb/42ftjgf9c

Since which MariaDB version does this work?

5.2 throws as there's no support for precision on the UTC_TIME command. It still works with just UTC_TIME() but the last three digits are equal to zero. 5.3 runs it fine:

Your MariaDB connection id is 1
Server version: 5.3.12-MariaDB-mariadb122~wheezy (MariaDB - http://mariadb.com/)

Copyright (c) 2000, 2012, Oracle, Monty Program Ab and others.

Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.

MariaDB [(none)]> SELECT UNIX_TIMESTAMP()*1000+FLOOR(MICROSECOND(UTC_TIME(3))*0.001);
+-------------------------------------------------------------+
| UNIX_TIMESTAMP()*1000+FLOOR(MICROSECOND(UTC_TIME(3))*0.001) |
+-------------------------------------------------------------+
|                                               1718031502718 |
+-------------------------------------------------------------+
1 row in set (0.00 sec)
Al2Klimov commented 1 month ago

DEFAULT (UNIX_TIMESTAMP()*1000)) works since MariaDB 10.2, is this desired?

ncosta-ic commented 1 month ago

DEFAULT (UNIX_TIMESTAMP()*1000)) works since MariaDB 10.2, is this desired?

No, as this only saves it with a precision to a second. We use the unix timestamp with millisecond precision on everything related to the icinga-notifications module. The whole default expression works starting with MariaDB 5.3. For every system with a version below 5.3, it would work with UNIX_TIMESTAMP()*1000+FLOOR(MICROSECOND(UTC_TIME()*0.001) but since we are loosing the millisecond precision anyways, that could then be simplified down to UNIX_TIMESTAMP()*1000

However you should keep in mind that MariaDB 5.2 went EOL on the 15/11/2015. I really do not think we should even try to support such old systems, but that might just be me. What would be your opinions about that, @nilmerg @julianbrost ?

Al2Klimov commented 1 month ago

What I just wanted to express is that even the simpler DEFAULT (UNIX_TIMESTAMP()*1000)) doesn't work in MariaDB 10.1 yet, which is supported by Icinga DB. IMAO Web should just insert the ts w/ whatever precision is desired.

ncosta-ic commented 1 month ago

I wasn't aware of the fact that you were talking about MariaDB 10.1 specifically. Even though all of the functions are properly supported since version 5.3, default non-constant values like expressions and functions were only introduced with 10.2 (https://mariadb.com/kb/en/create-table/#default-column-option):

Before MariaDB 10.2.1 you couldn't usually provide an expression or function to evaluate at insertion time. You had to provide a constant default value instead. The one exception is that you may use CURRENT_TIMESTAMP as the default value for a TIMESTAMP column to use the current timestamp at insertion time.

But again, 10.1 has been EOL since the 10/17/2020... I am happy to implement the insertion of the date in our PHP daemon code, but we really should let the database take care of it, starting with 10.2, as there's lots of room for error when working with database code libraries, adapters, timezones and collation settings.