Re: [HACKERS] pg_dump LOCK TABLE ONLY question

[Michael Paquier]

On Sat, Jan 2, 2016 at 12:28 PM, Noah Misch  wrote:
> On Sat, Oct 31, 2015 at 10:14:18AM +0100, Simon Riggs wrote:
>> I agree with Filip that this is a bug. pg_dump clearly doesn't work
>> correctly with inheritance.
>>
>> If I run this command
>>
>>   pg_dump -t tab1
>>
>> then I get a dump of "tab1".  No data is included from tables that inherit
>> tab1 because COPY refers only to the target table.
>>
>> Why should that action cause a lock to be taken on another table that
>> inherits from tab1?
>>
>> It seems clear that the user is requesting an action ONLY on tab1, so we
>> should use LOCK TABLE tab1 ONLY;
>
> +1

Hm. Looking one extra time at that, the patch upthread should as well
do the same for the parallel mode introduced in 9.3 as data is always
bumped using FROM ONLY. See for example the attached..
-- 
Michael
diff --git a/src/bin/pg_dump/parallel.c b/src/bin/pg_dump/parallel.c
index ff823e5..25d18ba 100644
--- a/src/bin/pg_dump/parallel.c
+++ b/src/bin/pg_dump/parallel.c
@@ -834,8 +834,12 @@ lockTableNoWait(ArchiveHandle *AH, TocEntry *te)
 			PQgetvalue(res, 0, 0),
 			PQgetvalue(res, 0, 1));
 
-	appendPQExpBuffer(query, "LOCK TABLE %s IN ACCESS SHARE MODE NOWAIT",
-	  qualId);
+	if (AHX->remoteVersion >= 80400)
+		appendPQExpBuffer(query, "LOCK TABLE ONLY %s IN ACCESS SHARE MODE NOWAIT",
+		  qualId);
+	else
+		appendPQExpBuffer(query, "LOCK TABLE %s IN ACCESS SHARE MODE NOWAIT",
+		  qualId);
 	PQclear(res);
 
 	res = PQexec(AH->connection, query->data);
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 36863df..755046f 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -5169,8 +5169,8 @@ getTables(Archive *fout, DumpOptions *dopt, int *numTables)
 		 * Read-lock target tables to make sure they aren't DROPPED or altered
 		 * in schema before we get around to dumping them.
 		 *
-		 * Note that we don't explicitly lock parents of the target tables; we
-		 * assume our lock on the child is enough to prevent schema
+		 * Note that we don't explicitly lock parents or children of the target
+		 * tables; we assume our lock on the child is enough to prevent schema
 		 * alterations to parent tables.
 		 *
 		 * NOTE: it'd be kinda nice to lock other relations too, not only
@@ -5179,11 +5179,18 @@ getTables(Archive *fout, DumpOptions *dopt, int *numTables)
 		if (tblinfo[i].dobj.dump && tblinfo[i].relkind == RELKIND_RELATION)
 		{
 			resetPQExpBuffer(query);
-			appendPQExpBuffer(query,
-			  "LOCK TABLE %s IN ACCESS SHARE MODE",
-			  fmtQualifiedId(fout->remoteVersion,
-		tblinfo[i].dobj.namespace->dobj.name,
-			 tblinfo[i].dobj.name));
+			if (fout->remoteVersion >= 80400)
+appendPQExpBuffer(query,
+  "LOCK TABLE ONLY %s IN ACCESS SHARE MODE",
+  fmtQualifiedId(fout->remoteVersion,
+			tblinfo[i].dobj.namespace->dobj.name,
+ tblinfo[i].dobj.name));
+			else
+appendPQExpBuffer(query,
+  "LOCK TABLE %s IN ACCESS SHARE MODE",
+  fmtQualifiedId(fout->remoteVersion,
+			tblinfo[i].dobj.namespace->dobj.name,
+ tblinfo[i].dobj.name));
 			ExecuteSqlStatement(fout, query->data);
 		}
 

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_dump LOCK TABLE ONLY question

