Hi, Koha version: 18.05.02.000 mysql Ver 15.1 Distrib 10.3.8-MariaDB create a "Label Template" and save. (any configuration ....) There is no error message. But it shows a message indicating that "there is no template" It looks like Koha is using some reserved words ( rows ) during the creation of a new label template... As i looked the sql commands that koha is using to create the template i figure out that its using "rows" - a new reserved word ... Could you verify it, please tia Henrique
... more info ... the SQL command is INSERT INTO creator_templates (creator, rows, template_code, profile_id, page_height, label_width, left_text_margin, units, cols, label_height, col_gap, row_gap, template_desc, page_width, top_margin, top_text_margin, left_margin) VALUES ('Labels','0','DEFAULT TEMPLATE','0','0','0','0','POINT','0','0','0','0','Default description','0','0','0','0') it does NOT work on 10.3.8-MariaDB-1:10.3.8+maria~stretch-log - mariadb.org binary distribution It WORKs on 10.0.30-MariaDB-0+deb8u2 ROWS is a reserved word .... tia, Henrique
The solution is to quote the keyword in question with backticks per the MariaDB knowledge base: "Reserved words cannot be used as Identifiers, unless they are quoted."[1] [1]https://mariadb.com/kb/en/library/reserved-words/
Hi Chris, yes, i know that. backticks is the solution ... but i dont want to mess with koha code .... koha uses some record object / classes / model ... ...not so easy to change i think koha should code backticks on all sql statments to solve this sittuation and prevent any others in the future .... tia
Created attachment 78292 [details] [review] Bug 21281: Surround creator_templates.rows with backquotes 'rows' is a reserved word since MariaDB 10.2.4 and MySQL https://mariadb.com/kb/en/library/mariadb-1024-release-notes/ https://dev.mysql.com/doc/refman/8.0/en/keywords.html Test plan: Do not apply this patch and make sure you recreate the reported issue Apply this patch and confirm that it is now fixed. QA will take care for the changes in installer and test files
Binding to UNIX socket file "/var/run/koha/kohadev/plack.sock" Setting gid to "1001 1001" Setting uid to "1001" Starman: Accepting connections at unix://localhost:/var/run/koha/kohadev/plack.sock/ DBD::mysql::st execute failed: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'rows) VALUES ('0','Labels','0','0','0','DEFAULT TEMPLATE','0','0','0','POINT','0' at line 1 [for Statement "INSERT INTO creator_templates (top_text_margin, creator, label_height, label_width, profile_id, template_code, page_height, row_gap, left_text_margin, units, page_width, left_margin, col_gap, top_margin, cols, template_desc, rows) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?);" with ParamValues: 0=0, 1='Labels', 2=0, 3=0, 4="0", 5="DEFAULT TEMPLATE", 6=0, 7=0, 8=0, 9="POINT", 10=0, 11=0, 12=0, 13=0, 14=0, 15="bleep!", 16=0] at /home/vagrant/kohaclone/C4/Creators/Template.pm line 199. Database returned the following error: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'rows) VALUES ('0','Labels','0','0','0','DEFAULT TEMPLATE','0','0','0','POINT','0' at line 1 at /home/vagrant/kohaclone/C4/Creators/Template.pm line 201. -- this is after applying the patch.
Created attachment 78448 [details] [review] Bug 21281: Correct SQL statement in C4/Creators/Template TEST PLAN --------- 1) Start a stretch kohgadevbox 2) Upgrade your MariaDB to 10.3 whatever (Google for instructions) 3) reset_all -- ignore the zebra problem. -- maybe include a sudo systemctl restart daemon-reload or whatever might have been mentioned in the output. 4) log in as admin/admin to staff client 5) Tool -> Label Creator -> New -> Layout Template 6) Change the description to something know like 'foobar'. 7) Save -- doesn't appear. 8) cat /var/log/koha/kohadev/plack-err.log -- mentions of errors like in comment #5 9) apply this patch 10) echo | sudo tee /var/log/koha/kohadev/plack-err.log -- this will make checking the log file easier. 11) restart_all -- maybe include a sudo systemctl restart daemon-reload or whatever might have been mentioned in the output. 12) reload the staff client page. 13) repeat steps 5-7 -- template shows up this time. 14) cat /var/log/koha/kohadev/plack-err.log -- no errors as before. 15) run koha qa test tools -- passes
Comment on attachment 78448 [details] [review] Bug 21281: Correct SQL statement in C4/Creators/Template busted prove case.
prove -v -r t/db_dependent/Creators/ current patch breaks. Patch needs tweaking still.
Created attachment 78449 [details] [review] Bug 21281: Correct t/db_dependent/Creators/Lib.t failures TEST PLAN --------- 1) apply first patch 2) kshell kohadev 3) prove -r -v t/db_dependent/Creators/ -- failures 4) apply this patch 5) repeat 2,3 -- no failures 6) run koha qa test tools
Hi, If I did folow tets plan, I get erroe after first patch: not ok 334 - get_batch_summary() raises warning if something went wrong with the sql request execution # Failed test 'get_batch_summary() raises warning if something went wrong with the sql request execution' # at t/db_dependent/Creators/Lib.t line 766. # found warning: DBD::mysql::st execute failed: Unknown column 'count(batch_id) as _item_count' in 'field list' [for Statement "SELECT `batch_id`, `count(batch_id) as _item_count` FROM creator_batches GROUP BY batch_id"] at /home/vagrant/kohaclone/C4/Creators/Lib.pm line 286. # found warning: Database returned the following error on attempted SELECT: something went wrong at /home/vagrant/kohaclone/C4/Creators/Lib.pm line 288. # expected to find warning: Database returned the following error on attempted SELECT: something went wrong ok 335 - $batches return -1 DBD::mysql::st execute failed: Unknown column 'count(batch_id) as _item_count' in 'field list' [for Statement "SELECT `batch_id`, `count(batch_id) as _item_count` FROM creator_batches WHERE 1 AND creator = ? GROUP BY batch_id" with ParamValues: 0='Labels'] at /home/vagrant/kohaclone/C4/Creators/Lib.pm line 286. Database returned the following error on attempted SELECT: Unknown column 'count(batch_id) as _item_count' in 'field list' at /home/vagrant/kohaclone/C4/Creators/Lib.pm line 288. ok 336 - There are 2 batches Can't use string ("-1") as an ARRAY ref while "strict refs" in use at t/db_dependent/Creators/Lib.t line 785. # Looks like your test exited with 255 just after 336. Dubious, test returned 255 (wstat 65280, 0xff00) Failed 310/645 subtests Test Summary Report ------------------- t/db_dependent/Creators/Lib.t (Wstat: 65280 Tests: 336 Failed: 1) Failed test: 334 Non-zero exit status: 255 Parse errors: Bad plan. You planned 645 tests but ran 336. Files=1, Tests=336, 2 wallclock secs ( 0.13 usr 0.04 sys + 1.82 cusr 0.13 csys = 2.12 CPU) Result: FAIL If I did apply both patch together, first test was without errors. I'm new in testing, maybe I did some mistake. Mike
After aplly of second patch I didi this error: Cannot detect source of '--no'! at /usr/share/perl/5.20/TAP/Parser/IteratorFactory.pm line 261. TAP::Parser::IteratorFactory::detect_source(TAP::Parser::IteratorFactory=HASH(0x12b1dd8), TAP::Parser::Source=HASH(0x12b1e98)) called at /usr/share/perl/5.20/TAP/Parser/IteratorFactory.pm line 211 TAP::Parser::IteratorFactory::make_iterator(TAP::Parser::IteratorFactory=HASH(0x12b1dd8), TAP::Parser::Source=HASH(0x12b1e98)) called at /usr/share/perl/5.20/TAP/Parser.pm line 470 TAP::Parser::_initialize(TAP::Parser=HASH(0x135d2d0), HASH(0x1051b70)) called at /usr/share/perl/5.20/TAP/Object.pm line 55 TAP::Object::new("TAP::Parser", HASH(0x1051b70)) called at /usr/share/perl/5.20/TAP/Object.pm line 130 TAP::Object::_construct(TAP::Harness=HASH(0xee3e20), "TAP::Parser", HASH(0x1051b70)) called at /usr/share/perl/5.20/TAP/Harness.pm line 793 TAP::Harness::make_parser(TAP::Harness=HASH(0xee3e20), TAP::Parser::Scheduler::Job=HASH(0x10519c0)) called at /usr/share/perl/5.20/TAP/Harness.pm line 592 TAP::Harness::_aggregate_single(TAP::Harness=HASH(0xee3e20), TAP::Parser::Aggregator=HASH(0xffb4a8), TAP::Parser::Scheduler=HASH(0xf036e0)) called at /usr/share/perl/5.20/TAP/Harness.pm line 684 TAP::Harness::aggregate_tests(TAP::Harness=HASH(0xee3e20), TAP::Parser::Aggregator=HASH(0xffb4a8), "t/db_dependent/Creators/Lib.t", "--no", "failures") called at /usr/share/perl/5.20/TAP/Harness.pm line 499 TAP::Harness::__ANON__() called at /usr/share/perl/5.20/TAP/Harness.pm line 512 TAP::Harness::runtests(TAP::Harness=HASH(0xee3e20), "t/db_dependent/Creators/Lib.t", "--no", "failures") called at /usr/share/perl/5.20/App/Prove.pm line 553 App::Prove::_runtests(App::Prove=HASH(0x96b720), HASH(0xebfc00), "TAP::Harness", "t/db_dependent/Creators/Lib.t", "--no", "failures") called at /usr/share/perl/5.20/App/Prove.pm line 511 App::Prove::run(App::Prove=HASH(0x96b720)) called at /usr/bin/prove line 13 Mike
Michal Denar, this is what I would expect you to see. Install a kohadevbox, vagrant up and then vagrant ssh. Get into the kohaclone directory. git checkout -b bug_21281 origin/master And then the following output should be practically identical. I did [SNIP] the excessive passing parts. vagrant@kohadevbox:kohaclone(bug_21281)$ git reset --hard origin/master HEAD is now at a9e859942c Bug 19550: (QA follow-up) Add missing [% USE %] vagrant@kohadevbox:kohaclone(bug_21281)$ git bz apply 21281 Bug 21281 - Label Template - Creation not working 78292 - Bug 21281: Surround creator_templates.rows with backquotes 78449 - Bug 21281: Correct t/db_dependent/Creators/Lib.t failures Apply? [(y)es, (n)o, (i)nteractive] i [Comment out second line, so only first one applies] Applying: Bug 21281: Surround creator_templates.rows with backquotes vagrant@kohadevbox:kohaclone(bug_21281)$ kshell kohadev kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove -r -v t/db_dependent/Creators/ t/db_dependent/Creators/Lib.t .. 1..645 ok 1 - use C4::Creators::Lib; [SNIP] ok 86 - creator is good DBD::mysql::st execute failed: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'rows = '7'' at line 1 [for Statement "SELECT * FROM creator_templates WHERE 1 AND rows = ? " with ParamValues: 0=7] at /home/vagrant/kohaclone/C4/Creators/Lib.pm line 179. Database returned the following error: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'rows = '7'' at line 1 at /home/vagrant/kohaclone/C4/Creators/Lib.pm line 181. ok 87 - There is 1 template matching Can't use string ("-1") as an ARRAY ref while "strict refs" in use at t/db_dependent/Creators/Lib.t line 376. # Looks like your test exited with 11 just after 87. Dubious, test returned 11 (wstat 2816, 0xb00) Failed 558/645 subtests Test Summary Report ------------------- t/db_dependent/Creators/Lib.t (Wstat: 2816 Tests: 87 Failed: 0) Non-zero exit status: 11 Parse errors: Bad plan. You planned 645 tests but ran 87. Files=1, Tests=87, 2 wallclock secs ( 0.02 usr 0.01 sys + 1.06 cusr 0.33 csys = 1.42 CPU) Result: FAIL kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ exit exit vagrant@kohadevbox:kohaclone(bug_21281)$ git bz apply 21281 Bug 21281 - Label Template - Creation not working 78292 - Bug 21281: Surround creator_templates.rows with backquotes 78449 - Bug 21281: Correct t/db_dependent/Creators/Lib.t failures Apply? [(y)es, (n)o, (i)nteractive] i [Comment out first one, so both will be applied] Applying: Bug 21281: Correct t/db_dependent/Creators/Lib.t failures vagrant@kohadevbox:kohaclone(bug_21281)$ kshell kohadev kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove -r -v t/db_dependent/Creators/ t/db_dependent/Creators/Lib.t .. 1..645 ok 1 - use C4::Creators::Lib; [SNIP] ok 645 - categorycode is good ok All tests successful. Files=1, Tests=645, 2 wallclock secs ( 0.06 usr 0.05 sys + 1.04 cusr 0.39 csys = 1.54 CPU) Result: PASS kohadev-koha@kohadevbox:/home/vagrant/kohaclone$
Created attachment 78668 [details] [review] Bug 21281: Surround creator_templates.rows with backquotes 'rows' is a reserved word since MariaDB 10.2.4 and MySQL https://mariadb.com/kb/en/library/mariadb-1024-release-notes/ https://dev.mysql.com/doc/refman/8.0/en/keywords.html Test plan: Do not apply this patch and make sure you recreate the reported issue Apply this patch and confirm that it is now fixed. QA will take care for the changes in installer and test files Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Created attachment 78669 [details] [review] Bug 21281: Correct t/db_dependent/Creators/Lib.t failures TEST PLAN --------- 1) apply first patch 2) kshell kohadev 3) prove -r -v t/db_dependent/Creators/ -- failures 4) apply this patch 5) repeat 2,3 -- no failures 6) run koha qa test tools Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
- my $query = "SELECT `$params[0]` FROM $params[1]"; + my $fields_list = $params[0]; If I am not mistaken, params[0] is a single field, not a field list. So the new variable name is confusing. If it would be a list of fields, this code does not look good: + if (index($fields_list, ' ')==-1 && index($fields_list,',')==-1 && $fields_list ne '*') { + $fields_list = "`$fields_list`"; + } If you would pass 'rows, something', it would not add backtics.
Working on a follow-up..
Created attachment 79192 [details] [review] Bug 21281: Surround creator_templates.rows with backquotes 'rows' is a reserved word since MariaDB 10.2.4 and MySQL https://mariadb.com/kb/en/library/mariadb-1024-release-notes/ https://dev.mysql.com/doc/refman/8.0/en/keywords.html Test plan: Do not apply this patch and make sure you recreate the reported issue Apply this patch and confirm that it is now fixed. QA will take care for the changes in installer and test files Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 79193 [details] [review] Bug 21281: Correct t/db_dependent/Creators/Lib.t failures TEST PLAN --------- 1) apply first patch 2) kshell kohadev 3) prove -r -v t/db_dependent/Creators/ -- failures 4) apply this patch 5) repeat 2,3 -- no failures 6) run koha qa test tools Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 79194 [details] [review] Bug 21281: (QA follow-up) Introduce _add_backtics Added a (initially trivial) test in Creators/Lib.t too. Sometimes you start something, but you end somewhere else ;) The test seems a bit slower now (regex, lookahead, etc). But this should not hurt label creators in daily use. Advantages: code is even more solid, consolidated in one place and can be tested on its own. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
> QA will take care for the changes in installer and test files Sorry. Same lack amount of time. Warm invitation to add them now :)
(In reply to Marcel de Rooy from comment #19) > The test seems a bit slower now (regex, lookahead, etc). But this should Not even lookahead btw
(In reply to Marcel de Rooy from comment #20) > > QA will take care for the changes in installer and test files > > Sorry. Same lack amount of time. > Warm invitation to add them now :) It was part of the test plan: the changes to .sql and .t cannot be tested by everybody. Nothing is missing. Thanks for the follow-up
Comment on attachment 79194 [details] [review] Bug 21281: (QA follow-up) Introduce _add_backtics Review of attachment 79194 [details] [review]: ----------------------------------------------------------------- Thanks for this. I didn't get around to writing something similar.
Awesome work all! Pushed to master for 18.11
Pushed to 18.05.x for 18.05.05
Pushed to 17.11.x for 17.11.11
The bug is still present in 19.05.03-1. I am using Debian package with MariaDB 10.4.7. As far as I can see the patch won't work with 19.05. Did anyone manage to fix it? [Wed Sep 04 10:38:34.359103 2019] [cgi:error] [pid 1214] [client 127.0.0.1:35854] AH01215: [Wed Sep 4 10:38:34 2019] label-edit-template.pl: DBD::mysql::st execute failed: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'rows, row_gap, label_height, col_gap, label_width, template_code, left_text_marg' at line 1 [for Statement "INSERT INTO creator_templates (creator, template_desc, left_margin, rows, row_gap, label_height, col_gap, label_width, template_code, left_text_margin, top_margin, page_width, top_text_margin, profile_id, page_height, cols, units) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?);" with ParamValues: 0='Labels', 1="Default description", 2="8", 3="8", 4=0, 5="34", 6="3", 7="63", 8="DEFAULT TEMPLATE", 9=0, 10="12", 11="209", 12=0, 13="0", 14="296", 15="3", 16="MM"] at /usr/share/koha/lib/C4/Creators/Template.pm line 199.: /usr/share/koha/intranet/cgi-bin/labels/label-edit-template.pl, referer: https://intra.koha.wsjo.pl/cgi-bin/koha/labels/label-edit-template.pl?op=new
(In reply to Krzysztof K. from comment #27) > The bug is still present in 19.05.03-1. I am using Debian package with > MariaDB 10.4.7. As far as I can see the patch won't work with 19.05. Did > anyone manage to fix it? > > [Wed Sep 04 10:38:34.359103 2019] [cgi:error] [pid 1214] [client > 127.0.0.1:35854] AH01215: [Wed Sep 4 10:38:34 2019] label-edit-template.pl: > DBD::mysql::st execute failed: You have an error in your SQL syntax; check > the manual that corresponds to your MariaDB server version for the right > syntax to use near 'rows, row_gap, label_height, col_gap, label_width, > template_code, left_text_marg' at line 1 [for Statement "INSERT INTO > creator_templates (creator, template_desc, left_margin, rows, row_gap, > label_height, col_gap, label_width, template_code, left_text_margin, > top_margin, page_width, top_text_margin, profile_id, page_height, cols, > units) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?);" with ParamValues: > 0='Labels', 1="Default description", 2="8", 3="8", 4=0, 5="34", 6="3", > 7="63", 8="DEFAULT TEMPLATE", 9=0, 10="12", 11="209", 12=0, 13="0", > 14="296", 15="3", 16="MM"] at /usr/share/koha/lib/C4/Creators/Template.pm > line 199.: /usr/share/koha/intranet/cgi-bin/labels/label-edit-template.pl, > referer: > https://intra.koha.wsjo.pl/cgi-bin/koha/labels/label-edit-template.pl?op=new Hi, you might be seeing bug 23289 that was just pushed to master and will probably make it into the next maintenance release of 19.05.