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

[jrmu <jrmu@xxxxxxxxxx>]

So my proposed diff would be:

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

If this seems ok, I'll send a patch also for botnow_db_fixer.pl

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

On Sat, Jun 17, 2023 at 11:10:57PM -0700, jrmu wrote:
> 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