[Noah Misch]

On Sat, Oct 31, 2015 at 10:14:18AM +0100, Simon Riggs wrote:
> I agree with Filip that this is a bug. pg_dump clearly doesn't work
> correctly with inheritance.
> 
> If I run this command
> 
>   pg_dump -t tab1
> 
> then I get a dump of "tab1".  No data is included from tables that inherit
> tab1 because COPY refers only to the target table.
> 
> Why should that action cause a lock to be taken on another table that
> inherits from tab1?
> 
> It seems clear that the user is requesting an action ONLY on tab1, so we
> should use LOCK TABLE tab1 ONLY;

+1


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_dump LOCK TABLE ONLY question

[Simon Riggs]

On 2 October 2015 at 01:19, Michael Paquier 
wrote:

> On Thu, Oct 1, 2015 at 10:43 PM, Filip Rembiałkowski
>  wrote:
> > I just want to understand why there is LOCK TABLE not LOCK TABLE ONLY.
>
> It seems to me that you'd still want to use LOCK TABLE particularly if
> the dump is only done on a subset of tables, using --table for
> example.
>

I agree with Filip that this is a bug. pg_dump clearly doesn't work
correctly with inheritance.

If I run this command

  pg_dump -t tab1

then I get a dump of "tab1".  No data is included from tables that inherit
tab1 because COPY refers only to the target table.

Why should that action cause a lock to be taken on another table that
inherits from tab1?

It seems clear that the user is requesting an action ONLY on tab1, so we
should use LOCK TABLE tab1 ONLY;

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [HACKERS] pg_dump LOCK TABLE ONLY question

[Filip Rembiałkowski]

Please take it as a very naive and basic approach :-)

What could go wrong here?





diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 36863df..57a50b5 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -5169,9 +5169,9 @@ getTables(Archive *fout, DumpOptions *dopt, int
*numTables)
 * Read-lock target tables to make sure they aren't DROPPED or altered
 * in schema before we get around to dumping them.
 *
-* Note that we don't explicitly lock parents of the target tables; we
-* assume our lock on the child is enough to prevent schema
-* alterations to parent tables.
+* Note that we don't explicitly lock neither parents nor children of
+* the target tables; we assume our lock on the child is enough to
+* prevent schema alterations to parent tables.
 *
 * NOTE: it'd be kinda nice to lock other relations too, not only
 * plain tables, but the backend doesn't presently allow that.
@@ -5179,11 +5179,18 @@ getTables(Archive *fout, DumpOptions *dopt,
int *numTables)
if (tblinfo[i].dobj.dump && tblinfo[i].relkind == RELKIND_RELATION)
{
resetPQExpBuffer(query);
-   appendPQExpBuffer(query,
- "LOCK TABLE %s IN ACCESS SHARE MODE",
- fmtQualifiedId(fout->remoteVersion,
-   tblinfo[i].dobj.namespace->dobj.name,
-tblinfo[i].dobj.name));
+   if (fout->remoteVersion >= 80400)
+   appendPQExpBuffer(query,
+   "LOCK TABLE ONLY %s IN ACCESS SHARE MODE",
+   fmtQualifiedId(fout->remoteVersion,
+
tblinfo[i].dobj.namespace->dobj.name,
+   tblinfo[i].dobj.name));
+   else
+   appendPQExpBuffer(query,
+   "LOCK TABLE %s IN ACCESS SHARE MODE",
+   fmtQualifiedId(fout->remoteVersion,
+
tblinfo[i].dobj.namespace->dobj.name,
+   tblinfo[i].dobj.name));
ExecuteSqlStatement(fout, query->data);
}





On Fri, Oct 16, 2015 at 5:06 PM, Robert Haas  wrote:
> On Thu, Oct 15, 2015 at 9:13 PM, Jim Nasby  wrote:
>> OTOH, now that the catalog is MVCC capable, do we even still need to lock
>> the objects for a schema-only dump?
>
> Yes.  The MVCC snapshots used for catalog reads are stable only for
> the duration of one particular catalog read.  We're not using the
> transaction snapshot.
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_dump LOCK TABLE ONLY question

