Thread: pg_dump versus views and opclasses

Hey all,

I recently had pg_dump produce a non-restorable dump for one of my
databases.  I can't share the dump itself, but I can describe what
went wrong.

This database had a user-defined composite type in it.  I wanted to be
able to GROUP BY this type, so I added a family of comparison
functions and operators, and created an opclass for btree.  Then I
created a view which grouped on the composite type.  All was well.

Then I tried to create a copy of the database.  Nothing out of the
ordinary, I just did a pg_dump and piped the results into a psql
connecting to a different database name.  The restore failed on
creation of the view; the opclass hadn't been created yet and
therefore the view couldn't figure out how to group on that composite
type.

I had a closer look at the output of pg_dump, and yes indeed, the
opclass and all its operators were listed after the view which needed
them to live.

Then I poked around in the pg_dump source code to get a better feel
for how it orders objects.  That lead me to poke around in pg_depend.
A couple of observations that might be relevant to this case:
* It seems there's no pg_depend entry for
types/functions/operators/opclasses that the view depends on, unless
they are part of the SELECT list.  I don't yet know enough about the
dependency system to understand if this is really a defect (and if so
whether there is a pratical fix for it), but it does seem a bit
limited.  If, for example, you call a user-defined operator in the
WHERE clause of a view, Postgres has no idea about that dependency.
* pg_dump queries the information from pg_depend sorting first by
classid -- the oid of the system catalog the dependent object lives
in, then by the oid of the dependent object.  I ran a similar query in
psql and the results were reminiscent of the order of objects produced
by pg_dump.  pg_opclass and pg_operator have larger oids than
pg_class, and in the absence of dependency information to guide it,
pg_dump output the view (pg_class) first, followed by the opclass.
Coincidence?

Assuming for the moment that I'm on the right track with these
hunches, it seems that pg_dump needs some help.  We could try to make
the dependency information about views more complete, in which case
everything should Just Work in pg_dump.  If that's not practical (and
perhaps even if it is practical) we could also try to give opclasses
and operators a higher default priority in the dump order than views.
This seems logical since a view can depend on an opclass, but an
opclass can't depend on a view.  At least, not directly.

So, am I on to something here, or is it just the ridiculously hot .au
weather talking?

If it would be helpful, I could write up a proper minimal test case to
demonstrate the error.  Let me know.

Cheers,
BJ

"Brendan Jurd" <direvus@gmail.com> writes:
> I recently had pg_dump produce a non-restorable dump for one of my
> databases.  I can't share the dump itself, but I can describe what
> went wrong. ...
>  * It seems there's no pg_depend entry for
> types/functions/operators/opclasses that the view depends on, unless
> they are part of the SELECT list.

What PG version exactly?  We've been moving towards fuller
representation of the semantics in the parse tree over time,
so that's a very relevant question.

FWIW I think this should be pretty much fixed as of CVS HEAD, because
all of the sorting/grouping semantics are now normalized in
SortGroupClauses and find_expr_references() does know about them.
Can you extract a test case from your problem DB so we can verify
nothing got missed?
        regards, tom lane

On Fri, Jan 16, 2009 at 9:01 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> "Brendan Jurd" <direvus@gmail.com> writes:
>>  * It seems there's no pg_depend entry for
>> types/functions/operators/opclasses that the view depends on, unless
>> they are part of the SELECT list.
>
> What PG version exactly?  We've been moving towards fuller
> representation of the semantics in the parse tree over time,
> so that's a very relevant question.
>

This is all on 8.3.3.

> FWIW I think this should be pretty much fixed as of CVS HEAD, because
> all of the sorting/grouping semantics are now normalized in
> SortGroupClauses and find_expr_references() does know about them.
> Can you extract a test case from your problem DB so we can verify
> nothing got missed?

Okay, I'll work on getting a test case together and try it against
HEAD and 8.3.5 for good measure.  I'll post the results here.

Cheers,
BJ

"Brendan Jurd" <direvus@gmail.com> writes:
> On Fri, Jan 16, 2009 at 9:01 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> "Brendan Jurd" <direvus@gmail.com> writes:
>>> * It seems there's no pg_depend entry for
>>> types/functions/operators/opclasses that the view depends on, unless
>>> they are part of the SELECT list.
>> 
>> What PG version exactly?

