Make JDatabaseDriver::splitSql() handle comments and remove trimSql functions #9369

richard67 · 2016-03-10T17:57:04Z

Summary of Changes

This pull request makes JDatabaseDriver::splitSql() strip off comments beginning with "--" or "#" until the end of line and c style comments "/* ... */" (also inline and multi-line) from the sql statements, trim the query and not return empty queries.

For the c style comments there are exceptions for mysql specials, which will not be stripped off, see http://dev.mysql.com/doc/refman/5.7/en/comments.html.

At the moment those specials are allowed regardless of the database kind, but if this is desired I can with a later PR make overrides for the mysql drivers and remove these specials then from base driver here.

In addition this PR removes the local functions trimQuery() and their usage whereever they were recently added to work around the missing handling of comments (except single line "#") of JDatabaseDriver::splitSql().

The handling of comments in sql statements retrieved from sql files (e.g. for schema updates or extensions) is not correct in current staging because it does not respect if comment characters appear in quoted text (and so not are comments). This PR corrects also this.

Testing Instructions

Overview, preconditions

Because the changed splitSql function and the replacement of the local workarounds (trimSql functions) has impact on any kind of installation action, there are several tests necessary to make sure that all still works well:

New installation of Joomla!
Updating Joomla! with Joomla! Update component
Running Database Schema updates
Installing extensions which include sql scripts for updating database
Parsing a test sql script with all handled kinds of comments

All tests should be done for all supported kinds of databases, and in case of mysql databases also old without and new with utf8mb4 support.

Hint on utf8mb4 support of mysql: It is NOT supported, if database server version is lower than 5.5.3, or if mysqlnd client used with a version lower than 5.0.9, or if other database client used with a version lower than 5.5.3.

Test 1: New installation of Joomla!

Perform a new installation of Joomla! using the latest staging + this PR as installation source.

You can find it here: https://github.com/richard67/joomla-cms/archive/correct-split-sql-in-db-driver.zip.

Verify that all worked well and all Joomla! database tables have been created, and the database is shown as updated and having no problems in "Extensions -> Manager -> Database":

Database is up to date
93 changes checked
145 changes skipped

Test 2: Updating Joomla! with Joomla! Update component

Before you update, make an export of your database so you can later use it in Test 3.

Set "Custom URL" as test channel in the update component's options and enter following as custom URL:

For updating a 3.x <= 3.4.8: http://test5.richard-fath.de/list_test1.xml
For updating a 2.5: http://test5.richard-fath.de/list_test1_j25.xml

Start the update and wait until it has finished.

Verify that all worked well and the database is shown as updated and having no problems in "Extensions -> Manager -> Database":

Database is up to date
93 changes checked
145 changes skipped

Test 3: Running Database Schema updates

After having made the previous Test 2, delete all database tables with and restored the old pre-update database using the export file made at the beginning of Test 2.

Goto "Extensions -> Manager -> Database" and verify that updated according to the version you have saved the old data from are shown as to be done, and also the utf8mb4 or utf8 conversion is shown as to be done as the last open problem.

Click the "Fix" button.

Result:

Database is up to date
93 changes checked
145 changes skipped

Test 4: Installing extensions which include sql scripts for updating database

With either the new installed 3.5.0 latest staging + this patch from Test 1 or the updated one from Test 2, verify that installing extensions works still well.

An example with an sql script containing a hashtag comment and a column with default value '#fff' can be found here: http://test5.richard-fath.de/sampleerror.zip. This file was once provided with issue #9251 and installs a table #__bla (replace #__ by your db prefix) if all goes well. If you use this example, check after installation that column bla of this new table has a default value of '#fff', so the '#' has not been stripped off by the splitSql function.

Test 5: Parsing the test sql script.

If you have no 3.5.0 latest staging + this patch available from the previous tests 1 and 2, apply this patch on latest staging or a 3.5.0 RC.

Download following test script and store it on a location on your server which is accessibly for Joomla!:

