I've been sifting through the database schema, and have come accross a couple of inconsistancies:

- The node table has some keys that should be updated to reflect the versioning changes introduced with the addition of thenode_revisions table. The primary key is nid and vid, which means we should also add an index for the nid column.

- nid in node_counter was set to a value of int (11). In all other places the nid type is int (10), and I've heard rumors that mysql indices work better if the field lengths are the same.

CommentFileSizeAuthor
#4 postgresql_updates_180_181_182-62478.diff.txt4.06 KBCvbge
schema-clean.patch3.51 KBmatt westgate
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Needs review » Fixed

Committed to CVS HEAD. Thanks Matt!

Zen’s picture

Zen CreditAttribution: Zen commented
Status: Fixed » Needs work

What about mysqli?

-K

fgm’s picture

fgm
Primary language French
Location Paris, France
CreditAttribution: fgm commented

FWIW, alternate database schema available here.

Cvbge’s picture

Cvbge CreditAttribution: Cvbge commented
Status: Needs work » Needs review
FileSize
postgresql_updates_180_181_182-62478.diff.txt4.06 KB

Some fixes from afterworld ;)

- add mysqli and postgresql support for update 180
- add forgotten not null and default for postgresql in update 181
- fix postgresql update 182

I've tested it in 90%, but it'd be nice if someone with postgresql could test it too.

Dries’s picture

Dries CreditAttribution: Dries commented

Code looks good to me, but I don't have a PostgreSQL installation to test it against.

drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm commented
Status: Needs review » Reviewed & tested by the community

Looks okay to me too. I don't think we should wait for antother Postgres tester, but not committing to be safe.

killes@www.drop.org’s picture

killes@www.drop.org CreditAttribution: killes@www.drop.org commented

applied to 4.7

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed Cvbge's patch to CVS HEAD. All done.

killes@www.drop.org’s picture

killes@www.drop.org CreditAttribution: killes@www.drop.org commented
Status: Fixed » Reviewed & tested by the community

not all done, there was no index on vid in the node table that could be dropped. Patch attached, committed to 4.7.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Needs work

Does not apply against CVS HEAD.

ax’s picture

ax CreditAttribution: ax commented

this patch (applied to both 4.7 and HEAD) introduces bugs/mysql errors: see http://drupal.org/node/65456 .

by the way: i don't think "the primary key [of the node table] is nid and vid" (it isn't in the referenced schema, neither). it still is nid only. there cannot be two rows with the same nid. the vid might change, but just references another row in node_revisions then.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Version: x.y.z » 6.x-dev

Giving this a version, might well have been fixed by now but not marking as such.

dpearcefl’s picture

dpearcefl CreditAttribution: dpearcefl commented
Status: Needs work » Postponed (maintainer needs more info)

Is this still a problem in current D6?

fgm’s picture

fgm
Primary language French
Location Paris, France
CreditAttribution: fgm commented

FWIW, this was a 4.7.x and 5.x-dev issue at the time.

In 6.x, ax' comment is correct: the PK of {node} is just nid, not nid/vid.

dpearcefl’s picture

dpearcefl CreditAttribution: dpearcefl commented
Status: Postponed (maintainer needs more info) » Closed (won't fix)

Considering this issue was reported on a non-release version and it is unknown if this issue even exists any more, I'm closing this ticket.