Re: Updatable views

--On Mittwoch, August 30, 2006 12:01:25 -0400 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
wrote:

> Bernd Helmle <mailings(at)oopsware(dot)de> 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