[srirampatil]

MXS-2: Week 4

This post is a part of a series of posts that I am writing weekly while fixing MXS-2 issue for MaxScale as part of Google Summer of Code - 2015.

I made a number of changes this week. First of them all was changing the HASHTABLE in BGD_SESSION. Previously, it was used for mapping file name to file handle, but, it needs to store much more information than that. For this purpose, I added a new structure named table_info. The commit with these changes can be sen here.

Followed a small tweak where, me and my mentor(Markus) decided to eliminate the regular expressions from the filter all togther. Since, the filter expects a list of tables for which the data should be logged, we introduced a parameter named tables which expects a list of comma separated unique table names. Here, unique table names signify the format <dbname>.<table name>. Only those tables which are specified in this list are processed and others are ignored. The changes for this are spread over two commits which can be observed here and here.

I also made a small optimization where I avoid generating the data file name over and over again by storing it in a new variable in BGD_SESSION named current_table_data_file. The changes can be seen here.

For extracting data from CREATE TABLE command, I needed to store the schema and column definitions. query_classifier deals with extracting data from queries. So two new structures were introduced namely TableSchema and ColunDef. Their definitions are as follows,

typedef struct column_def ColumnDef; typedef struct table_schema { char *dbname; // database name char *tblname; // table name int ncolumns; // number of columns ColumnDef *head; // head of list of columns ColumnDef *tail; // tail of list of columns } TableSchema; struct column_def { enum enum_field_types type; // columns data type char *colname; // column name void *defval; // default value ColumnDef *next; // next column };

Currently, TableSchema stores a linked list of ColumnDef. I intend to change it in next week with something which will give a little better mapping and access. A new function in query_classifier named skygw_get_schema_from_create extracts the required data from CREATE TABLE queries and returns a TableSchema object. bgdfilter handles the QUERY_OP_CREATE_TABLE in clientReply. All the changes for this can be reviewed here.

I had not handled a case in my implementation until now, wher a db name is specified in the query, e.g. CREATE TABLE test.t1(c1 int). The db name should be taken from the query in this case. So, I changed current_db and new_db in BGD_SESSION to default_db and active_db. active_db will always hold the db name for current query in execution whereas default_db will hold the db name specified in mysql command on terminal while connecting to MaxScale or the db name after USE DATABASE command. Change with respect to this can be seen here.

I still have a few tasks pending, they are as follows,

Creating a metadata file for tables

Logging data from INSERT queries

Making use of the "format" parameter for data logging. (JSON/XML)

Feel free to point out mistakes, if any. Also, suggestions and/or reviews are welcome!

[srirampatil]

MXS-2: Week 2 and 3

This post involves all the changes I made in past two weeks.

As mentioned in the previous post, the options parameter now expects a path of the directory in which all the data files should be stored. For creating directory and checking all the related errors, I have added a function named create_dir. With this, the file pointer in the BGD_INSTANCE was no longer required. A new variable named current_db has been added in BGD_SESSION which will be required for naming the data files. All these changes can be reviewed here.

We have finalized the naming convetion for data files. The data files will be named as <database>.<table>.data. For example, if we have a database named test and a table inside it named t1, then the data file would be named as test.t1.data. It will be stored in the directory provided by options parameter. Now, we can have multiple files open in a single session. I used HASHTABLE for mapping file names to their respective file descriptors. It is added in BGD_INSTANCE. Since, we do not have a closeInstance or freeInstance method, it is not possible to decide when and where to close these file descriptors. Closing them in closeSession or freeSession is not possible as these descriptors are shared by multiple sessions under single instance. To manage this, a static linked list of instances is created which is freed only when MaxScale goes down. The code changes for this can be reviewed here.

To make the code a little modular, log_insert_data function was added. I plan to write a generic function to log the data from any query, if possible. With this I also discoveed that the data should not be logged if there is some error when executing the query. So, instead of logging data from routeQuery, which was done before, it is logged from clientReply function only if the query is executed successfully. Two new filter function were added for this functionality to work, namely, setUpstream and clientReply. Also, the query buffer (GWBUF) is stored in BGD_SESSION to process it in clientReply. These changes can be reviewed here.

To keep current_db updated, I had to handle USE DATABASE query. Its query type byte is set to MYSQL_COM_INIT_DB. A variable named, new_db is added in BGD_SESSION for updating current_db only if it is changed successfully. These changes can be reviewed here.

I am yet to retrieve data from INSERT query. I ran into a lot of problems when trying to do so. In this week, I am targetting to complete following functionalities

Retrieving data from INSERT query

Make use of match regex or specify tables names directly. (To be decided)

Create data files when CREATE TABLE is executed.

Feel free to point out mistakes, if any. Also, suggestions and/or reviews are welcome!

[srirampatil]

MXS-2: Week 1

For the first week my target was to write a simple filter for MaxScale which will log only INSERT queries in a file. I have named the filter as bgdfilter for now which stands for BigData Filter. Following are filter INSTANCE and SESSION data structures

typedef struct {    char *format;   /* Storage format JSON or XML (Default: JSON */    char *path;     /* Path to a folder where to store all data files */    char *match;    /* Mandatory regex to match against table names */    regex_t re;     /*  Compiled regex text */    FILE *fp;    char *filebase;

} BGD_INSTANCE; typedef struct { DOWNSTREAM down; int active; } BGD_SESSION;

A few of these fields are not used for this simple filter. Also, these are not final data structures, I will keep on updating them as the features are implemented. Also the path variable currently represents the log file.

I have forked MaxScale repo and created a new branch named MXS-2 from develop branch. My latest code changes can be seen here.

For checking the bgdfilter, configuration can be written as

[BGD] type=filter module=bgdfilter [RWSplitRouter] type=service router=readwritesplit servers=master,s1,s2,s3 user=maxuser passwd=C8315EB77701CED103285274D0E022FB max_slave_connections=100% localhost_match_wildcard_host=1 filters=BGD

The path parameter is optional. By default the log file is created at /tmp/bgd.

Next up is retrieving data from INSERT queries and writing it into data files. Also, will decide a naming convention for data files.

Feel free to point out mistakes, if any. Also, suggestions and/or reviews are welcome!

[srirampatil]

MaxScale Installation and Configuration

I have started working with MariaDB as part of Google Sumer of Code 2015. I will be working on MaxScale issue MXS-2. The first step before starting work was to install and configure MaxScale. This post will give a general idea about the MaxScale configuration.

MaxScale works with almost all versions of MariaDB (>= 5.5 and <= 10.0, Galera) and MYSQL. Following are the steps and reqources I followed for configuring MariaDB and MaxScale

Running multiple MariaDB instances on single machine. Using this blog I configured 4 instances on my machine. It also provides a simple script for the same.

Setting up master-slave replication for MariaDB. I configured one master and three slaves on single machine. There are two resources for setting up the replication - MariaDB official Blog post and Ivan’s blog.

It is better to install MaxScale from source code. The official documentation for building MaxScale from source code gives all the requirements for several Linux distributions. I am using Ubuntu 14.04 and apart from the listed packages in the documentation, I had to install libcurl4-openssl-dev and libpcre3-dev for running cmake.

And again for configuring MaxScale, the official blog post and Ivan’s blog are sufficient. One can also check if it is working fine by executing example commands mentioned in the blogs.

All the above links give enough instructions on how to get started with MariaDB replication and setting up MaxScale. Apart from these, following is some useful stuff that I discovered while setting up.

To check replication error (if any) one can use show slave status\G command on slave database server. The error field will be empty if replication setup is fine.

Always refer “localhost” by a loopback address (e.g. 127.0.0.1) in configuration files as well as when specifying host to connect to in mysql command.

User localhost_match_wildcard_host=1 when specifying router service. This allows MaxScale to match ‘%’ with ‘localhost’.

Turn on debugging to spot errors in MaxScale setup. Add the log flags in configuration file under [maxscale] section. The log flags are log_trace, log_messages and log_debug. Set all these to 1. The log files are stored in folder $MAXSCALE_HOME/log.

Feel free to point out mistakes, if any. Also, suggestions and/or reviews are welcome! 

[srirampatil]

Adding support for OR REPLACE, IF NOT EXISTS and IF EXISTS (Pencils down)

Hello All,

Yesterday was the the hard 'pencils down' date for Google Summer of Code'2014. If you have been following my blog, you would know that I had been working on adding support for OR REPLACE, IF NOT EXISTS and DROP...IF EXISTS commands into MariaDB for all objects. MDEV-5359 gives details about the project. This blog post is to summarize the work I have done in last three months.

I started with studying the MariaDB code base. I started off with bazaar and launchpad but immediately moved to github repo as I am more comfortable with git. Also, my projects changes will be applied to MariaDB version 10.1, so it allowed me to switch to github.

After getting into the coding standards and the work flow of the "query execution" in MariaDB in the first week. I picked up the most simple command of them all which required very little code changes (at least this is what I thought at that time), CREATE OR REPLACE DATABASE.

At the end of second week I was all into coding. I thought of writing blog posts every week to keep track of what I do each week and my mentor, Alexander, also gave a thumbs up to that. Believe me, writing blog was the first most important decision I took, this helped me a lot. The response that I got for my blog was also great. Many developers from MariaDB mailing lists joined in and gave their inputs about what should be there in the blog and what should not be there. So, thanks to all of them, my blog got better and better.

Again, making use of the blog that I have written and simplifying my job, following are the links to the work I have done each week starting from first week to the last. Please, visit them all and get yourself acquainted with all the changes that I have made.

CREATE OR REPLACE DATABASE, CREATE USER/ROLE/SERVER IF NOT EXISTS and DROP USER/ROLE IF EXISTS

CREATE PROCEDURE/VIEW IF NOT EXISTS

CREATE TRIGGER/FUNCTION(STORED and UDF) IF NOT EXISTS, DROP FUNCTION(UDF) IF EXISTS

All about bin logging

Fixed MDEV-6409

CREATE OR REPLACE SERVER/TRIGGER

CREATE OR REPLACE PROCEDURE/FUNCTION(STORED)/EVENT

CREATE OR REPLACE FUNCTION(UDF)/USER/ROLE, CREATE DATABASE IF NOT EXISTS and permission checks

Test cases and bug fixes

CREATE OR REPLACE INDEX, Replication test cases and bug fixes

I have also made some changes in the last week which can be checked on my repository. You can find my repository here.

Ending this, I would like to thank everyone for allowing we to work on such an amazing project, and everyone who helped me out, on IRC, on the mailing lists, and everywhere else. :)

