Does not work on pgsql
| Project: | Backup and Migrate |
| Version: | 6.x-1.2 |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
I have to agree with other comments, for a module to be prime time, it should support both official databases. I notice that someone has offered to help you with this issue, I sincerely hope you take the offer, I am excited to try your module on my pg-drupal systems. Lastly, since I had to install your module, find that it fails to work correctly, come and submit a bug only to find that it never did support PGSQL, I would suggest that you add that fact to the description of the module in bold. Pehaps "This module does not support PostgreSQL (yet)". Thanks for contributing and keep up the good work, I look forward to your module when it supports PGSQL.
For the record, this is the error returned when this module is installed and run on PGSQL
* warning: pg_query() [function.pg-query]: Query failed: ERROR: syntax error at or near "table" LINE 1: show table status ^ in C:\Program Files\Apache Software Foundation\Apache2.2\htdocs\cms\includes\database.pgsql.inc on line 138.
* user warning: query: show table status in C:\Program Files\Apache Software Foundation\Apache2.2\htdocs\cms\sites\all\modules\backup_migrate\backup_migrate.module on line 669.
* warning: Invalid argument supplied for foreach() in C:\Program Files\Apache Software Foundation\Apache2.2\htdocs\cms\includes\form.inc on line 1417.
* warning: Invalid argument supplied for foreach() in C:\Program Files\Apache Software Foundation\Apache2.2\htdocs\cms\includes\form.inc on line 1417.
#1
I am working on adding postgres and multiple databases to this module.
Some functionality to do this is already in db_maintenance module, and I am creating a dependency between the two modules. Give me a few days and we will see where we get to.
#2
This module now seems to work with postgres and mysql (I didn't test mysql since I am using postgres). It also works if you have one or more than one database defined, and should even work if some of the databases are postgres and others are mysql.
Unfortunately, the magic for this intelligence comes from the db_maintenance module, and I am not going to cut-n-paste. So I have added that module as a dependency to this one. You need to use the patched version of that module, here:
http://drupal.org/node/306872#comment-1204027
#3
Thanks for the hard work. I'm not going to commit a patch that introduces a module dependency to a patched module as it would raise the barrier of entry a little high, but this could be very useful for posgres users who want this functionality.
I'd love to hear PGSQL users' take on whether this works and if it's a good solution, as there maybe a way forward using some sort of conditional dependency.
Thanks again.
r
#4
#5
Hi Charles/Ronan.
I've just given the patch a test. It did not quite apply cleanly against the latest dev, but it was close. I've also applied the patch to db_maintainance, and that went smoothly.
I've just attempted to do a backup of my Postgres database. However when I do I get a 0 byte file back. So something is not right. I had compression turned off and it made no difference if I downloaded the file or have it save to my files directory.
I'm running postgres 8.3 on windows
#6
Hi Aaron
I'm getting the empty backup now too - even though the module worked perfectly on Friday. It may have something to do with the latest dev, because I applied it to my version before using the module this morning. Will let you know.
In one manual test, I got a timeout on a very large database, so that might be one of the issues.
Ronan, I understand the dependency issue and suspected I might be chastised for doing such a thing. One workaround would be to copy the functions across into this module, but we know that refactoring is the proper solution in the long term. Or maybe we can pool resources and have this module supercede db_maintenance? Especially since this one has more functionality and now (theoretically) supports all the databases.
I'll do a new patch when I find the problem.
#7
I made some changes to the latest dev on friday, so that might be the issue. I am trying to keep the 1.x branch stable, but I applied a patch that helps fix some fairly serious issues with temp files not being deleted. That'll hopefully be last big change to the 1.x branch so that won't get in your way.
As to the dependency, I certainly didn't mean to chastise. I'm very happy that you're taking the time and the initiative to work on this issue. I don't mind the dependency either as long as it doesn't complicate life for MySQL users. A conditional warning that checks for the db_maintenance module if the current db is postgres should work. That way, pgsql users would get info on how to enable the module for their situation but MySQL users would be none the wiser, and would not have to install an unnecessary module just to satisfy a dependency.
Once you get something working and stable and vetted by the pgsql community (I don't use postgres it much at all, and am extremely unfamiliar with it's dump formats, so I won't be much use) then I'll either commit it to the 1.x branch with the dependency warning, or refactor it for inclusion in the 2.x branch.
Thanks again for your hard work.
#8
Fixed the problem where the database is created empty. Two problems, one a bug, the other the fact that pg_dump does not accept passwords in the parameter list. In situations where the db connection requires a password, this is what now happens:
This patch also includes the latest of Ronan's releases, so temp files are being deleted as intended.
Remember to satisfy the dependency of this module with db_maintenance, patched to http://drupal.org/node/306872#comment-1210575
#9
Hi Charles,
Results of testing:
However the files generated were good and contained (at first glace at least) the right stuff.
Thanks for the work your putting into this!
Aaron
#10
Aaron
My apologies. Firstly for not monitoring this thread over the last two weeks, and secondly for having completely forgotten the issues with running on Windows. I switched to linux 6 years ago and can no longer even imagine the difficulty of getting anything to work there. I propose two fixes to your problems:
Our system is going into testing soon, and only then will I be likely to work on this code again. If I do I will submit a new patch, or, if the module allows, integrate my changes into the new version.
#11
Thanks Charles,
I've got a couple of pressing projects on the go at the moment, but once I'm through those I'll get a patch together. I should switch to linux but haven't had the time/inclination to get used to a new environment yet.
#12
Hi,
first of all thanks for this patch, but i had to modify several things to get it work in backup and migrate and in db_maintenance :
- environment variable is neither DBPASSWORDFILE nor PGPASSWORDFILE but PGPASSFILE according to libpq documentation: http://www.postgresql.org/docs/8.1/static/libpq-pgpass.html
- the .pgpass should be in the user home directory (apache user, so normally /var/www) with 600 for permissions
i don't post a patch 'cause i modified more than that and it concerns both modules.
but if needed i can clean my code and post it.
--
Vincent
#13
good call vinzentt, but this is a better URL becuase 8.1 has fundamental differences in syntax than recent versions, so this is a better URL so as to not introduce accidental bugs becuase of old syntax...
http://www.postgresql.org/docs/8.3/static/libpq-pgpass.html#14
Sorry I am not a Patcher.... I have run the latest release version and this is the result upon trying to config the module:
* warning: pg_query() [function.pg-query]: Query failed: ERROR: syntax error at or near "table" LINE 1: show table status ^ in /var/www/sparcs/includes/database.pgsql.inc on line 139.* user warning: query: _backup_migrate_get_table_names show table status in /var/www/sparcs/sites/all/modules/backup_migrate/backup_migrate.module on line 682.
* warning: Invalid argument supplied for foreach() in /var/www/sparcs/includes/form.inc on line 1420.
* warning: Invalid argument supplied for foreach() in /var/www/sparcs/includes/form.inc on line 1420.
Dont know if this helps or not..... If you folks release something I can use (a dev or a release) I would be happy to feed back...
#15
Last patch still wrond against 6-1.2 if someone already have a working copy please make a new patch
#16
subscribing
#17
Hi and thanks for your work,
I'm running a fresh install of Drupal 6.14 on pgsql and I'm also running poormanscron.
My error msg:
* warning: pg_query() [function.pg-query]: Query failed: ERROR: syntax error at or near "table" at character 6 in /var/www/vhosts/lordnelsonyachts.org/httpdocs/includes/database.pgsql.inc on line 139.
* user warning: query: show table status in /var/www/vhosts/lordnelsonyachts.org/httpdocs/sites/all/modules/backup_migrate/backup_migrate.module on line 682.
* warning: Invalid argument supplied for foreach() in /var/www/vhosts/lordnelsonyachts.org/httpdocs/includes/form.inc on line 1428.
* warning: Invalid argument supplied for foreach() in /var/www/vhosts/lordnelsonyachts.org/httpdocs/includes/form.inc on line 1428.
I'll be happy to assist in any way possible!