From 8ba6eb692f78c0899db470e4ad592a2d976c8396 Mon Sep 17 00:00:00 2001 From: Abhijit Subramanya Date: Mon, 22 Feb 2016 14:30:37 -0800 Subject: [PATCH] gp_truncate_error_log() function should first check for permissions on the master before dispatching the command to segments. gp_truncate_error_log() function first dispatches the permissions check to the segments and prints out the results before performing the check on the master. This makes it hard to test this function if the cluster is configured with a different number of segments than the test since different number of segments produce different number of lines in the output. Performing the check first on the master will cause the transaction to error out immediately and will be easier to test. --- src/backend/cdb/cdbsreh.c | 84 +++++++++---------- src/test/regress/output/external_table.source | 50 +++-------- 2 files changed, 52 insertions(+), 82 deletions(-) diff --git a/src/backend/cdb/cdbsreh.c b/src/backend/cdb/cdbsreh.c index 03c03b165f..a9771efaf1 100644 --- a/src/backend/cdb/cdbsreh.c +++ b/src/backend/cdb/cdbsreh.c @@ -978,48 +978,6 @@ gp_truncate_error_log(PG_FUNCTION_ARGS) relname = PG_GETARG_TEXT_P(0); - /* - * Dispatch the work to segments. - */ - if (Gp_role == GP_ROLE_DISPATCH) - { - int i, resultCount = 0; - StringInfoData sql, errbuf; - PGresult **results; - - initStringInfo(&sql); - initStringInfo(&errbuf); - - appendStringInfo(&sql, - "SELECT pg_catalog.gp_truncate_error_log('%s')", - text_to_cstring(relname)); - - results = cdbdisp_dispatchRMCommand(sql.data, true, &errbuf, - &resultCount); - - if (errbuf.len > 0) - elog(ERROR, "%s", errbuf.data); - Assert(resultCount > 0); - - for (i = 0; i < resultCount; i++) - { - Datum value; - bool isnull; - - if (PQresultStatus(results[i]) != PGRES_TUPLES_OK) - ereport(ERROR, - (errmsg("unexpected result from segment: %d", - PQresultStatus(results[i])))); - - value = ResultToDatum(results[i], 0, 0, boolin, &isnull); - allResults &= (!isnull && DatumGetBool(value)); - PQclear(results[i]); - } - - pfree(errbuf.data); - pfree(sql.data); - } - relname_str = text_to_cstring(relname); if (strcmp(relname_str, "*.*") == 0) { @@ -1066,6 +1024,48 @@ gp_truncate_error_log(PG_FUNCTION_ARGS) ErrorLogDelete(MyDatabaseId, relid); } + /* + * Dispatch the work to segments. + */ + if (Gp_role == GP_ROLE_DISPATCH) + { + int i, resultCount = 0; + StringInfoData sql, errbuf; + PGresult **results; + + initStringInfo(&sql); + initStringInfo(&errbuf); + + appendStringInfo(&sql, + "SELECT pg_catalog.gp_truncate_error_log(%s)", + quote_literal_internal(text_to_cstring(relname))); + + results = cdbdisp_dispatchRMCommand(sql.data, true, &errbuf, + &resultCount); + + if (errbuf.len > 0) + elog(ERROR, "%s", errbuf.data); + Assert(resultCount > 0); + + for (i = 0; i < resultCount; i++) + { + Datum value; + bool isnull; + + if (PQresultStatus(results[i]) != PGRES_TUPLES_OK) + ereport(ERROR, + (errmsg("unexpected result from segment: %d", + PQresultStatus(results[i])))); + + value = ResultToDatum(results[i], 0, 0, boolin, &isnull); + allResults &= (!isnull && DatumGetBool(value)); + PQclear(results[i]); + } + + pfree(errbuf.data); + pfree(sql.data); + } + /* Return true iif all segments return true. */ PG_RETURN_BOOL(allResults); } diff --git a/src/test/regress/output/external_table.source b/src/test/regress/output/external_table.source index 9285753b84..4a1efab89e 100644 --- a/src/test/regress/output/external_table.source +++ b/src/test/regress/output/external_table.source @@ -1044,25 +1044,13 @@ ERROR: permission denied for relation exttab_permissions_1 (seg2 slice1 @hostn SELECT COUNT(*) FROM gp_read_error_log('exttab_permissions_2'); ERROR: permission denied for relation exttab_permissions_2 (seg1 slice1 @hostname@:40001 pid=50437) SELECT gp_truncate_error_log('exttab_permissions_1'); -ERROR: permission denied for relation exttab_permissions_1 (seg0 @hostname@:40000 pid=50430) (cdbsreh.c:1081) -DETAIL: -permission denied for relation exttab_permissions_1 (seg1 @hostname@:40001 pid=50437) -permission denied for relation exttab_permissions_1 (seg2 @hostname@:40002 pid=50448) +ERROR: permission denied for relation exttab_permissions_1 SELECT gp_truncate_error_log('exttab_permissions_2'); -ERROR: permission denied for relation exttab_permissions_2 (seg0 @hostname@:40000 pid=50430) (cdbsreh.c:1081) -DETAIL: -permission denied for relation exttab_permissions_2 (seg1 @hostname@:40001 pid=50437) -permission denied for relation exttab_permissions_2 (seg2 @hostname@:40002 pid=50448) +ERROR: permission denied for relation exttab_permissions_2 SELECT gp_truncate_error_log('*'); -ERROR: must be owner of database regression (seg0 @hostname@:40000 pid=50430) (cdbsreh.c:1081) -DETAIL: -must be owner of database regression (seg1 @hostname@:40001 pid=50437) -must be owner of database regression (seg2 @hostname@:40002 pid=50448) +ERROR: must be owner of database regression SELECT gp_truncate_error_log('*.*'); -ERROR: must be superuser to delete all error log files (seg0 @hostname@:40000 pid=50430) (cdbsreh.c:1081) -DETAIL: -must be superuser to delete all error log files (seg1 @hostname@:40001 pid=50437) -must be superuser to delete all error log files (seg2 @hostname@:40002 pid=50448) +ERROR: must be superuser to delete all error log files RESET ROLE; DROP ROLE IF EXISTS exttab_superuser; CREATE ROLE exttab_superuser WITH SUPERUSER LOGIN; @@ -1146,29 +1134,17 @@ SET ROLE exttab_user2; SELECT COUNT(*) FROM gp_read_error_log('exttab_permissions_1'); ERROR: permission denied for relation exttab_permissions_1 (seg0 slice1 @hostname@:40000 pid=51060) SELECT gp_truncate_error_log('*'); -ERROR: must be owner of database exttab_db (seg0 @hostname@:40000 pid=51060) (cdbsreh.c:1081) -DETAIL: -must be owner of database exttab_db (seg1 @hostname@:40001 pid=51061) -must be owner of database exttab_db (seg2 @hostname@:40002 pid=51062) +ERROR: must be owner of database exttab_db SELECT gp_truncate_error_log('*.*'); -ERROR: must be superuser to delete all error log files (seg0 @hostname@:40000 pid=51060) (cdbsreh.c:1081) -DETAIL: -must be superuser to delete all error log files (seg1 @hostname@:40001 pid=51061) -must be superuser to delete all error log files (seg2 @hostname@:40002 pid=51062) +ERROR: must be superuser to delete all error log files SELECT gp_truncate_error_log('exttab_permissions_1'); -ERROR: permission denied for relation exttab_permissions_1 (seg0 @hostname@:40000 pid=51060) (cdbsreh.c:1081) -DETAIL: -permission denied for relation exttab_permissions_1 (seg1 @hostname@:40001 pid=51061) -permission denied for relation exttab_permissions_1 (seg2 @hostname@:40002 pid=51062) +ERROR: permission denied for relation exttab_permissions_1 SET ROLE exttab_user1; -- Database owner can still not perform read / truncate on specific tables. This follows the same mechanism as TRUNCATE table. SELECT COUNT(*) FROM gp_read_error_log('exttab_permissions_1'); ERROR: permission denied for relation exttab_permissions_1 (seg1 slice1 @hostname@:40001 pid=51061) SELECT gp_truncate_error_log('exttab_permissions_1'); -ERROR: permission denied for relation exttab_permissions_1 (seg0 @hostname@:40000 pid=51060) (cdbsreh.c:1081) -DETAIL: -permission denied for relation exttab_permissions_1 (seg1 @hostname@:40001 pid=51061) -permission denied for relation exttab_permissions_1 (seg2 @hostname@:40002 pid=51062) +ERROR: permission denied for relation exttab_permissions_1 SELECT gp_truncate_error_log('*'); gp_truncate_error_log ----------------------- @@ -1177,10 +1153,7 @@ SELECT gp_truncate_error_log('*'); -- should fail SELECT gp_truncate_error_log('*.*'); -ERROR: must be superuser to delete all error log files (seg0 @hostname@:40000 pid=51060) (cdbsreh.c:1081) -DETAIL: -must be superuser to delete all error log files (seg1 @hostname@:40001 pid=51061) -must be superuser to delete all error log files (seg2 @hostname@:40002 pid=51062) +ERROR: must be superuser to delete all error log files RESET ROLE; SELECT * FROM gp_read_error_log('exttab_permissions_1'); cmdtime | relname | filename | linenum | bytenum | errmsg | rawdata | rawbytes @@ -1217,10 +1190,7 @@ SET ROLE errlog_exttab_user4; SELECT COUNT(*) FROM gp_read_error_log('exttab_permissions_3'); ERROR: permission denied for relation exttab_permissions_3 (seg0 slice1 @hostname@:40000 pid=51087) SELECT gp_truncate_error_log('exttab_permissions_3'); -ERROR: permission denied for relation exttab_permissions_3 (seg0 @hostname@:40000 pid=51087) (cdbsreh.c:1081) -DETAIL: -permission denied for relation exttab_permissions_3 (seg1 @hostname@:40001 pid=51088) -permission denied for relation exttab_permissions_3 (seg2 @hostname@:40002 pid=51089) +ERROR: permission denied for relation exttab_permissions_3 -- should go through fine with table owner SET ROLE errlog_exttab_user3; SELECT gp_truncate_error_log('exttab_permissions_3'); -- GitLab