Thanks a lot!

[srirampatil]

Week Twelve!

Update: CREATE OR REPLACE INDEX implemented. Needs review and some small changes. You can check the commit here.

The test cases phase continues from the previous week. I have written test cases for replication for almost all the objects. Only EVENTS is left for the same. There are no replication test cases for SERVER as the query is not written into the log for SERVERs.

During this I was able to find a bug in TRIGGER replication and have fixed it by adding a private method named build_trig_stmt_query. 

Following are the commits from the last week.

Replication tests for DB and USER

Replication tests for all other objects

TRIGGER replication bug fix

I am still working on CREATE OR REPLACE INDEX. It is a little complicated to handle cause CREATE, DROP and ALTER all the three commands for INDEX are handled by the same function. Will be giving a logical conclusion to it by EOD (11th August).

Suggestions and/or reviews are welcome!

[srirampatil]

Week Eleven!

I wrote basic test cases for the commands I have implemented until now in the last week. These are all very basic test cases and I will be adding into these in the coming weeks. The naming convention for the test files is create_drop_<object>.test. There is one separate file for checking CREATE OR REPLACE permissions named create_or_replace_permission.test.

Found two bugs in my code while testing. Both were in CREATE FUNCTION (stored). The return type of a stored function was not getting stored in the procedures table. So when invoked it would give an error saying that the table is corrupted.

