Skip to content

Fix database (MySQL) importer / exporter#191

Merged
mbabker merged 9 commits intojoomla-framework:2.0-devfrom
twister65:fix-import
Dec 29, 2019
Merged

Fix database (MySQL) importer / exporter#191
mbabker merged 9 commits intojoomla-framework:2.0-devfrom
twister65:fix-import

Conversation

@twister65
Copy link
Contributor

@twister65 twister65 commented Sep 11, 2019

Fix bugs with database (MySQL) exporter / importer.

Summary of Changes

  • fix null integer insertion.

  • fix search for current_timestamp.

Testing Instructions

  • Go to the cli folder of your website.

  • Export all tables and data to the folder:
    php joomla.php database:export --all --folder <folder_path>

  • Import all from folder:
    php joomla.php database:import --all --folder <folder_path>

Or

@twister65
Copy link
Contributor Author

Travis fails with PHP 7.4:

Function ReflectionType::__toString() is deprecated

https://travis-ci.org/joomla-framework/database/jobs/583842640

@mbabker
Copy link
Contributor

mbabker commented Sep 11, 2019

Travis fails with PHP 7.4:

Yup, well aware. The entire package test suite needs to be rewritten to not rely on the deprecated DbUnit extension for PHPUnit, which blocks upgrading to a PHPUnit version that does not emit these deprecation notices.

@wilsonge
Copy link
Contributor

The change to postgres is wrong. See #180 (tl/dr: calling reset in jtable resets the values with the default values from the table instance) - so historically here the default property here is just the table default with none of the bonus information.

@twister65
Copy link
Contributor Author

When you install a fresh nightly build, the default value is:
'1970-01-01 00:00:00'::timestamp without time zone
and not: '1970-01-01 00:00:00'

"the bonus information" is just a cast.

@twister65
Copy link
Contributor Author

Example of Dump with phpPgAdmin:

CREATE TABLE public.pmrah_action_logs ( id integer NOT NULL, message_language_key character varying(255) DEFAULT ''::character varying NOT NULL, message text DEFAULT ''::text NOT NULL, log_date timestamp without time zone DEFAULT '1970-01-01 00:00:00'::timestamp without time zone NOT NULL, extension character varying(50) DEFAULT ''::character varying NOT NULL, user_id integer DEFAULT 0 NOT NULL, item_id integer DEFAULT 0 NOT NULL, ip_address character varying(40) DEFAULT '0.0.0.0'::character varying NOT NULL );

@twister65
Copy link
Contributor Author

ping @alikon , @richard67

@richard67
Copy link
Contributor

Example of Dump with phpPgAdmin:

@twister65 Do you mean with "dump" a database export done by phpPgAdmin? If so, then this is maybe not exactly the same as the SQL statement used by the driver here:

