Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Schema: browser_session: let the code insert authenticated_at #195

Closed
Al2Klimov opened this issue Jun 3, 2024 · 7 comments · Fixed by Icinga/icinga-notifications#215
Closed
Assignees

Comments

@Al2Klimov
Copy link
Member

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
Copy link
Member

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
Copy link
Member Author

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

Since which MariaDB version does this work?

@ncosta-ic
Copy link
Member

ncosta-ic commented Jun 10, 2024

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
Copy link
Member Author

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

@ncosta-ic
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants