Page MenuHomePhabricator

waste through varchar in database- use CHAR not DATETIME
Closed, DeclinedPublic

Description

Author: tomk32

Description:
all timestamps should be type char and not varchar.


Version: 1.5.x
Severity: normal
OS: Linux
Platform: PC
URL: http://dev.mysql.com/doc/mysql/en/datetime.html

Details

Reference
bz1855

Event Timeline

bzimport raised the priority of this task from to Lowest.Nov 21 2014, 8:19 PM
bzimport set Reference to bz1855.
bzimport added a subscriber: Unknown Object (MLST).

(In reply to comment #0)

all timestamps should be type char and not varchar.

At the moment, all timestamps for 1) ENotif email notification
http://bugzilla.wikipedia.org/show_bug.cgi?id=454 and 2) EAuthent e-mail address
authentication http://bugzilla.wikipedia.org/show_bug.cgi?id=866 _uses_ indeed
varchar(14), because I currently need to use more values than timestamp and NULL.

However, I fully admit, that the use of varchar can and need to be changed
(which I will take care of in due time for the 454 and 866 ).

I added dependencies in order to track the bugzilla.

Tom - for ENotif and EAuthent

they should not be chars, they should be timestamps ;-) anyway, 1.5 has the
timestamp code and switching all chars/varchars into timestamp is possible on
transition...

eh, mea culpa, mea maxima culpa, not timestamps, DATETIME's :)

(In reply to comment #2)

they should not be chars, they should be timestamps ;-) anyway, 1.5 has the
DATETIME code and switching all chars/varchars into timestamp is possible on
transition...

Domas:

What you said is (currently) not okay for ENotif - I discussed this two days ago
with Brion.

Somone (might be you) has fully broken my code in CVS HEAD without saying me
this by adding $dbr->timestamp() all over (which broke my code, because it also
use 0 and 1 for certain actions.

So my next steps are:

  • to restore ENotif and EAuthent in CVS HEAD (and REL1_4 locally) by restoring

my way of implementation including all your and Brion's valuable suggestions !

I personally do not like, if other persons "wrap" database call without exactly
knowning what they potentially break (like in the
http://bugzilla.wikipedia.org/show_bug.cgi?id=454 case) and without contacting
the owner of the bug (here: me)

Tom

river wrote:

WONTFIX. there's no point changing varchar to char when we could just
correctly move to datetime.

The use of CHAR [sic] has the advantage to be independent of UNIX TIMESTAMP
rollover on 03:14:07 Tuesday, January 19, 2038 (UTC).
That is, why I propose NOT to change to DATETIME and to stick to CHAR (instead
of VARCHAR ).

If someone knows better, pls. reply.

river wrote:

If MySQL hasn't added 64-bit time_t support in the next 33 years, we're
going to have more problems than MediaWiki not working.

tomk32 wrote:

Thanks Tom for adding the links, maybe I should have added earlier that varchar
wastes a byte for the length information. I don't know how many entries are in
the db right now, but it should be a few megabytes that are wasted.

I'm not exactly sure how mysql reads the entries, but a fixed width is always a
performance advantage to a variable width.

I made a survey about the useful datatypes, the survey being missing in this
bugzilla
Storage requirements http://dev.mysql.com/doc/mysql/en/storage-requirements.html

VARCHAR(14) 15 bytes ***
CHAR(14) 14 bytes * proposed by Thomas R. Koll
DATETIME 8 bytes
TIMESTAMPS 4 bytes

  • currently used by Mediawiki software

Roadmap for this:

I need to know

  • where to go ?
  • What datatype to be used in the future ?

OKAY, very good.

I have updaters.inc ready for ENotif and EAuthent.
The code goes like that and can be adapted for converting other internal mediawiki timestamps.

$meta = $wgDatabase->fieldInfo( 'watchlist', 'wl_notificationtimestamp' );
if ($meta->type != 'datetime') {
echo "ENOTIF: Converting wl_notificationtimestamp field to datetime datatype.\n";

/* ALTER TABLE /*$wgDBprefix*/watchlist MODIFY (wl_notificationtimestamp datetime); - convert on-the-fly */
dbsource( "maintenance/archives/patch-email-notification-changetype.sql", $wgDatabase );
} else {
echo "no database changes needed.\n";
}

I propose to add in database.php

 	/** 
	 * Compatibility function 
	 * Converts a timestamp in old format into the new Datetime format 
	 * in:  $ts string char(14) YYYYMMDDHHMMSS 
	 * out: $ts string in datetime format YYYY-MM-DD HH:MM:SS 
	 */

function convTimestampChar14ToDatetime( $ts ) {

		if (strlen($ts) != 14) { 
			return '0000-00-00 00:00:00'; 
		} else { 
			return substr($ts,0,4).'-'.substr($ts,4,2).'-'.substr($ts,6,2).'

'.substr($ts,8,2).':'.substr($ts,10,2).':'.substr($ts,12,2);

		}

}

/**

  • Compatibility function:
  • Converts a timestamp in old format into the new Datetime format
  • in: $ts string char(14) YYYYMMDDHHMMSS
  • out: $ts string in datetime format YYYY-MM-DD HH:MM:SS
	 */

function convTimestampChar14ToDatetime( $ts ) {

		if (strlen($ts) != 14) {  
			return '0000-00-00 00:00:00';  
		} else {  
			return substr($ts,0,4).'-'.substr($ts,4,2).'-'.substr($ts,6,2).' '.  
                           substr($ts,8,2).':'.substr($ts,10,2).':'.substr($ts,12,2);  
		}

}

This function is needed inter alia when you compare timestamps in varchar14 format YYYYMMDDHHMMSS to timestamps in the DATETIME
format, which comes as YYYY-MM-DD HH:MM:SS

unless we use in every SELECT statement retrieving timestamp value the (database-dependent) DATE_FORMAT MySQL function.

avarab wrote:

We already have a function for this if I understand what you're trying to do
correctly, take a look at wfTimestamp().

(In reply to comment #14)

We already have a function for this

You are right, function wfTimestamp(TS_MW, $ts) is exactly, what I need.

My comment #13 is misleading and obsolete.
Tom