MySQL Bugs: #40092: Storage Engine API uses time

Bug #40092	Storage Engine API uses time_t datatype
Submitted:	16 Oct 2008 22:20	Modified:	14 Jul 2009 16:04
Reporter:	Vladislav Vaintroub	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: Storage Engine API	Severity:	S3 (Non-critical)
Version:	5.1	OS:	Any
Assigned to:	V Venkateswaran	CPU Architecture:	Any

Description:
This is followup to Bug#39802.

Storage engine API should use datatypes with known size, to be independent from compiler and compiler versions.

time_t does not belong here. One compiler can decide to make it 32 bit, another  compiler 64 bit. The size can be influenced by compiler flags, as in case of new Visual Studio. The size can change from one compiler release to another, as also in Visual Studio case.

How to repeat:
The following three are part of class ha_statistics:

  time_t create_time;			/* When table was created */
  time_t check_time;
  time_t update_time;

Suggested fix:
use int32 (or int) , or use int64 (long long), or even use intptr_t. Size of those datatypes is predictable, size of time_t not.

I'd raise the point that this is designed to be struct stat type data (if not just a cache of fstat data). For engines that store table in file (e.g. myisam), it just copies the stat info across.

So we should either be consistent with struct stat on the platform (so the copy is simple), remove these from the handler interface (which I'm not convinced is a bad idea) or have our own arbitrary time type (not a good idea).

So i'm not convinced this really is a bug.... if your OS isn't going to deal with 2039, then neither is the mysqld :)

Stewart, struct stat != struct stat on some platforms;)

Here is a full zoo if stat http://msdn.microsoft.com/en-us/library/14h5k7ff.aspx, depending on whether ot not they can handle 64 bit file sizes and return time in 32 or 64 bit.It produces strange creatures stat32i64 or stat64i32.

But the point was really that: suppose pluggable storage engine exists on Windows it is external API. People start writing DLL that is dynamically loaded into server, as they can do today on Unix.

Suppose further, somebody uses Visual Studio 2003 and somebody will use VS2005.
The definition of time_t differs in those 2 compilers (one has 32 bit and another 64 bit). Dependent on how server was compiled, one guy will loose and his storage engine will crash.

It might be different on unixes, and time_t is always the same as size_t and struct stat is absolutely portable. On Windows, it is a matter of compiler and its flags.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/60804

2729 V Narayanan	2008-12-06
      Bug#40092 Storage Engine API uses time_t datatype
      
      Change the usage of time_t in the Storage engine API
      to datatypes with known size.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/62806

2728 V Narayanan	2009-01-09
      Bug#40092 Storage Engine API uses time_t datatype
            
      Change the usage of time_t in the Storage engine API
      to datatypes with known size.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/63081

2962 V Narayanan	2009-01-13
      Bug#40092 Storage Engine API uses time_t datatype
      
      Change the usage of time_t in the Storage engine API
      to datatypes with known size.

Pushed into 6.0.10-alpha (revid:joro@sun.com-20090119171328-2hemf2ndc1dxl0et) (version source revid:timothy.smith@sun.com-20090114143745-x2dvnmix6gjlt6z6) (merge vers: 6.0.10-alpha) (pib:6)

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/73143

2880 Narayanan V	2009-04-30
      Bug#39802 On Windows, 32-bit time_t should be enforced
      Bug#40092 Storage engine API uses time_t datatype
      
      Change the usage of time_t in the storage
      engine API to datatypes with known size
     @ sql/handler.h
        Bug#39802 On Windows, 32-bit time_t should be enforced
        Bug#40092 Storage engine API uses time_t datatype
        
        Changed create_time, check_time, update_time in the
        ha_statistics and PARTITION_INFO structures to ulong.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/73148

2880 Narayanan V	2009-04-30
      Bug#40092 Storage engine API uses time_t datatype
      
      Change the usage of time_t in the storage
      engine API to datatypes with known size.
     @ sql/handler.h
        Bug#40092 Storage engine API uses time_t datatype
        
        Changed create_time, check_time, update_time in the ha_statistics and PARTITION_INFO structures to ulong.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/73151

2880 Narayanan V	2009-04-30
      Bug#39802 On Windows, 32-bit time_t should be enforced
      Bug#40092 Storage engine API uses time_t datatype
      
      Change the usage of time_t in the storage
      engine API to datatypes with known size
     @ sql/handler.h
        Bug#39802 On Windows, 32-bit time_t should be enforced
        Bug#40092 Storage engine API uses time_t datatype
        
        Change create_time, check_time, update_time
        in the ha_statistics and PARTITION_INFO
        structures to ulong.

Please disregard commits made on Apr 30th

Internal change only. No changelog entry required.

no, this isn't "internal".
Suggested changelog entry: "Storage engine plugins on windows could've been built with a definition of time_t which was different from the server expectations. It would cause affected plugins to crash."

A note has been added to the 6.0.10 changelog: 

Storage engine plugins on Windows could've been built with a definition of time_t which was different from the server expectations. The difference could cause affected plugins to crash. In addition, the use of the time_t type in the storage engine API layer has been enforced.