= The feature

The feature generalizes all the possible ways to set the default value, and to update automatically the value of both TIMESTAMP and DATETIME fields. It makes the clauses DEFAULT CURRENT_TIMESTAMP and ON UPDATE CURRENT_TIMESTAMP applicable to both TIMESTAMP and DATETIME fields, and allows to specify the default value as well as the update value on any TIMESTAMP and DATETIME field in the same table.

For full usability, the feature should be paired together with the system variable sysvar_explicit_defaults_for_timestamp from MySQL 5.6.6 that deprecates the implicit default values of TIMESTAMP fileds, and results in NULL behavior typical for other data types.

Therefore from feature perspective, this seems to useful. Backporting this feature will also allow MariaDB to be compatible in this respect with future MySQL versions.

= The code

The part of the diff related to code changes is 6096 lines, however only 4697 lines are related to changes in the server, the rest is unit testing code. Unfortunately until looking at this code I was totally unfamiliar with this area, therefore my analysis is not very deep.

From what I could tell by reading the patch:

• it seems to be self-contained. I tracked several new/changed data structures, and found them to be defined in the same patch. The patch performs some refactoring, for instance it transforms struct st_copy_info into a class COPY_INFO. There also are some minor refactoring/formatting changes.

• it doesn't change the FRM format (at least there are no changed or new enum values in "utype").

• it doesn't modify the parser (the YACC file)

Backporting the patch mechanically should be possible in 2-3 days, then we need to allocate time to backport the test cases, and to test the final result. With my current background I am not able to tell if there is a simpler and/or more ellegant way to implement this functionality.

Potential problems:

• the patch has to scan all fields to find the ones that are automatically update, can be high overhead if many fields
• use an additional array of fields to be autoupdated

Second opinion after a first attempt for a backport.

In summary: the MySQL patch cannot be used as it is.

Problems with the MySQL patch:

• The implementation of how timestamp values are updated is based on MySQL's WL# 946 (fractional seconds precision), while MariaDB has its own implementation in 5.5 - MWL#173. We need to use our own implementation.

• The new static method Item_func_now_local::store_in() is very strange, it should be either a Field method, or a C function.

• There is some big refactoring of struct COPY_INFO into a class that needs to be revised further how much of it is needed. This refactoring is tied to other changes I have not fully understood if they are required for this functionality, or they are some form of refactoring of related code.

• The set of fields that need to be auto updated is filtered by a bitmap that allows to represent the subset of all fields that need to be autoupdated. This requires a loop over all table fields for each update operation. As suggested by Monty, normally there aren't many timestap/datetime fields in a table. It will be much more efficient (especially for tables with many fields) to keep an explicit list of the fields instead of a bitmap. Not using a bitmap will also make the implementation shorter because it will not require the changes/additions to bitmaps done by MySQL.

• There also are quite a few:
• cosmetic changes that are irrelevant to the functionality,
• comments that are irrelevant, and others that need to be verified

Therefore I suggest that the backport is done it several steps:

• First make the changes necessary to construct the list of all fields for auto-updating and auto-inserting, and remove the old code. This change may follow more or less the changes from the MySQL patch.
• Next implement the logic to update a Field with the current timestamp.
• Finally introduce the update logic everywhere updates/inserts are done. Consider all places where MySQL had to change/add the same logic.

Current state:

On hasky:/tmp there is an archive mdev-452-mysql-timestamp.tar.gz with the files

mysql-wl-5874.diff - original MySQL patch
mysql-wl-5874-part0-code.diff - the part of the patch that actually implements the feature
mysql-wl-5874-part1-test-cases.diff - the part with test cases
mysql-wl-5874-part2-unittest.diff - the part with unit test changes
mysql-wl-5874-part0-code.working.diff - a copy of the code patch with my comments, and deleted those lines that were ported
md452.test - a small test case useful for debugging
mdev-452.v01.diff - my changes over 10.0-base that implement the first step of the feature - remove the limitation of a single timestamp column.

Suggestion how to implement the feature in MariaDB.

• Add a DYNAMIC_ARRAY to TABLE_SHARE that would contain all the fields that need to be auto-updated/initialized. Serg proposed that this array should contain Field indexes, and not actual Field pointers.
• When a field subject to auto-update/insert is created, it can add itself to this array.
• One thing that needs care, is to take into account during actual INSERT/UPDATE the fields that have an explicit value. Those fields should be updated with that value instead of the automatic timestamp.
• Make sure that our behavior is compatible with MySQL's changes to Create_field::init()
• For the actual update of the timestamp value reuse the implementation Field::set_time. Consider implementing this method for the remaining datetime Field types that can be auto-updated.

The above is in contrast with MySQL, where the set of fields to be auto-updated/inserted is determined by a loop over all table fields for each INSERT/UPDATE operation.
In MySQL the set of auto-update fields is kept in a bitmap over all table fields.

We can in the .frm table store the number of fields that have an init function/timestamp.
This allows us to allocate exactly as many fields we really need.
If this is an old table from MySQL we can allocate max amount of fields. 2 bytes / field is not that much for for old tables.

We need also to think how we continue this task and handle arbitrary init expression. Should not be that hard as we can reuse the virtual columns code for this.

I wondering if it is a good idea to store in .frm file things that can be easily calculated when opening the table.

Experience with virtual columns (lots of small bugs here-and-there, and finally - not much use in production so far) speaks against implementing anything else of this sort.

Changing Due date since Timour is on vacation. Will talk to Monty about it since this task was initiated by him.

Following Serg's advice, I reverted the change:
http://bazaar.launchpad.net/~maria-captains/maria/5.5-serg/revision/2502.567.13
Because MySQL fixed the problem in bug https://mariadb.atlassian.net/browse/MDEV-438
(Microseconds: Precision is ignored in CURRENT_TIMESTAMP(N) when it is given as a default column value)
in a different way. In order for MariaDB to be compatible with MySQL, and pass MySQL's regression tests,
the above fix was removed.

Since this feature is reimplemented, because we thought that MySQL's implementation is both too complicated, and not efficient, it is quite interesting to compare the two implementations code wise.
Since we didn't backport MySQL's unit tests, below I provide statistics of the MySQL patch without the unit tests. This makes both patches fully comparable via diffstat.

MariaDB:

• 61 files changed, 5376 insertions, 541 deletions
• total new lines: 4835

MySQL:

• 59 files changed, 6916 insertions, 989 deletions
• total new lines: 5927

I plan to add some more comments, and do some more code cleanup, so the number above may change a bit, but not likely too much.

Reassigned to Serg for review.
The code is pushed to lp:~maria-captains/maria/10.0-mdev452.

This MySQL bug describes the problem with LOAD semantics wrt default functions/values:
http://bugs.mysql.com/bug.php?id=14770

Pushed to 10.0-base.