Re: [COMMITTERS] pgsql: Code review for row security.

2014-09-25 Thread Stephen Frost
* Andres Freund (and...@2ndquadrant.com) wrote:
> I've not really looked at the code, but I doubt
>   if (policy1->hassublinks != policy2->hassublinks);
>   return false;
> was what you intended. Note the trailing ";".

Gah, quite right.  Will fix.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [COMMITTERS] pgsql: Code review for row security.

2014-09-25 Thread Andres Freund
On 2014-09-24 20:33:54 +, Stephen Frost wrote:
> Code review for row security.
> 
> Buildfarm member tick identified an issue where the policies in the
> relcache for a relation were were being replaced underneath a running
> query, leading to segfaults while processing the policies to be added
> to a query.  Similar to how TupleDesc RuleLocks are handled, add in a
> equalRSDesc() function to check if the policies have actually changed
> and, if not, swap back the rsdesc field (using the original instead of
> the temporairly built one; the whole structure is swapped and then
> specific fields swapped back).  This now passes a CLOBBER_CACHE_ALWAYS
> for me and should resolve the buildfarm error.

I've not really looked at the code, but I doubt
if (policy1->hassublinks != policy2->hassublinks);
return false;
was what you intended. Note the trailing ";".

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


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


[COMMITTERS] pgsql: Code review for row security.

2014-09-24 Thread Stephen Frost
Code review for row security.

Buildfarm member tick identified an issue where the policies in the
relcache for a relation were were being replaced underneath a running
query, leading to segfaults while processing the policies to be added
to a query.  Similar to how TupleDesc RuleLocks are handled, add in a
equalRSDesc() function to check if the policies have actually changed
and, if not, swap back the rsdesc field (using the original instead of
the temporairly built one; the whole structure is swapped and then
specific fields swapped back).  This now passes a CLOBBER_CACHE_ALWAYS
for me and should resolve the buildfarm error.

In addition to addressing this, add a new chapter in Data Definition
under Privileges which explains row security and provides examples of
its usage, change \d to always list policies (even if row security is
disabled- but note that it is disabled, or enabled with no policies),
rework check_role_for_policy (it really didn't need the entire policy,
but it did need to be using has_privs_of_role()), and change the field
in pg_class to relrowsecurity from relhasrowsecurity, based on
Heikki's suggestion.  Also from Heikki, only issue SET ROW_SECURITY in
pg_restore when talking to a 9.5+ server, list Bypass RLS in \du, and
document --enable-row-security options for pg_dump and pg_restore.

Lastly, fix a number of minor whitespace and typo issues from Heikki,
Dimitri, add a missing #include, per Peter E, fix a few minor
variable-assigned-but-not-used and resource leak issues from Coverity
and add tab completion for role attribute bypassrls as well.

Branch
--
master

Details
---
http://git.postgresql.org/pg/commitdiff/6550b901fe7c47c03775400e0c790c6c1234a017

Modified Files
--
doc/src/sgml/catalogs.sgml   |   11 ++-
doc/src/sgml/config.sgml |4 +-
doc/src/sgml/ddl.sgml|  168 ++
doc/src/sgml/ref/alter_table.sgml|2 +-
doc/src/sgml/ref/create_policy.sgml  |2 +-
doc/src/sgml/ref/pg_dump.sgml|   17 
doc/src/sgml/ref/pg_restore.sgml |   23 +
src/backend/catalog/heap.c   |2 +-
src/backend/catalog/system_views.sql |2 +-
src/backend/commands/policy.c|   17 ++--
src/backend/commands/tablecmds.c |4 +-
src/backend/rewrite/rowsecurity.c|   31 ---
src/backend/utils/adt/ri_triggers.c  |4 +-
src/backend/utils/cache/relcache.c   |   91 +-
src/bin/pg_dump/pg_backup_archiver.c |   11 ++-
src/bin/pg_dump/pg_dump.c|   36 
src/bin/pg_dump/pg_dump.h|2 +-
src/bin/pg_dump/pg_restore.c |2 +-
src/bin/psql/describe.c  |   79 +---
src/bin/psql/tab-complete.c  |   38 
src/include/catalog/catversion.h |2 +-
src/include/catalog/pg_class.h   |4 +-
src/include/commands/policy.h|7 +-
src/test/regress/expected/rules.out  |2 +-
24 files changed, 439 insertions(+), 122 deletions(-)


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