For mysql: http://test5.richard-fath.de/test_pr9369_mysql.sql
For postgresql: http://test5.richard-fath.de/test_pr9369_postgresql.sql
For sqlsrv/sqlazure: http://test5.richard-fath.de/test_pr9369_sqlazure.sql

Add following code snippet below the body tag in the index.php of your site template (e.g. protostar).

'; } echo '
'; ?>

Replace the path by the path where you stored the test sql script, and in case of non-mysql db, change db type in the file name.

Now go to your site and verify that the sql statements have been correctly extracted from the sql script, removing all comments "--" and "#" for complete lines and for parts until line end.

The output should look as follows on mysql and similar on the other databases:

ALTER TABLE #__test_table_1 ADD COLUMN header1 smallint(3) NOT NULL DEFAULT 1;
ALTER TABLE #__test_table_2 ADD COLUMN header2 smallint(3) NOT NULL DEFAULT 2;
ALTER TABLE #__test_table_3 ADD COLUMN header3 smallint(3) NOT NULL DEFAULT 3;
ALTER TABLE #__test_table_4 ADD COLUMN header4 smallint(3) NOT NULL DEFAULT 4;
ALTER TABLE #__test_table_5 ADD COLUMN header5 smallint(3) NOT NULL DEFAULT 5;
ALTER TABLE /*! test special 1 / #__test_table_6 ADD COLUMN header6 smallint(3) NOT NULL DEFAULT 6;
ALTER TABLE /+ test special 2 / #__test_table_7 ADD COLUMN header7 smallint(3) NOT NULL DEFAULT 7;
CREATE TABLE IF NOT EXISTS #__test_table_8 ( id int(10) unsigned NOT NULL AUTO_INCREMENT COMMENT 'Primary Key', name varchar(50) NOT NULL COMMENT 'The unique name for the asset.', title varchar(100) NOT NULL COMMENT 'The descriptive title for the asset.', rules varchar(5120) NOT NULL COMMENT 'JSON encoded access control.', PRIMARY KEY (id), UNIQUE KEY idx_asset_name (name) KEY idx_lft_rgt (lft,rgt), KEY idx_parent_id (parent_id) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 DEFAULT COLLATE=utf8mb4_unicode_ci;
UPDATE #__test_table_9 SET textcol9 = 'Single quoted text "x" \n # dada / no comment / -- yeah ';
UPDATE #__test_table_10 SET textcol10 = "Double quoted text 'x' \n # dada / no comment */ -- yeah ";

Names should be properly quoted (not shown properly here because name quotes are used here as markup).

You will see in my sql that I did not end the file with a new line and the last statement not with a semicolon.

This is compatible to the old behavior.

You can play around with adding the semicolon or the newline or both, the statements shown by the test should still be the same (all end with a semicolon).

You can also use the test sql script and the code snippet on an unpatched 3.5.0 RC or staging and see that the output of queries shows a mess which will very likely cause SQL syntax errors.

Make JDatabaseDriver::splitSql handle single line and multi line comments and end of line comments and trim the query and not return empty queries.

Remove local trimQuery functions and calls to them for queries obtained with JDatabaseDriver::splitSql(), since this strips off comments now.

wilsonge · 2016-03-11T00:03:33Z

administrator/components/com_admin/script.php

+						$converted = 0;
+
+						// Still render the error message from the Exception object
+						JFactory::getApplication()->enqueueMessage($e->getMessage(), 'error');


Can we use the string I added last week here (check the database fixer thing)

I will not add it in the loop because there it will be repeated. I leave the sql error there for more info, and add the new message below as summary in case if some error happened before in the loop.

richard67 · 2016-03-11T00:39:20Z

administrator/components/com_admin/script.php

+						$converted = 0;
+
+						// Show an error message telling to check database problems
+						JFactory::getApplication()->enqueueMessage(JText::_('JLIB_DATABASE_ERROR_DATABASE_UPGRADE_FAILED'), 'error');


@wilsonge Done.