Another bug was in permission checks, where I was checking DELETE_ACL instead of ALTER_ROUTINE. DELETE_ACL permission is checked for UDF and ALTER_ROUTINE for procedures and stored functions.

Following are the commits of last week, you can check the test files and their results, plus the bug fixes there.

Permission checks for create or replace

Basic tests for all objects

Return type bug in stored functions

Wrong permission check bug for stored function

With respect to implementation, only CREATE OR REPLACE INDEX is left and may be refactoring of some code. Apart from this it is going to be all about testing now on.

Suggestion and reviews are welcome!

[srirampatil]

Week Nine and Ten!

Continuing with CREATE OR REPLACE implementation from week eight. First, CREATE OR REPLACE USER/ROLE. You can check the changes in commit 02cefab. In the implementation, instead of calling complete drop function for USER/ROLE, I have just used the part that actually drops the USER/ROLE. Some code related to permission checks in this commit has been changed in the next commits, keep reading to know about it.

Moving to some more important stuff. CREATE OR REPLACE FUNCTION(UDF). My first attempt to write the code for this was not that great. It had some redundant code which could have been modulated for better understanding and code structure. Thanks to Alexander (my mentor) for a detailed review and now the code looks better. Here is how the final code looks like.I wrote a new private method in sql_udf.cc named mysql_drop_function_internal. Checkout the code below

