Content-Type: multipart/alternative;
 boundary="------------8e98QqIpzOVnMoaPW4TkESoM"
Message-ID: <79ea98ff-3bd7-03ff-be40-70e851c38640@migops.com>
Date: Wed, 4 Jan 2023 14:25:13 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: fix and document CLUSTER privileges
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Andrew Dunstan <andrew@dunslane.net>, Justin Pryzby
 <pryzby@telsasoft.com>, pgsql-hackers@postgresql.org,
 Robert Haas <robertmhaas@gmail.com>
References: <20221207223924.GA4182184@nathanxps13>
 <20221208022559.GA27893@telsasoft.com> <20221208041313.GA216874@nathanxps13>
 <b4d634cf-d893-f0f1-74ce-07675317575c@dunslane.net>
 <20221208181324.GA4385@nathanxps13>
 <CA+TgmoaWhAjTP3a9xjA2P=xkF-RV9UOYT11JVWgQq5A8AB9ZrA@mail.gmail.com>
 <20221214173435.GA690225@nathanxps13> <20221216045700.GA804311@nathanxps13>
Content-Language: fr, en-US
From: Gilles Darold <gilles@migops.com>
In-Reply-To: <20221216045700.GA804311@nathanxps13>
Archived-At: <https://www.postgresql.org/message-id/79ea98ff-3bd7-03ff-be40-70e851c38640%40migops.com>
Precedence: bulk

This is a multi-part message in MIME format.
--------------8e98QqIpzOVnMoaPW4TkESoM
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

Le 16/12/2022 à 05:57, Nathan Bossart a écrit :
> Here is a new version of the patch.  I've moved the privilege checks to a
> new function, and I added a note in the docs about clustering partitioned
> tables in a transaction block (it's not allowed).
>

Getting into review of this patch I wonder why the CLUSTER command do 
not react as VACUUM FULL command when there is insuffisant privileges. 
For example with a partitioned table (ptnowner) and two partitions 
(ptnowner1 and ptnowner2) with the second partition owned by another 
user, let' say usr2. We have the following report when executing vacuum 
as usr2:

testdb=> VACUUM FULL ptnowner;
WARNING:  permission denied to vacuum "ptnowner", skipping it
WARNING:  permission denied to vacuum "ptnowner1", skipping it
VACUUM

Here only ptnowner2 have been vacuumed which is correct and expected.

For the cluster command:

testdb=> CLUSTER;
CLUSTER


I would have expected something like:

testdb=> CLUSTER;
WARNING:  permission denied to cluster "ptnowner1", skipping it
CLUSTER

I mean that the silent behavior is not very helpful.


This is the current behavior of the CLUSTER command and current patch 
adds a sentence about the silent behavior in the documentation. This is 
good but I just want to ask if we could want to fix this behavior too or 
just keep things like that with the lack of noise.


Best regards,

-- 
Gilles Darold

--------------8e98QqIpzOVnMoaPW4TkESoM
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 8bit

<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <div class="moz-cite-prefix">Le 16/12/2022 à 05:57, Nathan Bossart a
      écrit :<br>
    </div>
    <blockquote type="cite"
      cite="mid:20221216045700.GA804311@nathanxps13">
      <pre class="moz-quote-pre" wrap="">Here is a new version of the patch.  I've moved the privilege checks to a
new function, and I added a note in the docs about clustering partitioned
tables in a transaction block (it's not allowed).

</pre>
    </blockquote>
    <p><br>
    </p>
    <p>Getting into review of this patch I wonder why the CLUSTER
      command do not react as VACUUM FULL command when there is
      insuffisant privileges. For example with a partitioned table
      (ptnowner) and two partitions (ptnowner1 and ptnowner2) with the
      second partition owned by another user, let' say usr2. We have the
      following report when executing vacuum as usr2:</p>
    <p>testdb=&gt; VACUUM FULL ptnowner;<br>
      WARNING:  permission denied to vacuum "ptnowner", skipping it<br>
      WARNING:  permission denied to vacuum "ptnowner1", skipping it<br>
      VACUUM</p>
    <p>Here only ptnowner2 have been vacuumed which is correct and
      expected.</p>
    <p>For the cluster command:</p>
    <p>testdb=&gt; CLUSTER;<br>
      CLUSTER</p>
    <p><br>
    </p>
    <p>I would have expected something like:</p>
    <p>testdb=&gt; CLUSTER;<br>
      WARNING:  permission denied to cluster "ptnowner1", skipping it<br>
      CLUSTER<br>
      <br>
    </p>
    <p>I mean that the silent behavior is not very helpful.</p>
    <p><br>
    </p>
    <p>This is the current behavior of the CLUSTER command and current
      patch adds a sentence about the silent behavior in the
      documentation. This is good but I just want to ask if we could
      want to fix this behavior too or just keep things like that <span
        class="HwtZe" lang="en"><span class="jCAhz ChMk0b"><span
            class="ryNqvb">with the lack of noise</span></span></span>.
      <br>
    </p>
    <p><br>
    </p>
    <p>Best regards,<br>
    </p>
    <pre class="moz-signature" cols="72">-- 
Gilles Darold</pre>
  </body>
</html>

--------------8e98QqIpzOVnMoaPW4TkESoM--