@wilsonge Hmm, meanwhile I think this message here is not a good idea. Above it maybe makes sense because not in a look and handling with this special new table nobody should care about. But here in the loop if multiple errors we repeat it multiple times. We don't exit the loop in case of error, and we shouldn't, so it was ok with the exception text I think, it could be helpful information. And this new text we should use in addition at the end, after the loop, if $converted == 0 (means somethign failed) only 1 time. I will change this in that way now.

richard67 · 2016-03-11T01:06:07Z

Corrected typo in testing instructions, code snippet for test 5 used $sql instead of $buffer in call to splitSql.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9369.}

Change back previous commit but add the new mesage to the end for the case if something failed

richard67 · 2016-03-11T01:40:29Z

administrator/components/com_admin/script.php

+			// Show an error message telling to check database problems
+			JFactory::getApplication()->enqueueMessage(JText::_('JLIB_DATABASE_ERROR_DATABASE_UPGRADE_FAILED'), 'error');
+		}
+


I added it here instead, after the loop.

richard67 · 2016-03-11T11:49:20Z

Please find me some testers.

Not every tester has to perform all tests, but then he/she should not in coment to test result what was tested and what not, e.g. "Tested all except Test 5" or so.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9369.}

Twincarb · 2016-03-11T11:59:56Z

@richard67 I am happy to test this afternoon if it hasn't been completed.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9369.}

richard67 · 2016-03-11T12:01:21Z

Great, thanks in advance.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9369.}

Twincarb · 2016-03-11T18:17:17Z

I have tested this item 🔴 unsuccessfully on 46fdca1

Having a bit of a run around here.

Test 1 Pass

Test 2 Problem - Joomla 2.5.28 upgrade to J3.5-richard-version
With this test, there seems to be issues with saving items, the examples I can use are installed kunena but the save and save and close wouldn't work when creating a module. Also having installed the patch tester entered my github parameters and save failed to work.
Test 2 Pass Joomla 3.4.8 upgrade to J3.5-richard-version

Test 3 I am getting an error when importing the database error is below

"CREATE TABLE h057e_assets (
id int(10) UNSIGNED NOT NULL COMMENT 'Primary Key',
parent_id int(11) NOT NULL DEFAULT '0'COMMENT AS Nested set parent.,
lft int(11) NOT NULL DEFAULT '0'COMMENT AS Nested set lft.,
rgt int(11) NOT NULL DEFAULT '0'COMMENT AS Nested set rgt.,
level int(10) UNSIGNED NOT NULL COMMENT 'The cached level in the nested tree.',
name varchar(50) COLLATE utf8mb4_unicode_ci NOT NULL COMMENT 'The unique name for the asset.\n',
title varchar(100) COLLATE utf8mb4_unicode_ci NOT NULL COMMENT 'The descriptive title for the asset.',
rules varchar(5120) COLLATE utf8mb4_unicode_ci NOT NULL COMMENT 'JSON encoded access control.'
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci
MySQL said: Documentation

#1064 - You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'AS Nested set parent.,
lft int(11) NOT NULL DEFAULT '0'COMMENT AS `Nested ' at line 3"

Test 4 Pass Joomla 2.5 upgraded and 3.4 upgraded (the table is inserted correctly both times)

Test 5 Joomla 3.4.8 upgrade to J3.5-richard-version
I had to install bug tester and install patch #9369 in order to get it to work I didn't expect to need to do this.

If you would like me to run any test again please let me know

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9369.}

richard67 · 2016-03-11T18:26:48Z

Thanks for testing, I'll have a look on it when I have time, later tonight or tomorrow.

richard67 · 2016-03-11T18:35:58Z

@Twincarb Did you upgrade with Joomla! Update component as descibed? Or did you use the extension installer to ugrade Joomla!? In this case please try again with Joomla! Update component.

Twincarb · 2016-03-11T18:43:21Z

@richard67 I used the Joomla Update component each time, I am just going to do a job in the garage then I will take another look as well.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9369.}

richard67 · 2016-03-11T18:50:27Z

@Twincarb How did you import your database? When I use my test code snippet from Test 5 to show the content of installation/sql/mysql/joomla.sql from a 2.5 (where the defaults where '0' and not 0), the statement for creating the assets table is shown by moy code snipped as follows:

CREATE TABLE IF NOT EXISTS #__assets ( id int(10) unsigned NOT NULL AUTO_INCREMENT COMMENT 'Primary Key', parent_id int(11) NOT NULL DEFAULT '0' COMMENT 'Nested set parent.', lft int(11) NOT NULL DEFAULT '0' COMMENT 'Nested set lft.', rgt int(11) NOT NULL DEFAULT '0' COMMENT 'Nested set rgt.', level int(10) unsigned NOT NULL COMMENT 'The cached level in the nested tree.', name varchar(50) NOT NULL COMMENT 'The unique name for the asset.\n', title varchar(100) NOT NULL COMMENT 'The descriptive title for the asset.', rules varchar(5120) NOT NULL COMMENT 'JSON encoded access control.', PRIMARY KEY (id), UNIQUE KEY idx_asset_name (name), KEY idx_lft_rgt (lft,rgt), KEY idx_parent_id (parent_id) ) DEFAULT CHARSET=utf8;

So as you can see there is not this syntax error with "... DEFAULT '0'COMMENT ...".

How did you export and import your pre-updated 2.5 database? Did you run the joomla.sql? Or did you export / import with phpmyadmin? Or Akeba backup? Maybe they have a problem with old 2.5?

I cannot replicate this here up to now.

richard67 · 2016-03-11T18:52:02Z

@Twincarb If you do the same test for updating with an unpatched 3.5.0 RC. Do you have the same problems? Can you check this? If so, they are not relatef tho this PR.

richard67 · 2016-03-11T18:59:10Z

@Twincarb Hmm, I've just processed the complete joomla.sql of a 2.5.28 with the patched splitSql function as shown in my test code snippet for Test 5 for the test scripts. Then I copied the complete sql from my frontpage into phpmyadmin and run the SQL. It all run without any error, and at the end all core tables of a 2.5.28 were created. So I have no idea what happened in your test. Please try what you did again with unpatched 3.5.0 RC and let me know if ok or same errors. Thanks in advance.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9369.}

Twincarb · 2016-03-11T19:30:04Z

@richard67 Doing a test upgrade to 3.5rc was my next plan, although I am not sure where to find the xml file for it.

With the sql, I did a quick export in phpmyadmin and then used the import to restore the dump. But it gave the error above

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9369.}

richard67 · 2016-03-11T19:45:33Z

@Twincarb Hmm, I also have a problem to update 2.5.28 to 3.5.0 RC using testing channel. Get told I am already at max version.

I prepared a custom URL like for my patched version but with "3" instead of "1" in the file name: http://test5.richard-fath.de/list_test3_j25.xml. This points to an original 3.5.0 RC.

But @wilsonge do you know why 2.5 cannot be updated to 3.5.0 RC via testing channel of Joomla! Update Component? Mistake? Or by purpose?

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9369.}

richard67 · 2016-03-11T19:50:35Z

@Twincarb The problem with syntax error in your export I cannot reproduce here. Did export from a 2.5.28 and it looks as it should, space between "DEFAULT '0'" and "COMMENT". Maybe this is a problem with your export/import?

What I can reproduce is that editing and saving modules does not work after updating the 2.5.28 to my patched 3.5.0 RC, so I want to test if this is also the case with unpatched 3.5.0 RC. I let you know my result, but if you find time, pleas test, too.

Thansk for your help.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9369.}

mbabker · 2016-03-11T19:50:57Z

It's purposeful because 2.5 and 3.x share the same testing channel. So when we were pushing 2.5 and 3.x releases together, we couldn't put the beta/RC for 2.5 to 3.x upgrades on that testing channel because that would override testing 2.5 to 2.5 updates.

Not as much of an issue now that 2.5's EOL I guess. In theory this change does it with the custom XML file we've got in the repo, or instead of the list_test_25to3x.xml change you change this line in the main testing channel's XML to use the 3.x releases.

richard67 · 2016-03-11T19:55:43Z

@mbabker Well, meanwhile I helpe myself with a separate custom URL on my domain serving the 3.5.0 RF for 2.5.28, http://test5.richard-fath.de/list_test3_j25.xml. If you want and have time you could help with testing this PR here.

@Twincarb I tested with this URL and it serves the unpatched RC, and it has the same problem: Saving changes when editing modules does not work.

So this problem is not related to this PR here. Can you test and confirm?

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9369.}

Twincarb · 2016-03-11T20:04:39Z

@richard67 No worries will test with your xml there, I need to look into why I am not able to restore databases on my vps the only change on this server is I set db defaults to utf8mb4 , I have a second one rebuilding at the moment so will be able to test again when that is set up. In the meantime I will do the 2.5.28 upgrade using your xml.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9369.}

richard67 · 2016-03-11T20:08:30Z

Before upgrading the 2.5.28, export its db again and then check in the export file if for the very first table at the top, assets, you see "DEFAULT '0' COMMENT ..." (= ok) or "DEFAULT '0'COMMENT ..." (=error) for columns lft and rgt, and let me know the result. If it is wrong in your export, the problem is also not related to this PR. Thanks in advance.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9369.}

Twincarb · 2016-03-11T20:22:59Z

@richard67 Just had a look at the sql dump, the first line is missing the space which should be in here "DEFAULT '0'COMMENT ...
the same applies to lft and rgt columns.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9369.}

richard67 · 2016-03-14T00:34:45Z

Changed description and example sql script output back for c style comment handling, but with exceptions for mysql specials.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9369.}

andrepereiradasilva · 2016-03-14T00:40:33Z

richard, a question for the future: wouldn't it be easy to remove the comments with regular expressions? see for instance http://stackoverflow.com/questions/7100127/regex-to-match-mysql-comments/13823363#answer-13823363 (didn't test)

richard67 · 2016-03-14T00:53:25Z

@andrepereiradasilva I have thought about it and exactly had this stackoverflow answer in mind, but I remember from past discussions about comment strippings in other issues (with the "#" comments) that complex regexes are not so welcome for some of the maintainers, and I also find them not easy to maintain, even if well commented, and the solutoon from stackoverflow did not work as it was for our purpose.

And so I decided to do it based on the existing solution.

Regarding preformance I think the one is like the other ... the regex parser also has to loop and check backwards, so i think there is not a big difference in performance.

So at the end it was a matter of taste, and what could be better to understand for others.

PR is ready for test now by the way, with last commit I added back the handling of c style comments but with exceptions for the mysql specials.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9369.}

andrepereiradasilva · 2016-03-14T00:55:32Z

ok thanks for the awnser

richard67 · 2016-03-14T00:57:24Z

Would you have preferred the regex way, andre?

andrepereiradasilva · 2016-03-14T01:00:05Z

always prefer to write less code :)

richard67 · 2016-03-14T01:09:36Z

I see .. well the problem with the stackoverflow solution was that it did not do what we need, split the complete filecontent into statement with borders at semicolons (which are not in quoted text) AND strip off the comments, inline, single line and multi line. This one you linked did not work for the complete file with multiple queries. I found another one for splitting but this then did not handle the comments. An all in one solution I did not find with regex, but found an answer in stackexchange telling that this would not be a trivial thing.

andrepereiradasilva · 2016-03-14T01:14:01Z

but found an answer in stackexchange telling that this would not be a trivial thing.

yeah, with complex regex it never is simple

but this PR is ready for test and that is what matters

Twincarb · 2016-03-14T07:32:42Z

I will be able to test again this evening.
Is it possible for Joomla to set a single standard for comments in sql, and other files too which is perhaps then adopted in J4.0 but encouraged in J3.5+ which gives plenty of time for coders to comment in the single Joomla way.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9369.}

richard67 · 2016-03-14T08:31:53Z

At the moment we have this single standard for sql (which works somehow for lucky circumstances), and with this PR we will also have same standard for all kinds of databases (but this time working well), because it not makes a difference between the databas kinds.

So I think this PR would be a good step in your direction.

Or did you mean we shall support only 1 kind of comment for all kinds of files?

I think this will not come true because sql guys not wanna miss their "--", while the shell scrip guys prefer their "#" ... not even to talk aout the c programmers.

Thanks in avance for later testing.

Twincarb · 2016-03-14T09:53:49Z

I was thinking of having a "standard" way for each of the file types..
but on my drive to work thought again, as it's only sql at the moment that would benefit.

Testing wise do I need to run all 5 tests? I presume you have updated your hosted files?

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9369.}

richard67 · 2016-03-14T09:59:33Z

Yes, I have updated my files, the test sql scripts and also the update container behind the custom url.

I think about doing 1 more commit for making code better without changing functionality, but I can also do this later if time gets too tight. So if you or someone will test this before, I will not do that 1 more commit, in order not to need tests again, so it can go into 3.5.0 as it is now.

Otherwise, if I see nobody tested yet when I have it prepared later, then I will check it in when my files are updated again.

Make not append 1 char in each loop if not comment or comment end but save the start index as it was in old code without this PR and then copy to buffer only if necessary. Also restore old behavior of not ingnoring the complete statement if it has unclosed quote at file end, so that as before with this PR people will see the sql error and not see nothing like with this PR before this commit.

joomla-cms-bot · 2016-03-14T12:48:50Z

This PR has received new commits.

CC: @Twincarb, @wilsonge

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9369.}

richard67 · 2016-03-14T12:51:25Z

Ready for test. Test update container is already updated, same custom URL. Comment to latest commit describes the changes. The test update container still is based on RC1 and not RC2 because with RC2 I had troubles with the changed handling of the manifest file.

richard67 · 2016-03-14T12:52:19Z

Code is even closer now to code before the PR, so diff (comparison) for the complete PR is better understandable now, even if diff for the last commit might be less clear.

brianteeman · 2016-03-14T15:58:25Z

Test One
Test Two
Test Three
Test Four

I didnt do test five - didnt quite understand

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9369.}

richard67 · 2016-03-14T16:01:55Z

Thanks for testing, Brian. Test 5 is just using a code snippet to read in a test sql file full of comments and stuff and statements, use the splitSql function modified by this PR to split the file content into sql statements array, and then output the extracted sql statements to the web site's page and check if it looks like the shown output, i.e. only contains the queries.

richard67 · 2016-03-14T16:04:46Z

P.S.: The statements in test 5 are of course not executed. It is only to show what is result of reading the test sql file. So you can also do this test with sql file for postgresql when using a mysql.

richard67 · 2016-03-14T17:21:47Z

P.P.S.: @brianteeman Ah, I just see that the "
" was messed up in my code snippet here because GutHub treats it as html, maybe that's what you did not understand? Have corrected it in the testing instrucions, so the code snippet is readable now and still can be copied into text editor.

Twincarb · 2016-03-14T18:54:49Z

I have tested this item ✅ successfully on 02462ed

All Tests passed.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9369.}

wilsonge · 2016-03-14T22:51:54Z

I finished the final test for @brianteeman successfully. So with 2 good tests i'm merging this

Make JDatabaseDriver::splitSql() handle comments and remove trimSql functions

richard67 · 2016-03-14T22:57:24Z

Thanks everybody for testing.

wilsonge · 2016-03-14T22:58:10Z

Thankyou richard!

richard67 added 2 commits March 10, 2016 18:25

Make JDatabaseDriver::splitSql handle comments

3983259

Make JDatabaseDriver::splitSql handle single line and multi line comments and end of line comments and trim the query and not return empty queries.

Remove local trimQuery functions

d2a1ff8

Remove local trimQuery functions and calls to them for queries obtained with JDatabaseDriver::splitSql(), since this strips off comments now.

joomla-cms-bot added the PR-staging label Mar 10, 2016

Code style

1b85647

wilsonge reviewed Mar 11, 2016
View reviewed changes

wilsonge added the 3.5.0-blocker label Mar 11, 2016

Show special error message

199a11c

richard67 reviewed Mar 11, 2016
View reviewed changes

Correct again error messages

46fdca1

Change back previous commit but add the new mesage to the end for the case if something failed

richard67 reviewed Mar 11, 2016
View reviewed changes

infograf768 mentioned this pull request Mar 11, 2016

ModulesViewModules::$filterForm error after session timeout #9368 #9371

Merged

wilsonge added a commit that referenced this pull request Mar 14, 2016

Merge pull request #9369 from richard67/correct-split-sql-in-db-driver

f9034e4

Make JDatabaseDriver::splitSql() handle comments and remove trimSql functions

wilsonge merged commit f9034e4 into joomla:staging Mar 14, 2016

wilsonge added this to the Joomla! 3.5.0 milestone Mar 14, 2016

wilsonge removed the 3.5.0-blocker label Mar 14, 2016

richard67 deleted the correct-split-sql-in-db-driver branch March 14, 2016 22:57

mbabker added a commit to joomla-framework/database that referenced this pull request Apr 23, 2016

Port joomla/joomla-cms#9369

d82d629

Make JDatabaseDriver::splitSql() handle comments and remove trimSql functions #9369

Make JDatabaseDriver::splitSql() handle comments and remove trimSql functions #9369

Conversation

richard67 commented Mar 10, 2016

Summary of Changes

Testing Instructions

Overview, preconditions

Test 1: New installation of Joomla!

Test 2: Updating Joomla! with Joomla! Update component

Test 3: Running Database Schema updates

Test 4: Installing extensions which include sql scripts for updating database

Test 5: Parsing the test sql script.

wilsonge Mar 11, 2016

Choose a reason for hiding this comment

richard67 Mar 11, 2016

Choose a reason for hiding this comment

richard67 Mar 11, 2016

Choose a reason for hiding this comment

richard67 Mar 11, 2016

Choose a reason for hiding this comment

richard67 commented Mar 11, 2016

richard67 Mar 11, 2016

Choose a reason for hiding this comment

richard67 commented Mar 11, 2016

Twincarb commented Mar 11, 2016

richard67 commented Mar 11, 2016

Twincarb commented Mar 11, 2016

richard67 commented Mar 11, 2016

richard67 commented Mar 11, 2016

Twincarb commented Mar 11, 2016

richard67 commented Mar 11, 2016

richard67 commented Mar 11, 2016

richard67 commented Mar 11, 2016

Twincarb commented Mar 11, 2016

richard67 commented Mar 11, 2016

richard67 commented Mar 11, 2016

mbabker commented Mar 11, 2016

richard67 commented Mar 11, 2016

Twincarb commented Mar 11, 2016

richard67 commented Mar 11, 2016

Twincarb commented Mar 11, 2016

richard67 commented Mar 14, 2016

andrepereiradasilva commented Mar 14, 2016

richard67 commented Mar 14, 2016

andrepereiradasilva commented Mar 14, 2016

richard67 commented Mar 14, 2016

andrepereiradasilva commented Mar 14, 2016

richard67 commented Mar 14, 2016

andrepereiradasilva commented Mar 14, 2016

Twincarb commented Mar 14, 2016

richard67 commented Mar 14, 2016

Twincarb commented Mar 14, 2016

richard67 commented Mar 14, 2016

joomla-cms-bot commented Mar 14, 2016

richard67 commented Mar 14, 2016

richard67 commented Mar 14, 2016

brianteeman commented Mar 14, 2016

richard67 commented Mar 14, 2016

richard67 commented Mar 14, 2016

richard67 commented Mar 14, 2016

Twincarb commented Mar 14, 2016

wilsonge commented Mar 14, 2016

richard67 commented Mar 14, 2016

wilsonge commented Mar 14, 2016