> This is all on 8.3.3.

Okay.

Looking at it some more, I notice that the SortGroupClause dependencies
are on the individual operators, which probably isn't good enough: the
operator *classes* have to exist or the parser will complain when trying
to make sense of the view.  So that would be a good thing to change for
8.4 (and it's not too late yet).  However --- it's also the case that
pg_dump should dump all operators *and* operator classes before it gets
to views.  So either you were doing something funny with the dump/reload
or else there's a circular dependency in your DB that pg_dump is
breaking in a bad place.  I look forward to the test case ;-)
        regards, tom lane

On Fri, Jan 16, 2009 at 10:23 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Looking at it some more, I notice that the SortGroupClause dependencies
> are on the individual operators, which probably isn't good enough: the
> operator *classes* have to exist or the parser will complain when trying
> to make sense of the view.  So that would be a good thing to change for
> 8.4 (and it's not too late yet).

I haven't come up with a stripped-down test case for this yet, but I
do have some further results to report from dumping and restoring my
database into different versions of postgres.

I wanted to check whether the problem was actually reproducible or
some freak occurence in the original database.  So I loaded my dump
(with the opclass created after the view) into a fresh 8.3.5 database.This failed to create the view as expected.  I
thenmanually created
 
the view, which worked fine because the opclass was now present.  I
then ran pg_dump on the new database and got the same results as
before.  The opclass and its operators were listed *after* the view.
So it's not just some kind of one-off weirdness, there really is a
dependency problem somewhere in here.

I'm not doing anything funny with the dump/reload.  The dump is using
the same pg_dump version as the server, with no command line options,
just the name of the database.  The reload is just piping the contents
of the dump file into psql, again the same version as the server and
no special options.

I tried the same experiment on HEAD and found something very
surprising.  The dump was loaded 100% successfully.  Note that this is
the very same dump, and it does indeed create the objects in the wrong
order.  But for some reason, HEAD doesn't seem to have a problem with
that.

This bodes well for anyone wanting to upgrade a database with such a
dependency glitch from 8.3 to 8.4 ... but I'm confused as to why 8.4
doesn't freak out when told to create a view it doesn't actually know
how to execute.  Have the rules for evaluating views changed?

Note also that after creating the database on HEAD, the pg_dump from
HEAD still outputs the objects in the wrong order.

I will continue working on getting a test case together.

Cheers,
BJ

On Sun, Jan 18, 2009 at 1:41 AM, Brendan Jurd <direvus@gmail.com> wrote:
> ... but I'm confused as to why 8.4
> doesn't freak out when told to create a view it doesn't actually know
> how to execute.  Have the rules for evaluating views changed?
>

Update.  Turns out that 8.4 DOES know how to execute the view.  If you
try to group on a user-defined composite type, 8.4 just goes ahead and
groups it, rather than giving the old "could not identify an ordering
operator" error.

Which renders the whole idea of creating my own opclass for the type
very much bunk.

What sorcery is this?  Does 8.4 add some kind of default ordering
strategy for composite types?

Cheers,
BJ

"Brendan Jurd" <direvus@gmail.com> writes:
> Update.  Turns out that 8.4 DOES know how to execute the view.  If you
> try to group on a user-defined composite type, 8.4 just goes ahead and
> groups it, rather than giving the old "could not identify an ordering
> operator" error.

Is there a hash opclass for the type?  8.4 can group types that have
hash but not btree opclasses, but prior versions insisted on btree.
        regards, tom lane

On Sun, Jan 18, 2009 at 2:52 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> "Brendan Jurd" <direvus@gmail.com> writes:
>> Update.  Turns out that 8.4 DOES know how to execute the view.  If you
>> try to group on a user-defined composite type, 8.4 just goes ahead and
>> groups it, rather than giving the old "could not identify an ordering
>> operator" error.
>
> Is there a hash opclass for the type?  8.4 can group types that have
> hash but not btree opclasses, but prior versions insisted on btree.

Well I sure didn't create one.  I've only been attempting to create a
btree opclass.  Is there some kind of default hash opclass for
composites?

Cheers,
BJ

"Brendan Jurd" <direvus@gmail.com> writes:
> On Sun, Jan 18, 2009 at 2:52 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Is there a hash opclass for the type?  8.4 can group types that have
>> hash but not btree opclasses, but prior versions insisted on btree.

> Well I sure didn't create one.  I've only been attempting to create a
> btree opclass.  Is there some kind of default hash opclass for
> composites?

No ... but after poking around in the system catalogs I notice there
*is* a default btree opclass for composites, as of 8.4 ...
        regards, tom lane

On Fri, Jan 16, 2009 at 10:23 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> However --- it's also the case that
> pg_dump should dump all operators *and* operator classes before it gets
> to views.  So either you were doing something funny with the dump/reload
> or else there's a circular dependency in your DB that pg_dump is
> breaking in a bad place.  I look forward to the test case ;-)

