NikhilS <[EMAIL PROTECTED]> writes: > As per discussion on -hackers, a patch which allows updates to use > subselects is attached with this mail.
I looked over this patch briefly, and I'm afraid it still needs a lot of work. The way you altered transformUpdateStmt is a mess; it's duplicating logic and also using inapplicable tests. (In particular, invoking checkInsertTargets on a subset of the complete target list is just not sensible --- it's supposed to be catching conflicts. I think you really don't need it at all anyway, because the needed tests are done downstream in the rewriter.) I think the right way is to expand out the row-assignment portions of the SET target list and then process the whole SET list the same as before. The real problem though is that the patch creates a new storage method and processing path for ROWEXPR_SUBLINK sublinks that's got nothing to do with the other kinds of sublinks. This is going to be a mess and a fertile source of bugs. (You already have quite a few stemming from having overlooked all the places that have to be touched to add a field to Query, eg backend/nodes/, optimizer/util/clauses.c, ruleutils.c...) The basic idea is not bad, but I think if we're going to make a list of subqueries to attach to Query, we should try to do all the forms of SubLink that way, not just one. From a logical point of view all the single-result-row forms of SubLink are about the same, and we should strive to unify them not make them even more different. Also there are a lot of kluges that could be got rid of if subqueries were pulled out of expression trees leaving only Param references behind. We'd need something a bit more general than Param, perhaps, depending on how we wanted to distinguish output values from different subqueries. Right now we don't have to worry about that at parse time, but can leave it to the planner to assign globally unique Param IDs; but that work would have to be pushed further upstream. (BTW, I'm fairly certain your patch won't work for multiple subselects in one UPDATE, because it fails to deal with this point. The test cases in the patch do NOT exercise the case because you don't have multiple multiple-output subselects in any of them.) I thought about trying to fix this stuff but soon concluded that it was more work than I could justify spending on one patch during commit fest. It has to be bounced back for now. regards, tom lane -- Sent via pgsql-patches mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches