Обсуждение: CLUSTER, REINDEX, VACUUM in "read only" transaction?

So I was looking for other omissions in utility.c, and I noticed that
check_xact_readonly() doesn't reject CLUSTER, REINDEX, or VACUUM.
Now the notion of "read only" that we're trying to enforce is pretty
weak (I think it's effectively "no writes to non-temp tables").
But I can't see that CLUSTER is a read-only operation even under the
weakest definitions, and I'm not seeing the rationale for REINDEX or
VACUUM here either.

Comments?
        regards, tom lane

On Fri, Oct 10, 2008 at 09:41:39AM -0400, Tom Lane wrote:
> So I was looking for other omissions in utility.c, and I noticed that
> check_xact_readonly() doesn't reject CLUSTER, REINDEX, or VACUUM.
> Now the notion of "read only" that we're trying to enforce is pretty
> weak (I think it's effectively "no writes to non-temp tables").
> But I can't see that CLUSTER is a read-only operation even under the
> weakest definitions, and I'm not seeing the rationale for REINDEX or
> VACUUM here either.
> 
> Comments?
> 
>             regards, tom lane
> 

CLUSTER, REINDEX, and VACUUM are read-only to me because they do not
change what I will call the actual working data in the database. Also,
how could you address problems with index imbalance or heap organization?

Regards,
Ken

Kenneth Marshall <ktm@rice.edu> writes:
> On Fri, Oct 10, 2008 at 09:41:39AM -0400, Tom Lane wrote:
>> But I can't see that CLUSTER is a read-only operation even under the
>> weakest definitions, and I'm not seeing the rationale for REINDEX or
>> VACUUM here either.

> CLUSTER, REINDEX, and VACUUM are read-only to me because they do not
> change what I will call the actual working data in the database. Also,
> how could you address problems with index imbalance or heap organization?

Why would you expect to be able to address such things in a read-only
transaction?
        regards, tom lane

On Fri, 2008-10-10 at 09:41 -0400, Tom Lane wrote:

> So I was looking for other omissions in utility.c, and I noticed that
> check_xact_readonly() doesn't reject CLUSTER, REINDEX, or VACUUM.
> Now the notion of "read only" that we're trying to enforce is pretty
> weak (I think it's effectively "no writes to non-temp tables").
> But I can't see that CLUSTER is a read-only operation even under the
> weakest definitions, and I'm not seeing the rationale for REINDEX or
> VACUUM here either.

I think you should add a few more to the list.

* LISTEN, UNLISTEN, NOTIFY
* nextval()
* ANALYZE
* CHECKPOINT
* GRANT, REVOKE, REASSIGN
* DISCARD ALL should not run UnlistenAll

These are all gaps I will have to plug for Hot Standby, and in fact
wrote code this morning for some of those before I saw this post.

(Yes, I went through every command).

If you're gonna do it, do it soon please, so I know not to bother any
further.

-- Simon Riggs           www.2ndQuadrant.comPostgreSQL Training, Services and Support

Tom Lane wrote:
> So I was looking for other omissions in utility.c, and I noticed that
> check_xact_readonly() doesn't reject CLUSTER, REINDEX, or VACUUM.
> Now the notion of "read only" that we're trying to enforce is pretty
> weak (I think it's effectively "no writes to non-temp tables").
> But I can't see that CLUSTER is a read-only operation even under the
> weakest definitions, and I'm not seeing the rationale for REINDEX or
> VACUUM here either.

I think the way the SQL standard meant the read-only flag is that the 
transaction doesn't change the structure of or the data in the database 
as seen by the next guy.  So all of these commands are OK, I think.

A theoretical use case is that you should be able to do the maximum set 
of useful work in read-only mode on a Slony-I slave.  No I haven't 
checked what Slony does with these three commands, so let me have it. :-)

Other definitions might be OK, but I can't see one offhand that is based 
on the current behavior but disallows these three commands.  "No disk 
writes" or "no big locks" is probably not what the SQL standard meant.

On Tue, 2008-10-14 at 19:18 +0300, Peter Eisentraut wrote:
> Tom Lane wrote:
> > So I was looking for other omissions in utility.c, and I noticed that
> > check_xact_readonly() doesn't reject CLUSTER, REINDEX, or VACUUM.
> > Now the notion of "read only" that we're trying to enforce is pretty
> > weak (I think it's effectively "no writes to non-temp tables").
> > But I can't see that CLUSTER is a read-only operation even under the
> > weakest definitions, and I'm not seeing the rationale for REINDEX or
> > VACUUM here either.
> 
> I think the way the SQL standard meant the read-only flag is that the 
> transaction doesn't change the structure of or the data in the database 
> as seen by the next guy.  So all of these commands are OK, I think.
> 
> A theoretical use case is that you should be able to do the maximum set 
> of useful work in read-only mode on a Slony-I slave.  No I haven't 
> checked what Slony does with these three commands, so let me have it. :-)

Well, read-only applies to queries on the Slony slave, not to other
necessary work, which cannot be read only.

In general, if one transaction is fully read-only I don't see why that
should prevent other parts of the system from working normally.

So I would say ban all the utilities mentioned from read-only
transactions, and don't be influenced by what non-read only transactions
do.

-- Simon Riggs           www.2ndQuadrant.comPostgreSQL Training, Services and Support