Обсуждение: Can I assume there's only one _RETURN rule?
Hi, I'm reworking Bernd's updatable views patch. I haven't found any showstoppers yet, which doesn't mean there aren't any. On the other hand, the functionality is severely limited, per spec, so really the thing is not all that difficult. I'm rewriting several parts of the patch, and have a question. Can I assume that there is only one rule on the CMD_SELECT event, for any given view? This seems awfully obvious to me (i.e. it doesn't make any sense two have two), and I see no way to make things otherwise using SQL commands. Also it seems only rules can have CMD_SELECT rules anyway; is that correct? Something else -- on the rewrite code I see mentioned a lot of times the acronym RIR (like in fireRIRrules); what does RIR stand for? I can't find it anywhere. Thanks. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> writes:
> I'm rewriting several parts of the patch, and have a question. Can I
> assume that there is only one rule on the CMD_SELECT event, for any
> given view?
Yeah. See the restrictions enforced by DefineQueryRewrite() for
details. There's also a unique index on pg_rewrite that would
have answered $SUBJECT for you ;-)
> Something else -- on the rewrite code I see mentioned a lot of times the
> acronym RIR (like in fireRIRrules); what does RIR stand for? I can't
> find it anywhere.
Damifino ... Jan might remember.
regards, tom lane
On Thu, Aug 17, 2006 at 08:52:45PM -0400, Tom Lane wrote: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > Something else -- on the rewrite code I see mentioned a lot of times the > > acronym RIR (like in fireRIRrules); what does RIR stand for? I can't > > find it anywhere. > > Damifino ... Jan might remember. Retrieve-Instead-Retrieve, according to this message from Jan: http://archives.postgresql.org/pgsql-hackers/2003-03/msg01219.php -- Michael Fuhr
Tom Lane wrote: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > I'm rewriting several parts of the patch, and have a question. Can I > > assume that there is only one rule on the CMD_SELECT event, for any > > given view? > > Yeah. See the restrictions enforced by DefineQueryRewrite() for > details. There's also a unique index on pg_rewrite that would > have answered $SUBJECT for you ;-) Doh -- I actually had even noticed there was a syscache for it. But actually, I wanted to know if there can be more than one rule for the CMD_SELECT event, and I see this is enforced in the code rather than restrictions on the catalog. Another question. The patch does a lot of scanning of the range table searching for specific aliases for things -- usually for *NEW* or *OLD*, but also to get the entry for the view we are modifying, etc. This seems pretty fragile -- it breaks if I create a table named *OLD*, for example. Is this acceptable? alvherre=# create table "*OLD*" (a int); CREATE TABLE alvherre=# create view aaa as select * from "*OLD*"; ERROR: could not open relation with OID 0 On the other hand it doesn't break CVS tip at all, i.e. I can perfectly create the view and it works. (I can even call it "*NEW*"). Is this a showstopper? I don't see any other way to walk the RT -- unless I were to change the RTE node representation, I think. Something else that's bothering me is that we mark *NEW* and *OLD* with rtekind RTE_RELATION, but a comment at the top of RangeTblEntry appears to say that they should be marked with RTE_SPECIAL. On the other hand, UpdateRangeTableOfViewParse calls the parser code (addRangeTableEntryForRelation) to insert the RTEs, which forcibly puts RTE_RELATION on them. I tried changing them to RTE_SPECIAL on UpdateRangeTableOfViewParse, but apparently that breaks code somewhere else because I get some regression failures. Do we want to fix that other code, so that *OLD* and *NEW* can always be marked RTE_SPECIAL? -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Another question. The patch does a lot of scanning of the range table
> searching for specific aliases for things -- usually for *NEW* or *OLD*,
> but also to get the entry for the view we are modifying, etc. This
> seems pretty fragile -- it breaks if I create a table named *OLD*, for
> example. Is this acceptable?
The rule stuff already depends on "*NEW*" and "*OLD*" being unique as
table aliases, though IIRC not as actual table names. Maybe it'd be
worth adding something to reject SELECT ... FROM foo "*NEW*" ...
> Is this a showstopper? I don't see any other way to walk the RT --
> unless I were to change the RTE node representation, I think.
The *NEW*/*OLD* stuff is pretty ugly, but it was there already for
many years, and now seems not the time to fix it.
However, I'm not sure you need to be depending on those textual names.
IIRC much of that code depends on rangetable indexes, instead --- look
for references to PRS2_OLD_VARNO and PRS2_NEW_VARNO. That's
considerably safer because the rule creation code can actually guarantee
what's in those rtable positions.
> Something else that's bothering me is that we mark *NEW* and *OLD* with
> rtekind RTE_RELATION, but a comment at the top of RangeTblEntry appears
> to say that they should be marked with RTE_SPECIAL.
I think they have to be RTE_RELATION because they serve in part as OID
references to the table-having-the-rule for dependency purposes. Again,
this is stuff to be cleaning up during development, not three weeks
after feature freeze.
regards, tom lane
Alvaro Herrera wrote: > On the other hand it doesn't break CVS tip at all, i.e. I can perfectly > create the view and it works. (I can even call it "*NEW*"). Actually CVS tip seems a bit broken, alvherre=# \d "*NEW*" Vista «public.*NEW*»Columna | Tipo | Modificadores ---------+---------+---------------a | integer | Definición de vista:SELECT public.old.a FROM "*OLD*"; note the "public.old.a". The culprit seems to be get_target_list, but I see that 8.1 behaves the same (and given your reply I guess it's been like that for a long time.) -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Thu, Aug 17, 2006 at 10:39:14PM -0400, Alvaro Herrera wrote: > Another question. The patch does a lot of scanning of the range table > searching for specific aliases for things -- usually for *NEW* or *OLD*, > but also to get the entry for the view we are modifying, etc. This > seems pretty fragile -- it breaks if I create a table named *OLD*, for > example. Is this acceptable? > > alvherre=# create table "*OLD*" (a int); > CREATE TABLE > alvherre=# create view aaa as select * from "*OLD*"; > ERROR: could not open relation with OID 0 > > On the other hand it doesn't break CVS tip at all, i.e. I can perfectly > create the view and it works. (I can even call it "*NEW*"). If we're going to allow this we should add a regression test for it, since this seems like something that's likely to break. Personally, I'd just say you can't name (or alias) something as "*OLD*" or "*NEW*" and be done with it. -- Jim C. Nasby, Sr. Engineering Consultant jnasby@pervasive.com Pervasive Software http://pervasive.com work: 512-231-6117 vcard: http://jim.nasby.net/pervasive.vcf cell: 512-569-9461