When enabling the 'biblio' module on a site that works on a PostgreSQL database, I got a couple of errors - I disregarded them (my bad, yes). But it didn't work out - In some (administrative) pages, I get:
* warning: pg_query() [function.pg-query]: Query failed: ERROR: relation "biblio_fields" does not exist in /usr/share/drupal5/includes/database.pgsql.inc on line 125.
* user warning: query: SELECT name,title FROM biblio_fields in /usr/share/drupal5/includes/database.pgsql.inc on line 144.
Looking into biblio.install, it seems PostgreSQL is simply not supported - but this lack of support is not documented (or at least, not visibly). And, IMHO, I don't see a reason for it.
I'm willing to translate the table creation so it works with PostgreSQL, of course, but please tell me - Is there a rationale on why it is not supported?
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | biblio.install.last_fix.diff | 830 bytes | gwolf |
| #2 | biblio.install.diff | 52.23 KB | gwolf |
Comments
Comment #1
rjerome commentedHi,
Your right it is not supported, and there is no reason other than I've never taken the time to setup a PostgreSQL installatoin and test it.
If you would like to do the translation, that would be great! Post it here and I will include it with the distribution.
Ron.
Comment #2
gwolf commentedOk, so it was somewhat harder than what I expected! :-) Anyway, here is the patch - And a brief explanation. It is a bit invasive, but I think it's for the better overall. And, of course, warning precedes explanation: I am not a PHP coder, and I think it shows. I'm barely familiar with its basics, so I'm sure there are better ways of doing many things (I'm specially bothered by my repeated calls to
db_data_types();).I started by copying your declarations and fixing them, but that'd lead to maintenance hell - I think it's a real bad idea to repeat the DB declarations, as it would become impossible for future maintenance to keep both sets synchronized. I tried, then, to just replace whatever peculiarities I found with calls to either a function or an interpolated variable to make it work with a single code base.
MySQL is peculiar, as it hugely deviates from the SQL standards - The most invasive change I did (which makes following the diff a tough task) is to remove all the
`quoting around field names - MySQL is the only RDBMS which uses this nomenclature. I dropped all the backticks. MySQL does not complain, and PostgreSQL is way happier.I added a couple of commodity functions at the end of the file: They all decide their behaviour based on
$GLOBALS['db_type']. Begin withdb_data_types(): It just creates an array of the data types (and other declarations used verbatim at table creation time).int(10)andint(11)becomeintegerin Postgres;intyint1andintyint4becomesmallint, andint(11) auto_incrementbecomesserial; I also added here the indications to make the tables UTF8 (and MyISAM), as they are not needed in Postgres (you declare the character set per database, not per table).Maybe this function should really chicken out if called from anything besides the supported RDBMSs? I'd like to, but as I said, this is the first time I write anything that touches the Drupal API. And it shows, I think.
There are five other functions, self-explanatory IMHO. They also don't do any work themselves, just hand back the right strings to be passed as SQL snippets.
enable_keys_sql($table)anddisable_keys_sql($table)are meaningless for Postgres, so they don't even both trying;add_key($table, $key_name, $flds)andadd_ft_key($table, $key_name, $flds)create the key in MySQL or the index in PostgreSQL (again, fulltext means nothing to Postgres);alter_column_sql($table, $column, $new_type)adds a column to an existing table.Besides this, my last change was not to use the multi-row inserts anymore, as they are only supported in MySQL. Whenever I found something like
db_query("INSERT INTO table (col1,col2, col3) VALUES ($so, $me, $thing), ($some, $more, $stuff);"), I replaced it forforeach (array("($so, $me, $thing)", "($some, $more, $stuff)") as $item) { db_query("INSERT INTO table (col1, col2, col3) VALUES $item;"); }As for the code quality: It works. I tested installing, removing and uninstalling the module several times from some Drupal sites I have both with MySQL and PostgreSQL, and it worked fine. There is still a hiccup somewhere, I think it is related to PostgreSQL executing nothing when you called disable/enable keys - I'll dig into it as soon as possible, but all the tables/fields get created, and the data gets loaded correctly. Oh! And I didn't support the updates number 15 and 16, as adding/dropping a primary key are completely different operations and I didn't find a way to represent them yet. If you think this is important (I don't think so - after all, your module has not yet supported PostgreSQL so far!), I'll dig into it.
So... Feel free to laugh at my code. But please, try to incorporate it into yours! ;-)
thanks a lot,
Comment #3
gwolf commentedOk, so this really-trivial patch to my patch fixes the remaining errors in Postgres. Executing a
SELECT true;is more correct than just executing nothing.Comment #4
rjerome commentedHoly Smokes! I haven't examined the code in detail, but clearly you did some serious work here. And it is for this reason I hesitate to tell you (I should have mentioned earlier) that Version 6 of Drupal has abstracted the entire database schema creation process so that (in theory) one size fits all DB's. See http://drupal.org/node/146843 and http://api.drupal.org/?q=api/group/schemaapi/HEAD
I will definitely review your code and include it in the 5.x version.
Ron.
Comment #5
gwolf commentedHeh - Thanks a lot. Still, I do expect -as 4.7 still lives on- 5.x to go on for a long time. There is too much installed base. I'm a bit worried about modules being slowly phased out... But as it is today, everything is 5.x-compatible... And almost nothing is ready for 6.x
I also didn't expect it to be so much work - but hey, thank you for the module! My work is but a tiny fraction of it.
Comment #6
murphy2atnetaxs commentedThanks for addressing this, gwolf, but unfortunately your patch does not help us PostgreSQL 7.4.X users.
PG 7.4 does not support ALTER TABLE ... ALTER COLUMN ... TYPE syntax. That first appeared in 8.0.
I'm not an expert on these matters, but apparently it is possible to kludge a workaround in 7.4.X:
http://www.dbforums.com/t1051977.html
I'm guessing you will make me do that :-( I haven't looked at any of the code yet.
Comment #7
murphy2atnetaxs commentedI have fixed the ALTER COLUMN ... TYPE problem for PostgreSQL 7.4. The workaround might be slow when updating large tables, so I would hate to apply it blindly to all versions of PG. Hmm.
Once this problem is fixed for PG 7.4, another one crops up. Biblio queries that look like this:
SELECT DISTINCT n.*, b.*
FROM node n left join biblio b on n.vid=b.vid
WHERE n.type='biblio'
ORDER BY b.biblio_year DESC, b.biblio_date DESC, SUBSTRING(n.title,1,1) ASC, b.biblio_type ASC
LIMIT 25 OFFSET 0
fail with this error message:
Query failed: ERROR: for SELECT DISTINCT, ORDER BY expressions must appear in select list
This type of error can be fixed by appending the ORDER BY expressions like 'SUBSTRING(...)' to the SELECT list, but depending on how the query results are used, this could cause errors (I'd have to see the code). This particular query could be fixed simply by ordering on n.title rather than the SUBSTRING expression, but there are probably a variety of other possible ORDER BY expressions that must be treated differently.
Comment #8
kjcole commentedWas the review/include for 5.x mentioned in #4 above ever done? If so, where might one find it? Thanx.
Comment #9
rjerome commentedUnfortunately (for you) the answer is no, it hasn't been done. I have spent what time I've had on the 6.x version which has some significant improvements over the 5.x version.
Ron.
Comment #10
kjcole commentedNo worries. I wasn't heavily invested in 5.x yet -- though I do like to stay with "packages" rather than tarballs, and as of the moment, Ubuntu Hardy doesn't have a 6.x... I made the jump to 6.8 in spite of that.
Comment #11
markusT commentedAre there any known issues with this biblio version and fitler settings?
I have applied the patches from gwolf and got biblio working (of a fashion) after a few minor hacks. (I am running postgresql-8.3). If I want to define filters via the webinterface, the filter-variables (e.g. author or year) seem not to be passed on correctly. I have traced that down until biblio_db_search(). In biblio_db_search the filters variables I've set are displayed (Filters:....correct values ... ) but the variables are somehow not replaced when the where_clause is constructed.
The construction of the where_clause (actually the entire query) should happen in db_rewrite_sql(...). ... and now the confusing bit ... The drupal API specifies db_rewrite_sql as accepting not only the query but also the arg_list array (and the table and prim. key). Nothing beside the query is passed in and even if I add all required fileds manually (or only the args) in accordance with the API ... nothing changes ...
I could work around that by manualy fetching all filter-variables from the args(_list) and do a nasty preg_replace ... but I am afraid that that will bite me somewhere else...
any suggestions?
thx
Comment #12
rjerome commentedI'm setting up PostgreSQL on my dev machine and I'll try to get to bottom of this issue once and for all.
Ron.
Comment #13
markusT commentedthanks - my modifications are still in progress but if it helps I could send you diff files
Comment #14
liam morlandThis version is no longer maintained. If this issue is still relevant to the Drupal 7 version, please re-open and provide details.