Re: Re: [BUGS] BUG #7873: pg_restore --clean tries to drop tables that don't exist

Hello

2014/1/22 Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>

> On 21 January 2014 22:28, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:
> > I have been mulling over this patch, and I can't seem to come to terms
> > with it. I first started making it look nicer here and there, thinking
> > it was all mostly okay, but eventually arrived at the idea that it seems
> > wrong to do what this does: basically, get_object_address() tries to
> > obtain an object address, and if that fails, return InvalidOid; then, in
> > RemoveObjects, we try rather hard to figure out why that failed, and
> > construct an error message.
> >
> > It seems to me that it would make more sense to have get_object_address
> > somehow return a code indicating what failed; then we don't have to go
> > all over through the parser code once more. Perhaps, for example, when
> > missing_ok is given as true to get_object_address it also needs to get a
> > pointer to ObjectType and a string; if some object does not exist then
> > fill the ObjectType with the failing object and the string with the
> > failing name. Then RemoveObjects can construct a string more easily.
> > Not sure how workable this exact idea is; maybe there is a better way.
> >
>
> Yeah, when I initially started reviewing this patch I had a very
> similar thought. But when I looked more deeply at the code beneath
> get_object_address, I started to doubt whether it could be done
> without rather extensive changes all over the place. Also
> get_object_address is itself called from a lot of places (not
> necessarily all in our code) and all the other places (in our code, at
> least) pass missing_ok=false. So it seemed rather ugly to change its
> signature and force a matching change in all those other places, which
> actually don't care about missing objects. Perhaps the answer would be
> to have a separate get_object_address_if_exists function, and remove
> the missing_ok flag from get_object_address, but that all felt like a
> much larger patch.
>
> In the end, I felt that Pavel's approach wasn't adding that much new
> code, and it's all localised in the one place that does actually
> tolerate missing objects.
>

I though about it too. But I didn't continue - reasons was named by Dean -
and RemoveObjects are not difficult code - lot of code is mechanical - and
it is not on critical path.

Regards

Pavel

>
> I admit though, that I didn't explore the other approach very deeply,
> so perhaps it might fall out more neatly than I feared.
>
> Regards,
> Dean
>