Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support GRANT/REVOKE on distributed database #3779

Merged
merged 1 commit into from Nov 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -17,6 +17,7 @@ accidentally triggering the load of a previous DB version.**
* #3784 Fix DISTINCT ON queries returning incorrect results on distributed hypertables
* #3801 Fail size utility functions when data nodes do not respond
* #3781 Segfault in fill_result_error()
* #3779 Support GRANT/REVOKE on distributed database

**Thanks**
* @cbisnett for reporting and fixing a typo in an error message
Expand Down
81 changes: 81 additions & 0 deletions tsl/src/deparse.c
Expand Up @@ -922,3 +922,84 @@ deparse_oid_function_call_coll(Oid funcid, Oid collation, unsigned int num_args,

return result;
}

const char *
deparse_grant_revoke_on_database(Node *node, const char *dbname)
{
GrantStmt *stmt = castNode(GrantStmt, node);
ListCell *lc;

/*
GRANT { { CREATE | CONNECT | TEMPORARY | TEMP } [, ...] | ALL [ PRIVILEGES ] }
ON DATABASE database_name [, ...]
TO role_specification [, ...] [ WITH GRANT OPTION ]
[ GRANTED BY role_specification ]

REVOKE [ GRANT OPTION FOR ]
{ { CREATE | CONNECT | TEMPORARY | TEMP } [, ...] | ALL [ PRIVILEGES ] }
ON DATABASE database_name [, ...]
FROM role_specification [, ...]
[ GRANTED BY role_specification ]
[ CASCADE | RESTRICT ]
*/
StringInfo command = makeStringInfo();

/* GRANT/REVOKE */
if (stmt->is_grant)
appendStringInfoString(command, "GRANT ");
else
appendStringInfoString(command, "REVOKE ");

/* privileges [, ...] | ALL */
if (stmt->privileges == NULL)
{
/* ALL */
appendStringInfoString(command, "ALL ");
}
else
{
foreach (lc, stmt->privileges)
{
AccessPriv *priv = lfirst(lc);

appendStringInfo(command,
"%s%s ",
priv->priv_name,
lnext_compat(stmt->privileges, lc) != NULL ? "," : "");
Comment on lines +966 to +968
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit

Suggested change
"%s%s ",
priv->priv_name,
lnext_compat(stmt->privileges, lc) != NULL ? "," : "");
"%s%c ",
priv->priv_name,
lnext_compat(stmt->privileges, lc) != NULL ? ';' : '');

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what it the proposal here, because format will emit error on the empty char string. Not sure if it is even supported for the C formatting function to accept '', I really doubt that it could have numeric equivalent.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be a whitespace. But nevermind, strings work too. But I usually prefer to work with correct types if you are adding a single char.

}
}

/* Set database name of the data node */
appendStringInfo(command, "ON DATABASE %s ", quote_identifier(dbname));

/* TO/FROM role_spec [, ...] */
if (stmt->is_grant)
appendStringInfoString(command, "TO ");
else
appendStringInfoString(command, "FROM ");

foreach (lc, stmt->grantees)
{
RoleSpec *role_spec = lfirst(lc);

appendStringInfo(command,
"%s%s ",
role_spec->rolename,
lnext_compat(stmt->grantees, lc) != NULL ? "," : "");
pmwkaa marked this conversation as resolved.
Show resolved Hide resolved
}

if (stmt->grant_option)
appendStringInfoString(command, "WITH GRANT OPTION ");

#if PG14
/* [ GRANTED BY role_specification ] */
if (stmt->grantor)
appendStringInfo(command, "GRANTED BY %s ", quote_identifier(stmt->grantor->rolename));
#endif

/* CASCADE | RESTRICT */
if (!stmt->is_grant && stmt->behavior == DROP_CASCADE)
appendStringInfoString(command, "CASCADE");

return command->data;
}
1 change: 1 addition & 0 deletions tsl/src/deparse.h
Expand Up @@ -49,5 +49,6 @@ DeparsedHypertableCommands *deparse_get_distributed_hypertable_create_command(Hy

const char *deparse_func_call(FunctionCallInfo finfo);
const char *deparse_oid_function_call_coll(Oid funcid, Oid collation, unsigned int num_args, ...);
const char *deparse_grant_revoke_on_database(Node *node, const char *dbname);

#endif
37 changes: 35 additions & 2 deletions tsl/src/remote/dist_commands.c
Expand Up @@ -84,8 +84,6 @@ ts_dist_cmd_collect_responses(List *requests)
* If "transactional" is false then it means that the SQL should be executed
* in autocommit (implicit statement level commit) mode without the need for
* an explicit 2PC from the access node.
*
* If "multiple_cmds" is false then "sql" and "params" are not iterated over.
*/
DistCmdResult *
ts_dist_multi_cmds_params_invoke_on_data_nodes(List *cmd_descriptors, List *data_nodes,
Expand Down Expand Up @@ -201,6 +199,41 @@ ts_dist_cmd_invoke_on_data_nodes_using_search_path(const char *sql, const char *
return results;
}

DistCmdResult *
ts_dist_multi_cmds_invoke_on_data_nodes_using_search_path(List *cmd_descriptors,
const char *search_path, List *node_names,
bool transactional)
{
DistCmdResult *set_result;
DistCmdResult *results;
bool set_search_path = search_path != NULL;

if (set_search_path)
{
char *set_request = psprintf("SET search_path = %s, pg_catalog", search_path);

set_result = ts_dist_cmd_invoke_on_data_nodes(set_request, node_names, transactional);
if (set_result)
ts_dist_cmd_close_response(set_result);

pfree(set_request);
}

results =
ts_dist_multi_cmds_params_invoke_on_data_nodes(cmd_descriptors, node_names, transactional);

if (set_search_path)
{
set_result = ts_dist_cmd_invoke_on_data_nodes("SET search_path = pg_catalog",
node_names,
transactional);
if (set_result)
ts_dist_cmd_close_response(set_result);
}

return results;
}

DistCmdResult *
ts_dist_cmd_invoke_on_all_data_nodes(const char *sql)
{
Expand Down
2 changes: 2 additions & 0 deletions tsl/src/remote/dist_commands.h
Expand Up @@ -26,6 +26,8 @@ extern DistCmdResult *ts_dist_cmd_invoke_on_data_nodes(const char *sql, List *no
bool transactional);
extern DistCmdResult *ts_dist_cmd_params_invoke_on_data_nodes(const char *sql, StmtParams *params,
List *data_nodes, bool transactional);
extern DistCmdResult *ts_dist_multi_cmds_invoke_on_data_nodes_using_search_path(
List *cmd_descriptors, const char *search_path, List *node_names, bool transactional);
extern DistCmdResult *ts_dist_cmd_invoke_on_data_nodes_using_search_path(const char *sql,
const char *search_path,
List *node_names,
Expand Down