Okay, here's the test case.  The attached file
test-view-opclass-deps.sql creates a simple database with a
user-defined composite type "comp", which consists of two integers
called "a" and "b".

It then puts together a simple btree opclass for that that type and
fills a table "stuff" with some generated values for the type.

It then creates a view "group_stuff" which groups on the type.

If you dump out the database, the view will be listed after the type,
but *before* the opclass and all its component operators and
underlying functions.  I've attached a copy of the faulty dump file
for reference.

If you try to load that dump, the view won't be created.

To reproduce the error on 8.3.5:

$ psql -f test-view-opclass-deps.sql postgres
$ psql -c "create database test_view_opclass_deps2;" postgres
$ pg_dump test_view_opclass_deps | psql test_view_opclass_deps2

On HEAD, the dump is still screwy, but in a slightly different way.
It lists the type first, followed by the = and <= operators, then the
table and the view, and finally the remainder of the operators and the
opclass.

If you load the dump into an 8.4 server, the view gets created without
error because 8.4 has a default btree opclass, so the order of objects
in the dump isn't important -- at least not with regard to this
particular scenario.  But I still find it a bit disturbing that the
order of objects in the 8.4 dump is so bizarre.  Why the special
treatment for = and <=?

Cheers,
BJ

"Brendan Jurd" <direvus@gmail.com> writes:
> On Fri, Jan 16, 2009 at 10:23 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> However --- it's also the case that
>> pg_dump should dump all operators *and* operator classes before it gets
>> to views.  So either you were doing something funny with the dump/reload
>> or else there's a circular dependency in your DB that pg_dump is
>> breaking in a bad place.  I look forward to the test case ;-)

> Okay, here's the test case.

Interesting --- it works as expected in 8.2.  The problem seems to have
been created by the introduction of arrays over composites in 8.3.
Although those array types don't get dumped separately, they still have
to participate in pg_dump's dependency sort (since they might form part
of dependency arcs).  pg_dump initially sorts them as if they are
ordinary types, which puts them pretty high up in the dependency order,
and then the tables or composite types that they depend on also get
pushed forward in the ordering.  (The topological sort for dependencies
basically causes items to get pushed forward to before anything that
depends on them; nothing ever moves backward.)

I'm surprised nobody noticed before that 8.3 is emitting considerably
different object ordering from 8.2.  In particular this mostly destroys
the intended behavior that operators and operator classes come out
before table definitions.

The right fix seems to be to treat auto-generated array types the same
way pg_dump already treats auto-generated table types, ie, mark them
with a different DumpableObject type code that causes them to sort much
lower in the initial ordering.  Fortunately, this is also a
back-patchable fix, since it's entirely internal to pg_dump.  I've got
some social obligations today but I'll have a patch in a day or so.

I'm thinking that adding operator family info to SortGroupClause in
HEAD might be a good thing too, but it is for a different issue than
this --- it's to prevent you dropping a family that a view has a
semantic dependency on.  However there might be room for a bit of
argument there: if there's both a btree and a hash opfamily then
the view could use either, so preventing you dropping one would be
a bit arbitrary.  In any case it wouldn't stop you doing an ALTER
OPERATOR FAMILY to remove the sort operators from the family, so
maybe there's not much point here.  Thoughts?
        regards, tom lane

On Sun, Jan 18, 2009 at 5:56 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Interesting --- it works as expected in 8.2.  The problem seems to have
> been created by the introduction of arrays over composites in 8.3.

Congratulations on identifying the source!