/* Drop user defined function. @param thd Thread handler. @param udf Existing udf_func pointer which is to be deleted. @param table mysql.func table reference (opened and locked) Assumption - udf is not null. - table is already opened and locked */ static int mysql_drop_function_internal(THD *thd, udf_func *udf, TABLE *table) { DBUG_ENTER("mysql_drop_function_internal"); char *exact_name_str= udf->name.str; uint exact_name_len= udf->name.length; del_udf(udf); /* Close the handle if this was function that was found during boot or CREATE FUNCTION and it's not in use by any other udf function */ if (udf->dlhandle && !find_udf_dl(udf->dl)) dlclose(udf->dlhandle); if (!table) DBUG_RETURN(1); table->use_all_columns(); table->field[0]->store(exact_name_str, exact_name_len, &my_charset_bin); if (!table->file->ha_index_read_idx_map(table->record[0], 0, (uchar*) table->field[0]->ptr, HA_WHOLE_KEY, HA_READ_KEY_EXACT)) { int error; if ((error = table->file->ha_delete_row(table->record[0]))) table->file->print_error(error, MYF(0)); } DBUG_RETURN(0); }

As one can understand from comments, this method drops an existing UDF and nothing else. This method is invoked from both mysql_drop_function and mysql_create_function (in case of CREATE OR REPLACE) to perform actual drop. All the error checks and opening and locking of mysql.func table and searching for existing UDF is done in the mysql_drop_function and mysql_create_function. The changes made for this can be seen here and here.

Then I did some refactoring with respect to permission checks for CREATE OR REPLACE in general. Instead of checking the DROP permission before calling respective REPLACE logic, we (me and my mentor) decided to check for the permission before invoking respective create function in sql_parse.cc. Which means, if a user does not have a DROP permission on certain object, s/he will not be able to execute a CREATE OR REPLACE command ever. This behavior is consistent with the current CREATE OR REPLACE TABLE implementation (check create_table_precheck method in sql_parse.cc). General structure for checking permissions is as follows

check_access(..., (lex->is_create_or_replace()) ? CREATE | DROP : CREATE, ...)

The changes can be observed in this commit.

I have also changed some code for readability and a small bug fix in CREATE DATABASE IF NOT EXISTS. Checkout the list of commits below, bottommost being the latest one.

CREATE OR REPLACE FUNCTION (UDF) before review

CREATE OR REPLACE USER/ROLE

CREATE OR REPLACE FUNCTION (UDF) after review

More readable code

Bug fix in CREATE DATABASE IF NOT EXISTS

Changes in permission checks

Apart from this I have also fixed MDEV-6409 in the last week on MariaDB 5.5. Thanks to Sergei for helping me with the patch.

Suggestions and reviews are welcome!

Update: Writing test cases for testing permissions logic will be the first thing I do in this week.

[srirampatil]

Week Eight!

Continuing with the CREATE OR REPLACE implementations, last week I implemented the same for procedures, stored functions and events. The approach has been the same for all. Added a parameter to drop methods PROCEDURE and STORED FUNCTION. The boolean parameter is names as drop_for_replace indicating if the drop function call is part of a replace command or not. I have changed the parameter name in the drop method for SERVER too from the one use in week seven. I thought this one is more readable.

In case of EVENTs, there is a little change, instead of calling the complete drop event routine, I just used the part from drop method which actually drops the existing event. The reason being, all the code in create event function and drop event function except for the create and drop part is the same. There is no need to execute the same code again by calling the complete drop event routine. So the CREATE OR REPLACE EVENT changes are made in event_db_repository.cc and are as follows

if (!find_named_event(parse_data->dbname, parse_data->name, table)) { if (thd->lex->is_create_or_replace()) { if ((ret= table->file->ha_delete_row(table->record[0]))) { table->file->print_error(ret, MYF(0)); goto end; } } else if (create_if_not) { *event_already_exists= true; push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE, ER_EVENT_ALREADY_EXISTS, ER(ER_EVENT_ALREADY_EXISTS), parse_data->name.str); ret= 0; goto end; } else { my_error(ER_EVENT_ALREADY_EXISTS, MYF(0), parse_data->name.str); goto end; } }

As can be observed in the first if condition after find_named_event there is a new function names is_create_or_replace. Last week I have addded two inline functions in struct LEX in order to make it a little intuitive to check for CREATE OR REPLACE and CREATE...IF NOT EXISTS flags. The functions are as follows

inline bool is_create_or_replace() { return (create_info.options & HA_LEX_CREATE_REPLACE); } inline bool is_create_if_not_exists() { return (create_info.options & HA_LEX_CREATE_IF_NOT_EXISTS); }

I have replaced the use of HA_LEX_CREATE_REPLACE and HA_LEX_CREATE_IF_NOT_EXISTS constants for checking the clause flags with these function calls in sql_yacc.yy too.

I will also look into other objects to check if it is possible to take a similar approach like EVENTs. That is, executing only the drop object part of the code instead of executing the complete drop object function.

Following are the commits for last week

CREATE OR REPLACE PROCEDURE

CREATE OR REPLACE FUNCTION

CREATE OR REPLACE EVENT and struct LEX changes

Suggestions and review are welcome! 

[srirampatil]

