Обсуждение: Only owners can ANALYZE tables...seems overly restrictive
Given the amount of damage a person with write access to a table can get into it seems pointless to not allow them to analyze the table after their updates - since best practices would say that normal work with a table should not be performed by an owner.
I should the check for whether a given user can or cannot analyze a table should be whether the user has INSERT, UPDATE, or DELETE privileges.
I suppose row-level-security might come into play here...
David J.
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> Given the amount of damage a person with write access to a table can get
> into it seems pointless to not allow them to analyze the table after their
> updates - since best practices would say that normal work with a table
> should not be performed by an owner.
> I should the check for whether a given user can or cannot analyze a table
> should be whether the user has INSERT, UPDATE, or DELETE privileges.
By that argument, we should allow anyone with any write access to do
TRUNCATE. Or perhaps even DROP TABLE. I'm not impressed.
regards, tom lane
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> Given the amount of damage a person with write access to a table can get
> into it seems pointless to not allow them to analyze the table after their
> updates - since best practices would say that normal work with a table
> should not be performed by an owner.
> I should the check for whether a given user can or cannot analyze a table
> should be whether the user has INSERT, UPDATE, or DELETE privileges.
By that argument, we should allow anyone with any write access to do
TRUNCATE. Or perhaps even DROP TABLE. I'm not impressed.
TRUNCATE indeed also seems overly restrictive. But in any case we have a GRANT for TRUNCATE. If you are saying that you'd be OK with adding a GRANT for ANALYZE that would probably suffice.
I'd place DROP TABLE into a different category simply because it alters the design of the database - some only owners in the database should be allowed to do. We also don't tell people to "run DROP TABLE" so that the planner can choose better query plans. We do tell people that after inserting or deleting lots of data they should run ANALYZE to ensure subsequent queries have good info to work with. Telling them to pray that (and/or wait an indefinite period of time for) the auto-vacuum process ran - or to call their DBA - it not a good solution to that problem.
As much as I respect your opinion I was hoping for something less trite.
ANALYZE, even on a large table (though statistics target would influence this), is seemingly fast due to the random sampling. It also operates fully concurrently with other activity.
David J.
On 2/28/2016 8:58 PM, Tom Lane wrote: >> >I should the check for whether a given user can or cannot analyze a table >> >should be whether the user has INSERT, UPDATE, or DELETE privileges. > By that argument, we should allow anyone with any write access to do > TRUNCATE. Or perhaps even DROP TABLE. I'm not impressed. I don't see why anyone with delete privileges shouldn't be able to truncate (after all, thats the same as deleting all records). analyze has arguably fewer side effects, its a performance enhancement, its neither altering the schema or changing the data. -- john r pierce, recycling bits in santa cruz
On 2/28/16, John R Pierce <pierce@hogranch.com> wrote: > I don't see why anyone with delete privileges shouldn't be able to > truncate (after all, thats the same as deleting all records). Firstly, because you can prevent deleting some rows by a trigger; TRUNCATE doesn't deal with rows. Secondary, TRUNCATE is _NOT_ MVCC. Even in a not yet finished transaction other connections can see empty table. Thirdly, TRUNCATE is often used for clearing most of (or even all) DB tables for tests. Splitting privileges is one of possible protection for running tests on a prod server (if config files are copied wrongly). > > -- > john r pierce, recycling bits in santa cruz -- Best regards, Vitaly Burovoy
John R Pierce wrote: > On 2/28/2016 8:58 PM, Tom Lane wrote: >>>> I should the check for whether a given user can or cannot analyze a table >>>> should be whether the user has INSERT, UPDATE, or DELETE privileges. >> By that argument, we should allow anyone with any write access to do >> TRUNCATE. Or perhaps even DROP TABLE. I'm not impressed. > I don't see why anyone with delete privileges shouldn't be able to > truncate (after all, thats the same as deleting all records). > > analyze has arguably fewer side effects, its a performance enhancement, > its neither altering the schema or changing the data. In a production environment you don't want a user to change your table statistics. They could just set default_statistics_target to something stupid, run ANALYZE and wreck the statistics for everyone. And then come back to the DBA and complain that things don't work. We have a policy that users are not table owners, and with the current behaviour we can be certain that any bad table statistics are the fault of the DBA or wrong configuration. Yours, Laurenz Albe
* David G. Johnston (david.g.johnston@gmail.com) wrote: > Given the amount of damage a person with write access to a table can get > into it seems pointless to not allow them to analyze the table after their > updates - since best practices would say that normal work with a table > should not be performed by an owner. > > I should the check for whether a given user can or cannot analyze a table > should be whether the user has INSERT, UPDATE, or DELETE privileges. Realistically, ANALYZE is a background/maintenance task that autovacuum should be handling for you. > I suppose row-level-security might come into play here... Yes, you may only have access to a subset of the table. If we had plenty more bits to allow ANALYZE to be independently GRANT'able, then maybe, but those are a limited resource. Thanks! Stephen
Вложения
2016-02-29 14:31 GMT+01:00 Stephen Frost <sfrost@snowman.net>:
* David G. Johnston (david.g.johnston@gmail.com) wrote:
> Given the amount of damage a person with write access to a table can get
> into it seems pointless to not allow them to analyze the table after their
> updates - since best practices would say that normal work with a table
> should not be performed by an owner.
>
> I should the check for whether a given user can or cannot analyze a table
> should be whether the user has INSERT, UPDATE, or DELETE privileges.
Realistically, ANALYZE is a background/maintenance task that autovacuum
should be handling for you.
Realistically, that can't happen every time. Think of temporary tables for example...
> I suppose row-level-security might come into play here...
Yes, you may only have access to a subset of the table.
If we had plenty more bits to allow ANALYZE to be independently
GRANT'able, then maybe, but those are a limited resource.
Agreed.
--
On 02/29/2016 03:15 PM, Guillaume Lelarge wrote: > 2016-02-29 14:31 GMT+01:00 Stephen Frost <sfrost@snowman.net > <mailto:sfrost@snowman.net>>: > > Realistically, ANALYZE is a background/maintenance task that autovacuum > should be handling for you. > > Realistically, that can't happen every time. Think of temporary tables > for example... Hmm. How are you not the owner of a temporary table? -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
2016-02-29 15:20 GMT+01:00 Vik Fearing <vik@2ndquadrant.fr>:
On 02/29/2016 03:15 PM, Guillaume Lelarge wrote:
> 2016-02-29 14:31 GMT+01:00 Stephen Frost <sfrost@snowman.net
> <mailto:sfrost@snowman.net>>:
>
> Realistically, ANALYZE is a background/maintenance task that autovacuum
> should be handling for you.
>
> Realistically, that can't happen every time. Think of temporary tables
> for example...
Hmm. How are you not the owner of a temporary table?
Oh, you obviously are :)
--
* David G. Johnston (david.g.johnston@gmail.com) wrote:
> Given the amount of damage a person with write access to a table can get
> into it seems pointless to not allow them to analyze the table after their
> updates - since best practices would say that normal work with a table
> should not be performed by an owner.
>
> I should the check for whether a given user can or cannot analyze a table
> should be whether the user has INSERT, UPDATE, or DELETE privileges.
Realistically, ANALYZE is a background/maintenance task that autovacuum
should be handling for you.
Then my recent experience of adding a bunch of records and having the subsequent select query take forever because the table wasn't analyzed is not supposed to happen? What am I doing wrong then that autovacuum didn't run for me?
> I suppose row-level-security might come into play here...
Yes, you may only have access to a subset of the table.
TBH, since you cannot see the data being analyzed I don't see a security implication here if you allow someone to ANALYZE the whole table even when RLS is in place.
If we had plenty more bits to allow ANALYZE to be independently
GRANT'able, then maybe, but those are a limited resource.
The planner and system performance seems important enough to give it such a resource. But as I stated initially I personally believe that a user with INSERT/DELETE/UPDATE permissions on a table (maybe require all three) should also be allowed to ANALYZE said table.
David J.
John R Pierce wrote:
> On 2/28/2016 8:58 PM, Tom Lane wrote:
>>>> I should the check for whether a given user can or cannot analyze a table
>>>> should be whether the user has INSERT, UPDATE, or DELETE privileges.
>> By that argument, we should allow anyone with any write access to do
>> TRUNCATE. Or perhaps even DROP TABLE. I'm not impressed.
> I don't see why anyone with delete privileges shouldn't be able to
> truncate (after all, thats the same as deleting all records).
>
> analyze has arguably fewer side effects, its a performance enhancement,
> its neither altering the schema or changing the data.
In a production environment you don't want a user to change your table
statistics.
They could just set default_statistics_target to something stupid,
run ANALYZE and wreck the statistics for everyone.
And then come back to the DBA and complain that things don't work.
We have a policy that users are not table owners, and with the
current behaviour we can be certain that any bad table statistics
are the fault of the DBA or wrong configuration.
Setting default_statistics_target and running ANALYZE are two entirely different things.
As it stands, from the standpoint of a DBA, it isn't much different for autovacuum to run ANALYZE compared to having a user run ANALYZE. In both cases the DBA's job is to ensure that the act of running ANALYZE is properly configured. All this does is allows an informed user to run ANALYZE when they suspect they performed sufficient changes to the table - just in case the thresholds for autovacuum are not met.
I really don't care to prevent legitimate uses of ANALYZE just because someone might do something stupid like.
INSERT INTO table VALUES (1);
ANALYZE;
INSERT INTO table VALUES(2);
ANALYZE;
etc...
There is enough gap between the volume of changes needed for auto-ANALYZE to kick in and a sufficiently large insertion of unique data to cause the planner to fail that allowing a user to act has merit. I'm looking for downsides and still haven't seen any that are serious enough that a well-meaning user can accidentally perform to hurt the system.
The only obvious thing they can do is run:
ANALYZE;
ANALYZE;
ANALYZE;
over and over again - but I'd call that malicious and they can already do worse.
For me it boils down to that we already have an auto-vacuum daemon so we've given up strict control of when the statistics are refreshed. We should acknowledge that such a mechanism is not perfect - which we do by having ANALYZE - but then also recognize that the user causing the statistics to become stale, if permissions are securely provisioned, has no way to correct the deficiency themselves but must rely upon a heuristic or a DBA. To me that is overly restrictive, discourages good security practices, and gains little to nothing in way of protecting the system.
David J.
* David G. Johnston (david.g.johnston@gmail.com) wrote: > On Mon, Feb 29, 2016 at 6:31 AM, Stephen Frost <sfrost@snowman.net> wrote: > > > * David G. Johnston (david.g.johnston@gmail.com) wrote: > > > Given the amount of damage a person with write access to a table can get > > > into it seems pointless to not allow them to analyze the table after > > their > > > updates - since best practices would say that normal work with a table > > > should not be performed by an owner. > > > > > > I should the check for whether a given user can or cannot analyze a table > > > should be whether the user has INSERT, UPDATE, or DELETE privileges. > > > > Realistically, ANALYZE is a background/maintenance task that autovacuum > > should be handling for you. > > Then my recent experience of adding a bunch of records and having the > subsequent select query take forever because the table wasn't analyzed is > not supposed to happen? What am I doing wrong then that autovacuum didn't > run for me? Perhaps nothing. Making autovacuum more aggressive is a trade-off and evidently there weren't enough changes or perhaps not enough time for autovacuum to realize it needed to kick in and re-analyze the table. One thought about how to address that might be to have a given backend, which is already sending stats info to the statistic collector, somehow also bump autovacuum to wake it up from its sleep to go analyze the tables just modified. This is all very hand-wavy as I don't have time at the moment to run it down, but I do think it'd be good to reduce the need to run ANALYZE by hand after every data load. > > > I suppose row-level-security might come into play here... > > > > Yes, you may only have access to a subset of the table. > > > > > TBH, since you cannot see the data being analyzed I don't see a security > implication here if you allow someone to ANALYZE the whole table even when > RLS is in place. I wasn't looking at it from a security implication standpoint as I suspect that any issue there could actually be addressed, if any exist. What I was getting at is that you're making an assumption that any user with DML rights on the table also has enough information about the table overall to know when it makes sense to ANALYZE the table or not. That's a bit of a stretch to begin with, but when you consider that RLS may be involved and the user may only have access to 1% (or less) of the overall table, it's that much more of a reach. > If we had plenty more bits to allow ANALYZE to be independently > > GRANT'able, then maybe, but those are a limited resource. > > > > The planner and system performance seems important enough to give it such > a resource. But as I stated initially I personally believe that a user > with INSERT/DELETE/UPDATE permissions on a table (maybe require all three) > should also be allowed to ANALYZE said table. I don't think requiring all three would make any sense and would, instead, simply be confusing. I'm not completely against your general idea, but let's keep it simple. Thanks! Stephen
Вложения
David G. Johnston wrote: > On Mon, Feb 29, 2016 at 2:52 AM, Albe Laurenz <laurenz.albe@wien.gv.at> wrote: >> John R Pierce wrote: >>> analyze has arguably fewer side effects, its a performance enhancement, >>> its neither altering the schema or changing the data. >> In a production environment you don't want a user to change your table >> statistics. >> >> They could just set default_statistics_target to something stupid, >> run ANALYZE and wreck the statistics for everyone. >> And then come back to the DBA and complain that things don't work. >> >> We have a policy that users are not table owners, and with the >> current behaviour we can be certain that any bad table statistics >> are the fault of the DBA or wrong configuration. > Setting default_statistics_target and running ANALYZE are two entirely different things. Setting default_statistics_target affects the statistics computed by ANALYZE, so I cannot follow you here. Yours, Laurenz Albe
* David G. Johnston (david.g.johnston@gmail.com) wrote:
> On Mon, Feb 29, 2016 at 6:31 AM, Stephen Frost <sfrost@snowman.net> wrote:
>
> > * David G. Johnston (david.g.johnston@gmail.com) wrote:
> > > Given the amount of damage a person with write access to a table can get
> > > into it seems pointless to not allow them to analyze the table after
> > their
> > > updates - since best practices would say that normal work with a table
> > > should not be performed by an owner.
> > >
> > > I should the check for whether a given user can or cannot analyze a table
> > > should be whether the user has INSERT, UPDATE, or DELETE privileges.
> >
> > Realistically, ANALYZE is a background/maintenance task that autovacuum
> > should be handling for you.
>
> Then my recent experience of adding a bunch of records and having the
> subsequent select query take forever because the table wasn't analyzed is
> not supposed to happen? What am I doing wrong then that autovacuum didn't
> run for me?
Perhaps nothing. Making autovacuum more aggressive is a trade-off and
evidently there weren't enough changes or perhaps not enough time for
autovacuum to realize it needed to kick in and re-analyze the table.
One thought about how to address that might be to have a given backend,
which is already sending stats info to the statistic collector, somehow
also bump autovacuum to wake it up from its sleep to go analyze the
tables just modified. This is all very hand-wavy as I don't have time
at the moment to run it down, but I do think it'd be good to reduce the
need to run ANALYZE by hand after every data load.
Improving it is desirable but it wouldn't preclude this desire.
> > > I suppose row-level-security might come into play here...
> >
> > Yes, you may only have access to a subset of the table.
> >
> >
> TBH, since you cannot see the data being analyzed I don't see a security
> implication here if you allow someone to ANALYZE the whole table even when
> RLS is in place.
I wasn't looking at it from a security implication standpoint as I
suspect that any issue there could actually be addressed, if any exist.
What I was getting at is that you're making an assumption that any user
with DML rights on the table also has enough information about the table
overall to know when it makes sense to ANALYZE the table or not. That's
a bit of a stretch to begin with, but when you consider that RLS may be
involved and the user may only have access to 1% (or less) of the
overall table, it's that much more of a reach.
So the typical user doesn't know or even care that what they just did needs to be analyzed. The situation is no worse than it is today. But as someone who writes many scripts and applications to perform bulk writing and data analysis I'd like those scripts to use restricted authorization credentials while still being able to run ANALYZE between performing the bulk DML and the running the SELECT statements needed to get the newly generated data out of the database.
> If we had plenty more bits to allow ANALYZE to be independently
> > GRANT'able, then maybe, but those are a limited resource.
> >
>
> The planner and system performance seems important enough to give it such
> a resource. But as I stated initially I personally believe that a user
> with INSERT/DELETE/UPDATE permissions on a table (maybe require all three)
> should also be allowed to ANALYZE said table.
I don't think requiring all three would make any sense and would,
instead, simply be confusing. I'm not completely against your general
idea, but let's keep it simple.
Agreed.
David J.
David G. Johnston wrote:
> On Mon, Feb 29, 2016 at 2:52 AM, Albe Laurenz <laurenz.albe@wien.gv.at> wrote:
>> John R Pierce wrote:
>>> analyze has arguably fewer side effects, its a performance enhancement,
>>> its neither altering the schema or changing the data.
>> In a production environment you don't want a user to change your table
>> statistics.
>>
>> They could just set default_statistics_target to something stupid,
>> run ANALYZE and wreck the statistics for everyone.
>> And then come back to the DBA and complain that things don't work.
>>
>> We have a policy that users are not table owners, and with the
>> current behaviour we can be certain that any bad table statistics
>> are the fault of the DBA or wrong configuration.
> Setting default_statistics_target and running ANALYZE are two entirely different things.
Setting default_statistics_target affects the statistics computed by ANALYZE,
so I cannot follow you here.
Just because I can run ANALYZE doesn't mean I should be able to update the statistic targets. While the features are related the permissions are not.
David J.
David G. Johnston wrote: >>>> In a production environment you don't want a user to change your table >>>> statistics. >>>> >>>> They could just set default_statistics_target to something stupid, >>>> run ANALYZE and wreck the statistics for everyone. >>>> And then come back to the DBA and complain that things don't work. >>> Setting default_statistics_target and running ANALYZE are two entirely different things. >> Setting default_statistics_target affects the statistics computed by ANALYZE, >> so I cannot follow you here. > Just because I can run ANALYZE doesn't mean I should be able to update the statistic targets. While > the features are related the permissions are not. See http://www.postgresql.org/docs/current/static/planner-stats.html "The amount of information stored in pg_statistic by ANALYZE, in particular the maximum number of entries in the most_common_vals and histogram_bounds arrays for each column, can be set on a column-by-column basis using the ALTER TABLE SET STATISTICS command, or globally by setting the default_statistics_target configuration variable." Yours, Laurenz Albe
On 02/29/2016 05:31 AM, Stephen Frost wrote: > * David G. Johnston (david.g.johnston@gmail.com) wrote: >> Given the amount of damage a person with write access to a table can get >> into it seems pointless to not allow them to analyze the table after their >> updates - since best practices would say that normal work with a table >> should not be performed by an owner. >> >> I should the check for whether a given user can or cannot analyze a table >> should be whether the user has INSERT, UPDATE, or DELETE privileges. > > Realistically, ANALYZE is a background/maintenance task that autovacuum > should be handling for you. Incorrect. That would be autoanalyze and although they are similar they are not the same. ANALYZE is used for a number of things, not the least is query profiling. Sincerely, JD -- Command Prompt, Inc. http://the.postgres.company/ +1-503-667-4564 PostgreSQL Centered full stack support, consulting and development.
David G. Johnston wrote:
>>>> In a production environment you don't want a user to change your table
>>>> statistics.
>>>>
>>>> They could just set default_statistics_target to something stupid,
>>>> run ANALYZE and wreck the statistics for everyone.
>>>> And then come back to the DBA and complain that things don't work.
>>> Setting default_statistics_target and running ANALYZE are two entirely different things.
>> Setting default_statistics_target affects the statistics computed by ANALYZE,
>> so I cannot follow you here.
> Just because I can run ANALYZE doesn't mean I should be able to update the statistic targets. While
> the features are related the permissions are not.
See http://www.postgresql.org/docs/current/static/planner-stats.html
"The amount of information stored in pg_statistic by ANALYZE, in particular the
maximum number of entries in the most_common_vals and histogram_bounds arrays
for each column, can be set on a column-by-column basis using the
ALTER TABLE SET STATISTICS command, or globally by setting the
default_statistics_target configuration variable."
Being able to run ANALYZE on a table in no way implies that I should be allowed to run ALTER TABLE SET STATISTICS on the same.
Only table owners should be allowed to execute ALTER TABLE while, in my opinion, anyone with write capabilities on a table should be allowed to execute ANALYZE. I would accept a GRANT permission if that could get committed but I find the status-quo mildly annoying.
David J.
On 02/29/16 06:20, Vik Fearing wrote: > > Hmm. How are you not the owner of a temporary table? After 'set session authorization ...'
On 02/29/2016 09:09 AM, David G. Johnston wrote: > > Being able to run ANALYZE on a table in no way implies that I should be > allowed to run ALTER TABLE SET STATISTICS on the same. > > > Only table owners should be allowed to execute ALTER TABLE while, in my > opinion, anyone with write capabilities on a table should be allowed to > execute ANALYZE. I would accept a GRANT permission if that could get > committed but I find the status-quo mildly annoying. I think a better question at this point is: What is the problem you are trying to solve? Think about the following: 1. When you run ANALYZE it will update the statistics. 2. Anyone can run SET, which means that if any user can run ANALYZE, any user can greatly modify the statistics. 3. This can already be handled by GRANT: * psql -U jd -h localhost; * create table foo (id text); * create role jd_role; * alter table foo owner to jd_role; * grant jd_role to boo; * \c jd boo * analyze foo; Sincerely, JD -- Command Prompt, Inc. http://the.postgres.company/ +1-503-667-4564 PostgreSQL Centered full stack support, consulting and development. Everyone appreciates your honesty, until you are honest with them.
On 02/29/2016 08:13 AM, David G. Johnston wrote:
> On Mon, Feb 29, 2016 at 8:28 AM, Stephen Frost <sfrost@snowman.net
> <mailto:sfrost@snowman.net>>wrote:
>
> * David G. Johnston (david.g.johnston@gmail.com
> <mailto:david.g.johnston@gmail.com>) wrote:
> > On Mon, Feb 29, 2016 at 6:31 AM, Stephen Frost <sfrost@snowman.net <mailto:sfrost@snowman.net>> wrote:
> >
> > > * David G. Johnston (david.g.johnston@gmail.com <mailto:david.g.johnston@gmail.com>) wrote:
> > > > Given the amount of damage a person with write access to a table can get
> > > > into it seems pointless to not allow them to analyze the table after
> > > their
> > > > updates - since best practices would say that normal work with a table
> > > > should not be performed by an owner.
> > > >
> > > > I should the check for whether a given user can or cannot analyze a table
> > > > should be whether the user has INSERT, UPDATE, or DELETE privileges.
> > >
> > > Realistically, ANALYZE is a background/maintenance task that autovacuum
> > > should be handling for you.
> >
> > Then my recent experience of adding a bunch of records and having the
> > subsequent select query take forever because the table wasn't analyzed is
> > not supposed to happen? What am I doing wrong then that autovacuum didn't
> > run for me?
>
> Perhaps nothing. Making autovacuum more aggressive is a trade-off and
> evidently there weren't enough changes or perhaps not enough time for
> autovacuum to realize it needed to kick in and re-analyze the table.
> One thought about how to address that might be to have a given backend,
> which is already sending stats info to the statistic collector, somehow
> also bump autovacuum to wake it up from its sleep to go analyze the
> tables just modified. This is all very hand-wavy as I don't have time
> at the moment to run it down, but I do think it'd be good to reduce the
> need to run ANALYZE by hand after every data load.
>
>
> Improving it is desirable but it wouldn't preclude this desire.
>
>
> > > > I suppose row-level-security might come into play here...
> > >
> > > Yes, you may only have access to a subset of the table.
> > >
> > >
> > TBH, since you cannot see the data being analyzed I don't see a security
> > implication here if you allow someone to ANALYZE the whole table even when
> > RLS is in place.
>
> I wasn't looking at it from a security implication standpoint as I
> suspect that any issue there could actually be addressed, if any exist.
>
> What I was getting at is that you're making an assumption that any user
> with DML rights on the table also has enough information about the table
> overall to know when it makes sense to ANALYZE the table or not. That's
> a bit of a stretch to begin with, but when you consider that RLS may be
> involved and the user may only have access to 1% (or less) of the
> overall table, it's that much more of a reach.
>
>
> So the typical user doesn't know or even care that what they just did
> needs to be analyzed. The situation is no worse than it is today. But
> as someone who writes many scripts and applications to perform bulk
> writing and data analysis I'd like those scripts to use restricted
> authorization credentials while still being able to run ANALYZE between
> performing the bulk DML and the running the SELECT statements needed to
> get the newly generated data out of the database.
Maybe?:
CREATE OR REPLACE FUNCTION public.analyze_test(tbl_name character varying)
RETURNS void
LANGUAGE plpgsql
SECURITY DEFINER
AS $function$
BEGIN
EXECUTE 'ANALYZE ' || quote_ident(tbl_name);
END;
$function$
>
> > If we had plenty more bits to allow ANALYZE to be independently
> > > GRANT'able, then maybe, but those are a limited resource.
> > >
> >
> > The planner and system performance seems important enough to give it such
> > a resource. But as I stated initially I personally believe that a user
> > with INSERT/DELETE/UPDATE permissions on a table (maybe require all three)
> > should also be allowed to ANALYZE said table.
>
> I don't think requiring all three would make any sense and would,
> instead, simply be confusing. I'm not completely against your general
> idea, but let's keep it simple.
>
>
> Agreed.
>
> David J.
--
Adrian Klaver
adrian.klaver@aklaver.com
"David G. Johnston" <david.g.johnston@gmail.com> writes: > On Mon, Feb 29, 2016 at 9:27 AM, Albe Laurenz <laurenz.albe@wien.gv.at> > wrote: >> See http://www.postgresql.org/docs/current/static/planner-stats.html >> "The amount of information stored in pg_statistic by ANALYZE, in >> particular the >> maximum number of entries in the most_common_vals and histogram_bounds >> arrays >> for each column, can be set on a column-by-column basis using the >> ALTER TABLE SET STATISTICS command, or globally by setting the >> default_statistics_target configuration variable." > Being able to run ANALYZE on a table in no way implies that I should be > allowed to run ALTER TABLE SET STATISTICS on the same. You're missing the point. If the table owner has *not* run ALTER TABLE SET STATISTICS, which surely is the typical situation, then whoever runs ANALYZE can control the volume of stats generated by setting default_statistics_target locally in his session. Thus, if we allow non-owners to run ANALYZE, they'd be able to mess things up by setting the stats target either much lower or much higher than the table owner expected. ("Much higher" would be bad in a different way than "much lower", but still bad.) I imagine this could be addressed by some rule about how if you don't own the table then your default_statistics_target is overridden by the global setting, but that would be a mess both conceptually and implementation-wise. regards, tom lane
Joshua D. Drake wrote: > On 02/29/2016 05:31 AM, Stephen Frost wrote: > >Realistically, ANALYZE is a background/maintenance task that autovacuum > >should be handling for you. > > Incorrect. That would be autoanalyze and although they are similar they are > not the same. ANALYZE is used for a number of things, not the least is query > profiling. I think you are confusing ANALYZE with EXPLAIN ANALYZE. I am not aware of any way in which ANALYZE is different from what you call "autoanalyze" (which is really just an autovacuum-invoked ANALYZE and for which we don't have any specific term). -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 02/29/2016 10:06 AM, Alvaro Herrera wrote: > Joshua D. Drake wrote: >> On 02/29/2016 05:31 AM, Stephen Frost wrote: > >>> Realistically, ANALYZE is a background/maintenance task that autovacuum >>> should be handling for you. >> >> Incorrect. That would be autoanalyze and although they are similar they are >> not the same. ANALYZE is used for a number of things, not the least is query >> profiling. > > I think you are confusing ANALYZE with EXPLAIN ANALYZE. Actually I am not but I can see how it wasn't clear. > > I am not aware of any way in which ANALYZE is different from what you > call "autoanalyze" (which is really just an autovacuum-invoked ANALYZE > and for which we don't have any specific term). One item that can be helpful from ANALYZE (not EXPLAIN ANALYZE) is modifying the statistics via SET, running Analyze and see if the query improves. You can then determine where best to set those statistics (usually ALTER TABLE). Sincerely, JD -- Command Prompt, Inc. http://the.postgres.company/ +1-503-667-4564 PostgreSQL Centered full stack support, consulting and development. Everyone appreciates your honesty, until you are honest with them.
On 02/29/2016 09:09 AM, David G. Johnston wrote:
Being able to run ANALYZE on a table in no way implies that I should be
allowed to run ALTER TABLE SET STATISTICS on the same.
Only table owners should be allowed to execute ALTER TABLE while, in my
opinion, anyone with write capabilities on a table should be allowed to
execute ANALYZE. I would accept a GRANT permission if that could get
committed but I find the status-quo mildly annoying.
I think a better question at this point is: What is the problem you are trying to solve? Think about the following:
1. When you run ANALYZE it will update the statistics.
2. Anyone can run SET, which means that if any user can run ANALYZE, any user can greatly modify the statistics.
OK - so the true problem has been identified.
3. This can already be handled by GRANT:
* psql -U jd -h localhost;
* create table foo (id text);
* create role jd_role;
* alter table foo owner to jd_role;
* grant jd_role to boo;
* \c jd boo
* analyze foo;
I dislike the fact that this solution involves giving someone who only cares about DML the capability to also perform DDL.
Given these two things it seems the least difficult solution that doesn't make things any worse is to make "ANALYZE" grantable. If you were going to give the user owner permissions anyway then having a less-inclusive permission cannot hurt.
The whole allowing session-local statistic targets seems suspect but I suppose it could be useful for development. I don't imagine it being very helpful in production since the autovacuum process is eventually just going to reset them back using the server default target at some point. Maybe for temporary tables which themselves are session-local.
David J.
So the typical user doesn't know or even care that what they just did
needs to be analyzed. The situation is no worse than it is today. But
as someone who writes many scripts and applications to perform bulk
writing and data analysis I'd like those scripts to use restricted
authorization credentials while still being able to run ANALYZE between
performing the bulk DML and the running the SELECT statements needed to
get the newly generated data out of the database.
Maybe?:
CREATE OR REPLACE FUNCTION public.analyze_test(tbl_name character varying)
RETURNS void
LANGUAGE plpgsql
SECURITY DEFINER
AS $function$
BEGIN
EXECUTE 'ANALYZE ' || quote_ident(tbl_name);
END;
$function$
Yes, a security definer function - and setting execute permissions appropriately - would work. But it is a hack to work around a restriction that, in theory, need not exist. I understand that our implementation - namely the presence of a publically visible GUC and uninhibitied SET usage
- may make reality more complicated than this.
I guess I don't see why anyone other than the database owner and superuser (if that, it probably could be made to be fixed at startup) should be allowed to SET default_statistics_target. If a table owner wants to play with different levels they can always just ALTER TABLE SET - which has the benefit (though maybe this is undesirable in some instances...) of making the alteration effective during auto-vacuum runs.
ANALYZE is something that needs to happen frequently and commonly on a running system for proper operation. In comparison the statistic targets are basically frozen values aside from periods of experimentation. We have our priorities backwards if SET is preventing more user-friendly usage of ANALYZE.
David J.
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Mon, Feb 29, 2016 at 9:27 AM, Albe Laurenz <laurenz.albe@wien.gv.at>
> wrote:
>> See http://www.postgresql.org/docs/current/static/planner-stats.html
>> "The amount of information stored in pg_statistic by ANALYZE, in
>> particular the
>> maximum number of entries in the most_common_vals and histogram_bounds
>> arrays
>> for each column, can be set on a column-by-column basis using the
>> ALTER TABLE SET STATISTICS command, or globally by setting the
>> default_statistics_target configuration variable."
> Being able to run ANALYZE on a table in no way implies that I should be
> allowed to run ALTER TABLE SET STATISTICS on the same.
You're missing the point. If the table owner has *not* run ALTER TABLE
SET STATISTICS, which surely is the typical situation, then whoever runs
ANALYZE can control the volume of stats generated by setting
default_statistics_target locally in his session. Thus, if we allow
non-owners to run ANALYZE, they'd be able to mess things up by setting
the stats target either much lower or much higher than the table owner
expected. ("Much higher" would be bad in a different way than "much
lower", but still bad.)
Yes, this particular implementation detail escaped me at first. I've since posted new thoughts having taking this mis-feature into account.
I imagine this could be addressed by some rule about how if you don't
own the table then your default_statistics_target is overridden by
the global setting, but that would be a mess both conceptually and
implementation-wise.
It seems easy enough to simply disallow session-local changes to this GUC...but barring that this does weigh the decision toward having a GRANT ANALYZE on TABLE since the issue isn't running the ANALYZE but rather its interaction with SET default_statistics_target. An explicit permission allows the table and/or database to choose to provide a user this capability if desired without also requiring the user to become a full-blown owner of the table and thus able to make even bigger and more permanent changes - including altering the default_statistics_target for the table permanently.
David J.
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Mon, Feb 29, 2016 at 9:27 AM, Albe Laurenz <laurenz.albe@wien.gv.at>
> wrote:
>> See http://www.postgresql.org/docs/current/static/planner-stats.html
>> "The amount of information stored in pg_statistic by ANALYZE, in
>> particular the
>> maximum number of entries in the most_common_vals and histogram_bounds
>> arrays
>> for each column, can be set on a column-by-column basis using the
>> ALTER TABLE SET STATISTICS command, or globally by setting the
>> default_statistics_target configuration variable."
> Being able to run ANALYZE on a table in no way implies that I should be
> allowed to run ALTER TABLE SET STATISTICS on the same.
You're missing the point. If the table owner has *not* run ALTER TABLE
SET STATISTICS, which surely is the typical situation, then whoever runs
ANALYZE can control the volume of stats generated by setting
default_statistics_target locally in his session. Thus, if we allow
non-owners to run ANALYZE, they'd be able to mess things up by setting
the stats target either much lower or much higher than the table owner
expected. ("Much higher" would be bad in a different way than "much
lower", but still bad.)Yes, this particular implementation detail escaped me at first. I've since posted new thoughts having taking this mis-feature into account.I imagine this could be addressed by some rule about how if you don't
own the table then your default_statistics_target is overridden by
the global setting, but that would be a mess both conceptually and
implementation-wise.It seems easy enough to simply disallow session-local changes to this GUC...but barring that this does weigh the decision toward having a GRANT ANALYZE on TABLE since the issue isn't running the ANALYZE but rather its interaction with SET default_statistics_target. An explicit permission allows the table and/or database to choose to provide a user this capability if desired without also requiring the user to become a full-blown owner of the table and thus able to make even bigger and more permanent changes - including altering the default_statistics_target for the table permanently.
Pondering further there is likely quite a bit more to the GUC dynamics that make re-working them then that desirable.
But I'm also still not totally convinced that our level of prohibition buys us much safety. I recognize how this specific combination could be made to cause problems but my gut reaction is that anyone we'd give write access to a table would implicitly have a sufficient enough level of trust to allow them to do both SET and ANALYZE in the rare instance they felt doing so was necessary - and likewise would trust them not to issue SET arbitrarily but rather just use the more useful ANALYZE command.
We've basically prohibited ANALYZE because we cannot prohibit SETting the complementary GUC - one which I agree ordinary users have no business messing with.
Are there any security (data exposure, denial of service) protections that this prohibition is also manifesting? I'm inclined to disallow the non-argumented (i.e., database-wide) version of ANALYZE - in fact I'd probably make it superuser-only - as a form of parental guidance.
David J.
If we had plenty more bits to allow ANALYZE to be independently
GRANT'able, then maybe, but those are a limited resource.
On Mon, Feb 29, 2016 at 10:35 AM, Joshua D. Drake <jd@commandprompt.com> wrote:3. This can already be handled by GRANT:
* psql -U jd -h localhost;
* create table foo (id text);
* create role jd_role;
* alter table foo owner to jd_role;
* grant jd_role to boo;
* \c jd boo
* analyze foo;
On 02/29/2016 09:09 AM, David G. Johnston wrote:
Given these two things it seems the least difficult solution that doesn't make things any worse is to make "ANALYZE" grantable. If you were going to give the user owner permissions anyway then having a less-inclusive permission cannot hurt.==================================================================
My last comment sums things up pretty well. I assume someone will insist that a security definer function is "the officially supported way to do this", and if the community wants to agree then fine. Otherwise, if you are going to tell me to give someone ownership of a table so that they can ANALYZE it then no intermediate solution I propose can be considered off-limits on security grounds because nothing - relative to the table in question - is less secure.
Which means that, aside from effort, the main blocking factors here are code complexity (which I understand) and limited grant "bits" as Stephen puts it. So I pose the question: do any of the committers consider a grant bit too valuable to consume on an ANALYZE grant?
If that and/or general code complexity means this will not be added even if a patch was proposed for 9.7 then I'll move on and institute one of the hacks that has been proffered. Otherwise I have (more than) half a mind to find some way to get a patch written.
David J.
David, * David G. Johnston (david.g.johnston@gmail.com) wrote: > Which means that, aside from effort, the main blocking factors here are > code complexity (which I understand) and limited grant "bits" as Stephen > puts it. So I pose the question: do any of the committers consider a grant > bit too valuable to consume on an ANALYZE grant? I wasn't referring to "bits" as "things" to do but rather as actual zeros and ones- AclMode is a 32bit integer, of which the bottom half are 'regular' grantable rights and the top half are "with grant option" indications, meanly we've only got 16 to work with, and every object uses AclMode, so we have to support *all* kinds of GRANTs with those 16 bits. See src/include/nodes/parsenodes.h, around line 63. > If that and/or general code complexity means this will not be added even if > a patch was proposed for 9.7 then I'll move on and institute one of the > hacks that has been proffered. Otherwise I have (more than) half a mind to > find some way to get a patch written. I don't see any reason why the patch itself would be terribly difficult, but are we sure we'd want just ANALYZE and not VACUUM also? Which would have to be another bit, since those are pretty different actions. The question really is- what other things might we want as grantable rights in the future? Once these 16 bits are gone, it's a whole bunch of work to get more. Thanks! Stephen
Вложения
David,
* David G. Johnston (david.g.johnston@gmail.com) wrote:
> Which means that, aside from effort, the main blocking factors here are
> code complexity (which I understand) and limited grant "bits" as Stephen
> puts it. So I pose the question: do any of the committers consider a grant
> bit too valuable to consume on an ANALYZE grant?
I wasn't referring to "bits" as "things" to do but rather as actual
zeros and ones- AclMode is a 32bit integer, of which the bottom half are
'regular' grantable rights and the top half are "with grant option"
indications, meanly we've only got 16 to work with, and every object
uses AclMode, so we have to support *all* kinds of GRANTs with those 16
bits.
Yes, that is how I understood "bits"...sorry for the poor phrasing.
See src/include/nodes/parsenodes.h, around line 63.
> If that and/or general code complexity means this will not be added even if
> a patch was proposed for 9.7 then I'll move on and institute one of the
> hacks that has been proffered. Otherwise I have (more than) half a mind to
> find some way to get a patch written.
I don't see any reason why the patch itself would be terribly difficult,
but are we sure we'd want just ANALYZE and not VACUUM also? Which would
have to be another bit, since those are pretty different actions.
In the limited experience that prompted this requested the benefit of performing a VACUUM is significantly less than the benefit of performing ANALYZE, and the cost of the former is considerably higher. I'm quite content to leave VACUUM decisions to the auto-vacuum process which balances the benefit of removing bloat with the I/O cost of doing so.
The question really is- what other things might we want as grantable
rights in the future? Once these 16 bits are gone, it's a whole bunch
of work to get more.
If I am reading parsenodes.h correctly we presently use only 12 of 16 bits and those that are present all seem ancient. With no other existing need to add a single additional grantable option, let alone 4, I'm not see this as being particularly concerning.
Let someone else argue for inclusion of VACUUM before considering adding it - all I believe that we need is ANALYZE. I want programs doing ETL to be able to get the system into "good-enough" shape to be functional; maintenance processes can deal with the rest.
David J.
David, * David G. Johnston (david.g.johnston@gmail.com) wrote: > On Thu, Mar 24, 2016 at 4:51 AM, Stephen Frost <sfrost@snowman.net> wrote: > > I don't see any reason why the patch itself would be terribly difficult, > > but are we sure we'd want just ANALYZE and not VACUUM also? Which would > > have to be another bit, since those are pretty different actions. > > In the limited experience that prompted this requested the benefit of > performing a VACUUM is significantly less than the benefit of performing > ANALYZE, and the cost of the former is considerably higher. I'm quite > content to leave VACUUM decisions to the auto-vacuum process which balances > the benefit of removing bloat with the I/O cost of doing so. I guess I don't entirely follow that logic. autovacuum, even though it's name doesn't imply it, is *also* quite responsible for ensuring that ANALYZE is done regularly on the tables and even has options to control when ANALYZE is run which would it to run more frequently than vacuums. Further, a lot of ETL could have very good reason to want to run a VACUUM, especially with the changes that we continue to make which make that process less and less expensive of an operation to run. > > The question really is- what other things might we want as grantable > > rights in the future? Once these 16 bits are gone, it's a whole bunch > > of work to get more. > > If I am reading parsenodes.h correctly we presently use only 12 of 16 bits > and those that are present all seem ancient. With no other existing need > to add a single additional grantable option, let alone 4, I'm not see this > as being particularly concerning. They're not all ancient- TRUNCATE was added not that long ago and took quite a few years of convincing before it was accepted (I asked for it when I first started working on PG, some 15-or-so years ago and it wasn't actually included until 3 or 4 years ago, iirc). Further, as we add new features, new kinds of GRANTs can be needed. Consider the case of auditing, for example. When we finally get around to adding support for proper in-core auditing, it may be desirable for individuals other than the owner of a relation to be able to control the auditing of the table. > Let someone else argue for inclusion of VACUUM before considering adding it > - all I believe that we need is ANALYZE. I want programs doing ETL to be > able to get the system into "good-enough" shape to be functional; > maintenance processes can deal with the rest. ANALYZE is a maintenance process too, really, so I don't entirely buy your argument here. Either we support having these maintanence-type actions being performed by non-owners, or we don't and encourage everyone to configure autovacuum to meet their needs. Thanks! Stephen