Does this also explain the extra-weird output from HEAD, with the <=
and = operators sorting earlier than the view, but the rest of the
operators sorting after it?

> I'm surprised nobody noticed before that 8.3 is emitting considerably
> different object ordering from 8.2.

Well I can't speak for anybody else, but I have noticed changes to the
dump order in the past, but I dismissed it as something that Just
Happens in a major release and got on with my life =).  I've long
since accepted that you can't expect dumps to be directly comparable
over the long term.

Plus, the difference in object ordering only manifests with composite
types, and that's probably something that only a slim portion of users
are actually playing with.

> In particular this mostly destroys
> the intended behavior that operators and operator classes come out
> before table definitions.

Yeah, that seems like a generally sane intention.

> The right fix seems to be to treat auto-generated array types the same
> way pg_dump already treats auto-generated table types, ie, mark them
> with a different DumpableObject type code that causes them to sort much
> lower in the initial ordering.

Sounds good.

> I'm thinking that adding operator family info to SortGroupClause in
> HEAD might be a good thing too, but it is for a different issue than
> this --- it's to prevent you dropping a family that a view has a
> semantic dependency on.  However there might be room for a bit of
> argument there: if there's both a btree and a hash opfamily then
> the view could use either, so preventing you dropping one would be
> a bit arbitrary.

Not to mention that, with the new default btree opclass in 8.4 that I
stumbled across earlier, dropping an opclass that a view uses may not
necessarily murder the view ... it may simply cause it to fall back to
the default opclass.  If falling back to the default opclass was
actually what the user wanted to achieve by dropping the user-defined
opclass, a dependency such as you describe might leave him with no way
to do that.

Worst case scenario is that you drop an opclass which causes a view to
die, and next time you try to run the view you get an intelligible
error message telling you why it doesn't work anymore.  That's
probably not the end of the world.

> In any case it wouldn't stop you doing an ALTER
> OPERATOR FAMILY to remove the sort operators from the family, so
> maybe there's not much point here.  Thoughts?

Honestly, I feel that messing around with operator classes is
something that doesn't happen with any great frequency, and you need
to be careful and hands-on when you're doing it anyway.  I could live
without the dependency.

Cheers,
BJ

"Brendan Jurd" <direvus@gmail.com> writes:
> Does this also explain the extra-weird output from HEAD, with the <=
> and = operators sorting earlier than the view, but the rest of the
> operators sorting after it?

No, that comes from the fact that we have dependencies on those two
operators (but not the whole opclass) in SortGroupClause.  So they
get forced to come out before the view, but the rest do not.
        regards, tom lane

"Brendan Jurd" <direvus@gmail.com> writes:
> On Sun, Jan 18, 2009 at 5:56 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Interesting --- it works as expected in 8.2.  The problem seems to have
>> been created by the introduction of arrays over composites in 8.3.

> Congratulations on identifying the source!

I've applied a patch for this to HEAD and 8.3.

>> I'm thinking that adding operator family info to SortGroupClause in
>> HEAD might be a good thing too, but it is for a different issue than
>> this --- it's to prevent you dropping a family that a view has a
>> semantic dependency on.  However there might be room for a bit of
>> argument there: if there's both a btree and a hash opfamily then
>> the view could use either, so preventing you dropping one would be
>> a bit arbitrary.

> Not to mention that, with the new default btree opclass in 8.4 that I
> stumbled across earlier, dropping an opclass that a view uses may not
> necessarily murder the view ... it may simply cause it to fall back to
> the default opclass.  If falling back to the default opclass was
> actually what the user wanted to achieve by dropping the user-defined
> opclass, a dependency such as you describe might leave him with no way
> to do that.

> Worst case scenario is that you drop an opclass which causes a view to
> die, and next time you try to run the view you get an intelligible
> error message telling you why it doesn't work anymore.  That's
> probably not the end of the world.

True.  I'll leave well enough alone for the moment, unless someone
comes up with additional arguments.
        regards, tom lane

On Mon, Jan 19, 2009 at 7:47 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I've applied a patch for this to HEAD and 8.3.
>

Cool, thanks Tom.

Just noting that I've tested your fix on both branches, and in both
cases pg_dump's output came out nice, clean and consistent.

Cheers,
BJ