$this->setQuery('
. But this is relevant. Could you try to execute that statement in phpPgAdmin and see if it also contains this "bonus information" stuff?

@twister65
Copy link
Contributor Author

twister65 commented Sep 15, 2019

It contains the "bonus information" stuff, but it was filtered after:

if (preg_match('/^\'(.*)\'::.*/', $field->Default, $matches))
{
$field->Default = $matches[1];
}

@richard67
Copy link
Contributor

I see. Well I'd prefer not to change that, it worked before, and who knows if this "bonus information" is there or not depends on seome server setting which might be different here and there?

Regarding the other changes: How shall there be tested? Was there any PHP notice before correction?

Regarding consistent code style: I see you use in your changes in PHP files sometimes "NULL" and sometimes "null" for value null. Is that by purpose?

@twister65
Copy link
Contributor Author

Without this fix, we have this issue after exporting and importing the postgreSQL database:
pgsqlDefProb
Default values are wrong...

@richard67
Copy link
Contributor

And on MySQL?

@twister65
Copy link
Contributor Author

twister65 commented Sep 15, 2019

And MySQL:

php joomla.php database:import --all --folder ~/tmp/mysqli/

Error merging the structure for t204a_contentitem_tag_map: Invalid default value for 'tag_date'

mysqlImpError

@richard67
Copy link
Contributor

@twister65 I've just tested with PosgreSQL (PDO) on current 4.0-dev but still have these warnings shown in Extensions -> Manage -> Database. The "Fix" button then can fix them.

@twister65
Copy link
Contributor Author

@richard67 , you have to apply this patch manually in your libraries/vendor/joomla/database/src/Pgsql folder.

@richard67
Copy link
Contributor

That's what I've done.

@richard67
Copy link
Contributor

Let's wait and see if @alikon can test. Maybe I made some mistake. And I have other priorities for the coming week and weekend. So it may take a while until I can continue with this PR here.

@wilsonge
Copy link
Contributor

wilsonge commented Sep 15, 2019

The problem here is that JTable in the CMS is expecting the default value to be a default value that it can insert into the DB (i.e. it doesn't have the cast)- and I believe this was the justification for this property when it was first created. I accept that this will break a dump of the db structure. So if this goes in it needs to be documented as a b/c break and we'll need to make appropriate fixes into the reset method of JTable

Also see https://github.com/joomla/joomla-cms/pull/6314/files#diff-cee5c179fc86adf6c499ee4641ae4780R396 for similar "Fixes" to postgresql that are likely going to cause descrepancies because getTableColumns was previously built for the CMS. If we're going to do this properly - we need to split up what the default value is and what the column properties are

@twister65
Copy link
Contributor Author

Clearly, what would you like to solve this problem with postgresql ? Do you want I rollback the postgreSQL commit ? Have you seen your postgreSQL tables after a new installation ? All default values have a cast (except integer values). I'm still waiting for @alikon review and test.

@richard67
Copy link
Contributor

@twister65 Yes, the tables have a cast after installation when exporting them with postgresql tools, but that cast is not necessary for anything to work, and it confuses our database checker and other things at other places, so yes, I would suggest you remove it.

Normalise default values like timestamp (Pgsql).
@alikon
Copy link
Contributor

alikon commented Oct 21, 2019

sorry folks for my unresponsiveness....it went out of my radar

@twister65 twister65 changed the title Fix database importer / exporter Fix database (MySQL) importer / exporter Oct 27, 2019
@alikon
Copy link
Contributor

alikon commented Oct 27, 2019

tested on latest staging with mysql(pdo) 8.0.17-0ubuntu2
Screenshot from 2019-10-27 12-06-53

@alikon
Copy link
Contributor

alikon commented Oct 27, 2019

i've made a test on postgresql 11.5
Screenshot from 2019-10-27 12-39-48

@twister65
Copy link
Contributor Author

twister65 commented Oct 27, 2019

tested on latest staging with mysql(pdo) 8.0.17-0ubuntu2

Staging ??? It works with 4.0-dev (beta1-dev).

@alikon , the database library for staging is here:
https://github.com/joomla/joomla-cms/tree/staging/libraries/joomla/database

@alikon
Copy link
Contributor

alikon commented Oct 28, 2019

yes i mean 4.0-dev (beta1-dev) cloned from github

@twister65
Copy link
Contributor Author

@alikon, non riesco a riprodurre questo errore con i dati di esempio beta1.

@alikon
Copy link
Contributor

alikon commented Oct 28, 2019

i've tested without any sample data just a fresh install

@alikon
Copy link
Contributor

alikon commented Oct 29, 2019

my last report #191 (comment) it is wrong
i've used the same folder with with different db's but with the same table prefix without, deleting the files in the folder

@mbabker
Copy link
Contributor

mbabker commented Nov 18, 2019

Can you sync your branch with the current 2.0-dev branch? There aren't any conflicts, but that'll get your changes to run on the updated test structure at least.

@twister65
Copy link
Contributor Author

RTC, please.

@richard67
Copy link
Contributor

I wish I could, but I'm not a maintainer of this repo nor have other privileges on it ;-)

@mbabker mbabker merged commit d82f210 into joomla-framework:2.0-dev Dec 29, 2019
@twister65 twister65 deleted the fix-import branch March 4, 2020 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants