Re: [HACKERS] [PATCHES] Updatable views
Where are on updatable views? --- Bernd Helmle wrote: --On Freitag, September 01, 2006 11:41:16 -0400 Tom Lane [EMAIL PROTECTED] wrote: So in other words, views on serial columns don't work? I don't think that's going to be acceptable. They work in such a case that someone isn't allowed to put a volatile function in an update query Not really worse than what the rewriter is doing already --- in fact, I think it's isomorphic to what would happen to the rule qual expressions in your existing patch. Currently you don't have to rewrite the rule conditions itself every time you apply them to the query tree since they are stored in pg_rewrite matching all various (reversed) varattno's, resno's and whatever. If i understand correctly you need to do that with constraints every time you fire an update query on a view for each underlying relation I'm about to propose that we should try to go beta next week (see forthcoming message). If you can strip down your patch to avoid the multi-eval problems in the next couple of days, I'm still willing to Depends on how many days couple of days are.if you mean the next three days then definitely not, since i'm afk for the whole upcoming weekend. Bad timing, but it's not deferrable :( consider it, but at the moment I'm assuming that it needs to be held for 8.3. Well, i'll see what i can do next weekit's a little bit disappointing that these problems raises so late, but that's no one's fault since there are many side effects of the rewriting system involved Many thanks for your comments. Bernd ---(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 -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-hackers
Re: [HACKERS] [PATCHES] Updatable views
--On Donnerstag, März 06, 2008 17:03:10 -0500 Bruce Momjian [EMAIL PROTECTED] wrote: Where are on updatable views? I really want to have this one ready for 8.4, but i have nothing appliable at the moment. Considering the amount of rework that needs to be done, i hope i can provide an updated patch version during next commit fest. -- Thanks Bernd -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-hackers
Re: [HACKERS] [PATCHES] Updatable views
On Thu, Mar 6, 2008 at 6:35 PM, Bernd Helmle [EMAIL PROTECTED] wrote: --On Donnerstag, März 06, 2008 17:03:10 -0500 Bruce Momjian [EMAIL PROTECTED] wrote: Where are on updatable views? I really want to have this one ready for 8.4, but i have nothing appliable at the moment. Considering the amount of rework that needs to be done, i hope i can provide an updated patch version during next commit fest. i will be waiting for it! maybe you can update the wiki with actual state or with the latest patch you have even if it doesn't apply... just to see what needs to be done... -- regards, Jaime Casanova -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] Updatable views
Here is the more recent email I have on this feature work. --- Bernd Helmle wrote: --On Freitag, September 01, 2006 11:41:16 -0400 Tom Lane [EMAIL PROTECTED] wrote: So in other words, views on serial columns don't work? I don't think that's going to be acceptable. They work in such a case that someone isn't allowed to put a volatile function in an update query Not really worse than what the rewriter is doing already --- in fact, I think it's isomorphic to what would happen to the rule qual expressions in your existing patch. Currently you don't have to rewrite the rule conditions itself every time you apply them to the query tree since they are stored in pg_rewrite matching all various (reversed) varattno's, resno's and whatever. If i understand correctly you need to do that with constraints every time you fire an update query on a view for each underlying relation I'm about to propose that we should try to go beta next week (see forthcoming message). If you can strip down your patch to avoid the multi-eval problems in the next couple of days, I'm still willing to Depends on how many days couple of days are.if you mean the next three days then definitely not, since i'm afk for the whole upcoming weekend. Bad timing, but it's not deferrable :( consider it, but at the moment I'm assuming that it needs to be held for 8.3. Well, i'll see what i can do next weekit's a little bit disappointing that these problems raises so late, but that's no one's fault since there are many side effects of the rewriting system involved Many thanks for your comments. Bernd ---(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 -- Bruce Momjian [EMAIL PROTECTED] http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(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: [HACKERS] [PATCHES] Updatable views
Where are we on this feature? --- Bernd Helmle wrote: --On Mittwoch, August 30, 2006 12:01:25 -0400 Tom Lane [EMAIL PROTECTED] wrote: Bernd Helmle [EMAIL PROTECTED] writes: [ latest views patch ] This is the first time I've actually looked at this patch, and I am dismayed. viewUpdate.c looks like nothing so much as a large program with a small program struggling to get out. What is all the stuff about handling multiple base rels? SQL92, at least, does not say that a join is updatable, and AFAICT this patch is rejecting that too ... though it's hard to tell with the conditions for allowing the join to be updatable scattered through a lot of different functions. And some of the code seems to be expecting multiple implicit rules and other parts not. I get the impression that a lot of this code is left over from a more ambitious first draft and ought to be removed in the name of readability/maintainability. I not sure what parts of the code you are refering to exactly, but I admit that there are code parts that could deal with multiple base relations and rules. get_base_base_relation() is an example, it is used to create lookup tables for reversed columns so we could break them down to the correct position in their base tables. Restricting that to only one base relation wouldn't make any difference. Furthermore, SQL99 allows at least updatable views with joined relations which preserve their keys in the view definition. So i don't think it's that bad to leave parts of the code that way for future improvements. I'm unclear as to why you've got DO INSTEAD NOTHING rules in there --- the spec says that a WITH CHECK OPTION violation results in an error, not in nothing happening, so it doesn't seem to me that we should need any NOTHING rules to implement the spec. It would probably help if Well, instead of something like ERROR: cannot insert into a view HINT: You need an unconditional ON INSERT DO INSTEAD rule. you will get ERROR: view update commands violates rule condition with the correct error code set, because the view update check function is fired before. The first one isn't very useful for someone who simply wants to insert data into the view which isn't allowed to get in. You never get the view update check function fired without the DO INSTEAD rule applied to a view created with a check option. there were some header documentation that explained exactly how the module intends to transform a SELECT to create the various action rules. I agree with you, maybe it's a good to add a README to src/backend/rewrite? The pg_dump changes seem pretty odd too. Why wouldn't you just ignore implicit rules during a dump, expecting the system to regenerate them when the view is reloaded? Uhm, you're right. It's easier to exclude them in the SELECT query directly instead of selecting them, iterating over and filter them out. I'll fix that. (Looks like this is a cannot see the wood for the trees-mistake) -- Thanks Bernd ---(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 -- Bruce Momjian [EMAIL PROTECTED] http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] Updatable views
This has been saved for the 8.3 release: http://momjian.postgresql.org/cgi-bin/pgpatches_hold --- Bernd Helmle wrote: --On Freitag, September 01, 2006 11:41:16 -0400 Tom Lane [EMAIL PROTECTED] wrote: So in other words, views on serial columns don't work? I don't think that's going to be acceptable. They work in such a case that someone isn't allowed to put a volatile function in an update query Not really worse than what the rewriter is doing already --- in fact, I think it's isomorphic to what would happen to the rule qual expressions in your existing patch. Currently you don't have to rewrite the rule conditions itself every time you apply them to the query tree since they are stored in pg_rewrite matching all various (reversed) varattno's, resno's and whatever. If i understand correctly you need to do that with constraints every time you fire an update query on a view for each underlying relation I'm about to propose that we should try to go beta next week (see forthcoming message). If you can strip down your patch to avoid the multi-eval problems in the next couple of days, I'm still willing to Depends on how many days couple of days are.if you mean the next three days then definitely not, since i'm afk for the whole upcoming weekend. Bad timing, but it's not deferrable :( consider it, but at the moment I'm assuming that it needs to be held for 8.3. Well, i'll see what i can do next weekit's a little bit disappointing that these problems raises so late, but that's no one's fault since there are many side effects of the rewriting system involved Many thanks for your comments. Bernd ---(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 -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] Updatable views
Bernd Helmle [EMAIL PROTECTED] writes: I would like to try to grab your idea to push down the CHECK OPTION logic down to the executor as a (table/view?) constraint. Would that be an idea worth to consider and, most important, is this doable? I don't have that much experience in the executor, so i fear this isn't something that will be done within a week or so.:( You're certainly welcome to work on it --- I don't have time at the moment. But I agree there's little chance of getting it done in time for 8.2. I have not read the spec's definition of WITH CHECK OPTION lately, so there may be something fundamentally wrong in what I'm about to say, but the way I'm envisioning this working is that W.C.O. is embodied as a check constraint (or something pretty similar --- a pg_constraint entry certainly) attached to the view. The rewriter would then be responsible for collecting all the check options that need to be enforced for a given rewritten query. This'd require some rework of the rewriter/planner/executor API: right now the executor is solely responsible for collecting check constraints to apply in an updating query, and we'd want to change that. My thought is we might as well move the collection responsibility over to the rewriter 100%: the rewriter decorates a Query with the list of constraint expressions to apply, and the executor just checks what it's told to. The planner probably need not do much with the constraint expressions beyond what it normally does with, say, targetlist expressions. Some thoughts: * It's too early in the morning for me to be clear about the difference between CASCADED and LOCAL CHECK OPTION --- I think that this would merely alter the set of check constraints collected for a particular query, but if there's something more fundamental there, this scheme might not work at all. So look into that first. * The reason we currently collect constraints at executor start is that ALTER TABLE ADD CONSTRAINT has no way to invalidate cached plans, so it's unsafe to store lists of constraints in plans. So this scheme *requires* that we have a plan invalidation mechanism in place (at least before we release, maybe not before the patch can go in). This doesn't bother me because I intend anyway to see to it that there's plan inval in 8.3. * With check constraints now passing through the planner, it'd become trivial to allow subqueries in check constraints. Just sayin'. * We'd probably eliminate the idea of storing constraints in TupleDescs, which would be a good simplification anyway (eg, eliminate the bogus distinction between CreateTupleDescCopy and CreateTupleDescCopyConstr). OTOH that might make it harder to allow rowtypes to have associated constraints? Needs thought. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] Updatable views
--On Donnerstag, August 31, 2006 11:10:47 -0400 Tom Lane
[EMAIL PROTECTED] wrote:
The problem is not with the view condition. Consider
CREATE TABLE data (id serial primary key, ...);
CREATE VIEW only_new_data AS SELECT * FROM data WHERE id 12345
WITH CHECK OPTION;
INSERT INTO only_new_data VALUES(nextval('data_id_seq'), ...);
The proposed implementation will execute nextval twice (bad), and will
apply the WITH CHECK OPTION test to the value that isn't the one stored
(much worse). It doesn't help if the id is defaulted.
*scratches head*i don't see a shortcoming solution for this in my
current implementation,
indeed. I admit that this is a serious containment of updatable views in
its current
incarnation (at least for the check option).
*thinking*
I would like to try to grab your idea to push down the CHECK OPTION logic
down to the executor as a (table/view?) constraint. Would that be an idea
worth to consider and,
most important, is this doable? I don't have that much experience in the
executor, so i fear
this isn't something that will be done within a week or so.:(
--
Thanks
Bernd
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] Updatable views
Bernd Helmle wrote: [EMAIL PROTECTED] wrote: * It's too early in the morning for me to be clear about the difference between CASCADED and LOCAL CHECK OPTION --- I think that this would merely alter the set of check constraints collected for a particular query, but if there's something more fundamental there, this scheme might not work at all. So look into that first. LOCAL checks the data to be updated against its own view WHERE condition only, where CASCADED involves all WHERE conditions of all underlying views. I don't understand this part very well. Say if you have a view WITH CHECK OPTION whose condition is foo 5, and then define a view WITH LOCAL CHECK OPTION on top of that, whose condition is bar 5. Does the local check option on the second view that I can insert a row with foo=4, bar=6? That doesn't violate the condition of bar 5, so it seems fine to me. But it also seems quite idiotic because it violated the original foo5 condition. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] Updatable views
Bernd Helmle [EMAIL PROTECTED] writes: What we can do is to restrict view updates that involves a volatile function completely. As soon as the rewriter wants to apply an implicit system rule to a current query which holds volatile functions, we could treat this as an error. So in other words, views on serial columns don't work? I don't think that's going to be acceptable. In order you want to do a CASCADED CHECK OPTION, you need to collect all expressions out of underlying relations and rewrite them to match the table you are selecting...that looks like a very expensive operation. Not really worse than what the rewriter is doing already --- in fact, I think it's isomorphic to what would happen to the rule qual expressions in your existing patch. So we need to stall this idea unless we have something workable in this area. So what's the plan for 8.2? Should we reject updatable views completely or is there some interest to apply this without CHECK OPTION? I'm about to propose that we should try to go beta next week (see forthcoming message). If you can strip down your patch to avoid the multi-eval problems in the next couple of days, I'm still willing to consider it, but at the moment I'm assuming that it needs to be held for 8.3. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] Updatable views
[Quick note: sorry if you received this mail multiple times, i've moved to a new workstation and my MUA gots hosed up with its identities (all of them has the same adress, suddenly) and I recognized that too late.i'm sorry] --On Freitag, September 01, 2006 10:03:42 -0400 Tom Lane [EMAIL PROTECTED] wrote: Bernd Helmle [EMAIL PROTECTED] writes: You're certainly welcome to work on it --- I don't have time at the moment. But I agree there's little chance of getting it done in time for 8.2. What we can do is to restrict view updates that involves a volatile function completely. As soon as the rewriter wants to apply an implicit system rule to a current query which holds volatile functions, we could treat this as an error. However, i haven't looked into that right now how doable that would be, but it seems correct in terms of data reliability (treat it as volatile in view update is evil ;) Maybe it's worth to look how other database systems solve this problem. I have not read the spec's definition of WITH CHECK OPTION lately, so there may be something fundamentally wrong in what I'm about to say, but the way I'm envisioning this working is that W.C.O. is embodied as a check constraint (or something pretty similar --- a pg_constraint entry certainly) attached to the view. The rewriter would then be responsible for collecting all the check options that need to be enforced for a given rewritten query. This'd require some rework of the rewriter/planner/executor API: right now the executor is solely responsible for collecting check constraints to apply in an updating query, and we'd want to change that. My thought is we might as well move the collection responsibility over to the rewriter 100%: the rewriter decorates a Query with the list of constraint expressions to apply, and the executor just checks what it's told to. The planner probably need not do much with the constraint expressions beyond what it normally does with, say, targetlist expressions. In order you want to do a CASCADED CHECK OPTION, you need to collect all expressions out of underlying relations and rewrite them to match the table you are selecting...that looks like a very expensive operation. Some thoughts: * It's too early in the morning for me to be clear about the difference between CASCADED and LOCAL CHECK OPTION --- I think that this would merely alter the set of check constraints collected for a particular query, but if there's something more fundamental there, this scheme might not work at all. So look into that first. LOCAL checks the data to be updated against its own view WHERE condition only, where CASCADED involves all WHERE conditions of all underlying views. That said, it's necessary to grep out all WHERE conditions of all relations involved in an update operation and apply them to the current query as a constraint expression. The current implementation passes this recursively via a conditional rule through the rewriter. It looked to me as an attractive implementation, but it has this annoying multiple evaluation side effects:( * The reason we currently collect constraints at executor start is that ALTER TABLE ADD CONSTRAINT has no way to invalidate cached plans, so it's unsafe to store lists of constraints in plans. So this scheme *requires* that we have a plan invalidation mechanism in place (at least before we release, maybe not before the patch can go in). This doesn't bother me because I intend anyway to see to it that there's plan inval in 8.3. So we need to stall this idea unless we have something workable in this area. So what's the plan for 8.2? Should we reject updatable views completely or is there some interest to apply this without CHECK OPTION? Some basic functionality could be simulated with table constraints, however, it's not what users out there would expect * With check constraints now passing through the planner, it'd become trivial to allow subqueries in check constraints. Just sayin'. That would be a nice feature, indeed ;) * We'd probably eliminate the idea of storing constraints in TupleDescs, which would be a good simplification anyway (eg, eliminate the bogus distinction between CreateTupleDescCopy and CreateTupleDescCopyConstr). OTOH that might make it harder to allow rowtypes to have associated constraints? Needs thought. regards, tom lane -- Thanks Bernd ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] Updatable views
On 9/1/06, Alvaro Herrera [EMAIL PROTECTED] wrote: Bernd Helmle wrote: [EMAIL PROTECTED] wrote: * It's too early in the morning for me to be clear about the difference between CASCADED and LOCAL CHECK OPTION --- I think that this would merely alter the set of check constraints collected for a particular query, but if there's something more fundamental there, this scheme might not work at all. So look into that first. LOCAL checks the data to be updated against its own view WHERE condition only, where CASCADED involves all WHERE conditions of all underlying views. I don't understand this part very well. Say if you have a view WITH CHECK OPTION whose condition is foo 5, and then define a view WITH LOCAL CHECK OPTION on top of that, whose condition is bar 5. Does the local check option on the second view that I can insert a row with foo=4, bar=6? That doesn't violate the condition of bar 5, so it yes. or at least that's the way i read that... seems fine to me. But it also seems quite idiotic because it violated the original foo5 condition. and that means that without the CHECK OPTION constraint you can insert anything into the base table no matter what the WHERE condition in the view is... again, this is the way informix implements it... ahhh, the great members of the SQL COMITTEE... -- regards, Jaime Casanova Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning. Richard Cook ---(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: [HACKERS] [PATCHES] Updatable views
--On Freitag, September 01, 2006 11:34:49 -0400 Alvaro Herrera [EMAIL PROTECTED] wrote: I don't understand this part very well. Say if you have a view WITH CHECK OPTION whose condition is foo 5, and then define a view WITH LOCAL CHECK OPTION on top of that, whose condition is bar 5. Does the local check option on the second view that I can insert a row with foo=4, bar=6? That doesn't violate the condition of bar 5, so it seems fine to me. But it also seems quite idiotic because it violated the original foo5 condition. That's exactly what i'm reading out there, too. If such a view definition is useful or not depends on its use case. Correct me if i'm wrong -- Thanks Bernd ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] Updatable views
--On Freitag, September 01, 2006 11:41:16 -0400 Tom Lane [EMAIL PROTECTED] wrote: So in other words, views on serial columns don't work? I don't think that's going to be acceptable. They work in such a case that someone isn't allowed to put a volatile function in an update query Not really worse than what the rewriter is doing already --- in fact, I think it's isomorphic to what would happen to the rule qual expressions in your existing patch. Currently you don't have to rewrite the rule conditions itself every time you apply them to the query tree since they are stored in pg_rewrite matching all various (reversed) varattno's, resno's and whatever. If i understand correctly you need to do that with constraints every time you fire an update query on a view for each underlying relation I'm about to propose that we should try to go beta next week (see forthcoming message). If you can strip down your patch to avoid the multi-eval problems in the next couple of days, I'm still willing to Depends on how many days couple of days are.if you mean the next three days then definitely not, since i'm afk for the whole upcoming weekend. Bad timing, but it's not deferrable :( consider it, but at the moment I'm assuming that it needs to be held for 8.3. Well, i'll see what i can do next weekit's a little bit disappointing that these problems raises so late, but that's no one's fault since there are many side effects of the rewriting system involved Many thanks for your comments. Bernd ---(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: [HACKERS] [PATCHES] Updatable views
Am Mittwoch, 30. August 2006 18:01 schrieb Tom Lane: This is the first time I've actually looked at this patch, and I am dismayed. viewUpdate.c looks like nothing so much as a large program with a small program struggling to get out. What is all the stuff about handling multiple base rels? SQL92, at least, does not say that a join is updatable, and AFAICT this patch is rejecting that too ... But later SQL versions allow some of that, so at least it shouldn't hurt to have some parts of the code to be more general in preparation of that. I'm unclear as to why you've got DO INSTEAD NOTHING rules in there --- You need to have one unconditional rule if you have a bunch of conditional ones. The system does not see through the fact that the conditional ones cover all cases. The pg_dump changes seem pretty odd too. Why wouldn't you just ignore implicit rules during a dump, expecting the system to regenerate them when the view is reloaded? Right. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(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: [HACKERS] [PATCHES] Updatable views
--On Mittwoch, August 30, 2006 12:01:25 -0400 Tom Lane [EMAIL PROTECTED] wrote: Bernd Helmle [EMAIL PROTECTED] writes: [ latest views patch ] This is the first time I've actually looked at this patch, and I am dismayed. viewUpdate.c looks like nothing so much as a large program with a small program struggling to get out. What is all the stuff about handling multiple base rels? SQL92, at least, does not say that a join is updatable, and AFAICT this patch is rejecting that too ... though it's hard to tell with the conditions for allowing the join to be updatable scattered through a lot of different functions. And some of the code seems to be expecting multiple implicit rules and other parts not. I get the impression that a lot of this code is left over from a more ambitious first draft and ought to be removed in the name of readability/maintainability. I not sure what parts of the code you are refering to exactly, but I admit that there are code parts that could deal with multiple base relations and rules. get_base_base_relation() is an example, it is used to create lookup tables for reversed columns so we could break them down to the correct position in their base tables. Restricting that to only one base relation wouldn't make any difference. Furthermore, SQL99 allows at least updatable views with joined relations which preserve their keys in the view definition. So i don't think it's that bad to leave parts of the code that way for future improvements. I'm unclear as to why you've got DO INSTEAD NOTHING rules in there --- the spec says that a WITH CHECK OPTION violation results in an error, not in nothing happening, so it doesn't seem to me that we should need any NOTHING rules to implement the spec. It would probably help if Well, instead of something like ERROR: cannot insert into a view HINT: You need an unconditional ON INSERT DO INSTEAD rule. you will get ERROR: view update commands violates rule condition with the correct error code set, because the view update check function is fired before. The first one isn't very useful for someone who simply wants to insert data into the view which isn't allowed to get in. You never get the view update check function fired without the DO INSTEAD rule applied to a view created with a check option. there were some header documentation that explained exactly how the module intends to transform a SELECT to create the various action rules. I agree with you, maybe it's a good to add a README to src/backend/rewrite? The pg_dump changes seem pretty odd too. Why wouldn't you just ignore implicit rules during a dump, expecting the system to regenerate them when the view is reloaded? Uhm, you're right. It's easier to exclude them in the SELECT query directly instead of selecting them, iterating over and filter them out. I'll fix that. (Looks like this is a cannot see the wood for the trees-mistake) -- Thanks Bernd ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] Updatable views
Peter Eisentraut [EMAIL PROTECTED] writes: Am Mittwoch, 30. August 2006 18:01 schrieb Tom Lane: This is the first time I've actually looked at this patch, and I am dismayed. viewUpdate.c looks like nothing so much as a large program with a small program struggling to get out. But later SQL versions allow some of that, so at least it shouldn't hurt to have some parts of the code to be more general in preparation of that. If it bloats the code to unreadability, it's bad. I'm unclear as to why you've got DO INSTEAD NOTHING rules in there --- You need to have one unconditional rule if you have a bunch of conditional ones. The system does not see through the fact that the conditional ones cover all cases. AFAICS, for the cases we are able to implement within the existing rule mechanism, there should be exactly one unconditional rule. If you propose more, then you are going to have insurmountable problems with the usual sorts of multiple-evaluation risks. The proposed WITH CHECK OPTION implementation is unworkable for exactly this reason --- it will give the wrong answers in the presence of volatile functions such as nextval(). I believe that we cannot implement WITH CHECK OPTION as a rule. It's a constraint, instead, and will have to be checked the way the executor presently checks constraints, ie after forming the finished new tuple(s). (Someday we're going to have to look into redesigning the rule system so that it can cope better with the kinds of situations that give rise to multiple-evaluation problems. But today is not that day.) It's possible that if we strip the patch down to SQL92-equivalent functionality (no multiple base rels) without WITH CHECK OPTION, we would have something that would work reliably atop the existing rule mechanism. It's getting mighty late in the 8.2 cycle to be doing major rework though. regards, tom lane ---(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: [HACKERS] [PATCHES] Updatable views
Am Donnerstag, 31. August 2006 15:55 schrieb Tom Lane: I'm unclear as to why you've got DO INSTEAD NOTHING rules in there --- You need to have one unconditional rule if you have a bunch of conditional ones. The system does not see through the fact that the conditional ones cover all cases. AFAICS, for the cases we are able to implement within the existing rule mechanism, there should be exactly one unconditional rule. If you propose more, then you are going to have insurmountable problems with the usual sorts of multiple-evaluation risks. I'm not sure what you are saying here ... The implementation creates, for each of the three actions INSERT, UPDATE, DELETE, one conditional rule that redirects the action from the view into the unterlying table, conditional on the view condition being fulfilled. The unconditional DO INSTEAD NOTHING rule then catches the cases where the view condition is not fulfilled. So there is, for each action, exactly one conditional and one unconditional rule. Which is consistent with what you said above, so I don't see the problem. The proposed WITH CHECK OPTION implementation is unworkable for exactly this reason --- it will give the wrong answers in the presence of volatile functions such as nextval(). I'm not sure why anyone would want to define a view condition containing a volatile function. At least it wouldn't put a major dent into this feature if such views were decreed not updatable. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] Updatable views
Peter Eisentraut [EMAIL PROTECTED] writes:
Am Donnerstag, 31. August 2006 15:55 schrieb Tom Lane:
The proposed WITH CHECK OPTION implementation is unworkable for exactly
this reason --- it will give the wrong answers in the presence of
volatile functions such as nextval().
I'm not sure why anyone would want to define a view condition containing a
volatile function. At least it wouldn't put a major dent into this feature
if such views were decreed not updatable.
The problem is not with the view condition. Consider
CREATE TABLE data (id serial primary key, ...);
CREATE VIEW only_new_data AS SELECT * FROM data WHERE id 12345
WITH CHECK OPTION;
INSERT INTO only_new_data VALUES(nextval('data_id_seq'), ...);
The proposed implementation will execute nextval twice (bad), and will
apply the WITH CHECK OPTION test to the value that isn't the one stored
(much worse). It doesn't help if the id is defaulted.
regards, tom lane
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] Updatable views
Bernd Helmle [EMAIL PROTECTED] writes: [ latest views patch ] This is the first time I've actually looked at this patch, and I am dismayed. viewUpdate.c looks like nothing so much as a large program with a small program struggling to get out. What is all the stuff about handling multiple base rels? SQL92, at least, does not say that a join is updatable, and AFAICT this patch is rejecting that too ... though it's hard to tell with the conditions for allowing the join to be updatable scattered through a lot of different functions. And some of the code seems to be expecting multiple implicit rules and other parts not. I get the impression that a lot of this code is left over from a more ambitious first draft and ought to be removed in the name of readability/maintainability. I'm unclear as to why you've got DO INSTEAD NOTHING rules in there --- the spec says that a WITH CHECK OPTION violation results in an error, not in nothing happening, so it doesn't seem to me that we should need any NOTHING rules to implement the spec. It would probably help if there were some header documentation that explained exactly how the module intends to transform a SELECT to create the various action rules. The pg_dump changes seem pretty odd too. Why wouldn't you just ignore implicit rules during a dump, expecting the system to regenerate them when the view is reloaded? regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] Updatable views
On Wed, Aug 30, 2006 at 12:01:25PM -0400, Tom Lane wrote: Bernd Helmle [EMAIL PROTECTED] writes: [ latest views patch ] This is the first time I've actually looked at this patch, and I am dismayed. viewUpdate.c looks like nothing so much as a large program with a small program struggling to get out. What is all the stuff about handling multiple base rels? SQL92, at least, does not say that a join is updatable, and AFAICT this patch is rejecting that too ... though it's hard to tell with the conditions for allowing the join to be updatable scattered through a lot of different functions. And some of the code seems to be expecting multiple implicit rules and other parts not. I get the impression that a lot of this code is left over from a more ambitious first draft and ought to be removed in the name of readability/maintainability. If that code is on the right path to allowing things like updates to the many side of a join then it would be worth adding comments to that effect. Or maybe a comment referencing whatever version of the file the code was yanked out of. -- Jim C. Nasby, Sr. Engineering Consultant [EMAIL PROTECTED] Pervasive Software http://pervasive.comwork: 512-231-6117 vcard: http://jim.nasby.net/pervasive.vcf cell: 512-569-9461 ---(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: [HACKERS] [PATCHES] Updatable views
Bernd Helmle [EMAIL PROTECTED] writes: While working on Alvaro's suggestions to fix the code i got the opinion that we need to reject any attempts to name a user defined rule as _INSERT _NOTHING_INSERT _DELETE _NOTHING_DELETE _UPDATE _NOTHING_UPDATE If the code is dependent on recognizing names to know what it's doing, then I'd say you have a fundamentally broken approach. Consider adding a flag column to pg_rewrite to distinguish these rules, instead. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Updatable views
On 8/24/06, Tom Lane [EMAIL PROTECTED] wrote: Bernd Helmle [EMAIL PROTECTED] writes: While working on Alvaro's suggestions to fix the code i got the opinion that we need to reject any attempts to name a user defined rule as _INSERT _NOTHING_INSERT _DELETE _NOTHING_DELETE _UPDATE _NOTHING_UPDATE If the code is dependent on recognizing names to know what it's doing, then I'd say you have a fundamentally broken approach. Consider adding a flag column to pg_rewrite to distinguish these rules, instead. Actually the code delete implicit rules based on a field added to pg_rewrite but that catalog has a unique index on ev_class, rulename: pg_rewrite_rel_rulename_index UNIQUE, btree (ev_class, rulename) i guess bernd's comment is about this index giving an error if we try to insert the new rule with the same name on the same event... -- regards, Jaime Casanova Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning. Richard Cook ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] Updatable views
--On Donnerstag, August 24, 2006 11:00:45 -0400 Tom Lane [EMAIL PROTECTED] wrote: If the code is dependent on recognizing names to know what it's doing, then I'd say you have a fundamentally broken approach. Consider adding a flag column to pg_rewrite to distinguish these rules, instead. This is the approach the code already follows (it uses an additional ev_kind column which distinguishes rules between implicit rules with no, local or cascaded check option and explicit ones). Turns out that i was thinking too difficult when looking at the code which drops implicit rulessorry for the noise. -- Thanks Bernd ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Updatable views
--On Donnerstag, August 24, 2006 11:02:43 -0500 Jaime Casanova [EMAIL PROTECTED] wrote: Actually the code delete implicit rules based on a field added to pg_rewrite but that catalog has a unique index on ev_class, rulename: pg_rewrite_rel_rulename_index UNIQUE, btree (ev_class, rulename) i guess bernd's comment is about this index giving an error if we try to insert the new rule with the same name on the same event... No, this wasn't the problem, since we are going to drop any implicit rule that collides with an user defined one (however, this approach is discussable, but nobody has put his comments on this yet and i think this is important for backwards compatibility). I don't think we need ev_kind in the index at all, in my opinion implicit and user defined rules of the same event shouldn't live together (_RETURN rules are marked as implicit ones now, too) -- Thanks Bernd ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
