[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] default values for localtime column, better-fitting types


For reference, here is IanJ's patch:

---
 table.sql | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/table.sql b/table.sql
index 1b71f51..d9fb362 100644
--- a/table.sql
+++ b/table.sql
@@ -7,7 +7,7 @@ CREATE TABLE bnc (
   username VARCHAR(32),
   email VARCHAR(100),
   password VARCHAR(100),
-  localtime VARCHAR(100),
+  localtime default current_timestamp,
   captcha INTEGER
 );
 CREATE TABLE shell (
@@ -19,7 +19,7 @@ CREATE TABLE shell (
   username VARCHAR(32),
   email VARCHAR(100),
   password VARCHAR(100),
-  localtime VARCHAR(100),
+  localtime default current_timestamp,
   captcha INTEGER
 );
 CREATE TABLE mail (
@@ -31,7 +31,7 @@ CREATE TABLE mail (
   username VARCHAR(32),
   email VARCHAR(100),
   password VARCHAR(100),
-  localtime VARCHAR(100),
+  localtime default current_timestamp,
   captcha INTEGER
 );
 CREATE TABLE www (
@@ -73,7 +73,7 @@ CREATE TABLE irc (
   identified INTEGER,
   ctcpversion VARCHAR(100),
   ctcptime VARCHAR(100),
-  localtime VARCHAR(100),
+  localtime default current_timestamp,
   oper INTEGER,
   idle INTEGER,
   ssl INTEGER,
--
2.37.3

I would prefer to have human readable strings stored in sqlite because
personally, I cannot remember all the SQL functions for conversion to human
readable strings, and I suspect other teammates would forget them as well.

The cost of storing an extra string is not worth worrying about, we have
plenty of cheap storage available.

However, I just realized that the irc table is already storing epochtime in
its localtime field

You can see this if you run the sql query: select localtime from irc;

The confusion caused by using different types with the identical variable
name localtime is going to cause a lot of confusion (it's confusing me even
now). So, I would recommend adding an additional field to the schema, maybe
timedate or humantime, which should be used for storing a human readable
string, and localtime can be used for storing the epochtime.

Would that be satisfactory to everyone? If not, please object soon,
otherwise I'm going to write a patch to do this.

-- 
jrmu
IRCNow (https://ircnow.org)

On Fri, Jun 02, 2023 at 02:12:05AM +0200, freeirc support wrote:
> adds a default value, in the same format as before (unix timestamp),
> so that complicated and failable migration scripts are not required.
> 
> we use an INTEGER because sqlite does not have a special type for
> timestamps. "current_timestamp" and similar are stored as large
> and inefficent TEXT (a UTF-8 or 16 string).
> 
> note that due to sqlite's weak typing, changing the column's declared
> type does not actually do much: it was already stored as an INTEGER
> to begin with, despite the declared VARCHAR type. changing this
> will just make the schema less misleading and confusing to those
> unfamiliar with sqlite's oddities.
> 
> we cannot give a default value to the date column, as botnow
> calculates the value with respect to the current timezone. despite
> the name, localtime does not consider the local timezone; this is
> unchanged from previous behavior.
> ---
>  table.sql | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/table.sql b/table.sql
> index 1b71f51..f0e4568 100644
> --- a/table.sql
> +++ b/table.sql
> @@ -7,7 +7,7 @@ CREATE TABLE bnc (
>    username VARCHAR(32),
>    email VARCHAR(100),
>    password VARCHAR(100),
> -  localtime VARCHAR(100),
> +  localtime INTEGER DEFAULT (unixepoch()),
>    captcha INTEGER
>  );
>  CREATE TABLE shell (
> @@ -19,7 +19,7 @@ CREATE TABLE shell (
>    username VARCHAR(32),
>    email VARCHAR(100),
>    password VARCHAR(100),
> -  localtime VARCHAR(100),
> +  localtime INTEGER DEFAULT (unixepoch()),
>    captcha INTEGER
>  );
>  CREATE TABLE mail (
> @@ -31,7 +31,7 @@ CREATE TABLE mail (
>    username VARCHAR(32),
>    email VARCHAR(100),
>    password VARCHAR(100),
> -  localtime VARCHAR(100),
> +  localtime INTEGER DEFAULT (unixepoch()),
>    captcha INTEGER
>  );
>  CREATE TABLE www (
> @@ -73,13 +73,13 @@ CREATE TABLE irc (
>    identified INTEGER,
>    ctcpversion VARCHAR(100),
>    ctcptime VARCHAR(100),
> -  localtime VARCHAR(100),
> +  localtime INTEGER DEFAULT (unixepoch()),
>    oper INTEGER,
>    idle INTEGER,
>    ssl INTEGER,
>    epochtime INTEGER,
>    chans VARCHAR(200),
> -  date VARCHAR(100)
> +  date INTEGER
>  );
>  CREATE TABLE smtp (
>    id INTEGER PRIMARY KEY,
> @@ -89,7 +89,7 @@ CREATE TABLE smtp (
>    deliveredto VARCHAR(100),
>    received VARCHAR(1000),
>    dkim VARCHAR(1000),
> -  date VARCHAR(100),
> +  date INTEGER,
>    other VARCHAR(5000),
>    content VARCHAR(100),
>    mime VARCHAR(100),
> -- 
> 2.35.1
> 

Attachment: signature.asc
Description: PGP signature


References:
[PATCH] default values for localtime column, better-fitting typesfreeirc support <support@xxxxxxxxxxx>