I have gotten the following errors when i tried upgrading my 6.0-rc1 site

I use postgres as my database

* warning: pg_query() [function.pg-query]: Query failed: ERROR: syntax error at or near "(" at character 34 in /mounts/max30/home/cath/1stsouth/www/includes/database.pgsql.inc on line 138.
* user warning: query: ALTER TABLE comments ADD KEY pid (pid) in /mounts/max30/home/cath/1stsouth/www/modules/comment/comment.install on line 65.

CommentFileSizeAuthor
#15 comment-index-pid-208938-14-1.patch2.05 KBscor
#14 comment-index-pid-208938-14.patch649 bytesbjaspan
#7 foo_bar_1.patch922 bytesscor
#4 foo_bar.patch691 byteschx
#3 foo_bar.patch457 byteschx
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bfo’s picture

bfo CreditAttribution: bfo commented
Component: postgresql database » comment.module

I tried to do the update to the database manually by running

create index comments_idx_pid on comments(pid);

It seems to work.

bfo’s picture

bfo CreditAttribution: bfo commented
Component: comment.module » postgresql database
chx’s picture

chx CreditAttribution: chx commented
Component: comment.module » postgresql database
Status: Active » Needs review
FileSize
foo_bar.patch457 bytes

Grumble grumble.

chx’s picture

chx CreditAttribution: chx commented
FileSize
foo_bar.patch691 bytes

Really..

HorsePunchKid’s picture

HorsePunchKid CreditAttribution: HorsePunchKid commented

Just curious... doesn't MySQL also support the CREATE INDEX syntax? 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.

chx’s picture

chx CreditAttribution: chx commented

pulled the syntax from system.install

scor’s picture

scor CreditAttribution: scor commented
FileSize
foo_bar_1.patch922 bytes

fixes some minor space issues (remains consistent with system_update_1022() in system.install)
also, is there a reason to use double quotes?

HorsePunchKid’s picture

HorsePunchKid CreditAttribution: HorsePunchKid commented

I 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?)

scor’s picture

scor CreditAttribution: scor commented

@HorsePunchKid: agreed. just a general best practice. not a big performance deal here.

chx’s picture

chx CreditAttribution: chx commented

It's good to see Drupal has so many postgresql testers that this critical issue has so many valuable feedback from them.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented

Do 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.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

agentrickard: yes we do support rc updates, this came up elsewhere when I asked the same question :)

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented
Status: Needs review » Reviewed & tested by the community

OK, then, I think RTBC, since the SQL syntax is good.

bjaspan’s picture

bjaspan CreditAttribution: bjaspan commented
Status: Reviewed & tested by the community » Needs review
FileSize
comment-index-pid-208938-14.patch649 bytes

The 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.

scor’s picture

scor CreditAttribution: scor commented
FileSize
comment-index-pid-208938-14-1.patch2.05 KB

following the same trend, we can also get rid of the other db-specific add index updates in core.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

Let's get this tested! :)

webernet’s picture

webernet CreditAttribution: webernet commented

Tested 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.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented

Tests fine upgrading 5.6 to 6.0rc2 and pgSQL 8.2.3.

scor’s picture

scor CreditAttribution: 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.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Needs review » Reviewed & tested by the community

Well 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.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Reviewed & tested by the community » Fixed

Great! Thanks Barry for stepping in! Committed.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.