Week Seven!

Now that CREATE...IF NOT EXISTS and DROP...IF EXISTS clauses are implemented for all the objects, I have moved on to work on CREATE OR REPLACE. Last week I implemented CREATE OR REPLACE SERVER and CREATE OR REPLACE TRIGGER. Both the implementations have some significant changes in function signatures and/or the execution flow.

Let me start with the basic workflow of CREATE OR REPLACE in general. It is going to be same as CREATE OR REPLACE DATABASE. Following is a small snippet for the same.

if object exists if CREATE OR REPLACE used drop the object but make sure deadlock does not occur and the bin log entry is not made for drop. Continue with the create algorithm in the respective function

In case of CREATE OR REPLACE SERVER, I had to deal with deadlock. There is mysql_rwlock_wrlock(&THR_LOCK_servers) statement at the beginning of both create_server and drop_server functions. Because of this statement if one does a CREATE OR REPLACE SERVER and it goes through both create_server and drop_server functions, then the next CREATE SERVER command enters deadlock.

For this I have added a parameter in drop_server function. The signature now is as follows

int drop_server(THD *thd, LEX_SERVER_OPTIONS *server_options, bool dont_lock);

The third parameter, dont_lock, indicates whether there is a need to lock the THR_LOCK_servers in drop_server. If dont_lock is true the it is not locked and is locked otherwise. You can find the changes for SERVER in this commit.

In case of CREATE OR REPLACE TRIGGER, there are two cases

1. Different name create table t1 (a int); create or replace trigger tr1 before insert on t1 for each row begin end; create or replace trigger tr2 before insert on t1 for each row begin end; --> error, multiple triggers with the same action 2. Same name create table t1 (a int); create or replace trigger tr1 before insert on t1 for each row begin end; create or replace trigger tr1 before insert on t1 for each row begin end; --> OK, the trigger is replaced

The previous code in create_trigger function handled only the first condition as there was no CREATE OR REPLACE before. It looked something as follows

if(bodies[lex->trg_chistics.event][lex->trg_chistics.action_time] != 0) { my_error(ER_NOT_SUPPORTED_YET, MYF(0), "multiple triggers with the same action time" " and event for one table"); return 1; }

bodies, is a 2D array of sp_head pointers which represents a trigger. So if there exists a trigger with same event and action time, ER_NOT_SUPPORTED_YET error is thrown. To handle the above mentioned cases, I had to compare the names of the existing triggers and the trigger name mentioned in the CREATE OR REPLACE query. I have changed the code as follows

sp_head *trg_head= bodies[lex->trg_chistics.event] [lex->trg_chistics.action_time]; ... ... /* We don't allow creation of several triggers of the same type yet. Note: Comparing the trigger names here to avoid throwing the error without considering the possibility of CREATE OR REPLACE and IF NOT EXISTS clauses in the query. */ if(trg_head != 0 && strcmp(trg_head->m_name.str, lex->spname->m_name.str) != 0) { my_error(ER_NOT_SUPPORTED_YET, MYF(0), "multiple triggers with the same action time" " and event for one table"); return 1; }

And to handle the CREATE OR REPLACE clause, that is, drop the existing trigger and then create a new one works as follows.

if (!access(trigname_buff, F_OK)) { if (lex->create_info.options & HA_LEX_CREATE_REPLACE) { String drop_trg_query; drop_trg_query.append("DROP TRIGGER "); drop_trg_query.append(lex->spname->m_name.str); if(drop_trigger(thd, tables, &drop_trg_query)) return 1; } else if (lex->create_info.options & HA_LEX_CREATE_IF_NOT_EXISTS) { push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE, ER_TRG_ALREADY_EXISTS, ER(ER_TRG_ALREADY_EXISTS), trigname_buff); return 0; } else { my_error(ER_TRG_ALREADY_EXISTS, MYF(0)); return 1; } }

drop_trg_query, holds the DROP TRIGGER command. You can find the changes for TRIGGER in this commit.

I have also done some minor changes for improving readability and removing some redundant permissions checks, you can find those here and here.

Suggestions and review are welcome!

[srirampatil]

Week Six!

Update: MDEV-6409 created for the CREATE VIEW bug.

Week six was all about replication test cases. I studied how test cases are written for replication. The test case files are stored at path mysql-test/suite/rpl/t.

I had to write a test case to verify the bug I had reported in fifth week. CREATE VIEW had bug with respect to replication. I wrote the test in file rpl_view_bug.test. Following is the test,

