BIG grant problem

Hi all

Minor detail, but when I did 'pg_dump -z -f dump.file dbname' and then
went to restore it, I found that the grant statments are like:
GRANT ALL on "tablename" to "tablename";
instead of
GRANT ALL on "tablename" to "username";

I used vim to edit the dump file, so am ok.
Just thought you might want to know.
Oh, ya, that is the release version 6.4, just downloaded this afternoon.

Yikes, confirmed here. We need to know how this got into the tree
without showing up on our tests, are there any more pg_dump bugs we have
not found, and a fix.

This is our first major 6.4 bug, aside from large objects!
Congradulations.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Bruce Momjian <maillist@candle.pha.pa.us> writes:

Terry Mackintosh wrote:

Minor detail, but when I did 'pg_dump -z -f dump.file dbname' and then
went to restore it, I found that the grant statments are like:
GRANT ALL on "tablename" to "tablename";
instead of
GRANT ALL on "tablename" to "username";

Yikes, confirmed here. We need to know how this got into the tree
without showing up on our tests,

Well, that's easy --- there are no regression tests that test pg_dump
at all, nor any that test multiple table owners and permissions.

FWIW, pg_dump -z works correctly for GRANT to PUBLIC --- otherwise
I would've noticed some time ago. But I hadn't had occasion
to check granting permission to specific users :-( ... and I don't
think most of the rest of the developers work with databases that
even have multiple users, let alone put access restrictions on
individual tables.

It's certainly true that pg_dump is pretty weak in the area of
table ownerships and permissions. We have fixed several bugs
in that area since 6.3.2, and I'm not particularly surprised to
hear of another one. We need someone who actually has occasion
to work with access-restricted databases to pound on pg_dump for
a while and flush out the bugs. (Terry, can you volunteer?)
I don't think the bugs will be hard to fix, it's just a matter
of not having done enough testing.

regards, tom lane

I said:

I don't think the bugs will be hard to fix,

And, indeed, this bug is pretty trivial. Fix will be applied
momentarily.

regards, tom lane

Hi

On Sun, 15 Nov 1998, Tom Lane wrote:

hear of another one. We need someone who actually has occasion
to work with access-restricted databases to pound on pg_dump for
a while and flush out the bugs. (Terry, can you volunteer?)

Sure.

I don't think the bugs will be hard to fix, it's just a matter
of not having done enough testing.

I started to look at it last night, found where the line is created in the
pg_dump.c file, looks ok there, so the problem is a little deeper then
just an easy typo.

Should be able to look at it today.

Have a great day
Terry Mackintosh <terry@terrym.com> http://www.terrym.com
sysadmin/owner Please! No MIME encoded or HTML mail, unless needed.

Proudly powered by R H Linux 4.2, Apache 1.3, PHP 3, PostgreSQL 6.3
-------------------------------------------------------------------
Success Is A Choice ... book by Rick Patino, get it, read it!

Hi Tom

On Sun, 15 Nov 1998, Tom Lane wrote:

I said:

I don't think the bugs will be hard to fix,

And, indeed, this bug is pretty trivial. Fix will be applied
momentarily.

regards, tom lane

Oh good, will there be a loose patch? so I and others don't have to
download the whole source again?

Thanks
Terry Mackintosh <terry@terrym.com> http://www.terrym.com
sysadmin/owner Please! No MIME encoded or HTML mail, unless needed.

Proudly powered by R H Linux 4.2, Apache 1.3, PHP 3, PostgreSQL 6.3
-------------------------------------------------------------------
Success Is A Choice ... book by Rick Patino, get it, read it!

Hi Tom

On Sun, 15 Nov 1998, Tom Lane wrote:

I said:

I don't think the bugs will be hard to fix,

And, indeed, this bug is pretty trivial. Fix will be applied
momentarily.

regards, tom lane

Oh good, will there be a loose patch? so I and others don't have to
download the whole source again?

I think I can say it will be in 6.4,1, which should be released in a
week or so. Hopefully, we can generate a 6.4.1, and a 6.4_to_6.4.1
patch.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Terry Mackintosh <terry@terrym.com> writes:

Oh good, will there be a loose patch? so I and others don't have to
download the whole source again?

If you need it here's the patch. (I already checked this into both
6.4 and 6.5 trees.) I found a second place that had the same problem,
btw. Calling fmtID() twice in one expression is no good 'cuz it uses
a static result area...

regards, tom lane

*** pg_dump.c.orig	Fri Nov  6 10:56:42 1998
--- pg_dump.c	Sun Nov 15 02:11:29 1998
***************
*** 2563,2577 ****
  	{
  		if (ACLlist[k].privledges != (char *) NULL)
  		{
  			if (ACLlist[k].user == (char *) NULL)
! 				fprintf(fout,
! 						"GRANT %s on %s to PUBLIC;\n",
! 						ACLlist[k].privledges, fmtId(tbinfo.relname));
  			else
! 				fprintf(fout,
! 						"GRANT %s on %s to %s;\n",
! 						ACLlist[k].privledges, fmtId(tbinfo.relname),
! 						fmtId(ACLlist[k].user));
  		}
  	}
  }
--- 2563,2578 ----
  	{
  		if (ACLlist[k].privledges != (char *) NULL)
  		{
+ 			/* If you change this code, bear in mind fmtId() can be
+ 			 * used only once per printf() call...
+ 			 */
+ 			fprintf(fout,
+ 					"GRANT %s on %s to ",
+ 					ACLlist[k].privledges, fmtId(tbinfo.relname));
  			if (ACLlist[k].user == (char *) NULL)
! 				fprintf(fout, "PUBLIC;\n");
  			else
! 				fprintf(fout, "%s;\n", fmtId(ACLlist[k].user));
  		}
  	}
  }
***************
*** 2851,2873 ****

strcpy(id1, fmtId(indinfo[i].indexrelname));
strcpy(id2, fmtId(indinfo[i].indrelname));
! sprintf(q, "CREATE %s INDEX %s on %s using %s (",
(strcmp(indinfo[i].indisunique, "t") == 0) ? "UNIQUE" : "",
id1,
id2,
indinfo[i].indamname);
if (funcname)
{
! sprintf(q, "%s %s (%s) %s );\n",
! q, fmtId(funcname), attlist, fmtId(classname[0]));
free(funcname);
free(classname[0]);
}
else
! sprintf(q, "%s %s );\n",
! q, attlist);
!
! fputs(q, fout);
}
}

--- 2852,2872 ----

strcpy(id1, fmtId(indinfo[i].indexrelname));
strcpy(id2, fmtId(indinfo[i].indrelname));
! fprintf(fout, "CREATE %s INDEX %s on %s using %s (",
(strcmp(indinfo[i].indisunique, "t") == 0) ? "UNIQUE" : "",
id1,
id2,
indinfo[i].indamname);
if (funcname)
{
! /* need 2 printf's here cuz fmtId has static return area */
! fprintf(fout, " %s", fmtId(funcname));
! fprintf(fout, " (%s) %s );\n", attlist, fmtId(classname[0]));
free(funcname);
free(classname[0]);
}
else
! fprintf(fout, " %s );\n", attlist);
}
}