Closed (fixed)
Project:
Drupal core
Version:
6.0-rc2
Component:
postgresql database
Priority:
Critical
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
11 Jan 2008 at 18:25 UTC
Updated:
30 Jan 2008 at 21:56 UTC
Jump to comment: Most recent file
Comments
Comment #1
bfo commentedI tried to do the update to the database manually by running
create index comments_idx_pid on comments(pid);
It seems to work.
Comment #2
bfo commentedComment #3
chx commentedGrumble grumble.
Comment #4
chx commentedReally..
Comment #5
HorsePunchKid commentedJust curious... doesn't MySQL also support the
CREATE INDEXsyntax? From the help text in MySQL 5.x, the syntax looks identical to what you've got for the Postgres case. If so, you can avoid the switch entirely, of course. Sorry, I don't have a patch to offer to test it.Comment #6
chx commentedpulled the syntax from system.install
Comment #7
scor commentedfixes some minor space issues (remains consistent with system_update_1022() in system.install)
also, is there a reason to use double quotes?
Comment #8
HorsePunchKid commentedI tend to use double quotes in this context because that allows you to easily use single quotes inside the query. MySQL may let you get away with using double quotes for string literals, but Postgres won't.
Is there a reason not to use double quotes? (Performance?)
Comment #9
scor commented@HorsePunchKid: agreed. just a general best practice. not a big performance deal here.
Comment #10
chx commentedIt's good to see Drupal has so many postgresql testers that this critical issue has so many valuable feedback from them.
Comment #11
agentrickardDo we support updates from rc1 to rc2? Is this a real issue?
Anyway, SQL syntax from #4 and #7 tests properly in pgsql 8.2.3. Use of single or double quotes is personal preference, though system.install tends to use double.
Comment #12
catchagentrickard: yes we do support rc updates, this came up elsewhere when I asked the same question :)
Comment #13
agentrickardOK, then, I think RTBC, since the SQL syntax is good.
Comment #14
bjaspan commentedThe bug here is that http://drupal.org/node/205465 introduced a mysql-specific ALTER TABLE statement instead of using the Schema API for its intended purpose. The solution is not to add pgsql-specific SQL but to use db_add_index(). New patch attached.
Comment #15
scor commentedfollowing the same trend, we can also get rid of the other db-specific add index updates in core.
Comment #16
gábor hojtsyLet's get this tested! :)
Comment #17
webernet commentedTested OK on MySQL -- although it might be better to leave out the change to system_update_1022() since it's a 5.x --> 5.x update.
Comment #18
agentrickardTests fine upgrading 5.6 to 6.0rc2 and pgSQL 8.2.3.
Comment #19
scor commented@webernet: the update is done in d6 environment so it should be ok.
I checked the patch for an update from D5.1 to D6, on both mysql and pgsql, compared the query logs with and without the patch.
No difference, except that mysql now uses the syntax 'ALTER TABLE {comments} ADD INDEX pid (pid)' instead of '... ADD KEY ...' which I believe is more consistent with the specs.
Comment #20
catchWell if it passed on pgsql and mysql, then it's rtbc. No harm using schema if we know we're in D6 environment, which we are.
Comment #21
gábor hojtsyGreat! Thanks Barry for stepping in! Committed.
Comment #22
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.