Обсуждение: Rethinking pg_dump's function sorting code

Поиск
Список
Период
Сортировка

Rethinking pg_dump's function sorting code

От
Tom Lane
Дата:
In bug #12832 Marko Tiikkaja points out that commit
7b583b20b1c95acb621c71251150beef958bb603 created a rather unnecessary
dump failure hazard, since it applies pg_get_function_identity_arguments()
to every function in the database, even those that won't get dumped.
I think we should fix this by getting rid of pg_dump's use of that
function altogether.  A low-tech way to sort functions of identical names
would be to compare argument type OIDs, as in the attached simple patch.
If people feel it's important to avoid depending on numerical OID order,
we could instead look up type names locally and compare them as in the
attached less-simple patch.  (Both patches neglect reverting the data
collection aspects of the prior commit, since that's mechanical; the only
interesting part is what we'll do to sort.)

Neither patch will exactly preserve the sort behavior of the current
code, but I don't think that's important.

Comments?

            regards, tom lane

diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index 4b9bba0..816c9f0 100644
*** a/src/bin/pg_dump/pg_dump_sort.c
--- b/src/bin/pg_dump/pg_dump_sort.c
*************** DOTypeNameCompare(const void *p1, const
*** 291,303 ****
      {
          FuncInfo   *fobj1 = *(FuncInfo *const *) p1;
          FuncInfo   *fobj2 = *(FuncInfo *const *) p2;

          cmpval = fobj1->nargs - fobj2->nargs;
          if (cmpval != 0)
              return cmpval;
!         cmpval = strcmp(fobj1->proiargs, fobj2->proiargs);
!         if (cmpval != 0)
!             return cmpval;
      }
      else if (obj1->objType == DO_OPERATOR)
      {
--- 291,307 ----
      {
          FuncInfo   *fobj1 = *(FuncInfo *const *) p1;
          FuncInfo   *fobj2 = *(FuncInfo *const *) p2;
+         int            i;

          cmpval = fobj1->nargs - fobj2->nargs;
          if (cmpval != 0)
              return cmpval;
!         for (i = 0; i < fobj1->nargs; i++)
!         {
!             cmpval = oidcmp(fobj1->argtypes[i], fobj2->argtypes[i]);
!             if (cmpval != 0)
!                 return cmpval;
!         }
      }
      else if (obj1->objType == DO_OPERATOR)
      {
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index 4b9bba0..de9407c 100644
*** a/src/bin/pg_dump/pg_dump_sort.c
--- b/src/bin/pg_dump/pg_dump_sort.c
*************** DOTypeNameCompare(const void *p1, const
*** 291,303 ****
      {
          FuncInfo   *fobj1 = *(FuncInfo *const *) p1;
          FuncInfo   *fobj2 = *(FuncInfo *const *) p2;

          cmpval = fobj1->nargs - fobj2->nargs;
          if (cmpval != 0)
              return cmpval;
!         cmpval = strcmp(fobj1->proiargs, fobj2->proiargs);
!         if (cmpval != 0)
!             return cmpval;
      }
      else if (obj1->objType == DO_OPERATOR)
      {
--- 291,313 ----
      {
          FuncInfo   *fobj1 = *(FuncInfo *const *) p1;
          FuncInfo   *fobj2 = *(FuncInfo *const *) p2;
+         int            i;

          cmpval = fobj1->nargs - fobj2->nargs;
          if (cmpval != 0)
              return cmpval;
!         for (i = 0; i < fobj1->nargs; i++)
!         {
!             TypeInfo   *argtype1 = findTypeByOid(fobj1->argtypes[i]);
!             TypeInfo   *argtype2 = findTypeByOid(fobj2->argtypes[i]);
!
!             if (argtype1 && argtype2)
!             {
!                 cmpval = strcmp(argtype1->dobj.name, argtype2->dobj.name);
!                 if (cmpval != 0)
!                     return cmpval;
!             }
!         }
      }
      else if (obj1->objType == DO_OPERATOR)
      {

Re: Rethinking pg_dump's function sorting code

От
Marko Tiikkaja
Дата:
On 2015-03-06 01:28, Tom Lane wrote:
> In bug #12832 Marko Tiikkaja points out that commit
> 7b583b20b1c95acb621c71251150beef958bb603 created a rather unnecessary
> dump failure hazard, since it applies pg_get_function_identity_arguments()
> to every function in the database, even those that won't get dumped.
> I think we should fix this by getting rid of pg_dump's use of that
> function altogether.  A low-tech way to sort functions of identical names
> would be to compare argument type OIDs, as in the attached simple patch.
> If people feel it's important to avoid depending on numerical OID order,
> we could instead look up type names locally and compare them as in the
> attached less-simple patch.  (Both patches neglect reverting the data
> collection aspects of the prior commit, since that's mechanical; the only
> interesting part is what we'll do to sort.)
>
> Neither patch will exactly preserve the sort behavior of the current
> code, but I don't think that's important.
>
> Comments?

I have my own cow in this ditch, but I would much prefer the sort to be 
done based on the type name.  That way the order is still consistent 
between two databases where the objects were created in a different order.


.m

Re: Rethinking pg_dump's function sorting code

От
Noah Misch
Дата:
On Thu, Mar 05, 2015 at 07:28:33PM -0500, Tom Lane wrote:
> In bug #12832 Marko Tiikkaja points out that commit
> 7b583b20b1c95acb621c71251150beef958bb603 created a rather unnecessary
> dump failure hazard, since it applies pg_get_function_identity_arguments()
> to every function in the database, even those that won't get dumped.
> I think we should fix this by getting rid of pg_dump's use of that
> function altogether.  A low-tech way to sort functions of identical names
> would be to compare argument type OIDs, as in the attached simple patch.
> If people feel it's important to avoid depending on numerical OID order,
> we could instead look up type names locally and compare them as in the
> attached less-simple patch.

Comparing argument type names sounds fine.  Comparing argument type OID does
not offer enough to justify the loss of cross-cluster sort equivalence.

> Neither patch will exactly preserve the sort behavior of the current
> code, but I don't think that's important.

Agreed.

> --- 291,313 ----
>       {
>           FuncInfo   *fobj1 = *(FuncInfo *const *) p1;
>           FuncInfo   *fobj2 = *(FuncInfo *const *) p2;
> +         int            i;
>   
>           cmpval = fobj1->nargs - fobj2->nargs;
>           if (cmpval != 0)
>               return cmpval;
> !         for (i = 0; i < fobj1->nargs; i++)
> !         {
> !             TypeInfo   *argtype1 = findTypeByOid(fobj1->argtypes[i]);
> !             TypeInfo   *argtype2 = findTypeByOid(fobj2->argtypes[i]);
> ! 
> !             if (argtype1 && argtype2)
> !             {
> !                 cmpval = strcmp(argtype1->dobj.name, argtype2->dobj.name);
> !                 if (cmpval != 0)
> !                     return cmpval;
> !             }
> !         }

So as to stably compare f(nsp0.t) to f(nsp1.t), this should also compare the
dobj.namespace name.

Re: Rethinking pg_dump's function sorting code

От
Tom Lane
Дата:
Noah Misch <noah@leadboat.com> writes:
> Comparing argument type names sounds fine.  Comparing argument type OID does
> not offer enough to justify the loss of cross-cluster sort equivalence.

Fair enough.

> So as to stably compare f(nsp0.t) to f(nsp1.t), this should also compare the
> dobj.namespace name.

Ah, good point.  Will fix.
        regards, tom lane