we consider stat object as invalid if it is not owned by current user (#6130)

src/backend/distributed/commands/utility_hook.c

@@ -549,11 +549,11 @@ ProcessUtilityInternal(PlannedStmt *pstmt,
 		 * unwanted citus related warnings or early error logs related to invalid address.
 		 * Therefore, we first check if any address in the given statement is valid.
 		 * Then, we do not execute qualify and preprocess if none of the addresses are valid
-		 * to prevent before-mentioned citus related messages. PG will complain about the
+		 * or any address violates ownership rules to prevent before-mentioned citus related
-		 * invalid address, so we are safe to not execute qualify and preprocess. Also
+		 * messages. PG will complain about the invalid address or ownership violation, so we
-		 * note that we should not guard any step after standardProcess_Utility with
+		 * are safe to not execute qualify and preprocess. Also note that we should not guard
-		 * the enum state distOpsValidationState because PG would have already failed the
+		 * any step after standardProcess_Utility with the enum state distOpsValidationState
-		 * transaction.
+		 * because PG would have already failed the transaction.
 		 */
 		distOpsValidationState = DistOpsValidityState(parsetree, ops);
 
@@ -565,15 +565,16 @@ ProcessUtilityInternal(PlannedStmt *pstmt,
 		 * and fill them out how postgres would resolve them. This makes subsequent
 		 * deserialize calls for the statement portable to other postgres servers, the
 		 * workers in our case.
-		 * If there are no valid objects, let's skip the qualify and
+		 * If there are no valid objects or any object violates ownership, let's skip
-		 * preprocess, and do not diverge from Postgres in terms of error messages.
+		 * the qualify and preprocess, and do not diverge from Postgres in terms of
+		 * error messages.
 		 */
-		if (ops && ops->qualify && distOpsValidationState != HasNoneValidObject)
+		if (ops && ops->qualify && DistOpsInValidState(distOpsValidationState))
 		{
 			ops->qualify(parsetree);
 		}
 
-		if (ops && ops->preprocess && distOpsValidationState != HasNoneValidObject)
+		if (ops && ops->preprocess && DistOpsInValidState(distOpsValidationState))
 		{
 			ddlJobs = ops->preprocess(parsetree, queryString, context);
 		}

src/backend/distributed/utils/citus_depended_object.c

@@ -39,7 +39,10 @@
 #include "nodes/makefuncs.h"
 #include "nodes/nodeFuncs.h"
 #include "nodes/parsenodes.h"
+#include "parser/parse_type.h"
+#include "storage/large_object.h"
 #include "utils/lsyscache.h"
+#include "utils/syscache.h"
 
 /*
  * GUC hides any objects, which depends on citus extension, from pg meta class queries,
@@ -50,6 +53,8 @@ bool HideCitusDependentObjects = false;
 static Node * CreateCitusDependentObjectExpr(int pgMetaTableVarno, int pgMetaTableOid);
 static List * GetCitusDependedObjectArgs(int pgMetaTableVarno, int pgMetaTableOid);
 static bool AlterRoleSetStatementContainsAll(Node *node);
+static bool HasDropCommandViolatesOwnership(Node *node);
+static bool AnyObjectViolatesOwnership(DropStmt *dropStmt);
 
 /*
  * IsPgLocksTable returns true if RTE is pg_locks table.
@@ -334,13 +339,20 @@ DistOpsValidityState(Node *node, const DistributeObjectOps *ops)
 		 */
 		return NoAddressResolutionRequired;
 	}
+	else if (HasDropCommandViolatesOwnership(node))
+	{
+		/*
+		 * found object with an invalid ownership, PG will complain if there is any object
+		 * with an invalid ownership.
+		 */
+		return HasObjectWithInvalidOwnership;
+	}
 
 	if (ops && ops->address)
 	{
 		bool missingOk = true;
 		bool isPostprocess = false;
 		List *objectAddresses = ops->address(node, missingOk, isPostprocess);
-
 		ObjectAddress *objectAddress = NULL;
 		foreach_ptr(objectAddress, objectAddresses)
 		{
@@ -362,6 +374,18 @@ DistOpsValidityState(Node *node, const DistributeObjectOps *ops)
 }
 
 
+/*
+ * DistOpsInValidState returns true if given state is valid to execute
+ * preprocess and qualify steps.
+ */
+bool
+DistOpsInValidState(DistOpsValidationState distOpsValidationState)
+{
+	return distOpsValidationState == HasAtLeastOneValidObject || distOpsValidationState ==
+		   NoAddressResolutionRequired;
+}
+
+
 /*
  * AlterRoleSetStatementContainsAll returns true if the statement is a
  * ALTER ROLE ALL (SET / RESET).
@@ -384,3 +408,88 @@ AlterRoleSetStatementContainsAll(Node *node)
 
 	return false;
 }
+
+
+/*
+ * HasDropCommandViolatesOwnership returns true if any object in the given
+ * statement violates object ownership.
+ *
+ * Currently there is only one test which fails due to object ownership.
+ * The command that is failing is DROP. If in the future we hit other
+ * commands like this, we should expand this function.
+ */
+static bool
+HasDropCommandViolatesOwnership(Node *node)
+{
+	if (!IsA(node, DropStmt))
+	{
+		return false;
+	}
+
+	DropStmt *dropStmt = castNode(DropStmt, node);
+	if (AnyObjectViolatesOwnership(dropStmt))
+	{
+		return true;
+	}
+
+	return false;
+}
+
+
+/*
+ * AnyObjectViolatesOwnership return true if given object in stmt violates ownership.
+ */
+static bool
+AnyObjectViolatesOwnership(DropStmt *dropStmt)
+{
+	volatile ObjectAddress objectAddress = { 0 };
+	Relation relation = NULL;
+	bool objectViolatesOwnership = false;
+	ObjectType objectType = dropStmt->removeType;
+	bool missingOk = dropStmt->missing_ok;
+
+	Node *object = NULL;
+	foreach_ptr(object, dropStmt->objects)
+	{
+		PG_TRY();
+		{
+			objectAddress = get_object_address(objectType, object,
+											   &relation, AccessShareLock, missingOk);
+
+
+			if (OidIsValid(objectAddress.objectId))
+			{
+				/*
+				 * if object violates ownership, check_object_ownership will throw error.
+				 */
+				check_object_ownership(GetUserId(),
+									   objectType,
+									   objectAddress,
+									   object, relation);
+			}
+		}
+		PG_CATCH();
+		{
+			if (OidIsValid(objectAddress.objectId))
+			{
+				/* ownership violation */
+				objectViolatesOwnership = true;
+			}
+		}
+		PG_END_TRY();
+
+		if (relation != NULL)
+		{
+			relation_close(relation, AccessShareLock);
+			relation = NULL;
+		}
+
+		/* we found ownership violation, so can return here */
+		if (objectViolatesOwnership)
+		{
+			return true;
+		}
+	}
+
+	return false;
+}

src/include/distributed/citus_depended_object.h

@@ -23,6 +23,7 @@ typedef enum DistOpsValidationState
 {
 	HasAtLeastOneValidObject,
 	HasNoneValidObject,
+	HasObjectWithInvalidOwnership,
 	NoAddressResolutionRequired
 } DistOpsValidationState;
 
@@ -33,5 +34,6 @@ extern bool HideCitusDependentObjectsOnQueriesOfPgMetaTables(Node *node, void *c
 extern bool IsPgLocksTable(RangeTblEntry *rte);
 extern DistOpsValidationState DistOpsValidityState(Node *node, const
 												   DistributeObjectOps *ops);
+extern bool DistOpsInValidState(DistOpsValidationState distOpsValidationState);
 
 #endif /* CITUS_DEPENDED_OBJECT_H */

src/test/regress/expected/grant_on_function_propagation.out

@@ -828,11 +828,7 @@ SELECT proname, pronargs, proacl FROM pg_proc WHERE proname IN ('dist_float_avg'
 \c - - - :master_port
 SET search_path TO grant_on_function, public;
 DROP AGGREGATE dist_float_avg(float8), dist_float_avg_2(float8), no_dist_float_avg(float8), not_propagated_aggregate_user_test(float8);
-SELECT run_command_on_coordinator_and_workers('DROP SCHEMA grant_on_function CASCADE');
+DROP SCHEMA grant_on_function CASCADE;
- run_command_on_coordinator_and_workers
----------------------------------------------------------------------
-
-(1 row)
-
 DROP USER function_user_1, function_user_2, function_user_3, not_propagated_function_user_4;
 DROP USER procedure_user_1, procedure_user_2, procedure_user_3, not_propagated_procedure_user_4;
+DROP USER aggregate_user_1, aggregate_user_2, aggregate_user_3, not_propagated_aggregate_user_4;

src/test/regress/sql/grant_on_function_propagation.sql

@@ -492,6 +492,7 @@ SET search_path TO grant_on_function, public;
 
 DROP AGGREGATE dist_float_avg(float8), dist_float_avg_2(float8), no_dist_float_avg(float8), not_propagated_aggregate_user_test(float8);
 
-SELECT run_command_on_coordinator_and_workers('DROP SCHEMA grant_on_function CASCADE');
+DROP SCHEMA grant_on_function CASCADE;
 DROP USER function_user_1, function_user_2, function_user_3, not_propagated_function_user_4;
 DROP USER procedure_user_1, procedure_user_2, procedure_user_3, not_propagated_procedure_user_4;
+DROP USER aggregate_user_1, aggregate_user_2, aggregate_user_3, not_propagated_aggregate_user_4;