[Bug 25040] New: Problematic current_timestamp syntax generated by DBIx::Class::Schema::Loader
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Bug ID: 25040 Summary: Problematic current_timestamp syntax generated by DBIx::Class::Schema::Loader Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: ASSIGNED Severity: major Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart@bugs.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org Caught by the changes made to Koha::Object->store by bug 23463 (especially 9c383aa286fee5a29c6f084873f2eb6644bad64f) 154 elsif ( not defined $self->$col 155 && $columns_info->{$col}->{datetime_undef_if_invalid} ) 156 { 157 # timestamp 158 $self->_result()->set_column($col => $columns_info->{$col}->{default_value}); 159 } If a timestamp not null column does not have a value when store is called, the default value is set. The expected default value is \"current_timestamp"(`git grep current_timestamp Koha/Schema/Result`) but for some reasons DBIx::Class::Schema::Loader sometimes generates "current_timestamp()" (certainly depending on versions of DBIx::Class::Schema::Loader and DBMS MySQL vs MariaDB) It ends up with an error on insert/update: DBD::mysql::st execute failed: Incorrect datetime value: 'current_timestamp()' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 102277 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102277&action=edit Bug 25040: Handle incorrect current_timestamp syntax There is an incorrect current_timestamp syntax generated by DBIx::Class::Schema::Loader Caught by the changes made to Koha::Object->store by bug 23463 (especially 9c383aa286fee5a29c6f084873f2eb6644bad64f) 154 elsif ( not defined $self->$col 155 && $columns_info->{$col}->{datetime_undef_if_invalid} ) 156 { 157 # timestamp 158 $self->_result()->set_column($col => $columns_info->{$col}->{default_value}); 159 } If a timestamp not null column does not have a value when store is called, the default value is set. The expected default value is \"current_timestamp"(`git grep current_timestamp Koha/Schema/Result`) but for some reasons DBIx::Class::Schema::Loader sometimes generates "current_timestamp()" (certainly depending on versions of DBIx::Class::Schema::Loader and DBMS MySQL vs MariaDB) It ends up with an error on insert/update: DBD::mysql::st execute failed: Incorrect datetime value: 'current_timestamp()' This should be a temporary patch, the root of the problem should be found and correct fixed. With this patch we want people to continue working with Koha until we investigate. Test plan: Apply this change: @ Koha/Schema/Result/Aqorder.pm:374 @ __PACKAGE__->add_columns( { data_type => "timestamp", datetime_undef_if_invalid => 1, - default_value => \"current_timestamp", + default_value => "current_timestamp()", is_nullable => 0, }, "rrp", and run: use Koha::Acquisition::Orders; use t::lib::TestBuilder; my $builder = t::lib::TestBuilder->new; my $order = $builder->build_object({ class => 'Koha::Acquisition::Orders' }); $order->timestamp(undef)->store; Without this patch you get an error -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|major |blocker --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Raising severity, we need that ASAP! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com, | |nick@bywatersolutions.com, | |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- This highlights a difference in recent MariaDB versions and I intend to submit a patch to DBIx::Class::Schema::Loader::DBI::mysql to account for it. To summarise, in the mysql drive for Schema::Loader there is an 'eq' match looking for 'current_timestamp' to add in special handling for it. But, MariaDB now returns 'current_timestamp()' from the DESCRIBE call and as such the 'eq' no longer matches and we fallback to just putting in whatever we are returned from the DB call. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 102339 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102339&action=edit Bug 25040: ALTERNATE You will need to install Sub::Override to test this. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 102340 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102340&action=edit Bug 25040: monkeypatch Schema::Loader for recent MariaDB Recent versions of MariaDB changed the output of 'DESCRIBE' for timestamp columns with defaults from `CURRENT_TIMESTAMP` to `current_timestamp()`. As such the code inside DBIx::Class::Schema::Loader which catches such cases and outputs `\"current_timestamp"` as a sensible cross platform default is missed and this leads of inconsistent class files and bugs with out default lookup code in Koha::Objects. This patch serves as a backport of the code I have submitted upstream such that out developers can continue to use update_dbix_class_files.pl to build their schema classes from the database and regardless of their db server version get a consistently correct output. You will need Sub::Override (libsub-override-perl) after this patch to use update_dbix_class_files. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102340|0 |1 is obsolete| | --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 102341 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102341&action=edit Bug 25040: monkeypatch Schema::Loader for recent MariaDB Recent versions of MariaDB changed the output of 'DESCRIBE' for timestamp columns with defaults from `CURRENT_TIMESTAMP` to `current_timestamp()`. As such the code inside DBIx::Class::Schema::Loader which catches such cases and outputs `\"current_timestamp"` as a sensible cross platform default is missed and this leads of inconsistent class files and bugs with out default lookup code in Koha::Objects. This patch serves as a backport of the code I have submitted upstream such that out developers can continue to use update_dbix_class_files.pl to build their schema classes from the database and regardless of their db server version get a consistently correct output. You will need Sub::Override (libsub-override-perl) after this patch to use update_dbix_class_files. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|jonathan.druart@bugs.koha-c |martin.renvoize@ptfs-europe |ommunity.org |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102339|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102277|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102341|0 |1 is obsolete| | --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 102342 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102342&action=edit Bug 25040: monkeypatch Schema::Loader for recent MariaDB Recent versions of MariaDB changed the output of 'DESCRIBE' for timestamp columns with defaults from `CURRENT_TIMESTAMP` to `current_timestamp()`. As such the code inside DBIx::Class::Schema::Loader which catches such cases and outputs `\"current_timestamp"` as a sensible cross platform default is missed and this leads of inconsistent class files and bugs with out default lookup code in Koha::Objects. This patch serves as a backport of the code I have submitted upstream such that out developers can continue to use update_dbix_class_files.pl to build their schema classes from the database and regardless of their db server version get a consistently correct output. You will need Sub::Override (libsub-override-perl) after this patch to use update_dbix_class_files. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 102343 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102343&action=edit Bug 25040: Add Sub::Override development dependancy This patch adds the Sub::Override dependancy to our cpanfile. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Testplan: 0/ Prior to applying this patchset run the update_dbix_class_files.pl script against a very new version of MariaDB - Note that \"current_timestamp2 will get replaced by "current_timestamp()" in the schema classes 1/ Apply the patch and install Sub::Override 2/ Run update_dbix_class_files.pl and note that we now stick to \"current_timestamp" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102342|0 |1 is obsolete| | Attachment #102343|0 |1 is obsolete| | --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 102344 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102344&action=edit Bug 25040: monkeypatch Schema::Loader for recent MariaDB Recent versions of MariaDB changed the output of 'DESCRIBE' for timestamp columns with defaults from `CURRENT_TIMESTAMP` to `current_timestamp()`. As such the code inside DBIx::Class::Schema::Loader which catches such cases and outputs `\"current_timestamp"` as a sensible cross platform default is missed and this leads of inconsistent class files and bugs with out default lookup code in Koha::Objects. This patch serves as a backport of the code I have submitted upstream such that out developers can continue to use update_dbix_class_files.pl to build their schema classes from the database and regardless of their db server version get a consistently correct output. You will need Sub::Override (libsub-override-perl) after this patch to use update_dbix_class_files. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 102345 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102345&action=edit Bug 25040: Add Sub::Override development dependancy This patch adds the Sub::Override dependancy to our cpanfile. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Martin Renvoize from comment #9)
Testplan:
0/ Prior to applying this patchset run the update_dbix_class_files.pl script against a very new version of MariaDB - Note that \"current_timestamp2 will get replaced by "current_timestamp()" in the schema classes 1/ Apply the patch and install Sub::Override 2/ Run update_dbix_class_files.pl and note that we now stick to \"current_timestamp"
Note that is not enough for me, I had to modify the structure of a table to make sure the file would have been generated. For instance with patches from bug 24161, that modifies aqorders, @ Koha/Schema/Result/Aqorder.pm:362 @ __PACKAGE__->add_columns( { data_type => "timestamp", datetime_undef_if_invalid => 1, - default_value => "current_timestamp()", + default_value => \"current_timestamp", is_nullable => 0, }, See also commit message from comment 1. Thanks Martin! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- My tests were wrong. I tested on My8, that's why I needed to apply 24161. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Back to the drawing board.. the override isn't working as expected :( -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102344|0 |1 is obsolete| | --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 102645 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102645&action=edit Bug 25040: monkeypatch Schema::Loader for recent MariaDB Recent versions of MariaDB changed the output of 'DESCRIBE' for timestamp columns with defaults from `CURRENT_TIMESTAMP` to `current_timestamp()`. As such the code inside DBIx::Class::Schema::Loader which catches such cases and outputs `\"current_timestamp"` as a sensible cross platform default is missed and this leads of inconsistent class files and bugs with out default lookup code in Koha::Objects. This patch serves as a backport of the code I have submitted upstream such that out developers can continue to use update_dbix_class_files.pl to build their schema classes from the database and regardless of their db server version get a consistently correct output. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102345|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Subclassing seems to have done the trick.. I just had to delve deeper into the dbic docs to understand how to call my subclass. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #17 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- This time I tested it correctly and it works! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 --- Comment #18 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Yeah for RTFM.. turned out subclassing was built right in to dbic, u was just doing it wrong the first couple of times I tried. Really glad that final patch is working and it even drops the Sub::Override dependancy I thought we needed to introduce. 🙂 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- S/u/I/.. silly mobile autocorrect -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- URL| |https://github.com/dbsrgits | |/dbix-class-schema-loader/p | |ull/23 --- Comment #20 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Upstream patch is at https://github.com/dbsrgits/dbix-class-schema-loader/pull/23 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102645|0 |1 is obsolete| | --- Comment #21 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 102696 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102696&action=edit Bug 25040: monkeypatch Schema::Loader for recent MariaDB Recent versions of MariaDB changed the output of 'DESCRIBE' for timestamp columns with defaults from `CURRENT_TIMESTAMP` to `current_timestamp()`. As such the code inside DBIx::Class::Schema::Loader which catches such cases and outputs `\"current_timestamp"` as a sensible cross platform default is missed and this leads of inconsistent class files and bugs with out default lookup code in Koha::Objects. This patch serves as a backport of the code I have submitted upstream such that out developers can continue to use update_dbix_class_files.pl to build their schema classes from the database and regardless of their db server version get a consistently correct output. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |20.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 --- Comment #22 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable CC| |joy@bywatersolutions.com Version(s)|20.05.00 |20.05.00, 19.11.06 released in| | --- Comment #23 from Joy Nelson <joy@bywatersolutions.com> --- backported to 19.11.x for 19.11.06 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #24 from Lucas Gass <lucas@bywatersolutions.com> --- Not to sure about backporting this one to 19.05.x. When I run the QA tool I get: FAIL Koha/Schema/Loader/mysql.pm OK critic OK forbidden patterns OK git manipulation OK pod FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 OK spelling OK valid Leaving it out of 19.05.x for now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25040 --- Comment #25 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Lucas, it's a POD warning, you can ignore it. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org