{"action":"created","issue":{"url":"https://api.github.com/repos/joomla/joomla-cms/issues/9369","repository_url":"https://api.github.com/repos/joomla/joomla-cms","labels_url":"https://api.github.com/repos/joomla/joomla-cms/issues/9369/labels{/name}","comments_url":"https://api.github.com/repos/joomla/joomla-cms/issues/9369/comments","events_url":"https://api.github.com/repos/joomla/joomla-cms/issues/9369/events","html_url":"https://github.com/joomla/joomla-cms/pull/9369","id":139964848,"number":9369,"title":"Make JDatabaseDriver::splitSql() handle comments and remove trimSql functions","user":{"login":"richard67","id":7413183,"avatar_url":"https://avatars.githubusercontent.com/u/7413183?v=3","gravatar_id":"","url":"https://api.github.com/users/richard67","html_url":"https://github.com/richard67","followers_url":"https://api.github.com/users/richard67/followers","following_url":"https://api.github.com/users/richard67/following{/other_user}","gists_url":"https://api.github.com/users/richard67/gists{/gist_id}","starred_url":"https://api.github.com/users/richard67/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/richard67/subscriptions","organizations_url":"https://api.github.com/users/richard67/orgs","repos_url":"https://api.github.com/users/richard67/repos","events_url":"https://api.github.com/users/richard67/events{/privacy}","received_events_url":"https://api.github.com/users/richard67/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/joomla/joomla-cms/labels/3.5.0-blocker","name":"3.5.0-blocker","color":"fbca04"},{"url":"https://api.github.com/repos/joomla/joomla-cms/labels/PR-staging","name":"PR-staging","color":"c7def8"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":15,"created_at":"2016-03-10T17:57:04Z","updated_at":"2016-03-11T19:50:58Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/joomla/joomla-cms/pulls/9369","html_url":"https://github.com/joomla/joomla-cms/pull/9369","diff_url":"https://github.com/joomla/joomla-cms/pull/9369.diff","patch_url":"https://github.com/joomla/joomla-cms/pull/9369.patch"},"body":"#### Summary of Changes\r\n\r\nThis pull request makes JDatabaseDriver::splitSql() handle single line, multi line\r\nand end of line comments, trim the query and not return empty queries.\r\n\r\nIn addition it 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().\r\n\r\n#### Testing Instructions\r\n\r\n##### Overview, preconditions\r\n\r\nBecause 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:\r\n\r\n1. New installation of Joomla!\r\n2. Updating Joomla! with Joomla! Update component\r\n3. Running Database Schema updates\r\n4. Installing extensions which include sql scripts for updating database\r\n5. Parsing a test sql script with all handled kinds of comments\r\n\r\nAll tests should be done for all supported kinds of databases, and in case of mysql databases also old without and new with utf8mb4 support.\r\n\r\nHint 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.\r\n\r\n##### Test 1: New installation of Joomla!\r\n\r\nPerform a new installation of Joomla! using the latest staging + this PR as installation source.\r\n\r\nYou can find it here: [https://github.com/richard67/joomla-cms/archive/correct-split-sql-in-db-driver.zip](https://github.com/richard67/joomla-cms/archive/correct-split-sql-in-db-driver.zip \"https://github.com/richard67/joomla-cms/archive/correct-split-sql-in-db-driver.zip\").\r\n\r\nVerify 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\":\r\n\r\n- Database is up to date\r\n- 93 changes checked\r\n- 145 changes skipped\r\n\r\n##### Test 2: Updating Joomla! with Joomla! Update component\r\n\r\nBefore you update, make an export of your database so you can later use it in Test 3.\r\n\r\nSet \"Custom URL\" as test channel in the update component's options and enter following as custom URL:\r\n\r\n- For updating a 3.x <= 3.4.8: [http://test5.richard-fath.de/list_test1.xml](http://test5.richard-fath.de/list_test1.xml \"http://test5.richard-fath.de/list_test1.xml\")\r\n- For updating a 2.5: [http://test5.richard-fath.de/list_test1_j25.xml](http://test5.richard-fath.de/list_test1_j25.xml \"http://test5.richard-fath.de/list_test1_j25.xml\")\r\n\r\nStart the update and wait until it has finished.\r\n\r\nVerify that all worked well and the database is shown as updated and having no problems in \"Extensions -> Manager -> Database\":\r\n\r\n- Database is up to date\r\n- 93 changes checked\r\n- 145 changes skipped\r\n\r\n##### Test 3: Running Database Schema updates\r\n\r\nAfter 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.\r\n\r\nGoto \"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.\r\n\r\nClick the \"Fix\" button.\r\n\r\nResult: Database is up to date.\r\n\r\n##### Test 4: Installing extensions which include sql scripts for updating database\r\n\r\nWith 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.\r\n\r\nAn example with an sql script containing a hashtag comment can be found here: [http://test5.richard-fath.de/sampleerror.zip](http://test5.richard-fath.de/sampleerror.zip \"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.\r\n\r\n##### Test 5: Parsing the test sql script.\r\n\r\nIf 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.\r\n\r\nDownload following test script and store it on a location on your server which is accessibly for Joomla!:\r\n\r\n- For mysql: [http://test5.richard-fath.de/test_pr9369_mysql.sql](http://test5.richard-fath.de/test_pr9369_mysql.sql \"http://test5.richard-fath.de/test_pr9369_mysql.sql\")\r\n- For postgresql: [http://test5.richard-fath.de/test_pr9369_postgresql.sql](http://test5.richard-fath.de/test_pr9369_postgresql.sql \"http://test5.richard-fath.de/test_pr9369_postgresql.sql\")\r\n- For sqlsrv/sqlazure: [http://test5.richard-fath.de/test_pr9369_sqlazure.sql](http://test5.richard-fath.de/test_pr9369_sqlazure.sql \"http://test5.richard-fath.de/test_pr9369_sqlazure.sql\")\r\n\r\nAdd following code snippet below the body tag in the index.php of your site template (e.g. protostar).\r\n\r\n> <?php\r\n$buffer = file_get_contents('/FULL_PATH_TO_THE_FOLDER_WITH_THE_TEST_SQL_SCRIPT/test_pr9369_mysql.sql');\r\n$statements = JDatabaseDriver::splitSql($buffer);\r\nforeach ($statements as $statement)\r\n{\r\n\techo $statement . '<br />';\r\n}\r\necho '<br />';\r\n?>\r\n\r\nReplace 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.\r\n\r\nNow 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 or before line end and also all c-style multiline comments.\r\n\r\nThe output should look as follows on mysql and similar on the other databases:\r\n\r\n> ALTER TABLE `#__test_table_1` ADD COLUMN `header1` smallint(3) NOT NULL DEFAULT 1;\r\nALTER TABLE `#__test_table_2` ADD COLUMN `header2` smallint(3) NOT NULL DEFAULT 2;\r\nALTER TABLE `#__test_table_3` ADD COLUMN `header3` smallint(3) NOT NULL DEFAULT 3;\r\nALTER TABLE `#__test_table_4` ADD COLUMN `header4` smallint(3) NOT NULL DEFAULT 4;\r\nALTER TABLE `#__test_table_5` ADD COLUMN `header5` smallint(3) NOT NULL DEFAULT 5;\r\nALTER TABLE `#__test_table_6` ADD COLUMN `header6` smallint(3) NOT NULL DEFAULT 6;\r\nCREATE TABLE IF NOT EXISTS `#__test_table_7` ( `id` int(10) unsigned NOT NULL AUTO_INCREMENT COMMENT 'Primary Key', `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;\r\nALTER TABLE `#__test_table_8` ADD COLUMN `header8` smallint(3) NOT NULL DEFAULT 7;\r\nUPDATE `#__test_table_9` SET `textcol9` = 'Single quoted text \"x\" \\n /*gaga*/ # dada -- yeah ';\r\nUPDATE `#__test_table_10` SET `textcol10` = \"Double quoted text 'x' \\n /*gaga*/ # dada -- yeah \";\r\n\r\nNames should be properly quoted (not shown properly here because name quotes are used here as markup).\r\n\r\nIn the last 2 lines, there should be c style comments around \"gaga\" in the quoted texts, here this is markup somehow so we see only the slashes.\r\n\r\nYou will see in my sql that I did not end the file with a new line and the last statement not with a semicolon.\r\n\r\nThis is compatible to the old behavior.\r\n\r\nYou 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).\r\n\r\nYou 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."},"comment":{"url":"https://api.github.com/repos/joomla/joomla-cms/issues/comments/195520162","html_url":"https://github.com/joomla/joomla-cms/pull/9369#issuecomment-195520162","issue_url":"https://api.github.com/repos/joomla/joomla-cms/issues/9369","id":195520162,"user":{"login":"mbabker","id":368545,"avatar_url":"https://avatars.githubusercontent.com/u/368545?v=3","gravatar_id":"","url":"https://api.github.com/users/mbabker","html_url":"https://github.com/mbabker","followers_url":"https://api.github.com/users/mbabker/followers","following_url":"https://api.github.com/users/mbabker/following{/other_user}","gists_url":"https://api.github.com/users/mbabker/gists{/gist_id}","starred_url":"https://api.github.com/users/mbabker/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mbabker/subscriptions","organizations_url":"https://api.github.com/users/mbabker/orgs","repos_url":"https://api.github.com/users/mbabker/repos","events_url":"https://api.github.com/users/mbabker/events{/privacy}","received_events_url":"https://api.github.com/users/mbabker/received_events","type":"User","site_admin":false},"created_at":"2016-03-11T19:50:57Z","updated_at":"2016-03-11T19:50:57Z","body":"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.\r\n\r\nNot as much of an issue now that 2.5's EOL I guess.  In theory [this change](https://github.com/joomla/update.joomla.org/commit/5192be544ff2d8b65cfbdd5b61c1541f8600209d) 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](https://github.com/joomla/update.joomla.org/blob/master/www/core/test/list_test.xml#L2) in the main testing channel's XML to use the 3.x releases."}}