From ea7aa6712dc8135b0aadd53943adcbbc4b63dd5b Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Tue, 29 Apr 2025 14:22:29 +0300 Subject: [PATCH 1/3] Move stat view implementations into a submodule (#7975) Also move serialize_distributed_ddls into commands submodule, seems like an oversight from last year (by me). --- src/backend/distributed/Makefile | 2 +- src/backend/distributed/README.md | 2 +- src/backend/distributed/commands/database.c | 2 +- src/backend/distributed/commands/multi_copy.c | 2 +- .../{ => commands}/serialize_distributed_ddls.c | 2 +- .../distributed/connection/connection_management.c | 2 +- src/backend/distributed/executor/adaptive_executor.c | 2 +- src/backend/distributed/executor/citus_custom_scan.c | 4 ++-- src/backend/distributed/executor/insert_select_executor.c | 2 +- src/backend/distributed/executor/local_executor.c | 2 +- src/backend/distributed/executor/merge_executor.c | 2 +- src/backend/distributed/planner/deparse_shard_query.c | 2 +- src/backend/distributed/planner/distributed_planner.c | 2 +- src/backend/distributed/shared_library_init.c | 6 +++--- .../distributed/sql/udfs/citus_stat_counters/13.1-1.sql | 8 ++++---- .../distributed/sql/udfs/citus_stat_counters/latest.sql | 8 ++++---- src/backend/distributed/{executor => stats}/query_stats.c | 2 +- src/backend/distributed/{ => stats}/stat_counters.c | 2 +- .../{utils/citus_stat_tenants.c => stats/stat_tenants.c} | 4 ++-- src/backend/distributed/test/citus_stat_tenants.c | 2 +- src/backend/distributed/utils/maintenanced.c | 2 +- .../{ => commands}/serialize_distributed_ddls.h | 0 src/include/distributed/{ => stats}/query_stats.h | 0 src/include/distributed/{ => stats}/stat_counters.h | 0 .../{utils/citus_stat_tenants.h => stats/stat_tenants.h} | 2 +- 25 files changed, 32 insertions(+), 32 deletions(-) rename src/backend/distributed/{ => commands}/serialize_distributed_ddls.c (99%) rename src/backend/distributed/{executor => stats}/query_stats.c (99%) rename src/backend/distributed/{ => stats}/stat_counters.c (99%) rename src/backend/distributed/{utils/citus_stat_tenants.c => stats/stat_tenants.c} (99%) rename src/include/distributed/{ => commands}/serialize_distributed_ddls.h (100%) rename src/include/distributed/{ => stats}/query_stats.h (100%) rename src/include/distributed/{ => stats}/stat_counters.h (100%) rename src/include/distributed/{utils/citus_stat_tenants.h => stats/stat_tenants.h} (99%) diff --git a/src/backend/distributed/Makefile b/src/backend/distributed/Makefile index 8f28b04b0..e0e23dc25 100644 --- a/src/backend/distributed/Makefile +++ b/src/backend/distributed/Makefile @@ -18,7 +18,7 @@ generated_downgrade_sql_files += $(patsubst %,$(citus_abs_srcdir)/build/sql/%,$( DATA_built = $(generated_sql_files) # directories with source files -SUBDIRS = . commands connection ddl deparser executor metadata operations planner progress relay safeclib shardsplit test transaction utils worker clock +SUBDIRS = . commands connection ddl deparser executor metadata operations planner progress relay safeclib shardsplit stats test transaction utils worker clock # enterprise modules SUBDIRS += replication diff --git a/src/backend/distributed/README.md b/src/backend/distributed/README.md index b372d20ff..b4e139a1a 100644 --- a/src/backend/distributed/README.md +++ b/src/backend/distributed/README.md @@ -2718,7 +2718,7 @@ And beside these, Citus itself also provides some additional statistic views to ### Citus stat counters Citus keeps track of several stat counters and exposes them via the `citus_stat_counters` view. The counters are tracked once `citus.enable_stat_counters` is set to true. Also, `citus_stat_counters_reset()` can be used to reset the counters for the given database if a database id different than 0 (default, InvalidOid) is provided, otherwise, it resets the counters for the current database. -Details about the implementation and its caveats can be found in the header comment of [stat_counters.c](/src/backend/distributed/stat_counters.c). However, at the high level; +Details about the implementation and its caveats can be found in the header comment of [stat_counters.c](/src/backend/distributed/stats/stat_counters.c). However, at the high level; 1. We allocate a shared memory array of length `MaxBackends` so that each backend has its own counter slot to reduce the contention while incrementing the counters at the runtime. 2. We also allocate a shared hash, whose entries correspond to individual databases. Then, when a backend exits, it first aggregates its counters to the relevant entry in the shared hash, and then it resets its own counters because the same counter slot might be reused by another backend later. diff --git a/src/backend/distributed/commands/database.c b/src/backend/distributed/commands/database.c index bbc3981b5..3586fa2cd 100644 --- a/src/backend/distributed/commands/database.c +++ b/src/backend/distributed/commands/database.c @@ -35,6 +35,7 @@ #include "distributed/adaptive_executor.h" #include "distributed/commands.h" +#include "distributed/commands/serialize_distributed_ddls.h" #include "distributed/commands/utility_hook.h" #include "distributed/comment.h" #include "distributed/deparse_shard_query.h" @@ -46,7 +47,6 @@ #include "distributed/metadata_utility.h" #include "distributed/multi_executor.h" #include "distributed/relation_access_tracking.h" -#include "distributed/serialize_distributed_ddls.h" #include "distributed/shard_cleaner.h" #include "distributed/worker_protocol.h" #include "distributed/worker_transaction.h" diff --git a/src/backend/distributed/commands/multi_copy.c b/src/backend/distributed/commands/multi_copy.c index d83e7c1fb..1f0abbb56 100644 --- a/src/backend/distributed/commands/multi_copy.c +++ b/src/backend/distributed/commands/multi_copy.c @@ -106,7 +106,7 @@ #include "distributed/resource_lock.h" #include "distributed/shard_pruning.h" #include "distributed/shared_connection_stats.h" -#include "distributed/stat_counters.h" +#include "distributed/stats/stat_counters.h" #include "distributed/transmit.h" #include "distributed/version_compat.h" #include "distributed/worker_protocol.h" diff --git a/src/backend/distributed/serialize_distributed_ddls.c b/src/backend/distributed/commands/serialize_distributed_ddls.c similarity index 99% rename from src/backend/distributed/serialize_distributed_ddls.c rename to src/backend/distributed/commands/serialize_distributed_ddls.c index 11d10905b..2cca64fb0 100644 --- a/src/backend/distributed/serialize_distributed_ddls.c +++ b/src/backend/distributed/commands/serialize_distributed_ddls.c @@ -26,9 +26,9 @@ #include "distributed/adaptive_executor.h" #include "distributed/argutils.h" +#include "distributed/commands/serialize_distributed_ddls.h" #include "distributed/deparse_shard_query.h" #include "distributed/resource_lock.h" -#include "distributed/serialize_distributed_ddls.h" PG_FUNCTION_INFO_V1(citus_internal_acquire_citus_advisory_object_class_lock); diff --git a/src/backend/distributed/connection/connection_management.c b/src/backend/distributed/connection/connection_management.c index 2f78a1ee9..407de776b 100644 --- a/src/backend/distributed/connection/connection_management.c +++ b/src/backend/distributed/connection/connection_management.c @@ -39,7 +39,7 @@ #include "distributed/remote_commands.h" #include "distributed/run_from_same_connection.h" #include "distributed/shared_connection_stats.h" -#include "distributed/stat_counters.h" +#include "distributed/stats/stat_counters.h" #include "distributed/time_constants.h" #include "distributed/version_compat.h" #include "distributed/worker_log_messages.h" diff --git a/src/backend/distributed/executor/adaptive_executor.c b/src/backend/distributed/executor/adaptive_executor.c index b429829a2..895f01ae7 100644 --- a/src/backend/distributed/executor/adaptive_executor.c +++ b/src/backend/distributed/executor/adaptive_executor.c @@ -171,7 +171,7 @@ #include "distributed/repartition_join_execution.h" #include "distributed/resource_lock.h" #include "distributed/shared_connection_stats.h" -#include "distributed/stat_counters.h" +#include "distributed/stats/stat_counters.h" #include "distributed/subplan_execution.h" #include "distributed/transaction_identifier.h" #include "distributed/transaction_management.h" diff --git a/src/backend/distributed/executor/citus_custom_scan.c b/src/backend/distributed/executor/citus_custom_scan.c index 89503b0c9..5ba60b5ad 100644 --- a/src/backend/distributed/executor/citus_custom_scan.c +++ b/src/backend/distributed/executor/citus_custom_scan.c @@ -43,9 +43,9 @@ #include "distributed/multi_executor.h" #include "distributed/multi_router_planner.h" #include "distributed/multi_server_executor.h" -#include "distributed/query_stats.h" #include "distributed/shard_utils.h" -#include "distributed/stat_counters.h" +#include "distributed/stats/query_stats.h" +#include "distributed/stats/stat_counters.h" #include "distributed/subplan_execution.h" #include "distributed/worker_log_messages.h" #include "distributed/worker_protocol.h" diff --git a/src/backend/distributed/executor/insert_select_executor.c b/src/backend/distributed/executor/insert_select_executor.c index d53f60594..9ed1962fa 100644 --- a/src/backend/distributed/executor/insert_select_executor.c +++ b/src/backend/distributed/executor/insert_select_executor.c @@ -50,7 +50,7 @@ #include "distributed/repartition_executor.h" #include "distributed/resource_lock.h" #include "distributed/shardinterval_utils.h" -#include "distributed/stat_counters.h" +#include "distributed/stats/stat_counters.h" #include "distributed/subplan_execution.h" #include "distributed/transaction_management.h" #include "distributed/version_compat.h" diff --git a/src/backend/distributed/executor/local_executor.c b/src/backend/distributed/executor/local_executor.c index d824d8f31..18563c763 100644 --- a/src/backend/distributed/executor/local_executor.c +++ b/src/backend/distributed/executor/local_executor.c @@ -104,8 +104,8 @@ #include "distributed/query_utils.h" #include "distributed/relation_access_tracking.h" #include "distributed/remote_commands.h" /* to access LogRemoteCommands */ +#include "distributed/stats/stat_tenants.h" #include "distributed/transaction_management.h" -#include "distributed/utils/citus_stat_tenants.h" #include "distributed/version_compat.h" #include "distributed/worker_protocol.h" diff --git a/src/backend/distributed/executor/merge_executor.c b/src/backend/distributed/executor/merge_executor.c index 2b2f20451..d0f01dcf2 100644 --- a/src/backend/distributed/executor/merge_executor.c +++ b/src/backend/distributed/executor/merge_executor.c @@ -26,7 +26,7 @@ #include "distributed/multi_partitioning_utils.h" #include "distributed/multi_router_planner.h" #include "distributed/repartition_executor.h" -#include "distributed/stat_counters.h" +#include "distributed/stats/stat_counters.h" #include "distributed/subplan_execution.h" static void ExecuteSourceAtWorkerAndRepartition(CitusScanState *scanState); diff --git a/src/backend/distributed/planner/deparse_shard_query.c b/src/backend/distributed/planner/deparse_shard_query.c index 6b8ad3fde..2542d931a 100644 --- a/src/backend/distributed/planner/deparse_shard_query.c +++ b/src/backend/distributed/planner/deparse_shard_query.c @@ -39,7 +39,7 @@ #include "distributed/multi_physical_planner.h" #include "distributed/multi_router_planner.h" #include "distributed/shard_utils.h" -#include "distributed/utils/citus_stat_tenants.h" +#include "distributed/stats/stat_tenants.h" #include "distributed/version_compat.h" diff --git a/src/backend/distributed/planner/distributed_planner.c b/src/backend/distributed/planner/distributed_planner.c index 7f8f827ea..193e2f250 100644 --- a/src/backend/distributed/planner/distributed_planner.c +++ b/src/backend/distributed/planner/distributed_planner.c @@ -66,7 +66,7 @@ #include "distributed/recursive_planning.h" #include "distributed/shard_utils.h" #include "distributed/shardinterval_utils.h" -#include "distributed/utils/citus_stat_tenants.h" +#include "distributed/stats/stat_tenants.h" #include "distributed/version_compat.h" #include "distributed/worker_shard_visibility.h" diff --git a/src/backend/distributed/shared_library_init.c b/src/backend/distributed/shared_library_init.c index 688e247ca..430eb8555 100644 --- a/src/backend/distributed/shared_library_init.c +++ b/src/backend/distributed/shared_library_init.c @@ -89,7 +89,6 @@ #include "distributed/placement_connection.h" #include "distributed/priority.h" #include "distributed/query_pushdown_planning.h" -#include "distributed/query_stats.h" #include "distributed/recursive_planning.h" #include "distributed/reference_table_utils.h" #include "distributed/relation_access_tracking.h" @@ -105,13 +104,14 @@ #include "distributed/shardsplit_shared_memory.h" #include "distributed/shared_connection_stats.h" #include "distributed/shared_library_init.h" -#include "distributed/stat_counters.h" #include "distributed/statistics_collection.h" +#include "distributed/stats/query_stats.h" +#include "distributed/stats/stat_counters.h" +#include "distributed/stats/stat_tenants.h" #include "distributed/subplan_execution.h" #include "distributed/time_constants.h" #include "distributed/transaction_management.h" #include "distributed/transaction_recovery.h" -#include "distributed/utils/citus_stat_tenants.h" #include "distributed/utils/directory.h" #include "distributed/worker_log_messages.h" #include "distributed/worker_manager.h" diff --git a/src/backend/distributed/sql/udfs/citus_stat_counters/13.1-1.sql b/src/backend/distributed/sql/udfs/citus_stat_counters/13.1-1.sql index 3db1325b1..3792bf7c8 100644 --- a/src/backend/distributed/sql/udfs/citus_stat_counters/13.1-1.sql +++ b/src/backend/distributed/sql/udfs/citus_stat_counters/13.1-1.sql @@ -1,14 +1,14 @@ -- See the comments for the function in --- src/backend/distributed/stat_counters.c for more details. +-- src/backend/distributed/stats/stat_counters.c for more details. CREATE OR REPLACE FUNCTION pg_catalog.citus_stat_counters( database_id oid DEFAULT 0, -- must always be the first column or you should accordingly update - -- StoreDatabaseStatsIntoTupStore() function in src/backend/distributed/stat_counters.c + -- StoreDatabaseStatsIntoTupStore() function in src/backend/distributed/stats/stat_counters.c OUT database_id oid, -- Following stat counter columns must be in the same order as the - -- StatType enum defined in src/include/distributed/stat_counters.h + -- StatType enum defined in src/include/distributed/stats/stat_counters.h OUT connection_establishment_succeeded bigint, OUT connection_establishment_failed bigint, OUT connection_reused bigint, @@ -16,7 +16,7 @@ CREATE OR REPLACE FUNCTION pg_catalog.citus_stat_counters( OUT query_execution_multi_shard bigint, -- must always be the last column or you should accordingly update - -- StoreDatabaseStatsIntoTupStore() function in src/backend/distributed/stat_counters.c + -- StoreDatabaseStatsIntoTupStore() function in src/backend/distributed/stats/stat_counters.c OUT stats_reset timestamp with time zone ) RETURNS SETOF RECORD diff --git a/src/backend/distributed/sql/udfs/citus_stat_counters/latest.sql b/src/backend/distributed/sql/udfs/citus_stat_counters/latest.sql index 3db1325b1..3792bf7c8 100644 --- a/src/backend/distributed/sql/udfs/citus_stat_counters/latest.sql +++ b/src/backend/distributed/sql/udfs/citus_stat_counters/latest.sql @@ -1,14 +1,14 @@ -- See the comments for the function in --- src/backend/distributed/stat_counters.c for more details. +-- src/backend/distributed/stats/stat_counters.c for more details. CREATE OR REPLACE FUNCTION pg_catalog.citus_stat_counters( database_id oid DEFAULT 0, -- must always be the first column or you should accordingly update - -- StoreDatabaseStatsIntoTupStore() function in src/backend/distributed/stat_counters.c + -- StoreDatabaseStatsIntoTupStore() function in src/backend/distributed/stats/stat_counters.c OUT database_id oid, -- Following stat counter columns must be in the same order as the - -- StatType enum defined in src/include/distributed/stat_counters.h + -- StatType enum defined in src/include/distributed/stats/stat_counters.h OUT connection_establishment_succeeded bigint, OUT connection_establishment_failed bigint, OUT connection_reused bigint, @@ -16,7 +16,7 @@ CREATE OR REPLACE FUNCTION pg_catalog.citus_stat_counters( OUT query_execution_multi_shard bigint, -- must always be the last column or you should accordingly update - -- StoreDatabaseStatsIntoTupStore() function in src/backend/distributed/stat_counters.c + -- StoreDatabaseStatsIntoTupStore() function in src/backend/distributed/stats/stat_counters.c OUT stats_reset timestamp with time zone ) RETURNS SETOF RECORD diff --git a/src/backend/distributed/executor/query_stats.c b/src/backend/distributed/stats/query_stats.c similarity index 99% rename from src/backend/distributed/executor/query_stats.c rename to src/backend/distributed/stats/query_stats.c index 319041b56..8e767d9d5 100644 --- a/src/backend/distributed/executor/query_stats.c +++ b/src/backend/distributed/stats/query_stats.c @@ -32,7 +32,7 @@ #include "distributed/hash_helpers.h" #include "distributed/multi_executor.h" #include "distributed/multi_server_executor.h" -#include "distributed/query_stats.h" +#include "distributed/stats/query_stats.h" #include "distributed/tuplestore.h" #include "distributed/version_compat.h" diff --git a/src/backend/distributed/stat_counters.c b/src/backend/distributed/stats/stat_counters.c similarity index 99% rename from src/backend/distributed/stat_counters.c rename to src/backend/distributed/stats/stat_counters.c index 7773b36b2..03151befd 100644 --- a/src/backend/distributed/stat_counters.c +++ b/src/backend/distributed/stats/stat_counters.c @@ -76,7 +76,7 @@ #include "distributed/argutils.h" #include "distributed/metadata_cache.h" -#include "distributed/stat_counters.h" +#include "distributed/stats/stat_counters.h" #include "distributed/tuplestore.h" diff --git a/src/backend/distributed/utils/citus_stat_tenants.c b/src/backend/distributed/stats/stat_tenants.c similarity index 99% rename from src/backend/distributed/utils/citus_stat_tenants.c rename to src/backend/distributed/stats/stat_tenants.c index 1ca4fc6f1..17cd3bf46 100644 --- a/src/backend/distributed/utils/citus_stat_tenants.c +++ b/src/backend/distributed/stats/stat_tenants.c @@ -1,6 +1,6 @@ /*------------------------------------------------------------------------- * - * citus_stat_tenants.c + * stat_tenants.c * Routines related to the multi tenant monitor. * * Copyright (c) Citus Data, Inc. @@ -35,9 +35,9 @@ #include "distributed/log_utils.h" #include "distributed/metadata_cache.h" #include "distributed/multi_executor.h" +#include "distributed/stats/stat_tenants.h" #include "distributed/tenant_schema_metadata.h" #include "distributed/tuplestore.h" -#include "distributed/utils/citus_stat_tenants.h" static void AttributeMetricsIfApplicable(void); diff --git a/src/backend/distributed/test/citus_stat_tenants.c b/src/backend/distributed/test/citus_stat_tenants.c index b8fe305c6..000e3fc02 100644 --- a/src/backend/distributed/test/citus_stat_tenants.c +++ b/src/backend/distributed/test/citus_stat_tenants.c @@ -15,7 +15,7 @@ #include "sys/time.h" -#include "distributed/utils/citus_stat_tenants.h" +#include "distributed/stats/stat_tenants.h" PG_FUNCTION_INFO_V1(sleep_until_next_period); diff --git a/src/backend/distributed/utils/maintenanced.c b/src/backend/distributed/utils/maintenanced.c index 9cef13539..e6bf3d00c 100644 --- a/src/backend/distributed/utils/maintenanced.c +++ b/src/backend/distributed/utils/maintenanced.c @@ -54,10 +54,10 @@ #include "distributed/maintenanced.h" #include "distributed/metadata_cache.h" #include "distributed/metadata_sync.h" -#include "distributed/query_stats.h" #include "distributed/resource_lock.h" #include "distributed/shard_cleaner.h" #include "distributed/statistics_collection.h" +#include "distributed/stats/query_stats.h" #include "distributed/transaction_recovery.h" #include "distributed/version_compat.h" diff --git a/src/include/distributed/serialize_distributed_ddls.h b/src/include/distributed/commands/serialize_distributed_ddls.h similarity index 100% rename from src/include/distributed/serialize_distributed_ddls.h rename to src/include/distributed/commands/serialize_distributed_ddls.h diff --git a/src/include/distributed/query_stats.h b/src/include/distributed/stats/query_stats.h similarity index 100% rename from src/include/distributed/query_stats.h rename to src/include/distributed/stats/query_stats.h diff --git a/src/include/distributed/stat_counters.h b/src/include/distributed/stats/stat_counters.h similarity index 100% rename from src/include/distributed/stat_counters.h rename to src/include/distributed/stats/stat_counters.h diff --git a/src/include/distributed/utils/citus_stat_tenants.h b/src/include/distributed/stats/stat_tenants.h similarity index 99% rename from src/include/distributed/utils/citus_stat_tenants.h rename to src/include/distributed/stats/stat_tenants.h index 573502606..c13ea59e7 100644 --- a/src/include/distributed/utils/citus_stat_tenants.h +++ b/src/include/distributed/stats/stat_tenants.h @@ -1,6 +1,6 @@ /*------------------------------------------------------------------------- * - * citus_stat_tenants.h + * stat_tenants.h * Routines related to the multi tenant monitor. * * Copyright (c) Citus Data, Inc. From d4dd44e715e337ad84edbabe3cf612c0c96c0c9d Mon Sep 17 00:00:00 2001 From: Colm Date: Wed, 30 Apr 2025 18:03:52 +0100 Subject: [PATCH 2/3] Propagate SECURITY LABEL on tables and columns. (#7956) Issue #7709 asks for security labels on columns to be propagated, to support the `anon` extension. Before, Citus supported security labels on roles (#7735) and this PR adds support for propagating security labels on tables and columns. All scenarios that involve propagating metadata for a Citus table now include the security labels on the table and on the columns of the table. These scenarios are: - When a table becomes distributed using `create_distributed_table()` or `create_reference_table()`, its security labels (if any) are propageted. - When a security label is defined on a distributed table, or one of its columns, the label is propagated. - When a node is added to a Citus cluster, all distributed tables have their security labels propagated. - When a column of a distributed table is dropped, any security labels on the column are also dropped. - When a column is added to a distributed table, security labels can be defined on the column and are propagated. - Security labels on a distributed table or its columns are not propagated when `citus.enable_metadata_sync` is enabled. Regress test `seclabel` is extended with tests to cover these scenarios. The implementation is somewhat involved because it impacts DDL propagation of Citus tables, but can be broken down as follows: - distributed_object_ops has `Role_SecLabel`, `Table_SecLabel` and `Column_SecLabel` to take care of security labels on roles, tables and columns. `Any_SecLabel` is used for all other security labels and is essentially a nop. - Deparser support - `DeparseRoleSecLabelStmt()`, `DeparseTableSecLabelStmt()` and `DeparseColumnSecLabelStmt()` take care of deparsing security label statements on roles, tables and columns respectively. - When reconstructing the DDL for a citus table, security labels on the table or its columns are included by having `GetPreLoadTableCreationCommands()` call a new function `CreateSecurityLabelCommands()` to take care of any security labels on the table or its columns. - When changing a distributed table name to a shard name before running a command locally on a worker, function `RelayEventExtendNames()` checks for security labels on a table or its columns. --- .../commands/distribute_object_ops.c | 53 +- src/backend/distributed/commands/seclabel.c | 98 +++- .../deparser/deparse_seclabel_stmts.c | 134 +++-- src/backend/distributed/metadata/distobject.c | 32 ++ .../distributed/operations/node_protocol.c | 109 ++++ .../distributed/relay/relay_event_utility.c | 53 +- src/include/distributed/commands.h | 4 +- src/include/distributed/deparser.h | 4 +- src/include/distributed/metadata/distobject.h | 1 + src/test/regress/expected/seclabel.out | 475 ++++++++++++++++-- src/test/regress/sql/seclabel.sql | 154 +++++- 11 files changed, 999 insertions(+), 118 deletions(-) diff --git a/src/backend/distributed/commands/distribute_object_ops.c b/src/backend/distributed/commands/distribute_object_ops.c index 0e8887905..8d1c6bc23 100644 --- a/src/backend/distributed/commands/distribute_object_ops.c +++ b/src/backend/distributed/commands/distribute_object_ops.c @@ -399,10 +399,37 @@ static DistributeObjectOps Any_Rename = { .markDistributed = false, }; static DistributeObjectOps Any_SecLabel = { - .deparse = DeparseSecLabelStmt, + .deparse = NULL, .qualify = NULL, .preprocess = NULL, - .postprocess = PostprocessSecLabelStmt, + .postprocess = PostprocessAnySecLabelStmt, + .operationType = DIST_OPS_ALTER, + .address = SecLabelStmtObjectAddress, + .markDistributed = false, +}; +static DistributeObjectOps Role_SecLabel = { + .deparse = DeparseRoleSecLabelStmt, + .qualify = NULL, + .preprocess = NULL, + .postprocess = PostprocessRoleSecLabelStmt, + .operationType = DIST_OPS_ALTER, + .address = SecLabelStmtObjectAddress, + .markDistributed = false, +}; +static DistributeObjectOps Table_SecLabel = { + .deparse = DeparseTableSecLabelStmt, + .qualify = NULL, + .preprocess = NULL, + .postprocess = PostprocessTableOrColumnSecLabelStmt, + .operationType = DIST_OPS_ALTER, + .address = SecLabelStmtObjectAddress, + .markDistributed = false, +}; +static DistributeObjectOps Column_SecLabel = { + .deparse = DeparseColumnSecLabelStmt, + .qualify = NULL, + .preprocess = NULL, + .postprocess = PostprocessTableOrColumnSecLabelStmt, .operationType = DIST_OPS_ALTER, .address = SecLabelStmtObjectAddress, .markDistributed = false, @@ -2119,7 +2146,27 @@ GetDistributeObjectOps(Node *node) case T_SecLabelStmt: { - return &Any_SecLabel; + SecLabelStmt *stmt = castNode(SecLabelStmt, node); + switch (stmt->objtype) + { + case OBJECT_ROLE: + { + return &Role_SecLabel; + } + + case OBJECT_TABLE: + { + return &Table_SecLabel; + } + + case OBJECT_COLUMN: + { + return &Column_SecLabel; + } + + default: + return &Any_SecLabel; + } } case T_RenameStmt: diff --git a/src/backend/distributed/commands/seclabel.c b/src/backend/distributed/commands/seclabel.c index 1d274a056..2d4f31d4a 100644 --- a/src/backend/distributed/commands/seclabel.c +++ b/src/backend/distributed/commands/seclabel.c @@ -15,19 +15,18 @@ #include "distributed/commands/utility_hook.h" #include "distributed/coordinator_protocol.h" #include "distributed/deparser.h" +#include "distributed/listutils.h" #include "distributed/log_utils.h" #include "distributed/metadata/distobject.h" #include "distributed/metadata_sync.h" - /* - * PostprocessSecLabelStmt prepares the commands that need to be run on all workers to assign - * security labels on distributed objects, currently supporting just Role objects. - * It also ensures that all object dependencies exist on all - * nodes for the object in the SecLabelStmt. + * PostprocessRoleSecLabelStmt prepares the commands that need to be run on all workers to assign + * security labels on distributed roles. It also ensures that all object dependencies exist on all + * nodes for the role in the SecLabelStmt. */ List * -PostprocessSecLabelStmt(Node *node, const char *queryString) +PostprocessRoleSecLabelStmt(Node *node, const char *queryString) { if (!EnableAlterRolePropagation || !ShouldPropagate()) { @@ -42,34 +41,91 @@ PostprocessSecLabelStmt(Node *node, const char *queryString) return NIL; } - if (secLabelStmt->objtype != OBJECT_ROLE) + EnsurePropagationToCoordinator(); + EnsureAllObjectDependenciesExistOnAllNodes(objectAddresses); + + const char *secLabelCommands = DeparseTreeNode((Node *) secLabelStmt); + List *commandList = list_make3(DISABLE_DDL_PROPAGATION, + (void *) secLabelCommands, + ENABLE_DDL_PROPAGATION); + return NodeDDLTaskList(REMOTE_NODES, commandList); +} + + +/* + * PostprocessTableOrColumnSecLabelStmt prepares the commands that need to be run on all + * workers to assign security labels on distributed tables or the columns of a distributed + * table. It also ensures that all object dependencies exist on all nodes for the table in + * the SecLabelStmt. + */ +List * +PostprocessTableOrColumnSecLabelStmt(Node *node, const char *queryString) +{ + if (!EnableAlterRolePropagation || !ShouldPropagate()) { - /* - * If we are not in the coordinator, we don't want to interrupt the security - * label command with notices, the user expects that from the worker node - * the command will not be propagated - */ - if (EnableUnsupportedFeatureMessages && IsCoordinator()) - { - ereport(NOTICE, (errmsg("not propagating SECURITY LABEL commands whose " - "object type is not role"), - errhint("Connect to worker nodes directly to manually " - "run the same SECURITY LABEL command."))); - } return NIL; } + SecLabelStmt *secLabelStmt = castNode(SecLabelStmt, node); + + List *objectAddresses = GetObjectAddressListFromParseTree(node, false, true); + if (!IsAnyParentObjectDistributed(objectAddresses)) + { + return NIL; + } EnsurePropagationToCoordinator(); EnsureAllObjectDependenciesExistOnAllNodes(objectAddresses); const char *secLabelCommands = DeparseTreeNode((Node *) secLabelStmt); - List *commandList = list_make3(DISABLE_DDL_PROPAGATION, (void *) secLabelCommands, ENABLE_DDL_PROPAGATION); + List *DDLJobs = NodeDDLTaskList(REMOTE_NODES, commandList); + ListCell *lc = NULL; - return NodeDDLTaskList(REMOTE_NODES, commandList); + /* + * The label is for a table or a column, so we need to set the targetObjectAddress + * of the DDLJob to the relationId of the table. This is needed to ensure that + * the search path is correctly set for the remote security label command; it + * needs to be able to resolve the table that the label is being defined on. + */ + Assert(list_length(objectAddresses) == 1); + ObjectAddress *target = linitial(objectAddresses); + Oid relationId = target->objectId; + Assert(relationId != InvalidOid); + + foreach(lc, DDLJobs) + { + DDLJob *ddlJob = (DDLJob *) lfirst(lc); + ObjectAddressSet(ddlJob->targetObjectAddress, RelationRelationId, relationId); + } + + return DDLJobs; +} + + +/* + * PostprocessAnySecLabelStmt is used for any other object types + * that are not supported by Citus. It issues a notice to the client + * if appropriate. Is effectively a nop. + */ +List * +PostprocessAnySecLabelStmt(Node *node, const char *queryString) +{ + /* + * If we are not in the coordinator, we don't want to interrupt the security + * label command with notices, the user expects that from the worker node + * the command will not be propagated + */ + if (EnableUnsupportedFeatureMessages && IsCoordinator()) + { + ereport(NOTICE, (errmsg("not propagating SECURITY LABEL commands whose " + "object type is not role or table or column"), + errhint("Connect to worker nodes directly to manually " + "run the same SECURITY LABEL command."))); + } + return NIL; } diff --git a/src/backend/distributed/deparser/deparse_seclabel_stmts.c b/src/backend/distributed/deparser/deparse_seclabel_stmts.c index ffe775b76..761aa7acf 100644 --- a/src/backend/distributed/deparser/deparse_seclabel_stmts.c +++ b/src/backend/distributed/deparser/deparse_seclabel_stmts.c @@ -10,37 +10,16 @@ #include "postgres.h" +#include "catalog/namespace.h" #include "nodes/parsenodes.h" #include "utils/builtins.h" #include "distributed/deparser.h" -static void AppendSecLabelStmt(StringInfo buf, SecLabelStmt *stmt); - -/* - * DeparseSecLabelStmt builds and returns a string representing of the - * SecLabelStmt for application on a remote server. - */ -char * -DeparseSecLabelStmt(Node *node) -{ - SecLabelStmt *secLabelStmt = castNode(SecLabelStmt, node); - StringInfoData buf = { 0 }; - initStringInfo(&buf); - - AppendSecLabelStmt(&buf, secLabelStmt); - - return buf.data; -} - - -/* - * AppendSecLabelStmt generates the string representation of the - * SecLabelStmt and appends it to the buffer. - */ static void -AppendSecLabelStmt(StringInfo buf, SecLabelStmt *stmt) +BeginSecLabel(StringInfo buf, SecLabelStmt *stmt) { + initStringInfo(buf); appendStringInfoString(buf, "SECURITY LABEL "); if (stmt->provider != NULL) @@ -49,31 +28,84 @@ AppendSecLabelStmt(StringInfo buf, SecLabelStmt *stmt) } appendStringInfoString(buf, "ON "); - - switch (stmt->objtype) - { - case OBJECT_ROLE: - { - appendStringInfo(buf, "ROLE %s ", quote_identifier(strVal(stmt->object))); - break; - } - - /* normally, we shouldn't reach this */ - default: - { - ereport(ERROR, (errmsg("unsupported security label statement for" - " deparsing"))); - } - } - - appendStringInfoString(buf, "IS "); - - if (stmt->label != NULL) - { - appendStringInfo(buf, "%s", quote_literal_cstr(stmt->label)); - } - else - { - appendStringInfoString(buf, "NULL"); - } +} + + +static void +EndSecLabel(StringInfo buf, SecLabelStmt *stmt) +{ + appendStringInfo(buf, "IS %s", (stmt->label != NULL) ? + quote_literal_cstr(stmt->label) : "NULL"); +} + + +/* + * DeparseRoleSecLabelStmt builds and returns a string representation of the + * SecLabelStmt for application on a remote server. The SecLabelStmt is for + * a role object. + */ +char * +DeparseRoleSecLabelStmt(Node *node) +{ + SecLabelStmt *secLabelStmt = castNode(SecLabelStmt, node); + char *role_name = strVal(secLabelStmt->object); + StringInfoData buf = { 0 }; + + BeginSecLabel(&buf, secLabelStmt); + appendStringInfo(&buf, "ROLE %s ", quote_identifier(role_name)); + EndSecLabel(&buf, secLabelStmt); + + return buf.data; +} + + +/* + * DeparseTableSecLabelStmt builds and returns a string representation of the + * SecLabelStmt for application on a remote server. The SecLabelStmt is for a + * table. + */ +char * +DeparseTableSecLabelStmt(Node *node) +{ + SecLabelStmt *secLabelStmt = castNode(SecLabelStmt, node); + List *names = (List *) secLabelStmt->object; + StringInfoData buf = { 0 }; + + BeginSecLabel(&buf, secLabelStmt); + appendStringInfo(&buf, "TABLE %s", quote_identifier(strVal(linitial(names)))); + if (list_length(names) > 1) + { + appendStringInfo(&buf, ".%s", quote_identifier(strVal(lsecond(names)))); + } + appendStringInfoString(&buf, " "); + EndSecLabel(&buf, secLabelStmt); + + return buf.data; +} + + +/* + * DeparseColumnSecLabelStmt builds and returns a string representation of the + * SecLabelStmt for application on a remote server. The SecLabelStmt is for a + * column of a distributed table. + */ +char * +DeparseColumnSecLabelStmt(Node *node) +{ + SecLabelStmt *secLabelStmt = castNode(SecLabelStmt, node); + List *names = (List *) secLabelStmt->object; + StringInfoData buf = { 0 }; + + BeginSecLabel(&buf, secLabelStmt); + appendStringInfo(&buf, "COLUMN %s.%s", + quote_identifier(strVal(linitial(names))), + quote_identifier(strVal(lsecond(names)))); + if (list_length(names) > 2) + { + appendStringInfo(&buf, ".%s", quote_identifier(strVal(lthird(names)))); + } + appendStringInfoString(&buf, " "); + EndSecLabel(&buf, secLabelStmt); + + return buf.data; } diff --git a/src/backend/distributed/metadata/distobject.c b/src/backend/distributed/metadata/distobject.c index daa51eb75..43bd3877a 100644 --- a/src/backend/distributed/metadata/distobject.c +++ b/src/backend/distributed/metadata/distobject.c @@ -566,6 +566,38 @@ IsAnyObjectDistributed(const List *addresses) } +/* + * IsAnyParentObjectDistributed - true if at least one of the + * given addresses is distributed. If an address has a non-zero + * objectSubId, it checks the parent object (the object with + * the same classId and objid, but with objectSubId = 0). For + * example, a column address will check the table address. + * If the address has a zero objectSubId, it checks the address + * itself. + */ +bool +IsAnyParentObjectDistributed(const List *addresses) +{ + bool isDistributed = false; + ListCell *lc = NULL; + foreach(lc, addresses) + { + ObjectAddress *address = (ObjectAddress *) lfirst(lc); + int32 savedObjectSubId = address->objectSubId; + address->objectSubId = 0; + isDistributed = IsObjectDistributed(address); + address->objectSubId = savedObjectSubId; + + if (isDistributed) + { + break; + } + } + + return isDistributed; +} + + /* * GetDistributedObjectAddressList returns a list of ObjectAddresses that contains all * distributed objects as marked in pg_dist_object diff --git a/src/backend/distributed/operations/node_protocol.c b/src/backend/distributed/operations/node_protocol.c index 8a633e3dc..8e253de41 100644 --- a/src/backend/distributed/operations/node_protocol.c +++ b/src/backend/distributed/operations/node_protocol.c @@ -36,6 +36,7 @@ #include "catalog/pg_constraint.h" #include "catalog/pg_index.h" #include "catalog/pg_namespace.h" +#include "catalog/pg_seclabel.h" #include "catalog/pg_type.h" #include "commands/sequence.h" #include "foreign/foreign.h" @@ -57,6 +58,7 @@ #include "distributed/citus_ruleutils.h" #include "distributed/commands.h" #include "distributed/coordinator_protocol.h" +#include "distributed/deparser.h" #include "distributed/listutils.h" #include "distributed/metadata_cache.h" #include "distributed/metadata_sync.h" @@ -83,6 +85,7 @@ static char * CitusCreateAlterColumnarTableSet(char *qualifiedRelationName, const ColumnarOptions *options); static char * GetTableDDLCommandColumnar(void *context); static TableDDLCommand * ColumnarGetTableOptionsDDL(Oid relationId); +static List * CreateSecurityLabelCommands(Oid relationId); /* exports for SQL callable functions */ PG_FUNCTION_INFO_V1(master_get_table_metadata); @@ -665,6 +668,9 @@ GetPreLoadTableCreationCommands(Oid relationId, List *policyCommands = CreatePolicyCommands(relationId); tableDDLEventList = list_concat(tableDDLEventList, policyCommands); + List *securityLabelCommands = CreateSecurityLabelCommands(relationId); + tableDDLEventList = list_concat(tableDDLEventList, securityLabelCommands); + /* revert back to original search_path */ PopEmptySearchPath(saveNestLevel); @@ -833,6 +839,109 @@ GetTableRowLevelSecurityCommands(Oid relationId) } +/* + * CreateSecurityLabelCommands - return the SECURITY LABEL commands on + * the table identified by relationId. It is used by GetPreLoadTableCreationCommands() + * to reconstruct the security labels on the table and its columns. + */ +static List * +CreateSecurityLabelCommands(Oid relationId) +{ + List *securityLabelCommands = NIL; + + if (!RegularTable(relationId)) /* should be an Assert ? */ + { + return securityLabelCommands; + } + + Relation pg_seclabel = table_open(SecLabelRelationId, AccessShareLock); + ScanKeyData skey[1]; + ScanKeyInit(&skey[0], Anum_pg_seclabel_objoid, BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(relationId)); + SysScanDesc scan = systable_beginscan(pg_seclabel, SecLabelObjectIndexId, + true, NULL, 1, &skey[0]); + HeapTuple tuple = NULL; + List *table_name = NIL; + Relation relation = NULL; + TupleDesc tupleDescriptor = NULL; + List *securityLabelStmts = NULL; + ListCell *lc; + + while (HeapTupleIsValid(tuple = systable_getnext(scan))) + { + SecLabelStmt *secLabelStmt = makeNode(SecLabelStmt); + + if (relation == NULL) + { + relation = relation_open(relationId, AccessShareLock); + if (!RelationIsVisible(relationId)) + { + char *nsname = get_namespace_name(RelationGetNamespace(relation)); + table_name = lappend(table_name, makeString(nsname)); + } + char *relname = get_rel_name(relationId); + table_name = lappend(table_name, makeString(relname)); + } + + Datum datumArray[Natts_pg_seclabel]; + bool isNullArray[Natts_pg_seclabel]; + + heap_deform_tuple(tuple, RelationGetDescr(pg_seclabel), datumArray, + isNullArray); + int subObjectId = DatumGetInt32( + datumArray[Anum_pg_seclabel_objsubid - 1]); + secLabelStmt->provider = TextDatumGetCString( + datumArray[Anum_pg_seclabel_provider - 1]); + secLabelStmt->label = TextDatumGetCString( + datumArray[Anum_pg_seclabel_label - 1]); + + if (subObjectId > 0) + { + /* Its a column; construct the name */ + secLabelStmt->objtype = OBJECT_COLUMN; + List *col_name = list_copy(table_name); + + if (tupleDescriptor == NULL) + { + tupleDescriptor = RelationGetDescr(relation); + } + + Form_pg_attribute attrForm = TupleDescAttr(tupleDescriptor, subObjectId - 1); + char *attributeName = NameStr(attrForm->attname); + col_name = lappend(col_name, makeString(attributeName)); + + secLabelStmt->object = (Node *) col_name; + } + else + { + Assert(subObjectId == 0); + secLabelStmt->objtype = OBJECT_TABLE; + secLabelStmt->object = (Node *) table_name; + } + + securityLabelStmts = lappend(securityLabelStmts, secLabelStmt); + } + + foreach(lc, securityLabelStmts) + { + Node *stmt = (Node *) lfirst(lc); + char *secLabelStmtString = DeparseTreeNode(stmt); + TableDDLCommand *secLabelCommand = makeTableDDLCommandString(secLabelStmtString); + securityLabelCommands = lappend(securityLabelCommands, secLabelCommand); + } + + systable_endscan(scan); + table_close(pg_seclabel, AccessShareLock); + + if (relation != NULL) + { + relation_close(relation, AccessShareLock); + } + + return securityLabelCommands; +} + + /* * IndexImpliedByAConstraint is a helper function to be used while scanning * pg_index. It returns true if the index identified by the given indexForm is diff --git a/src/backend/distributed/relay/relay_event_utility.c b/src/backend/distributed/relay/relay_event_utility.c index 630c783e5..f89dba138 100644 --- a/src/backend/distributed/relay/relay_event_utility.c +++ b/src/backend/distributed/relay/relay_event_utility.c @@ -591,6 +591,58 @@ RelayEventExtendNames(Node *parseTree, char *schemaName, uint64 shardId) break; } + case T_SecLabelStmt: + { + SecLabelStmt *secLabelStmt = (SecLabelStmt *) parseTree; + + /* Should be looking at a security label for a table or column */ + if (secLabelStmt->objtype == OBJECT_TABLE || secLabelStmt->objtype == + OBJECT_COLUMN) + { + List *qualified_name = (List *) secLabelStmt->object; + String *table_name = NULL; + + switch (list_length(qualified_name)) + { + case 1: + { + table_name = castNode(String, linitial(qualified_name)); + break; + } + + case 2: + case 3: + { + table_name = castNode(String, lsecond(qualified_name)); + break; + } + + default: + { + /* Unlikely, but just in case */ + ereport(ERROR, (errmsg( + "unhandled name type in security label; name is: \"%s\"", + NameListToString(qualified_name)))); + break; + } + } + + /* Now change the table name: -> */ + char *relationName = strVal(table_name); + AppendShardIdToName(&relationName, shardId); + strVal(table_name) = relationName; + } + else + { + ereport(WARNING, (errmsg( + "unsafe object type in security label statement"), + errdetail("Object type: %u", + (uint32) secLabelStmt->objtype))); + } + + break; /* End of handling Security Label */ + } + default: { ereport(WARNING, (errmsg("unsafe statement type in name extension"), @@ -846,7 +898,6 @@ AppendShardIdToName(char **name, uint64 shardId) { SafeSnprintf(extendedName, NAMEDATALEN, "%s%s", (*name), shardIdAndSeparator); } - /* * Otherwise, we need to truncate the name further to accommodate * a sufficient hash value. The resulting name will avoid collision diff --git a/src/include/distributed/commands.h b/src/include/distributed/commands.h index a6e6bf6ec..19919e32c 100644 --- a/src/include/distributed/commands.h +++ b/src/include/distributed/commands.h @@ -546,7 +546,9 @@ extern List * AlterSchemaRenameStmtObjectAddress(Node *node, bool missing_ok, bo isPostprocess); /* seclabel.c - forward declarations*/ -extern List * PostprocessSecLabelStmt(Node *node, const char *queryString); +extern List * PostprocessAnySecLabelStmt(Node *node, const char *queryString); +extern List * PostprocessRoleSecLabelStmt(Node *node, const char *queryString); +extern List * PostprocessTableOrColumnSecLabelStmt(Node *node, const char *queryString); extern List * SecLabelStmtObjectAddress(Node *node, bool missing_ok, bool isPostprocess); extern void citus_test_object_relabel(const ObjectAddress *object, const char *seclabel); diff --git a/src/include/distributed/deparser.h b/src/include/distributed/deparser.h index 66c697f03..362f0d59e 100644 --- a/src/include/distributed/deparser.h +++ b/src/include/distributed/deparser.h @@ -292,7 +292,9 @@ extern void QualifyTextSearchConfigurationCommentStmt(Node *node); extern void QualifyTextSearchDictionaryCommentStmt(Node *node); /* forward declarations for deparse_seclabel_stmts.c */ -extern char * DeparseSecLabelStmt(Node *node); +extern char * DeparseRoleSecLabelStmt(Node *node); +extern char * DeparseTableSecLabelStmt(Node *node); +extern char * DeparseColumnSecLabelStmt(Node *node); /* forward declarations for deparse_sequence_stmts.c */ extern char * DeparseDropSequenceStmt(Node *node); diff --git a/src/include/distributed/metadata/distobject.h b/src/include/distributed/metadata/distobject.h index e98e6ee86..9348f9e8c 100644 --- a/src/include/distributed/metadata/distobject.h +++ b/src/include/distributed/metadata/distobject.h @@ -21,6 +21,7 @@ extern bool ObjectExists(const ObjectAddress *address); extern bool CitusExtensionObject(const ObjectAddress *objectAddress); extern bool IsAnyObjectDistributed(const List *addresses); +extern bool IsAnyParentObjectDistributed(const List *addresses); extern bool ClusterHasDistributedFunctionWithDistArgument(void); extern void MarkObjectDistributed(const ObjectAddress *distAddress); extern void MarkObjectDistributedWithName(const ObjectAddress *distAddress, char *name, diff --git a/src/test/regress/expected/seclabel.out b/src/test/regress/expected/seclabel.out index ca6c6f984..bf85b3b89 100644 --- a/src/test/regress/expected/seclabel.out +++ b/src/test/regress/expected/seclabel.out @@ -1,8 +1,10 @@ -- -- SECLABEL -- --- Test suite for SECURITY LABEL ON ROLE statements +-- Test suite for SECURITY LABEL statements: +-- SECURITY LABEL ON IS -- +-- Citus can propagate ROLE, TABLE and COLUMN objects -- first we remove one of the worker nodes to be able to test -- citus_add_node later SELECT citus_remove_node('localhost', :worker_2_port); @@ -28,7 +30,8 @@ SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORD (2 rows) RESET citus.enable_metadata_sync; --- check that we only support propagating for roles +-- check that we only support propagating for roles, tables and columns; +-- support for VIEW and FUNCTION is not there (yet) SET citus.shard_replication_factor to 1; -- distributed table CREATE TABLE a (a int); @@ -43,22 +46,12 @@ CREATE VIEW v_dist AS SELECT * FROM a; -- distributed function CREATE FUNCTION notice(text) RETURNS void LANGUAGE plpgsql AS $$ BEGIN RAISE NOTICE '%', $1; END; $$; -SECURITY LABEL ON TABLE a IS 'citus_classified'; -NOTICE: not propagating SECURITY LABEL commands whose object type is not role -HINT: Connect to worker nodes directly to manually run the same SECURITY LABEL command. SECURITY LABEL ON FUNCTION notice IS 'citus_unclassified'; -NOTICE: not propagating SECURITY LABEL commands whose object type is not role +NOTICE: not propagating SECURITY LABEL commands whose object type is not role or table or column HINT: Connect to worker nodes directly to manually run the same SECURITY LABEL command. SECURITY LABEL ON VIEW v_dist IS 'citus_classified'; -NOTICE: not propagating SECURITY LABEL commands whose object type is not role +NOTICE: not propagating SECURITY LABEL commands whose object type is not role or table or column HINT: Connect to worker nodes directly to manually run the same SECURITY LABEL command. -SELECT node_type, result FROM get_citus_tests_label_provider_labels('a') ORDER BY node_type; - node_type | result ---------------------------------------------------------------------- - coordinator | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} - worker_1 | -(2 rows) - SELECT node_type, result FROM get_citus_tests_label_provider_labels('notice(text)') ORDER BY node_type; node_type | result --------------------------------------------------------------------- @@ -74,17 +67,9 @@ SELECT node_type, result FROM get_citus_tests_label_provider_labels('v_dist') OR (2 rows) \c - - - :worker_1_port -SECURITY LABEL ON TABLE a IS 'citus_classified'; SECURITY LABEL ON FUNCTION notice IS 'citus_unclassified'; SECURITY LABEL ON VIEW v_dist IS 'citus_classified'; \c - - - :master_port -SELECT node_type, result FROM get_citus_tests_label_provider_labels('a') ORDER BY node_type; - node_type | result ---------------------------------------------------------------------- - coordinator | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} - worker_1 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} -(2 rows) - SELECT node_type, result FROM get_citus_tests_label_provider_labels('notice(text)') ORDER BY node_type; node_type | result --------------------------------------------------------------------- @@ -99,10 +84,8 @@ SELECT node_type, result FROM get_citus_tests_label_provider_labels('v_dist') OR worker_1 | {"label": "citus_classified", "objtype": "view", "provider": "citus '!tests_label_provider"} (2 rows) -DROP TABLE a CASCADE; -NOTICE: drop cascades to view v_dist DROP FUNCTION notice; --- test that SECURITY LABEL statement is actually propagated for ROLES +-- test that SECURITY LABEL statement is actually propagated for ROLES, TABLES and COLUMNS SET citus.log_remote_commands TO on; SET citus.grep_remote_commands = '%SECURITY LABEL%'; -- we have exactly one provider loaded, so we may not include the provider in the command @@ -118,6 +101,41 @@ DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx SECURITY LABEL for "citus '!tests_label_provider" ON ROLE "user 2" IS 'citus_classified'; NOTICE: issuing SECURITY LABEL FOR "citus '!tests_label_provider" ON ROLE "user 2" IS 'citus_classified' DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +SECURITY LABEL ON TABLE a IS 'citus_classified'; +NOTICE: issuing SECURITY LABEL ON TABLE a IS 'citus_classified' +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +SECURITY LABEL for "citus '!tests_label_provider" ON COLUMN a.a IS 'citus_classified'; +NOTICE: issuing SECURITY LABEL FOR "citus '!tests_label_provider" ON COLUMN a.a IS 'citus_classified' +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +-- ROLE, TABLE and COLUMN should be propagated to the worker +SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_unclassified", "objtype": "role", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_unclassified", "objtype": "role", "provider": "citus '!tests_label_provider"} +(2 rows) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('"user 2"') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "role", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "role", "provider": "citus '!tests_label_provider"} +(2 rows) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} +(2 rows) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a.a') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} +(2 rows) + \c - - - :worker_1_port SET citus.log_remote_commands TO on; SET citus.grep_remote_commands = '%SECURITY LABEL%'; @@ -139,6 +157,23 @@ SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORD worker_1 | {"label": "citus_classified", "objtype": "role", "provider": "citus '!tests_label_provider"} (2 rows) +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a.a') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} +(2 rows) + +SECURITY LABEL for "citus '!tests_label_provider" ON COLUMN a.a IS 'citus ''!unclassified'; +NOTICE: issuing SECURITY LABEL FOR "citus '!tests_label_provider" ON COLUMN a.a IS 'citus ''!unclassified' +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a.a') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} +(2 rows) + RESET citus.log_remote_commands; SECURITY LABEL for "citus '!tests_label_provider" ON ROLE "user 2" IS 'citus ''!unclassified'; SELECT node_type, result FROM get_citus_tests_label_provider_labels('"user 2"') ORDER BY node_type; @@ -150,19 +185,229 @@ SELECT node_type, result FROM get_citus_tests_label_provider_labels('"user 2"') \c - - - :master_port SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type; - node_type | result + node_type | result --------------------------------------------------------------------- coordinator | {"label": "citus_classified", "objtype": "role", "provider": "citus '!tests_label_provider"} worker_1 | {"label": "citus_classified", "objtype": "role", "provider": "citus '!tests_label_provider"} (2 rows) -SELECT node_type, result FROM get_citus_tests_label_provider_labels('"user 2"') ORDER BY node_type; - node_type | result +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a') ORDER BY node_type; + node_type | result --------------------------------------------------------------------- - coordinator | {"label": "citus '!unclassified", "objtype": "role", "provider": "citus '!tests_label_provider"} - worker_1 | {"label": "citus '!unclassified", "objtype": "role", "provider": "citus '!tests_label_provider"} + coordinator | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} (2 rows) +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a.a') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} +(2 rows) + +SET citus.shard_replication_factor to 1; +-- Distributed table with delimited identifiers +CREATE TABLE "Dist T" ("col.1" int); +SELECT create_distributed_table('"Dist T"', 'col.1'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +SECURITY LABEL ON TABLE "Dist T" IS 'citus_classified'; +SECURITY LABEL ON COLUMN "Dist T"."col.1" IS 'citus_classified'; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('"Dist T"') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} +(2 rows) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('"Dist T".col.1') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} +(2 rows) + +-- Add and Drop column +CREATE TABLE tddl (a1 int, b1 int, c1 int); +SELECT create_distributed_table('tddl', 'c1'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +ALTER TABLE tddl ADD COLUMN d1 varchar(128); +-- Security label on tddl.d1 is propagated to all nodes +SECURITY LABEL ON COLUMN tddl.d1 IS 'citus_classified'; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('tddl.d1') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} +(2 rows) + +-- Drop column d1, security label should be removed from all nodes +ALTER TABLE tddl DROP COLUMN d1; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('tddl.d1') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | + worker_1 | +(2 rows) + +-- Define security labels before distributed table creation +CREATE TABLE tb (a1 int, b1 int, c1 int); +SECURITY LABEL ON TABLE tb IS 'citus_classified'; +SECURITY LABEL ON COLUMN tb.a1 IS 'citus_classified'; +SELECT create_distributed_table('tb', 'a1'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('tb') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} +(2 rows) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('tb.a1') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} +(2 rows) + +-- Similar test with reference table; security labels should be propagated to the worker. +CREATE TABLE tref (a1 int, b1 int, c1 int); +SECURITY LABEL ON TABLE tref IS 'citus_classified'; +SECURITY LABEL ON COLUMN tref.b1 IS 'citus_classified'; +SELECT create_reference_table('tref'); + create_reference_table +--------------------------------------------------------------------- + +(1 row) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('tref') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} +(2 rows) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('tref.b1') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} +(2 rows) + +-- Distributed table with delimited identifiers - 2 +CREATE TABLE "Dist T2" ("col one" int); +SELECT create_distributed_table('"Dist T2"', 'col one'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +SECURITY LABEL ON TABLE "Dist T2" IS 'citus_classified'; +SECURITY LABEL ON COLUMN "Dist T2"."col one" IS 'citus_classified'; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('"Dist T2"') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} +(2 rows) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('"Dist T2".col one') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} +(2 rows) + +-- Repeat the table and column tests using an explicit schema +CREATE SCHEMA label_test; +SET search_path TO label_test; +CREATE TABLE dist_test1 (a int); +SELECT create_distributed_table('dist_test1', 'a'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +-- Define security labels on a distributed table +SECURITY LABEL ON TABLE dist_test1 IS 'citus_classified'; +SECURITY LABEL ON COLUMN dist_test1.a IS 'citus ''!unclassified'; +SELECT node_type, result FROM public.get_citus_tests_label_provider_labels('label_test.dist_test1') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} +(2 rows) + +SELECT node_type, result FROM public.get_citus_tests_label_provider_labels('label_test.dist_test1.a') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} +(2 rows) + +CREATE TABLE dist_test2 (a int); +SECURITY LABEL on TABLE dist_test2 IS 'citus_unclassified'; +SECURITY LABEL on COLUMN dist_test2.a IS 'citus ''!unclassified'; +-- Distributing a table means security labels on the table and its columns are propagated +SELECT create_distributed_table('dist_test2', 'a'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +SELECT node_type, result FROM public.get_citus_tests_label_provider_labels('label_test.dist_test2') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_unclassified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_unclassified", "objtype": "table", "provider": "citus '!tests_label_provider"} +(2 rows) + +SELECT node_type, result FROM public.get_citus_tests_label_provider_labels('label_test.dist_test2.a') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} +(2 rows) + +-- Add and Drop column +CREATE TABLE tddl (a1 int, b1 int, c1 int); +SELECT create_distributed_table('tddl', 'c1'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +ALTER TABLE tddl ADD COLUMN d1 varchar(128); +-- Security label on tddl.d1 is propagated to all nodes +SECURITY LABEL ON COLUMN tddl.d1 IS 'citus ''!unclassified'; +SELECT node_type, result FROM public.get_citus_tests_label_provider_labels('label_test.tddl.d1') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} +(2 rows) + +-- Drop column d1, security label should be removed from all nodes +ALTER TABLE tddl DROP COLUMN d1; +SELECT node_type, result FROM public.get_citus_tests_label_provider_labels('label_test.tddl.d1') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | + worker_1 | +(2 rows) + +RESET search_path; -- add a new node and check that it also propagates the SECURITY LABEL statement to the new node SET citus.log_remote_commands TO on; SET citus.grep_remote_commands = '%SECURITY LABEL%'; @@ -170,6 +415,20 @@ SELECT 1 FROM citus_add_node('localhost', :worker_2_port); NOTICE: issuing SELECT worker_create_or_alter_role('user1', 'CREATE ROLE user1 NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT -1 PASSWORD NULL', 'ALTER ROLE user1 NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT -1 PASSWORD NULL');SECURITY LABEL FOR "citus '!tests_label_provider" ON ROLE user1 IS 'citus_classified' DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx NOTICE: issuing SELECT worker_create_or_alter_role('user 2', 'CREATE ROLE "user 2" NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT -1 PASSWORD NULL', 'ALTER ROLE "user 2" NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT -1 PASSWORD NULL');SECURITY LABEL FOR "citus '!tests_label_provider" ON ROLE "user 2" IS 'citus ''!unclassified' +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT pg_catalog.worker_drop_sequence_dependency('public.a');;DROP TABLE IF EXISTS public.a CASCADE;CREATE TABLE public.a (a integer) USING heap;ALTER TABLE public.a OWNER TO postgres;SECURITY LABEL FOR "citus '!tests_label_provider" ON TABLE public.a IS 'citus_classified';SECURITY LABEL FOR "citus '!tests_label_provider" ON COLUMN public.a.a IS 'citus ''!unclassified';SELECT worker_create_truncate_trigger('public.a') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT pg_catalog.worker_drop_sequence_dependency('public."Dist T"');;DROP TABLE IF EXISTS public."Dist T" CASCADE;CREATE TABLE public."Dist T" ("col.1" integer) USING heap;ALTER TABLE public."Dist T" OWNER TO postgres;SECURITY LABEL FOR "citus '!tests_label_provider" ON TABLE public."Dist T" IS 'citus_classified';SECURITY LABEL FOR "citus '!tests_label_provider" ON COLUMN public."Dist T"."col.1" IS 'citus_classified';SELECT worker_create_truncate_trigger('public."Dist T"') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT pg_catalog.worker_drop_sequence_dependency('public.tb');;DROP TABLE IF EXISTS public.tb CASCADE;CREATE TABLE public.tb (a1 integer, b1 integer, c1 integer) USING heap;ALTER TABLE public.tb OWNER TO postgres;SECURITY LABEL FOR "citus '!tests_label_provider" ON TABLE public.tb IS 'citus_classified';SECURITY LABEL FOR "citus '!tests_label_provider" ON COLUMN public.tb.a1 IS 'citus_classified';SELECT worker_create_truncate_trigger('public.tb') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT pg_catalog.worker_drop_sequence_dependency('public.tref');;DROP TABLE IF EXISTS public.tref CASCADE;CREATE TABLE public.tref (a1 integer, b1 integer, c1 integer) USING heap;ALTER TABLE public.tref OWNER TO postgres;SECURITY LABEL FOR "citus '!tests_label_provider" ON TABLE public.tref IS 'citus_classified';SECURITY LABEL FOR "citus '!tests_label_provider" ON COLUMN public.tref.b1 IS 'citus_classified';SELECT worker_create_truncate_trigger('public.tref') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT pg_catalog.worker_drop_sequence_dependency('public."Dist T2"');;DROP TABLE IF EXISTS public."Dist T2" CASCADE;CREATE TABLE public."Dist T2" ("col one" integer) USING heap;ALTER TABLE public."Dist T2" OWNER TO postgres;SECURITY LABEL FOR "citus '!tests_label_provider" ON TABLE public."Dist T2" IS 'citus_classified';SECURITY LABEL FOR "citus '!tests_label_provider" ON COLUMN public."Dist T2"."col one" IS 'citus_classified';SELECT worker_create_truncate_trigger('public."Dist T2"') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT pg_catalog.worker_drop_sequence_dependency('label_test.dist_test1');;DROP TABLE IF EXISTS label_test.dist_test1 CASCADE;CREATE TABLE label_test.dist_test1 (a integer) USING heap;ALTER TABLE label_test.dist_test1 OWNER TO postgres;SECURITY LABEL FOR "citus '!tests_label_provider" ON TABLE label_test.dist_test1 IS 'citus_classified';SECURITY LABEL FOR "citus '!tests_label_provider" ON COLUMN label_test.dist_test1.a IS 'citus ''!unclassified';SELECT worker_create_truncate_trigger('label_test.dist_test1') +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +NOTICE: issuing SELECT pg_catalog.worker_drop_sequence_dependency('label_test.dist_test2');;DROP TABLE IF EXISTS label_test.dist_test2 CASCADE;CREATE TABLE label_test.dist_test2 (a integer) USING heap;ALTER TABLE label_test.dist_test2 OWNER TO postgres;SECURITY LABEL FOR "citus '!tests_label_provider" ON TABLE label_test.dist_test2 IS 'citus_unclassified';SECURITY LABEL FOR "citus '!tests_label_provider" ON COLUMN label_test.dist_test2.a IS 'citus ''!unclassified';SELECT worker_create_truncate_trigger('label_test.dist_test2') DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx ?column? --------------------------------------------------------------------- @@ -177,7 +436,7 @@ DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx (1 row) SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type; - node_type | result + node_type | result --------------------------------------------------------------------- coordinator | {"label": "citus_classified", "objtype": "role", "provider": "citus '!tests_label_provider"} worker_1 | {"label": "citus_classified", "objtype": "role", "provider": "citus '!tests_label_provider"} @@ -192,11 +451,114 @@ SELECT node_type, result FROM get_citus_tests_label_provider_labels('"user 2"') worker_2 | {"label": "citus '!unclassified", "objtype": "role", "provider": "citus '!tests_label_provider"} (3 rows) +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_2 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} +(3 rows) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a.a') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_2 | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} +(3 rows) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('"Dist T"') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_2 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} +(3 rows) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('"Dist T".col.1') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_2 | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} +(3 rows) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('"Dist T2"') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_2 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} +(3 rows) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('"Dist T2"."col one"') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | + worker_1 | + worker_2 | +(3 rows) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('tb') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_2 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} +(3 rows) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('tb.a1') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_2 | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} +(3 rows) + +-- Check that security labels in the label_test schema are propagated to the newly added node +SELECT node_type, result FROM public.get_citus_tests_label_provider_labels('label_test.dist_test1') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_2 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} +(3 rows) + +SELECT node_type, result FROM public.get_citus_tests_label_provider_labels('label_test.dist_test1.a') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_2 | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} +(3 rows) + +SELECT node_type, result FROM public.get_citus_tests_label_provider_labels('label_test.dist_test2') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_unclassified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_unclassified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_2 | {"label": "citus_unclassified", "objtype": "table", "provider": "citus '!tests_label_provider"} +(3 rows) + +SELECT node_type, result FROM public.get_citus_tests_label_provider_labels('label_test.dist_test2.a') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_2 | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} +(3 rows) + -- disable the GUC and check that the command is not propagated SET citus.enable_alter_role_propagation TO off; SECURITY LABEL ON ROLE user1 IS 'citus_unclassified'; NOTICE: not propagating SECURITY LABEL commands to other nodes HINT: Connect to other nodes directly to manually assign necessary labels. +SECURITY LABEL ON TABLE a IS 'citus_unclassified'; +NOTICE: not propagating SECURITY LABEL commands to other nodes +HINT: Connect to other nodes directly to manually assign necessary labels. +SECURITY LABEL ON COLUMN a.a IS 'citus_classified'; +NOTICE: not propagating SECURITY LABEL commands to other nodes +HINT: Connect to other nodes directly to manually assign necessary labels. SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type; node_type | result --------------------------------------------------------------------- @@ -205,6 +567,22 @@ SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORD worker_2 | {"label": "citus_classified", "objtype": "role", "provider": "citus '!tests_label_provider"} (3 rows) +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_unclassified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_2 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} +(3 rows) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a.a') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_2 | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} +(3 rows) + \c - - - :worker_2_port SET citus.log_remote_commands TO on; SET citus.grep_remote_commands = '%SECURITY LABEL%'; @@ -212,6 +590,12 @@ SET citus.enable_alter_role_propagation TO off; SECURITY LABEL ON ROLE user1 IS 'citus ''!unclassified'; NOTICE: not propagating SECURITY LABEL commands to other nodes HINT: Connect to other nodes directly to manually assign necessary labels. +SECURITY LABEL ON TABLE a IS 'citus ''!unclassified'; +NOTICE: not propagating SECURITY LABEL commands to other nodes +HINT: Connect to other nodes directly to manually assign necessary labels. +SECURITY LABEL ON COLUMN a.a IS 'citus_unclassified'; +NOTICE: not propagating SECURITY LABEL commands to other nodes +HINT: Connect to other nodes directly to manually assign necessary labels. SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type; node_type | result --------------------------------------------------------------------- @@ -220,7 +604,36 @@ SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORD worker_2 | {"label": "citus '!unclassified", "objtype": "role", "provider": "citus '!tests_label_provider"} (3 rows) +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_unclassified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus_classified", "objtype": "table", "provider": "citus '!tests_label_provider"} + worker_2 | {"label": "citus '!unclassified", "objtype": "table", "provider": "citus '!tests_label_provider"} +(3 rows) + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a.a') ORDER BY node_type; + node_type | result +--------------------------------------------------------------------- + coordinator | {"label": "citus_classified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_1 | {"label": "citus '!unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} + worker_2 | {"label": "citus_unclassified", "objtype": "column", "provider": "citus '!tests_label_provider"} +(3 rows) + RESET citus.enable_alter_role_propagation; +\c - - - :master_port -- cleanup +DROP TABLE a CASCADE; +NOTICE: drop cascades to view v_dist +DROP TABLE "Dist T" CASCADE; +DROP TABLE "Dist T2" CASCADE; +DROP TABLE tb CASCADE; +DROP TABLE tref CASCADE; +DROP TABLE tddl CASCADE; RESET citus.log_remote_commands; DROP ROLE user1, "user 2"; +DROP SCHEMA label_test CASCADE; +NOTICE: drop cascades to 3 other objects +DETAIL: drop cascades to table label_test.dist_test1 +drop cascades to table label_test.dist_test2 +drop cascades to table label_test.tddl diff --git a/src/test/regress/sql/seclabel.sql b/src/test/regress/sql/seclabel.sql index d39e01183..366579edd 100644 --- a/src/test/regress/sql/seclabel.sql +++ b/src/test/regress/sql/seclabel.sql @@ -1,8 +1,10 @@ -- -- SECLABEL -- --- Test suite for SECURITY LABEL ON ROLE statements +-- Test suite for SECURITY LABEL statements: +-- SECURITY LABEL ON IS -- +-- Citus can propagate ROLE, TABLE and COLUMN objects -- first we remove one of the worker nodes to be able to test -- citus_add_node later @@ -22,7 +24,8 @@ SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORD RESET citus.enable_metadata_sync; --- check that we only support propagating for roles +-- check that we only support propagating for roles, tables and columns; +-- support for VIEW and FUNCTION is not there (yet) SET citus.shard_replication_factor to 1; -- distributed table CREATE TABLE a (a int); @@ -33,28 +36,23 @@ CREATE VIEW v_dist AS SELECT * FROM a; CREATE FUNCTION notice(text) RETURNS void LANGUAGE plpgsql AS $$ BEGIN RAISE NOTICE '%', $1; END; $$; -SECURITY LABEL ON TABLE a IS 'citus_classified'; SECURITY LABEL ON FUNCTION notice IS 'citus_unclassified'; SECURITY LABEL ON VIEW v_dist IS 'citus_classified'; -SELECT node_type, result FROM get_citus_tests_label_provider_labels('a') ORDER BY node_type; SELECT node_type, result FROM get_citus_tests_label_provider_labels('notice(text)') ORDER BY node_type; SELECT node_type, result FROM get_citus_tests_label_provider_labels('v_dist') ORDER BY node_type; \c - - - :worker_1_port -SECURITY LABEL ON TABLE a IS 'citus_classified'; SECURITY LABEL ON FUNCTION notice IS 'citus_unclassified'; SECURITY LABEL ON VIEW v_dist IS 'citus_classified'; \c - - - :master_port -SELECT node_type, result FROM get_citus_tests_label_provider_labels('a') ORDER BY node_type; SELECT node_type, result FROM get_citus_tests_label_provider_labels('notice(text)') ORDER BY node_type; SELECT node_type, result FROM get_citus_tests_label_provider_labels('v_dist') ORDER BY node_type; -DROP TABLE a CASCADE; DROP FUNCTION notice; --- test that SECURITY LABEL statement is actually propagated for ROLES +-- test that SECURITY LABEL statement is actually propagated for ROLES, TABLES and COLUMNS SET citus.log_remote_commands TO on; SET citus.grep_remote_commands = '%SECURITY LABEL%'; @@ -64,6 +62,15 @@ SECURITY LABEL ON ROLE user1 IS NULL; SECURITY LABEL ON ROLE user1 IS 'citus_unclassified'; SECURITY LABEL for "citus '!tests_label_provider" ON ROLE "user 2" IS 'citus_classified'; +SECURITY LABEL ON TABLE a IS 'citus_classified'; +SECURITY LABEL for "citus '!tests_label_provider" ON COLUMN a.a IS 'citus_classified'; + +-- ROLE, TABLE and COLUMN should be propagated to the worker +SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('"user 2"') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a.a') ORDER BY node_type; + \c - - - :worker_1_port SET citus.log_remote_commands TO on; SET citus.grep_remote_commands = '%SECURITY LABEL%'; @@ -72,13 +79,110 @@ SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORD SECURITY LABEL for "citus '!tests_label_provider" ON ROLE user1 IS 'citus_classified'; SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a.a') ORDER BY node_type; +SECURITY LABEL for "citus '!tests_label_provider" ON COLUMN a.a IS 'citus ''!unclassified'; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a.a') ORDER BY node_type; + RESET citus.log_remote_commands; SECURITY LABEL for "citus '!tests_label_provider" ON ROLE "user 2" IS 'citus ''!unclassified'; SELECT node_type, result FROM get_citus_tests_label_provider_labels('"user 2"') ORDER BY node_type; \c - - - :master_port SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type; -SELECT node_type, result FROM get_citus_tests_label_provider_labels('"user 2"') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a.a') ORDER BY node_type; + +SET citus.shard_replication_factor to 1; + +-- Distributed table with delimited identifiers +CREATE TABLE "Dist T" ("col.1" int); +SELECT create_distributed_table('"Dist T"', 'col.1'); + +SECURITY LABEL ON TABLE "Dist T" IS 'citus_classified'; +SECURITY LABEL ON COLUMN "Dist T"."col.1" IS 'citus_classified'; + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('"Dist T"') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('"Dist T".col.1') ORDER BY node_type; + +-- Add and Drop column +CREATE TABLE tddl (a1 int, b1 int, c1 int); +SELECT create_distributed_table('tddl', 'c1'); + +ALTER TABLE tddl ADD COLUMN d1 varchar(128); + +-- Security label on tddl.d1 is propagated to all nodes +SECURITY LABEL ON COLUMN tddl.d1 IS 'citus_classified'; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('tddl.d1') ORDER BY node_type; + +-- Drop column d1, security label should be removed from all nodes +ALTER TABLE tddl DROP COLUMN d1; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('tddl.d1') ORDER BY node_type; + +-- Define security labels before distributed table creation +CREATE TABLE tb (a1 int, b1 int, c1 int); +SECURITY LABEL ON TABLE tb IS 'citus_classified'; +SECURITY LABEL ON COLUMN tb.a1 IS 'citus_classified'; + +SELECT create_distributed_table('tb', 'a1'); + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('tb') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('tb.a1') ORDER BY node_type; + +-- Similar test with reference table; security labels should be propagated to the worker. +CREATE TABLE tref (a1 int, b1 int, c1 int); +SECURITY LABEL ON TABLE tref IS 'citus_classified'; +SECURITY LABEL ON COLUMN tref.b1 IS 'citus_classified'; + +SELECT create_reference_table('tref'); + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('tref') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('tref.b1') ORDER BY node_type; + +-- Distributed table with delimited identifiers - 2 +CREATE TABLE "Dist T2" ("col one" int); +SELECT create_distributed_table('"Dist T2"', 'col one'); + +SECURITY LABEL ON TABLE "Dist T2" IS 'citus_classified'; +SECURITY LABEL ON COLUMN "Dist T2"."col one" IS 'citus_classified'; + +SELECT node_type, result FROM get_citus_tests_label_provider_labels('"Dist T2"') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('"Dist T2".col one') ORDER BY node_type; + +-- Repeat the table and column tests using an explicit schema +CREATE SCHEMA label_test; +SET search_path TO label_test; + +CREATE TABLE dist_test1 (a int); +SELECT create_distributed_table('dist_test1', 'a'); +-- Define security labels on a distributed table +SECURITY LABEL ON TABLE dist_test1 IS 'citus_classified'; +SECURITY LABEL ON COLUMN dist_test1.a IS 'citus ''!unclassified'; +SELECT node_type, result FROM public.get_citus_tests_label_provider_labels('label_test.dist_test1') ORDER BY node_type; +SELECT node_type, result FROM public.get_citus_tests_label_provider_labels('label_test.dist_test1.a') ORDER BY node_type; + +CREATE TABLE dist_test2 (a int); +SECURITY LABEL on TABLE dist_test2 IS 'citus_unclassified'; +SECURITY LABEL on COLUMN dist_test2.a IS 'citus ''!unclassified'; +-- Distributing a table means security labels on the table and its columns are propagated +SELECT create_distributed_table('dist_test2', 'a'); +SELECT node_type, result FROM public.get_citus_tests_label_provider_labels('label_test.dist_test2') ORDER BY node_type; +SELECT node_type, result FROM public.get_citus_tests_label_provider_labels('label_test.dist_test2.a') ORDER BY node_type; + +-- Add and Drop column +CREATE TABLE tddl (a1 int, b1 int, c1 int); +SELECT create_distributed_table('tddl', 'c1'); + +ALTER TABLE tddl ADD COLUMN d1 varchar(128); + +-- Security label on tddl.d1 is propagated to all nodes +SECURITY LABEL ON COLUMN tddl.d1 IS 'citus ''!unclassified'; +SELECT node_type, result FROM public.get_citus_tests_label_provider_labels('label_test.tddl.d1') ORDER BY node_type; + +-- Drop column d1, security label should be removed from all nodes +ALTER TABLE tddl DROP COLUMN d1; +SELECT node_type, result FROM public.get_citus_tests_label_provider_labels('label_test.tddl.d1') ORDER BY node_type; + +RESET search_path; -- add a new node and check that it also propagates the SECURITY LABEL statement to the new node SET citus.log_remote_commands TO on; @@ -87,20 +191,52 @@ SELECT 1 FROM citus_add_node('localhost', :worker_2_port); SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type; SELECT node_type, result FROM get_citus_tests_label_provider_labels('"user 2"') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a.a') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('"Dist T"') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('"Dist T".col.1') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('"Dist T2"') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('"Dist T2"."col one"') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('tb') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('tb.a1') ORDER BY node_type; +-- Check that security labels in the label_test schema are propagated to the newly added node +SELECT node_type, result FROM public.get_citus_tests_label_provider_labels('label_test.dist_test1') ORDER BY node_type; +SELECT node_type, result FROM public.get_citus_tests_label_provider_labels('label_test.dist_test1.a') ORDER BY node_type; +SELECT node_type, result FROM public.get_citus_tests_label_provider_labels('label_test.dist_test2') ORDER BY node_type; +SELECT node_type, result FROM public.get_citus_tests_label_provider_labels('label_test.dist_test2.a') ORDER BY node_type; -- disable the GUC and check that the command is not propagated SET citus.enable_alter_role_propagation TO off; SECURITY LABEL ON ROLE user1 IS 'citus_unclassified'; +SECURITY LABEL ON TABLE a IS 'citus_unclassified'; +SECURITY LABEL ON COLUMN a.a IS 'citus_classified'; + SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a.a') ORDER BY node_type; \c - - - :worker_2_port SET citus.log_remote_commands TO on; SET citus.grep_remote_commands = '%SECURITY LABEL%'; SET citus.enable_alter_role_propagation TO off; SECURITY LABEL ON ROLE user1 IS 'citus ''!unclassified'; +SECURITY LABEL ON TABLE a IS 'citus ''!unclassified'; +SECURITY LABEL ON COLUMN a.a IS 'citus_unclassified'; + SELECT node_type, result FROM get_citus_tests_label_provider_labels('user1') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a') ORDER BY node_type; +SELECT node_type, result FROM get_citus_tests_label_provider_labels('a.a') ORDER BY node_type; + RESET citus.enable_alter_role_propagation; +\c - - - :master_port -- cleanup +DROP TABLE a CASCADE; +DROP TABLE "Dist T" CASCADE; +DROP TABLE "Dist T2" CASCADE; +DROP TABLE tb CASCADE; +DROP TABLE tref CASCADE; +DROP TABLE tddl CASCADE; RESET citus.log_remote_commands; DROP ROLE user1, "user 2"; +DROP SCHEMA label_test CASCADE; From a4040ba5da2203f976bd2f4f54ffc56cf0fa2ce8 Mon Sep 17 00:00:00 2001 From: Mehmet YILMAZ Date: Tue, 6 May 2025 17:45:49 +0300 Subject: [PATCH 3/3] =?UTF-8?q?Planner:=20lift=20volatile=20target?= =?UTF-8?q?=E2=80=91list=20items=20in=20`WrapSubquery`=20to=20coordinator?= =?UTF-8?q?=20(prevents=20sequence=E2=80=91leap=20in=20distributed?= =?UTF-8?q?=E2=80=AF`INSERT=E2=80=AF=E2=80=A6=E2=80=AFSELECT`)=20(#7976)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR fixes #7784 and refactors the `WrapSubquery(Query *subquery)` function to improve clarity and correctness when handling volatile expressions in subqueries during Citus insert-select rewriting. ### Background The `WrapSubquery` function rewrites a query of the form: ```sql INSERT INTO target_table SELECT ... FROM ... ``` ...by wrapping the `SELECT` in a subquery: ```sql SELECT FROM ( ) citus_insert_select_subquery ``` This transformation allows: * **Volatile expressions** (e.g., `nextval`, `now`) **not used in `GROUP BY` or `ORDER BY`** to be evaluated **exactly once on the coordinator**. * **Stable/immutable or sort-relevant expressions** to remain in the worker-executed subquery. * Placeholder `NULL`s to maintain column alignment in the inner subquery. ### Fix Details * Restructured the code into labeled logical sections: 1. Build wrapper query (`SELECT … FROM (subquery)`) 2. Rewrite target lists with volatility analysis 3. Assign and return updated query trees * Preserved existing behavior, focusing on clarity and maintainability. ### How the new code handles volatile items stage | what we look for | what we do | why -- | -- | -- | -- scan target list once | 1. `expr_is_volatile(te->expr)` 2. `te->ressortgroupref != 0` (is the column used in GROUP BY / ORDER BY?) | decide whether to hoist or keep | we must not hoist an expression the inner query still needs for sorting/grouping, otherwise its `SortGroupClause` breaks volatile & not used in sort/group | deep‑copy the expression into the outer target list | executes once on the coordinator |     | leave a typed `NULL `placeholder (visible, not `resjunk`) in the inner target list | keeps column numbering stable for helpers that already ran (reorder, cast); the worker sends a cheap constant |   stable / immutable, or volatile but used in sort/group | keep the original expression in the inner list; outer list references it via a `Var `| workers can evaluate it safely and, if needed, the inner ORDER BY still works |   ### Example Given this query: ```sql INSERT INTO t SELECT nextval('s'), 42 FROM generate_series(1, 2); ``` The planner rewrites it as: ```sql SELECT nextval('s'), col2 FROM (SELECT NULL::bigint AS col1, 42 AS col2 FROM generate_series(1, 2)) citus_insert_select_subquery; ``` This ensures `nextval('s')` is evaluated only once per row on the **coordinator**, not on each worker node, preserving correct sequence semantics. #### **Outer‑Var guard (`FindReferencedTableColumn`)** Because `WrapSubquery` adds an extra query level, lots of Vars that the old code never expected become “outer” Vars; without teaching `FindReferencedTableColumn` to climb that extra level reliably, Citus would intermittently reject valid foreign keys and even hit asserts. * Re‑implemented the outer‑Var guard so that the function: * **Walks deterministically up the query stack** when `skipOuterVars = false` (default for FK / UNION checks). A new while‑loop copies — rather than truncates — `parentQueryList` on each hop, eliminating list‑aliasing that made *issue 5248* fail intermittently in parallel regressions. * Handles multi‑level `varlevelsup` in a single loop; never mutates the caller’s list in place. --- .../planner/insert_select_planner.c | 160 +++++++++++++----- .../planner/multi_logical_optimizer.c | 45 +++-- .../regress/expected/multi_insert_select.out | 145 ++++++++++++++++ src/test/regress/sql/multi_insert_select.sql | 131 ++++++++++++++ 4 files changed, 415 insertions(+), 66 deletions(-) diff --git a/src/backend/distributed/planner/insert_select_planner.c b/src/backend/distributed/planner/insert_select_planner.c index db5c3d4ff..3bf0bb327 100644 --- a/src/backend/distributed/planner/insert_select_planner.c +++ b/src/backend/distributed/planner/insert_select_planner.c @@ -1247,7 +1247,7 @@ InsertPartitionColumnMatchesSelect(Query *query, RangeTblEntry *insertRte, RangeTblEntry *subqueryPartitionColumnRelationIdRTE = NULL; List *parentQueryList = list_make2(query, subquery); - bool skipOuterVars = true; + bool skipOuterVars = false; FindReferencedTableColumn(selectTargetExpr, parentQueryList, subquery, &subqueryPartitionColumn, @@ -1543,71 +1543,147 @@ InsertSelectResultIdPrefix(uint64 planId) /* - * WrapSubquery wraps the given query as a subquery in a newly constructed - * "SELECT * FROM (...subquery...) citus_insert_select_subquery" query. + * Return true if the expression tree can change value within a single scan + * (i.e. the planner must treat it as VOLATILE). + * We just delegate to PostgreSQL’s helper. + */ +static inline bool +expr_is_volatile(Node *node) +{ + /* contain_volatile_functions() also returns true for set-returning + * volatile functions and for nextval()/currval(). */ + return contain_volatile_functions(node); +} + + +/* + * WrapSubquery + * + * Build a wrapper query: + * + * SELECT + * FROM ( ) + * citus_insert_select_subquery + * + * Purpose: + * - Preserve column numbering while lifting volatile expressions to the coordinator. + * - Volatile (non-deterministic) expressions not used in GROUP BY / ORDER BY + * are lifted to the outer SELECT to ensure they are evaluated only once. + * - Stable/immutable expressions or volatile ones required by GROUP BY / ORDER BY + * stay in the subquery and are accessed via Vars in the outer SELECT. */ Query * WrapSubquery(Query *subquery) { + /* + * 1. Build the wrapper skeleton: SELECT ... FROM (subquery) alias + */ ParseState *pstate = make_parsestate(NULL); - List *newTargetList = NIL; - Query *outerQuery = makeNode(Query); outerQuery->commandType = CMD_SELECT; - /* create range table entries */ - Alias *selectAlias = makeAlias("citus_insert_select_subquery", NIL); - RangeTblEntry *newRangeTableEntry = RangeTableEntryFromNSItem( - addRangeTableEntryForSubquery( - pstate, subquery, - selectAlias, false, true)); - outerQuery->rtable = list_make1(newRangeTableEntry); + Alias *alias = makeAlias("citus_insert_select_subquery", NIL); + RangeTblEntry *rte_subq = + RangeTableEntryFromNSItem( + addRangeTableEntryForSubquery(pstate, + subquery, /* still points to original subquery */ + alias, + false, /* not LATERAL */ + true)); /* in FROM clause */ + + outerQuery->rtable = list_make1(rte_subq); #if PG_VERSION_NUM >= PG_VERSION_16 - /* - * This part of the code is more of a sanity check for readability, - * it doesn't really do anything. - * addRangeTableEntryForSubquery doesn't add permission info - * because the range table is set to be RTE_SUBQUERY. - * Hence we should also have no perminfos here. - */ - Assert(newRangeTableEntry->rtekind == RTE_SUBQUERY && - newRangeTableEntry->perminfoindex == 0); + /* Ensure RTE_SUBQUERY has proper permission handling */ + Assert(rte_subq->rtekind == RTE_SUBQUERY && + rte_subq->perminfoindex == 0); outerQuery->rteperminfos = NIL; #endif - /* set the FROM expression to the subquery */ - RangeTblRef *newRangeTableRef = makeNode(RangeTblRef); - newRangeTableRef->rtindex = 1; - outerQuery->jointree = makeFromExpr(list_make1(newRangeTableRef), NULL); + RangeTblRef *rtref = makeNode(RangeTblRef); + rtref->rtindex = 1; /* Only one RTE, so index is 1 */ + outerQuery->jointree = makeFromExpr(list_make1(rtref), NULL); - /* create a target list that matches the SELECT */ - TargetEntry *selectTargetEntry = NULL; - foreach_declared_ptr(selectTargetEntry, subquery->targetList) + /* + * 2. Create new target lists for inner (worker) and outer (coordinator) + */ + List *newInnerTL = NIL; + List *newOuterTL = NIL; + int nextResno = 1; + + TargetEntry *te = NULL; + foreach_declared_ptr(te, subquery->targetList) { - /* exactly 1 entry in FROM */ - int indexInRangeTable = 1; - - if (selectTargetEntry->resjunk) + if (te->resjunk) { + /* Keep resjunk entries only in subquery (not in outer query) */ + newInnerTL = lappend(newInnerTL, te); continue; } - Var *newSelectVar = makeVar(indexInRangeTable, selectTargetEntry->resno, - exprType((Node *) selectTargetEntry->expr), - exprTypmod((Node *) selectTargetEntry->expr), - exprCollation((Node *) selectTargetEntry->expr), 0); + bool isVolatile = expr_is_volatile((Node *) te->expr); + bool usedInSort = (te->ressortgroupref != 0); - TargetEntry *newSelectTargetEntry = makeTargetEntry((Expr *) newSelectVar, - selectTargetEntry->resno, - selectTargetEntry->resname, - selectTargetEntry->resjunk); + if (isVolatile && !usedInSort) + { + /* + * Lift volatile expression to outer query so it's evaluated once. + * In inner query, place a NULL of the same type to preserve column position. + */ + TargetEntry *outerTE = + makeTargetEntry(copyObject(te->expr), + list_length(newOuterTL) + 1, + te->resname, + false); + newOuterTL = lappend(newOuterTL, outerTE); - newTargetList = lappend(newTargetList, newSelectTargetEntry); + Const *nullConst = makeNullConst(exprType((Node *) te->expr), + exprTypmod((Node *) te->expr), + exprCollation((Node *) te->expr)); + + TargetEntry *placeholder = + makeTargetEntry((Expr *) nullConst, + nextResno++, /* preserve column position */ + te->resname, + false); /* visible, not resjunk */ + newInnerTL = lappend(newInnerTL, placeholder); + } + else + { + /* + * Either: + * - expression is stable or immutable, or + * - volatile but needed for sorting or grouping + * + * In both cases, keep it in subquery and reference it using a Var. + */ + TargetEntry *innerTE = te; /* reuse original node */ + innerTE->resno = nextResno++; + newInnerTL = lappend(newInnerTL, innerTE); + + Var *v = makeVar(/* subquery reference index is 1 */ + rtref->rtindex, /* same as 1, but self‑documenting */ + innerTE->resno, + exprType((Node *) innerTE->expr), + exprTypmod((Node *) innerTE->expr), + exprCollation((Node *) innerTE->expr), + 0); + + TargetEntry *outerTE = + makeTargetEntry((Expr *) v, + list_length(newOuterTL) + 1, + innerTE->resname, + false); + newOuterTL = lappend(newOuterTL, outerTE); + } } - outerQuery->targetList = newTargetList; + /* + * 3. Assign target lists and return the wrapper query + */ + subquery->targetList = newInnerTL; + outerQuery->targetList = newOuterTL; return outerQuery; } diff --git a/src/backend/distributed/planner/multi_logical_optimizer.c b/src/backend/distributed/planner/multi_logical_optimizer.c index 029de7707..c4c11f4eb 100644 --- a/src/backend/distributed/planner/multi_logical_optimizer.c +++ b/src/backend/distributed/planner/multi_logical_optimizer.c @@ -4478,46 +4478,43 @@ FindReferencedTableColumn(Expr *columnExpression, List *parentQueryList, Query * return; } - - if (candidateColumn->varlevelsup > 0) + /* Walk up varlevelsup as many times as needed */ + while (candidateColumn->varlevelsup > 0) { + /* Caller asked us to ignore any outer Vars → just bail out */ if (skipOuterVars) { - /* - * we don't want to process outer vars, so we return early. - */ return; } - /* - * We currently don't support finding partition keys in the subqueries - * that reference outer subqueries. For example, in correlated - * subqueries in WHERE clause, we don't support use of partition keys - * in the subquery that is referred from the outer query. - */ - - int parentQueryIndex = list_length(parentQueryList) - - candidateColumn->varlevelsup; - if (!(IsIndexInRange(parentQueryList, parentQueryIndex))) + /* Locate the parent query that owns this Var */ + int parentIdx = + list_length(parentQueryList) - candidateColumn->varlevelsup; + if (!IsIndexInRange(parentQueryList, parentIdx)) { - return; + return; /* malformed tree */ } - /* - * Before we recurse into the query tree, we should update the candidateColumn and we use copy of it. - * As we get the query from varlevelsup up, we reset the varlevelsup. - */ + /* Work on a fresh copy of the Var with varlevelsup reset */ candidateColumn = copyObject(candidateColumn); candidateColumn->varlevelsup = 0; /* - * We should be careful about these fields because they need to - * be updated correctly based on ctelevelsup and varlevelsup. + * Make a *completely private* copy of parentQueryList for the + * next recursion step. We copy the whole list and then truncate + * so every recursive branch owns its own list cells. */ - query = list_nth(parentQueryList, parentQueryIndex); - parentQueryList = list_truncate(parentQueryList, parentQueryIndex); + List *newParent = + list_copy(parentQueryList); /* duplicates every cell */ + newParent = list_truncate(newParent, parentIdx); + + query = list_nth(parentQueryList, parentIdx); + parentQueryList = newParent; /* hand private copy down */ + + /* Loop again if still pointing to an outer level */ } + if (candidateColumn->varattno == InvalidAttrNumber) { /* diff --git a/src/test/regress/expected/multi_insert_select.out b/src/test/regress/expected/multi_insert_select.out index 58d22583e..00e335a82 100644 --- a/src/test/regress/expected/multi_insert_select.out +++ b/src/test/regress/expected/multi_insert_select.out @@ -3492,5 +3492,150 @@ $$); Task Count: 1 (2 rows) +--------------------------------------------------------------------- +-- Regression Test Script for Issue #7784 +-- This script tests INSERT ... SELECT with a CTE for: +-- 1. Schema based sharding. +-- 2. A distributed table. +--------------------------------------------------------------------- +-- Enable schema-based sharding +SET citus.enable_schema_based_sharding TO ON; +-- Create a table for schema based sharding +CREATE TABLE version_sch_based ( + id bigserial NOT NULL, + description varchar(255), + PRIMARY KEY (id) +); +-- Insert an initial row. +INSERT INTO version_sch_based (description) VALUES ('Version 1'); +-- Duplicate the row using a CTE and INSERT ... SELECT. +WITH v AS ( + SELECT * FROM version_sch_based WHERE description = 'Version 1' +) +INSERT INTO version_sch_based (description) +SELECT description FROM v; +-- Expected output: +-- id | description +-- ----+------------- +-- 1 | Version 1 +-- 2 | Version 1 +-- Query the table and order by id for consistency. +SELECT * FROM version_sch_based ORDER BY id; + id | description +--------------------------------------------------------------------- + 1 | Version 1 + 2 | Version 1 +(2 rows) + +--------------------------------------------------------------------- +-- Case 2: Distributed Table Scenario +--------------------------------------------------------------------- +SET citus.enable_schema_based_sharding TO OFF; +-- Create a table for the distributed test. +CREATE TABLE version_dist ( + id bigserial NOT NULL, + description varchar(255), + PRIMARY KEY (id) +); +-- Register the table as distributed using the 'id' column as the distribution key. +SELECT create_distributed_table('version_dist', 'id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +-- Insert an initial row. +INSERT INTO version_dist (description) VALUES ('Version 1'); +-- Duplicate the row using a CTE and INSERT ... SELECT. +WITH v AS ( + SELECT * FROM version_dist WHERE description = 'Version 1' +) +INSERT INTO version_dist (description) +SELECT description FROM v; +-- Expected output: +-- id | description +-- ----+------------- +-- 1 | Version 1 +-- 2 | Version 1 +-- Query the table and order by id for consistency. +SELECT * FROM version_dist ORDER BY id; + id | description +--------------------------------------------------------------------- + 1 | Version 1 + 2 | Version 1 +(2 rows) + +--------------------------------------------------------------------- +-- Case 3: Distributed INSERT … SELECT with nextval() +-- Verifies that nextval() is evaluated on the coordinator only. +--------------------------------------------------------------------- +-- A fresh sequence for clarity +CREATE SEQUENCE seq_nextval_test START 100; +-- Table with DEFAULT nextval() +CREATE TABLE version_dist_seq ( + id bigint DEFAULT nextval('seq_nextval_test'), + description text, + PRIMARY KEY (id) +); +SELECT create_distributed_table('version_dist_seq', 'id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +-- Seed one row (id = 100) +INSERT INTO version_dist_seq (description) VALUES ('row‑0'); +-- CTE duplication – should produce **exactly one** new sequence value (id = 101) +WITH v AS ( + SELECT * FROM version_dist_seq WHERE description = 'row‑0' +) +INSERT INTO version_dist_seq (description) +SELECT description FROM v; +-- Expected: ids are 100 and 101 (no gaps, no duplicates) +SELECT id, description FROM version_dist_seq ORDER BY id; + id | description +--------------------------------------------------------------------- + 100 | row‑0 + 101 | row‑0 +(2 rows) + +--------------------------------------------------------------------- +-- Case 4: UNION ALL + nextval() in distributed INSERT … SELECT +--------------------------------------------------------------------- +CREATE SEQUENCE seq_union_test START 200; +CREATE TABLE version_dist_union ( + id bigint DEFAULT nextval('seq_union_test'), + val int, + PRIMARY KEY (id) +); +SELECT create_distributed_table('version_dist_union', 'id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +-- Seed rows +INSERT INTO version_dist_union (val) VALUES (1), (2); +-- UNION ALL duplication; each leg returns two rows -> four inserts total +WITH src AS ( + SELECT val FROM version_dist_union + UNION ALL + SELECT val FROM version_dist_union +) +INSERT INTO version_dist_union(val) +SELECT val FROM src; +-- Expected IDs: 200,201,202,203,204,205 +SELECT id, val FROM version_dist_union ORDER BY id; + id | val +--------------------------------------------------------------------- + 200 | 1 + 201 | 2 + 202 | 1 + 203 | 2 + 204 | 1 + 205 | 2 +(6 rows) + +-- End of Issue #7784 SET client_min_messages TO ERROR; DROP SCHEMA multi_insert_select CASCADE; diff --git a/src/test/regress/sql/multi_insert_select.sql b/src/test/regress/sql/multi_insert_select.sql index b773ce906..eabadda7c 100644 --- a/src/test/regress/sql/multi_insert_select.sql +++ b/src/test/regress/sql/multi_insert_select.sql @@ -2452,5 +2452,136 @@ SELECT coordinator_plan($$ SELECT id FROM dist_table_5 JOIN cte_1 USING(id); $$); +------------------------------- +-- Regression Test Script for Issue #7784 +-- This script tests INSERT ... SELECT with a CTE for: +-- 1. Schema based sharding. +-- 2. A distributed table. +------------------------------- + +-- Enable schema-based sharding +SET citus.enable_schema_based_sharding TO ON; + + +-- Create a table for schema based sharding +CREATE TABLE version_sch_based ( + id bigserial NOT NULL, + description varchar(255), + PRIMARY KEY (id) +); + +-- Insert an initial row. +INSERT INTO version_sch_based (description) VALUES ('Version 1'); + +-- Duplicate the row using a CTE and INSERT ... SELECT. +WITH v AS ( + SELECT * FROM version_sch_based WHERE description = 'Version 1' +) +INSERT INTO version_sch_based (description) +SELECT description FROM v; + +-- Expected output: +-- id | description +-- ----+------------- +-- 1 | Version 1 +-- 2 | Version 1 + +-- Query the table and order by id for consistency. +SELECT * FROM version_sch_based ORDER BY id; + +-------------------------------------------------- +-- Case 2: Distributed Table Scenario +-------------------------------------------------- +SET citus.enable_schema_based_sharding TO OFF; + +-- Create a table for the distributed test. +CREATE TABLE version_dist ( + id bigserial NOT NULL, + description varchar(255), + PRIMARY KEY (id) +); + +-- Register the table as distributed using the 'id' column as the distribution key. +SELECT create_distributed_table('version_dist', 'id'); + +-- Insert an initial row. +INSERT INTO version_dist (description) VALUES ('Version 1'); + +-- Duplicate the row using a CTE and INSERT ... SELECT. +WITH v AS ( + SELECT * FROM version_dist WHERE description = 'Version 1' +) +INSERT INTO version_dist (description) +SELECT description FROM v; + +-- Expected output: +-- id | description +-- ----+------------- +-- 1 | Version 1 +-- 2 | Version 1 + +-- Query the table and order by id for consistency. +SELECT * FROM version_dist ORDER BY id; + +------------------------------- +-- Case 3: Distributed INSERT … SELECT with nextval() +-- Verifies that nextval() is evaluated on the coordinator only. +------------------------------- + +-- A fresh sequence for clarity +CREATE SEQUENCE seq_nextval_test START 100; + +-- Table with DEFAULT nextval() +CREATE TABLE version_dist_seq ( + id bigint DEFAULT nextval('seq_nextval_test'), + description text, + PRIMARY KEY (id) +); +SELECT create_distributed_table('version_dist_seq', 'id'); + +-- Seed one row (id = 100) +INSERT INTO version_dist_seq (description) VALUES ('row‑0'); + +-- CTE duplication – should produce **exactly one** new sequence value (id = 101) +WITH v AS ( + SELECT * FROM version_dist_seq WHERE description = 'row‑0' +) +INSERT INTO version_dist_seq (description) +SELECT description FROM v; + +-- Expected: ids are 100 and 101 (no gaps, no duplicates) +SELECT id, description FROM version_dist_seq ORDER BY id; + + +------------------------------- +-- Case 4: UNION ALL + nextval() in distributed INSERT … SELECT +------------------------------- + +CREATE SEQUENCE seq_union_test START 200; + +CREATE TABLE version_dist_union ( + id bigint DEFAULT nextval('seq_union_test'), + val int, + PRIMARY KEY (id) +); +SELECT create_distributed_table('version_dist_union', 'id'); + +-- Seed rows +INSERT INTO version_dist_union (val) VALUES (1), (2); + +-- UNION ALL duplication; each leg returns two rows -> four inserts total +WITH src AS ( + SELECT val FROM version_dist_union + UNION ALL + SELECT val FROM version_dist_union +) +INSERT INTO version_dist_union(val) +SELECT val FROM src; + +-- Expected IDs: 200,201,202,203,204,205 +SELECT id, val FROM version_dist_union ORDER BY id; + +-- End of Issue #7784 + SET client_min_messages TO ERROR; DROP SCHEMA multi_insert_select CASCADE;