Re: [PATCHES] ADD/DROP INHERITS
Tom Lane <[EMAIL PROTECTED]> writes: > If you're happy with the code looking directly at pg_constraint then > I see no reason to change it. I just mentioned the relcache because > I thought you were concerned about the performance of a pg_constraint > search. I'm not concerned with the performance hit of doing a linear scan on pg_constraint or pg_attribute. I am slightly concerned about repeatedly calling SearchSysCacheExistsAttName But using relcache would mean a O(n^2) search across the attributes which might be even worse. I'm unclear how efficient the SysCache lookup function is. If it's a hash table lookup it might be slower but more scalable than an O(n^2) match against the relcache anyways. And I'm slightly concerned with the O(n^2) constraint matching. If someone has 100+ constraints it may be somewhat disappointing to have the operation have a noticeable delay. 1,000 constraints means a million calls to strcmp. Realistically though 1,000 check constraints would be pretty unlikely. 100 constraints might be on the edge of reasonableness and 10,000 calls to strcmp is probably also at the edge of reasonableness too. -- greg ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] ADD/DROP INHERITS
Greg Stark <[EMAIL PROTECTED]> writes: > So I'm thinking it's better to leave my implementation as is rather than > reimplement it using the relcache. Or would it be better to use the relcache > and then when and if it comes to making inherited tables inherit foreign key > constraints look into adding foreign keys and the deferrable and isdeffered > flags to the relcache? Well, it's reasonable to assume that the relcache entries for check constraints include everything that's semantically meaningful, because that's what the actual constraint enforcement code looks at. IE, if we ever did have deferrable check constraints then that flag would get added to the entries. However, we don't keep any info about FK constraints in the relcache (except indirectly via their triggers). At the moment I can't think of any reason why we should. If you're happy with the code looking directly at pg_constraint then I see no reason to change it. I just mentioned the relcache because I thought you were concerned about the performance of a pg_constraint search. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] ADD/DROP INHERITS
Tom Lane <[EMAIL PROTECTED]> writes: > Greg Stark <[EMAIL PROTECTED]> writes: > > So should I set up a nested scan, essentially implementing a nested loop? or > > should I gather together all the children in a list? > > I'd use the predigested form of the constraints attached to the Relation > tupledescs, cf. RelationBuildTupleDesc, equalTupleDescs. I've spent more time absorbing relcache.c and I see now that there is in fact a ccname array that contains the constraint names. The relcache does not however contain the deferrable and isdeferred flags which as you point out don't mean much for check constraints. But you said I should be checking those. Going with the relcache would mean not being able to check them. Going with the relcache would also mean I wouldn't be able to easily add foreign key constraints. I had hoped to add foreign key constraints to the list of things that inherited tables copy and checking for them in AddInherits. So I'm thinking it's better to leave my implementation as is rather than reimplement it using the relcache. Or would it be better to use the relcache and then when and if it comes to making inherited tables inherit foreign key constraints look into adding foreign keys and the deferrable and isdeffered flags to the relcache? > It might be worth refactoring equalTupleDescs so you could share code --- > ISTM what you're trying to implement is something like a "subsetTupleDesc". I still don't see how there's any refactoring possible here. I could move the code into a function in tupdesc.c but then it would mean an entirely redundant loop to bump attinhcount. -- greg ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] ADD/DROP INHERITS
Tom Lane <[EMAIL PROTECTED]> writes: > Greg Stark <[EMAIL PROTECTED]> writes: > > So should I set up a nested scan, essentially implementing a nested loop? or > > should I gather together all the children in a list? > > I'd use the predigested form of the constraints attached to the Relation > tupledescs, cf. RelationBuildTupleDesc, equalTupleDescs. It might be > worth refactoring equalTupleDescs so you could share code --- ISTM what > you're trying to implement is something like a "subsetTupleDesc". Unless I'm missing something that predigested form only has the conbin field. It doesn't have the name of the constraint nor the other fields like deferrable and deferred by default. It also doesn't have foreign key constraints which I'm ignoring now but suggesting that we will want to be copying to children and checking for in new children in the future. And subsetTupleDesc seems to be checking that the attributes are in the same specific order, not that they match by name. That seems like a very different kind of quality/subset nature than needed here. -- greg ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] ADD/DROP INHERITS
Tom Lane <[EMAIL PROTECTED]> writes: > I don't believe those attributes mean anything for check constraints > ATM, but you may as well compare them anyway. If we ever do implement > them then it'd be reasonable to expect parent and child to have > identical settings. I'm not sure. Does it have to have identical behaviour as long as it guarantees the same level of data integrity? Deferred constraints will still guarantee that the promises of the parent table are met. But in that case I guess I really have to store the whole tuple. I'll look into the stuff you suggested I look at to do that. > > Also, it seems to me that LIKE ought to copy constraints or at least have an > > option to. > > What does the spec say about that? It says: NOTE 245 \u2014 s, except for NOT NULL, are not included in CDi; s are effectively transformed to s and are thereby also excluded. We could still do an INCLUDING CONSTRAINTS option or something like that? It seems it would make it much more convenient for creating partitions. Then we could document that "CREATE TABLE child (LIKE parent INCLUDING CONSTRAINTS)" is guaranteed to create a suitable child table for your parent table. -- greg ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] ADD/DROP INHERITS
Greg Stark <[EMAIL PROTECTED]> writes: > So should I set up a nested scan, essentially implementing a nested loop? or > should I gather together all the children in a list? I'd use the predigested form of the constraints attached to the Relation tupledescs, cf. RelationBuildTupleDesc, equalTupleDescs. It might be worth refactoring equalTupleDescs so you could share code --- ISTM what you're trying to implement is something like a "subsetTupleDesc". > And are there any other fields of pg_constraint that I should be checking for > matches in? Do we care if a parent table has a non-deferrable constraint and > the child has a deferrable one, or if the parent's is deferred by default and > the child isn't? I don't believe those attributes mean anything for check constraints ATM, but you may as well compare them anyway. If we ever do implement them then it'd be reasonable to expect parent and child to have identical settings. > Also, it seems to me that LIKE ought to copy constraints or at least have an > option to. What does the spec say about that? regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] ADD/DROP INHERITS
Tom Lane <[EMAIL PROTECTED]> writes: > Greg Stark <[EMAIL PROTECTED]> writes: > >I also haven't checked the constraint name. To do so it would make sense > > to > >use a small hash table. > > No, it'd make sense to use strcmp(). It's unlikely that there will be > enough constraints attached to any one table to justify use of any but > the simplest algorithm. AFAICS you should just iterate through the > child constraints looking for matches ... and I'd suggest checking the > name first, as that will save a whole lot more work in reverse-compiling > than any amount of tenseness in the matching code. So should I set up a nested scan, essentially implementing a nested loop? or should I gather together all the children in a list? My inclination is to avoid the repeated scans and gather them together in a list of cons cells of the two strings. Or can I stuff the whole tuple in the list elements? I'm unclear on the memory management of tuples in the midst of a scan; would I have to copy them? Are the scans less expensive than I imagine and there's no point in storing the results? And are there any other fields of pg_constraint that I should be checking for matches in? Do we care if a parent table has a non-deferrable constraint and the child has a deferrable one, or if the parent's is deferred by default and the child isn't? > >I'm ignoring unique, primary key, and foreign key constraints on the > > theory > >that these things don't really work on inherited tables yet > >anyways. > > Yeah, the consistent thing to do with these is nothing, until something > is done about the generic problem. It seems to me that foreign key constraints ought to be copied even now though. Also, it seems to me that LIKE ought to copy constraints or at least have an option to. Otherwise it's not really suitable for creating partitions which would be sad since it seems perfect for that task. -- greg ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] ADD/DROP INHERITS
Greg Stark <[EMAIL PROTECTED]> writes: >I also haven't checked the constraint name. To do so it would make sense to >use a small hash table. No, it'd make sense to use strcmp(). It's unlikely that there will be enough constraints attached to any one table to justify use of any but the simplest algorithm. AFAICS you should just iterate through the child constraints looking for matches ... and I'd suggest checking the name first, as that will save a whole lot more work in reverse-compiling than any amount of tenseness in the matching code. >I'm ignoring unique, primary key, and foreign key constraints on the theory >that these things don't really work on inherited tables yet >anyways. Yeah, the consistent thing to do with these is nothing, until something is done about the generic problem. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