source include/master-slave.inc; --disable_warnings drop table if exists t1; drop view if exists v1,v2; sync_slave_with_master; reset master; --enable_warnings --echo # --echo # Check the bug where if n error occurs in mysql_register_view the view --echo # is still replicated to the slave --echo # connection master; create table t1 (a int); insert into t1 values (1); create view v1 as select a from t1; insert into v1 values (2); select * from v1 order by a; sync_slave_with_master; # view already have to be on slave show tables; connection master; set @@debug_dbug="d,simulate_register_view_failure"; --error 1041 CREATE VIEW v2 as SELECT * FROM t1; show tables; sync_slave_with_master; show tables; --source include/rpl_end.inc

The line set @@debug_dbug="d,simulate_register_view_failure" is important here. The debug mechanism works on this flag. There is a macro DBUG_EXECUTE_IF(<flag>, <code>). If @@debug_dbug variable is set to <flag>, <code> is executed. Otherwise <code> is not executed.

I wrote three DBUG_EXECUTE_IF statements in mysql_register_view to return ER_OUT_OF_RESOURCES error. These statements ensure a failure in mysql_register_view method. So, CREATE VIEW statement fails always on master server when debug_dbug variable is set to simulate_register_view_failure. I wrote following debug statements in mysql_register_view method(sql_view.cc).

DBUG_EXECUTE_IF("simulate_register_view_failure", my_error(ER_OUT_OF_RESOURCES, MYF(0));); DBUG_EXECUTE_IF("simulate_register_view_failure", error= -1;); DBUG_EXECUTE_IF("simulate_register_view_failure", goto err;);

The CREATE VIEW query gets logged even though this error occurs in mysql_register_view. So, the view is not created on master server but, since the query is written in the bin log, the query is executed at slave servers too. And if same error does not occur at slave server the replication is aborted.

The test is executed from mysql-test directory with following command

./mtr rpl.rpl_view_bug

I also fixed bin logging issues int CREATE OR REPLACE DATABASE. Since,in current approach (explained here) the database is first dropped and then new one is created, CREATE OR REPLACE should be logged only once. The mysql_rm_db methods' fourth parameter is a flag which decides whether to log the query or not. So, had to change the call to this function for correct logging, passing true as the fourth parameter.

/* Removing old database. */ if (mysql_rm_db(thd, db, 0, 1)) DBUG_RETURN(1);

Suggestions and reviews are welcome!

[srirampatil]

Week Five!

Last week was all about bin logging and replication. The commands I have implemented until now were writing wrong queries in the bin log, because I had not programmed them to write the correct ones (pretty obvious, isn't it? :D). So, my focus was to read about bin logging, understand the mechanism and fix my code to write correct queries into bin log.

Let me start with what is bin log and why is it created? Well, it is mainly used for replication. MariaDB supports replication with one master server and multiple slave servers. The master server writes the bin log. Almost all successfully executed commands are written into the bin log. And the slave servers make use of this bin log created by master to reproduce the results on their respective machines. This is very basic idea of replication and bin logging. More about it can be read here.

Now, what if I write incorrect queries into the bin log? The replication mechanism will go crazy. Though the complete replication process will work fine (if the bin logged queries are syntactically correct), the replicated databases at slaves will not be consistent with the master servers' database. This can cause havoc.

In case of IF NOT EXISTS and IF EXISTS commands if the object in consideration exists or does not exist respectively the query is considered to be executed successfully with a warning.

I have changed the code for IF NOT EXISTS and/or IF EXISTS of VIEW, USER, ROLE, PROCEDURE, FUNCTION and FUNCTION(UDF). For TRIGGER correct queries were written into the log. The changes are in the work flow of respective methods. I found it a little difficult to explain my source code changes without pasting complete method code here. So, instead, you can visit this commit to go through the changes I have made. Please ignore the changes in udf.test and udf.result files.

*** Important ***

Last week, I have also reported a probable bug in VIEW replication. I call it a probable bug cause, though it seems to be a bug from the source code, I have not yet written the tests to verify the same. You can see the mail thread from the maria-developers mailing list here.

Writing tests for verifying this bug and the code that I have written is going to be my first priority this week. Suggestions and review are welcome!

[srirampatil]

Week Four!

Update: MDEV-5359 has been updated to include User Defined Functions.

Well, it's been a complete month now. Though the third week was a little rough, last week was pretty productive.

I was a little stuck in the previous week because of CREATE TRIGGER IF NOT EXISTS. I am gonna explain the problem in detail. The problem was in the way I was interpreting the YACC rules. trigger_tail is the rule used for parsing and storing the properties for the TRIGGER command. A snippet of the previous rule is as follows

trigger_tail: TRIGGER_SYM remember_name sp_name trg_action_time trg_event ON remember_name ...

The remember_name token here was the culprit. This token returns a char * pointer pointing to the symbol right before this token. So, the first remember_name token here will return a char * pointer pointing to a string in the query from "TRIGGER trig_name...". This is where I went wrong. I had put the opt_if_not_exists token in between TRIGGER_SYM and remember_name. Because of this even after creating a TRIGGER successfully, the .TRG file for a table was written in wrong format. The DROP TRIGGER command was not able to find any valid triggers and printing out a "Trigger does not exists" error message. Figuring this out took a while. So, the correct way to write the rule was to put opt_if_not_exists after the first remember_name token which kept the .TRG file intact and then all was back on track. Made the following change in the Table_triggers_list::create_trigger method in sql_trigger.cc

if (!access(trigname_buff, F_OK)) { if (lex->create_info.options & HA_LEX_CREATE_IF_NOT_EXISTS) { push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE, ER_TRG_ALREADY_EXISTS, ER(ER_TRG_ALREADY_EXISTS), trigname_buff); return 0; } my_error(ER_TRG_ALREADY_EXISTS, MYF(0)); return 1; }

I have added the test cases in file trigger.test. You can see all the changes made for this command here. 

Next in the queue was CREATE FUNCTION IF NOT EXISTS for both stored functions and user defined functions. I had to change all the FUNCTION related rules at once in the parser because changing only one of the three rules gives shift/reduce conflicts for obvious reasons. I have changed the udf_tail and sf_tail rules in the parser. Like CREATE PROCEDURE, sp_create_routine function from file sp.cc is invoked for creating stored functions. So, I had to tweak the CREATE PROCEDURE IF NOT EXISTS implementation explained in my previous post. The change is as follows

if(db_find_routine_aux(thd, type, thd->lex->spname, table) == SP_OK) { if (thd->lex->create_info.options & HA_LEX_CREATE_IF_NOT_EXISTS) { push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE, ER_SP_ALREADY_EXISTS, ER(ER_SP_ALREADY_EXISTS), (type == TYPE_ENUM_FUNCTION) ? "FUNCTION" : "PROCEDURE", thd->lex->spname->m_name.str); ret= SP_OK; } else ret= SP_WRITE_ROW_FAILED; goto done; }

I have also added a few test cases in proc_fun.test.

And now the User Defined Functions (UDF). I have implemented both CREATE.. IF NOT EXISTS and DROP...IF EXISTS for UDF. Had to change two functions for these, mysql_create_function and mysql_drop_function respectively from file sql_udf.cc. Following are the code snippets for both commands respectively

if ((my_hash_search(&udf_hash,(uchar*) udf->name.str, udf->name.length))) { if (thd->lex->create_info.options & HA_LEX_CREATE_IF_NOT_EXISTS) { push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE, ER_UDF_EXISTS, ER(ER_UDF_EXISTS), udf->name.str); mysql_rwlock_unlock(&THR_LOCK_udf); DBUG_RETURN(0); } my_error(ER_UDF_EXISTS, MYF(0), udf->name.str); goto err; }

if (!(udf=(udf_func*) my_hash_search(&udf_hash,(uchar*) udf_name->str, (uint) udf_name->length))) { if (thd->lex->check_exists) { push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE, ER_FUNCTION_NOT_DEFINED, ER(ER_FUNCTION_NOT_DEFINED), udf_name->str); mysql_rwlock_unlock(&THR_LOCK_udf); DBUG_RETURN(0); } my_error(ER_FUNCTION_NOT_DEFINED, MYF(0), udf_name->str); goto err; }

I have appended a few tests into the file udf.test.

Now, about some pending discussions.

I will be working on a more careful implementation of CREATE OR REPLACE DATABASE in the month of August. For now current implementation of removing and then creating the database will be kept as it is. We will test it for replication and bin logging though.

I had a doubt about bin logging when CREATE IF NOT EXISTS or DROP IF EXISTS is used. There might be some changes in the workflow of the previously implemented commands but the core functionality will remain intact.

Suggestions and review are welcome!

[srirampatil]

Week Three!

As of last week I have completed DROP...IF NOT EXISTS for all the objects. This week, my target was to close off CREATE...IF NOT EXISTS for all the objects. Was not able to close it off though because I got stuck in CREATE TRIGGER...IF NOT EXISTS and some suggestions on last weeks CREATE OR REPLACE code.

Started off this week with CREATE PROCEDURE IF NOT EXISTS. Changed the sp_tail rule in the yacc parser (sql_yacc.yy). And wrote the following lines of code in the sp_create_routine function in file sp.cc. The commit details regarding this change can be seen here.

/* Checking if the routine already exists */ if (db_find_routine_aux(thd, type, thd->lex->spname, table) == SP_OK) { if (thd->lex->create_info.options & HA_LEX_CREATE_IF_NOT_EXISTS) { push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE, ER_SP_ALREADY_EXISTS, ER(ER_SP_ALREADY_EXISTS), "PROCEDURE", thd->lex->spname->m_name.str); ret= SP_OK; } else ret= SP_WRITE_ROW_FAILED; goto done; }

The db_find_routine_aux function returns SP_OK if the procedure exists. I have not written any tests for this yet.

Well, after this there is not much to show in terms of code. I got stuck while implementing CREATE TRIGGER IF NOT EXISTS. Will be working on it in this week.

Also, got a few suggestions on last weeks CREATE OR REPLACE DATABASE implementation. Yes, there are a few things that need some tweaking. Last weeks code will almost change completely for that matter. You can observe respective changes in MDEV-5359. Will be working on it this week and will write the details in the next blog.

Suggestions and review are welcome!

[srirampatil]

First two weeks into coding!

It's been about two weeks since I started making changes in the MariaDB source in my local repository. I intend to implement CREATE OR REPLACE, IF NOT EXISTS and DROP IF EXISTS for all the objects in MariaDB. The detailed description and specification of the things to be implemented can be seen here.

I was first working on the launchpad MariaDB repository but have migrated to GitHub now. I have forked the repo and pushed all my changes on create_drop  branch.

So, let's get started with the real stuff!

I chose to implement CREATE OR REPLACE DATABASE to begin with. The reason being it is simple to understand and the flow was pretty straight forward. CREATE OR REPLACE is essentially DROP IF EXISTS and CREATE combined into one command. The tricky thing is it has to be atomic. It executes completely or does not execute at all. So, before dropping the existing database I had to check both CREATE_ACL as well as DROP_ACL permissions. (The need of doing so is explained in detail in MDEV-5359.) Following is a snippet from function mysql_create_db which essentially does the mentioned operations.

if (mysql_file_stat(key_file_misc, path, &stat_info, MYF(0))) { if (create_options & HA_LEX_CREATE_REPLACE) { /* Need to check access permissions before dropping database. */ if (check_access(thd, DROP_ACL, db, NULL, NULL, 1, 0)) DBUG_RETURN(1); /* Checking if create permission is granted before deleting the existing db. This makes sure that CREATE OR REPLACE happens completely or nothing changes. */ if(check_access(thd, CREATE_ACL, db, NULL, NULL, 1, 0)) DBUG_RETURN(1); /* Removing old database. */ if (mysql_rm_db(thd, db, 0, 0)) DBUG_RETURN(1); /* Reset the diagnostics m_status. It might be set ot DA_OK in mysql_rm_db. */ thd->get_stmt_da()->reset_diagnostics_area(); goto mkdir; } else if (!(create_options & HA_LEX_CREATE_IF_NOT_EXISTS)) { my_error(ER_DB_CREATE_EXISTS, MYF(0), db); error= -1; goto exit; } else { push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE, ER_DB_CREATE_EXISTS, ER(ER_DB_CREATE_EXISTS), db); error= 0; goto not_silent; } }

USER and ROLE were the only two objects which do not have the DROP IF EXISTS command. So, I implemented those first. If the USER or ROLE exists then a warning must be printed. I added one error code for this in errmsg-utf8.txt, namely ER_DROP_USER_EXISTS. The check_exists flag in LEX class indicates that IF EXISTS is used or not.

if (handle_grant_data(tables, 1, user_name, NULL) <= 0) { if (!thd->lex->check_exists) { append_user(thd, &wrong_users, user_name); result= TRUE; continue; } else { push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE, ER_USER_DROP_EXISTS, ER(ER_USER_DROP_EXISTS), tmp_user_name->user.str); continue; } }

I also implemented CREATE IF NOT EXISTS for USER, ROLE and SERVER. The existence of IF NOT EXISTS in the command is indicated by create_info.options in LEX class. The option is set to HA_LEX_CREATE_IF_NOT_EXISTS. The flow for all the three objects is pretty much the same. It can be summarized as follows

if (object name exists) { if (create_info.options & HA_LEX_CREATE_IF_NOT_EXISTS) { // Print warning } else { // Output error. // If a more than one of new object names are given in // the command then continue with the next object // names } }

Of course, I have changed the respective rules for all these commands in sql_yacc.yy and have set all the flags and options when necessary while parsing. I have also taken care of errors like CREATE OR REPLACE and IF NOT EXISTS cannot occur together.

I have also written test cases for all these changes the test case files are db_create.test, user_role.test and servers.test which can be found in mysql-test/t folder.

Suggestions and review on the code are welcome.