Re: [PATCHES] costly foreign key ri checks (4)
Fabien COELHO [EMAIL PROTECTED] writes: I'll keep that in mind. However, from other projects I've worked on, I found that large regression tests are not wasted. Maybe two level of tests wouldn't be bad, as when you're about to release a new version, it's better to pass large tests, but when installing some light checks are enough just to check that all functionnalities are there. I think I already saw something like that somewhere in the sources? There is a provision for big tests in the regression test infrastructure. The only additional test at present is a rerun of the numeric test with greater precision choices; which personally I think is quite useless. But we could imagine using the mechanism for other stuff. regards, tom lane ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [PATCHES] costly foreign key ri checks (4)
Fabien COELHO [EMAIL PROTECTED] writes: Following the discussion about previous versions of this patch, please find attached a new patch candidate for warning about costly foreign key referential integrity checks. I have reviewed and applied this, with some tweaking. I attach the patch as applied. Some comments on the changes: * You were careless about updating the comments to match the code. This is an essential activity to keep things intelligible for future developers. * The operator lookup needed to have the left and right operand types switched; as it stood, the test for opclass membership failed in many more cases than it was supposed to, because you were feeding it the wrong member of a commutator pair. * I changed the message wording to conform to the message style guidelines. I also made it complain about costly sequential scans instead of costly cross-type conversion, since ISTM that's what's really at issue here. I'm not completely wedded to that wording though, if anyone feels the previous version was better. * BTW, you were generating the type names in the error message in the wrong way --- format_type_be is preferred for this, as it is easier to call and generates nicer output too. * It seemed to me that while we were at it, we should improve the message for complete failure (no available equality operator) to complain about the foreign key constraint rather than the operator per se. That is, -- This next should fail, because inet=int does not exist ALTER TABLE FKTABLE ADD FOREIGN KEY(ftest1) references pktable; ! ERROR: operator does not exist: inet = integer ! HINT: No operator matches the given name and argument type(s). You may need to add explicit type casts. becomes -- This next should fail, because inet=int does not exist ALTER TABLE FKTABLE ADD FOREIGN KEY(ftest1) references pktable; ! ERROR: foreign key constraint $1 cannot be implemented ! DETAIL: Key columns ftest1 and ptest1 are of incompatible types: inet and integer. Again, I'm not wedded to this wording, but it seems like a step forward to me. The old way's HINT was certainly pretty inappropriate for the context of a foreign key. * The number of regression cases you added seemed excessive for such a minor feature. We do need to have some consideration for the runtime of the regression tests, because they are used so often by so many developers. I pared it down a little, and made sure it exercised both promotion and crosstype-index-operator cases. Overall though, a good effort. This was your first backend patch, wasn't it? Nice job. regards, tom lane bin0.bin Description: costly_ri.patch.gz ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] costly foreign key ri checks (4)
Applied by Tom. Thanks. --- Fabien COELHO wrote: Dear patchers, Following the discussion about previous versions of this patch, please find attached a new patch candidate for warning about costly foreign key referential integrity checks. 1/ it generates a WARNING 2/ it DETAILs the attributes and types 3/ some regression tests are also appended to foreign_key.sql It validates for me against current head. Have a nice day, -- Fabien Coelho - [EMAIL PROTECTED] Content-Description: [ Attachment, skipping... ] ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] costly foreign key ri checks (4)
Tom Lane wrote: * I changed the message wording to conform to the message style guidelines. I also made it complain about costly sequential scans instead of costly cross-type conversion, since ISTM that's what's really at issue here. I'm not completely wedded to that wording though, if anyone feels the previous version was better. So the issue wasn't that the conversion was costly, but that an index couldn't be used to look up the primary key? -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] costly foreign key ri checks (4)
Your patch has been added to the PostgreSQL unapplied patches list at: http://momjian.postgresql.org/cgi-bin/pgpatches I will try to apply it within the next 48 hours. --- Fabien COELHO wrote: Dear patchers, Following the discussion about previous versions of this patch, please find attached a new patch candidate for warning about costly foreign key referential integrity checks. 1/ it generates a WARNING 2/ it DETAILs the attributes and types 3/ some regression tests are also appended to foreign_key.sql It validates for me against current head. Have a nice day, -- Fabien Coelho - [EMAIL PROTECTED] Content-Description: [ Attachment, skipping... ] ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 8: explain analyze is your friend
[PATCHES] costly foreign key ri checks (4)
Dear patchers, Following the discussion about previous versions of this patch, please find attached a new patch candidate for warning about costly foreign key referential integrity checks. 1/ it generates a WARNING 2/ it DETAILs the attributes and types 3/ some regression tests are also appended to foreign_key.sql It validates for me against current head. Have a nice day, -- Fabien Coelho - [EMAIL PROTECTED] costly_ri_notice.patch.gz Description: Binary data ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