[Robert Haas]

On Thu, Oct 15, 2015 at 9:13 PM, Jim Nasby  wrote:
> OTOH, now that the catalog is MVCC capable, do we even still need to lock
> the objects for a schema-only dump?

Yes.  The MVCC snapshots used for catalog reads are stable only for
the duration of one particular catalog read.  We're not using the
transaction snapshot.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_dump LOCK TABLE ONLY question

[Jim Nasby]

On 10/7/15 6:44 AM, Filip Rembiałkowski wrote:

Oct 2 2015 01:19 "Michael Paquier" > wrote:
 >
 > On Thu, Oct 1, 2015 at 10:43 PM, Filip Rembiałkowski
>
wrote:
 > > I just want to understand why there is LOCK TABLE not LOCK TABLE ONLY.
 >
 > It seems to me that you'd still want to use LOCK TABLE particularly if
 > the dump is only done on a subset of tables, using --table for
 > example.

Right. But please consider this use case, when I have to dunp only given
schema, nothing more and nothing less.

Is --schema option not just for that?

Locking child tables seems a bit counter-intuitive.

COPY does not touch child tables, also.


I agree this seems unnecessary.

OTOH, now that the catalog is MVCC capable, do we even still need to 
lock the objects for a schema-only dump?

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_dump LOCK TABLE ONLY question

[Filip Rembiałkowski]

Oct 2 2015 01:19 "Michael Paquier"  wrote:
>
> On Thu, Oct 1, 2015 at 10:43 PM, Filip Rembiałkowski  <
filip.rembialkow...@gmail.com> wrote:
> > I just want to understand why there is LOCK TABLE not LOCK TABLE ONLY.
>
> It seems to me that you'd still want to use LOCK TABLE particularly if
> the dump is only done on a subset of tables, using --table for
> example.

Right. But please consider this use case, when I have to dunp only given
schema, nothing more and nothing less.

Is --schema option not just for that?

Locking child tables seems a bit counter-intuitive.

COPY does not touch child tables, also.

Re: [HACKERS] pg_dump LOCK TABLE ONLY question

[Filip Rembiałkowski]

(sorry I lost the original thread somehow)

tgl wrote:

> Filip wrote:

> > I'm running pg_dump constrained to one schema. It appears that pg_dump runs
> > "LOCK TABLE %s IN ACCESS SHARE MODE" for each table.
> > Naturally it makes sense, but...

> > This schema has a table that serves as parent for thousands of child
> > tables (via INHERITS).

> > So effectively, to dump this schema, I have to LOCK all these tables
> > not only parent.

> They'd all end up locked anyway wouldn't they?

I would like to dump the whole schema in ONLY mode, including table
data for only that schema, excluding data for child tables in other
schemas.

Why would they be locked then?

Which part of pg_dump requires locking child tables?

Per the docs, "COPY only deals with the specific table named; it does
not copy data to or from child tables. "


I just want to understand why there is LOCK TABLE not LOCK TABLE ONLY.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_dump LOCK TABLE ONLY question

[Michael Paquier]

On Thu, Oct 1, 2015 at 10:43 PM, Filip Rembiałkowski
 wrote:
> I just want to understand why there is LOCK TABLE not LOCK TABLE ONLY.

It seems to me that you'd still want to use LOCK TABLE particularly if
the dump is only done on a subset of tables, using --table for
example.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_dump LOCK TABLE ONLY question

[Tom Lane]

=?UTF-8?Q?Filip_Rembia=C5=82kowski?=  writes:
> I'm running pg_dump constrained to one schema. It appears that pg_dump runs

> "LOCK TABLE %s IN ACCESS SHARE MODE" for each table.

> Naturally it makes sense, but...

> This schema has a table that serves as parent for thousands of child
> tables (via INHERITS).

> So effectively, to dump this schema, I have to LOCK all these tables
> not only parent.

They'd all end up locked anyway wouldn't they?

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers