Bug 5366 - Koha does not work right with MariaDB
Summary: Koha does not work right with MariaDB
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Galen Charlton
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-11-04 18:04 UTC by Chris Nighswonger
Modified: 2017-06-14 22:10 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 5366 - Standing enh req for code changes enabling database agnosticism (2.07 KB, patch)
2012-11-12 03:07 UTC, Chris Nighswonger
Details | Diff | Splinter Review
[2/2] Bug 5366 - Standing enh req for code changes enabling database agnosticism (3.68 KB, patch)
2012-11-12 03:07 UTC, Chris Nighswonger
Details | Diff | Splinter Review
Bug 5366 - Standing enh req for code changes enabling database agnosticism (2.17 KB, patch)
2012-11-16 05:48 UTC, Mason James
Details | Diff | Splinter Review
Bug 5366 - [2/2] Standing enh req for code changes enabling database agnosticism (2.18 KB, patch)
2012-11-16 05:52 UTC, Mason James
Details | Diff | Splinter Review
Bug 5366 - [2/2] Standing enh req for code changes enabling database agnosticism (3.57 KB, patch)
2012-11-16 05:55 UTC, Mason James
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Nighswonger 2010-11-04 18:04:24 UTC
Please include any specific databases code changes are made for in the comment section when updating this req and submitting patches.
Comment 1 Chris Cormack 2010-11-08 07:06:42 UTC
Branch on git.koha-community.org/koha.git  called new/enh/bug_5366

I will push any patches for this bug to that branch
Comment 3 Chris Nighswonger 2012-11-12 03:07:20 UTC Comment hidden (obsolete)
Comment 4 Chris Nighswonger 2012-11-12 03:07:33 UTC Comment hidden (obsolete)
Comment 5 Chris Nighswonger 2012-11-12 03:10:31 UTC
These patches may or may not still be applicable. This work is quite old; however, I've attached them for posterity's sake if for nothing else.
Comment 6 Mason James 2012-11-16 05:48:16 UTC
Created attachment 13478 [details] [review]
Bug 5366 - Standing enh req for code changes enabling database agnosticism

MariaDB apparently will not fudge when one attempts to insert a null value into a
NOT NULL field with no default defined. This probably should have been fixed in
any case, but must be fixed if Koha is to run over MariaDB.

This patch simple supplies a single blank space as the value passed in on the
initial insert.
Comment 7 Mason James 2012-11-16 05:52:56 UTC Comment hidden (obsolete)
Comment 8 Mason James 2012-11-16 05:55:21 UTC
Created attachment 13480 [details] [review]
Bug 5366 - [2/2] Standing enh req for code changes enabling database agnosticism

MariaDB apparently will not fudge when one attempts to insert a null value into a
    NOT NULL field with no default defined. This probably should have been fixed in
    any case, but must be fixed if Koha is to run over MariaDB.

    This patch simple supplies a single blank space as the value passed in on the
    initial insert.
