Column fid in table {files} should have been serial type for PostgreSQL database [#100143]

Comment	File	Size	Author
#13	system.install_20061130_1640_patch.txt	770 bytes	sammys
#12	system.install_9.patch	2.85 KB	havran

#9	system.install_8.patch	2.92 KB	havran

#7	system.install_7.patch	2.86 KB	havran

#3	system.install_6.patch	2.86 KB	havran

#1	system.install_5.patch	815 bytes	havran

Comment #1

havran CreditAttribution: havran commented 28 November 2006 at 13:50

Assigned:	Unassigned	» havran
Status:	Active	» Reviewed & tested by the community

File	Size
system.install_5.patch	815 bytes

Here is patch for this issue.

Log in or register to post comments

Comment #2

webchick

she/they

English

Vancouver 🇨🇦

CreditAttribution: webchick commented 28 November 2006 at 14:00

Status:

Reviewed & tested by the community

» Needs work

You need to create an update as well, for old installations.

Also, please don't mark your own patches "ready to be committed." They need community review.

Log in or register to post comments

Comment #3

havran CreditAttribution: havran commented 28 November 2006 at 14:43

File	Size
system.install_6.patch	2.86 KB

Sorry. Ok i created update function (i look in function system_update_173() which is similar). Please check this. For my installation working ok, but i have only one record in files table (and file_revisions table).

Log in or register to post comments

Comment #4

pwolanin CreditAttribution: pwolanin commented 28 November 2006 at 16:16

Do you need to copy and recreate the table, rather than just doing an update for this one column definition?

Log in or register to post comments

Comment #5

havran CreditAttribution: havran commented 28 November 2006 at 16:45

I have PostgreSQL version 7.4. In PostgreSQL version < 8 there is no query like ALTER TABLE table ALTER COLUMN column TYPE newtype.

Log in or register to post comments

Comment #6

havran CreditAttribution: havran commented 28 November 2006 at 16:48

(ups i use >)

I have PostgreSQL version 7.4. In PostgreSQL version lower than 8 there is no query for altering column type.

Log in or register to post comments

Comment #7

havran CreditAttribution: havran commented 28 November 2006 at 21:30

Version:	5.x-dev	» 5.0-beta2
Status:	Needs work	» Needs review

File	Size
system.install_7.patch	2.86 KB

Still same bug in 5.0-beta2. Patch attached.

Log in or register to post comments

Comment #8

havran CreditAttribution: havran commented 28 November 2006 at 22:56

And now i have found same bug here http://drupal.org/node/97726?

Log in or register to post comments

Comment #9

havran CreditAttribution: havran commented 29 November 2006 at 07:13

File	Size
system.install_8.patch	2.92 KB

I realise, we don't need any table operation on MySQL on this update. Modified patch attached.

Log in or register to post comments

Comment #10

chx CreditAttribution: chx commented 29 November 2006 at 08:23

Status:

Needs review

» Needs work

uh huh. what about adding a fid_serial column instead , then UPDATE so that fid_serial equals fid , then dropping fid columnd and renaming fid_serial finally to fid? At the end, you likely will want to VACUUM ANALYZE the poor table but still I would find this better than copying the whole thing around a few times.

Log in or register to post comments

Comment #11

havran CreditAttribution: havran commented 29 November 2006 at 09:44

(I use PostgreSQL 7.4)

For recreate fid as serial i must use:

DELETE FROM drupal_files WHERE fid=0;
DELETE FROM drupal_file_revisions WHERE fid=0;

ALTER TABLE drupal_files ADD COLUMN fid_serial integer;
UPDATE drupal_files SET fid_serial=fid;

ALTER TABLE drupal_files DROP COLUMN fid;
ALTER TABLE drupal_files RENAME COLUMN fid_serial TO fid;

ALTER TABLE drupal_files ALTER COLUMN fid SET NOT NULL;
ALTER TABLE drupal_files ADD CONSTRAINT drupal_files_fid CHECK (fid >= 0);
ALTER TABLE drupal_files ADD CONSTRAINT drupal_files_pkey PRIMARY KEY (fid);

CREATE SEQUENCE drupal_files_fid_seq;
SELECT setval('drupal_files_seq', max(fid)) FROM drupal_files;
ALTER TABLE drupal_files ALTER COLUMN fid SET DEFAULT nextval('drupal_files_fid_seq');

If column fid exists as serial i give error (i think it is possible). This is not very good. May i try drop drupal_files_fid_seq after droping previous fid column? I think recreate table through temporary table is simpler, safer and cleaner way for this job. And if i use transaction, there is no possibility for destroy {files} data. Maybe someone create better code?

Any other opinions?

Log in or register to post comments

Comment #12

havran CreditAttribution: havran commented 29 November 2006 at 10:09

Status:

Needs work

» Needs review

File	Size
system.install_9.patch	2.85 KB

For safer execution I added TRANSACTION in patch. I have tested it on my installation without problem.

Log in or register to post comments

Comment #13

sammys CreditAttribution: sammys commented 30 November 2006 at 05:43

Status:

Needs review

» Reviewed & tested by the community

File	Size
system.install_20061130_1640_patch.txt	770 bytes

Ok... seems there is a lot of action going on in here. The patch is incorrect as the CHECK condition tests the nid field rather than fid. Another thing is that we're not supplying update code for this because 4.7 already has the serial column. Update code is, after all, making a transition from 4.7 to 5.0 and not 5.0 beta to 5.0 release.

Attached is the replacement patch for system.install. This is RTBC.

As for all you wonderful people out there using the beta release, please use the manual update SQL provided in this issue to update your {files} table.

--
Sammy Spets
Synerger
http://synerger.com

Log in or register to post comments

Comment #14

sammys CreditAttribution: sammys commented 30 November 2006 at 05:45

Oh and thank you for getting to the root of the problem. I screwed up in migrating the code across because I used the MySQL schema and mapped it over to PostgreSQL. There is no mention in there about the sequence needed for files table so it was stuffed. Apologies for the inconvenience.

Log in or register to post comments

Comment #15

sammys CreditAttribution: sammys commented 30 November 2006 at 06:32

One other thing while i'm here. The way you guys are deleting all entries from files and file_revisions may work, but it's only half the picture because node and node_revisions tables have entries in them as well. May I suggest using something like:

CREATE SEQUENCE files_fid_seq;
UPDATE files SET fid = nextval('files_fid_seq');
UPDATE file_revisions SET fid = files.fid FROM files, node_revisions WHERE files.nid = node_revisions.nid AND file_revisions.vid = node_revisions.vid;

Please take a backup of the files and file_revisions tables before performing these queries as i've only tested it with a small number of entries.

You shouldn't have too many entries in there anyway since the PRIMARY KEY constraint would have stopped you from having more than one fid == zero entry.

Cheers,
-- sammys