Comment 9 Mason James 2012-11-16 05:57:32 UTC
(In reply to comment #5)
> These patches may or may not still be applicable. This work is quite old;
> however, I've attached them for posterity's sake if for nothing else.

hi Chris, 

i've just rebased them on master 411d8c3c8, 

(but i havent tested them yet)
Comment 10 Jared Camins-Esakov 2013-03-23 16:08:20 UTC
Chris, what are the symptoms of this problem?
Comment 11 Owen Leonard 2013-04-09 18:54:39 UTC
> Chris, what are the symptoms of this problem?

In other words, please supply a test plan.
Comment 12 Chris Nighswonger 2013-04-09 18:58:18 UTC
Jared and I chated about this a bit on IRC. I do not have Koha running over MariaDB atm nor do I have the time to look back into this atm. So we can do one of two things:

1. Mark this bug WONTFIX because it appears at least one person cannot duplicate it.

Or

2. Push the patches anyway because they net zero changes in the functionality of the code.

Either way is fine with me. I'm sure if these represent real problems with MariaDB the bug will be reopened at some point in time.
Comment 13 Chris Cormack 2013-04-09 19:24:56 UTC
Id like to test this, I kinda understand the commit message on the patch, so I will try to get time to test this. Ill put back to assigned, and will switch to needs signoff when i have mariadb going to test on.
Comment 14 Chris Cormack 2013-04-10 02:53:03 UTC
I now have Koha running under MariaDB, I am going to attempt to recreate the problem.
Comment 15 Chris Cormack 2013-04-10 03:13:15 UTC
I can recreate, to do so

Install Mariadb (this will remove mysql) Koha will keep working fine
Click around everything is good
Try to add a biblio, you will end up on the additems screen, and a biblio has been created, but the biblioitem part failed, leaving your db a mess
Apply the patch, try adding a new one again

I get
Can't call method "fields" on an undefined value at /home/chrisc/git/catalyst-koha/C4/Biblio.pm line 2509.
Comment 16 Chris Cormack 2013-04-10 03:19:31 UTC
It now inserts the biblioitem but explodes. I don't think we can push the patch as is.

I switched back to mysql and get that same error, if I remove the patch, it works again
Comment 17 Chris Nighswonger 2013-04-10 10:05:28 UTC
(In reply to comment #16)
> It now inserts the biblioitem but explodes. I don't think we can push the
> patch as is.
> 
> I switched back to mysql and get that same error, if I remove the patch, it
> works again

I think that I remember now that you mention it that there were other problems which is probably why I did not pursue this further at the time.

I seem to recall a discussion about the best way to approach a fix for the no null fields without a default could be fixed in the schema itself.

Maybe fixing it there in all of its occurrences would fix the symptoms without breaking things too badly. But then if we have code expecting null values returned from some of these fields then things might just break anyway.
Comment 18 Galen Charlton 2013-06-19 18:57:30 UTC
As a data point, I'm currently running MariaDB 5.5.31, using mariadb.org's Debian Wheezy package.

So far, I am *not* running into any glitches with inserting into tables that don't set explicit defaults for non-NULL columns.  Here's an example session:

MariaDB [koha]> CREATE TABLE foo (
    ->    id INTEGER auto_increment,
    ->    field1 VARCHAR(50) NOT NULL,
    ->    field2 TEXT NOT NULL,
    ->    PRIMARY KEY (id)
    -> ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
Query OK, 0 rows affected (0.06 sec)

MariaDB [koha]> INSERT INTO foo (field1) VALUES ('field1 set, field2 not');
Query OK, 1 row affected, 1 warning (0.00 sec)

MariaDB [koha]> show warnings;
+---------+------+---------------------------------------------+
| Level   | Code | Message                                     |
+---------+------+---------------------------------------------+
| Warning | 1364 | Field 'field2' doesn't have a default value |
+---------+------+---------------------------------------------+
1 row in set (0.00 sec)

MariaDB [koha]> INSERT INTO foo (field2) VALUES ('field2 set, field1 not');
Query OK, 1 row affected, 1 warning (0.03 sec)

MariaDB [koha]> show warnings ;
+---------+------+---------------------------------------------+
| Level   | Code | Message                                     |
+---------+------+---------------------------------------------+
| Warning | 1364 | Field 'field1' doesn't have a default value |
+---------+------+---------------------------------------------+
1 row in set (0.00 sec)

MariaDB [koha]> select * from foo;
+----+------------------------+------------------------+
| id | field1                 | field2                 |
+----+------------------------+------------------------+
|  1 | field1 set, field2 not |                        |
|  2 |                        | field2 set, field1 not |
+----+------------------------+------------------------+
2 rows in set (0.00 sec)

MariaDB [koha]> select * from foo where field2 is null;
Empty set (0.00 sec)

MariaDB [koha]> select * from foo where field2 = '';
+----+------------------------+--------+
| id | field1                 | field2 |
+----+------------------------+--------+
|  1 | field1 set, field2 not |        |
+----+------------------------+--------+
1 row in set (0.00 sec)


I wonder if, back when this was originally tested, whether MariaDB was shipping with a default value of SQL_MODE that included 'STRICT_TRANS_TABLES':

ariaDB [koha]> SET SESSION sql_mode = 'STRICT_TRANS_TABLES';
Query OK, 0 rows affected (0.00 sec)

MariaDB [koha]> INSERT INTO foo (field2) VALUES ('field2 set, field1 not');
ERROR 1364 (HY000): Field 'field1' doesn't have a default value

If that's the case, perhaps a solution is to set SQL_MODE explicitly whenever Koha makes a database connection.  The initial value would almost certainly have to be '', for compatibility reasons, but it would be a very worthy code to then start tightening the screws and work our way up to setting SQL_MODE to TRADITIONAL or the like -- and this would be an improvement regardless of whether any given Koha site uses MariaDB or MySQL.
Comment 19 Chris Nighswonger 2013-06-19 19:12:18 UTC
(In reply to comment #18)
> As a data point, I'm currently running MariaDB 5.5.31, using mariadb.org's
> Debian Wheezy package.
> 
> So far, I am *not* running into any glitches with inserting into tables that
> don't set explicit defaults for non-NULL columns. 

<snip>
 
> I wonder if, back when this was originally tested, whether MariaDB was
> shipping with a default value of SQL_MODE that included
> 'STRICT_TRANS_TABLES':
> 
> ariaDB [koha]> SET SESSION sql_mode = 'STRICT_TRANS_TABLES';
> Query OK, 0 rows affected (0.00 sec)
> 
> MariaDB [koha]> INSERT INTO foo (field2) VALUES ('field2 set, field1 not');
> ERROR 1364 (HY000): Field 'field1' doesn't have a default value

This is entirely possible as this last error is what I was seeing.

> If that's the case, perhaps a solution is to set SQL_MODE explicitly
> whenever Koha makes a database connection.  The initial value would almost
> certainly have to be '', for compatibility reasons, but it would be a very
> worthy code to then start tightening the screws and work our way up to
> setting SQL_MODE to TRADITIONAL or the like -- and this would be an
> improvement regardless of whether any given Koha site uses MariaDB or MySQL.

This sounds like a good approach, and one which I would certainly support (fwiw).
Comment 20 Jonathan Druart 2015-02-16 12:46:48 UTC
(In reply to Mason James from comment #6)
> Created attachment 13478 [details] [review] [review]
> Bug 5366 - Standing enh req for code changes enabling database agnosticism

The first patch will be useless with bug 13523.
Comment 21 Jonathan Druart 2015-02-16 12:48:50 UTC
Comment on attachment 13480 [details] [review]
Bug 5366 - [2/2] Standing enh req for code changes enabling database agnosticism

Review of attachment 13480 [details] [review]:
-----------------------------------------------------------------

::: C4/ImportBatch.pm
@@ +1354,1 @@
>  

A better would be to remove the NOT NULL condition in the DB structure for the import_records table.
Comment 22 Manos PETRIDIS 2015-10-16 10:27:47 UTC
It seems not to be a MySQL/MiariaDB issue, but a koha one. If no nulls may be entered in a column, then the code should check for nulls and exclude such records from the import procedure, creating perhaps an exceprion report of records that failed the insertion criteria. If blanks are considered sufficient Information, then the code should check for nulls and convert them to empty strings (''). If on the other hand nulls are to be allowed as such, the column(s) should not have the NOT NULL attribute in the First place.
Comment 23 Marc Véron 2015-12-24 14:01:25 UTC
Is this bug still valid?
Comment 24 Mark Tompsett 2016-11-22 17:06:35 UTC
(In reply to Marc Véron from comment #23)
> Is this bug still valid?

*timeout*
*ping*
I'm not sure this is still valid, because if we have Jenkins running MariaDB, then someone must have got it working.
Though, http://jenkins.koha-community.org/ doesn't seem like it is working, so I am wondering too: is this bug still valid?
Comment 25 Chris Cormack 2016-11-22 22:02:39 UTC
It's not, we have almost all of our production sites running with mariaDB. 
I will close this