Address the issues/comments from the original PR# 6315

1) Regular users fail to use clock UDF with permission issue.
2) Clock functions were declared as STABLE, whereas by definition they are VOLATILE. By design, any clock/time
   functions will return different results for each call even within a single SQL statement.

Note: UDF citus_get_transaction_clock() is a misnomer as it internally calls the clock tick which always returns
      different results for every invocation in the same transaction.
pull/6489/merge
Teja Mupparti 2022-11-21 19:00:03 -08:00 committed by Teja Mupparti
parent 65f256eec4
commit e14dc5d45d
7 changed files with 81 additions and 7 deletions

View File

@ -44,10 +44,15 @@
#define SAVE_AND_PERSIST(c) \
do { \
Oid savedUserId = InvalidOid; \
int savedSecurityContext = 0; \
LogicalClockShmem->clusterClockValue = *(c); \
GetUserIdAndSecContext(&savedUserId, &savedSecurityContext); \
SetUserIdAndSecContext(CitusExtensionOwner(), SECURITY_LOCAL_USERID_CHANGE); \
DirectFunctionCall2(setval_oid, \
ObjectIdGetDatum(DistClockLogicalSequenceId()), \
Int64GetDatum((c)->logical)); \
SetUserIdAndSecContext(savedUserId, savedSecurityContext); \
} while (0)
PG_FUNCTION_INFO_V1(citus_get_node_clock);
@ -61,7 +66,7 @@ PG_FUNCTION_INFO_V1(citus_get_transaction_clock);
typedef enum ClockState
{
CLOCKSTATE_INITIALIZED,
CLOCKSTATE_UNINITIALIZED,
CLOCKSTATE_UNINITIALIZED
} ClockState;
/*
@ -504,9 +509,17 @@ InitClockAtFirstUse(void)
* a higher rate than once every 32 seconds.
*
*/
Oid saveUserId = InvalidOid;
int savedSecurityCtx = 0;
GetUserIdAndSecContext(&saveUserId, &savedSecurityCtx);
SetUserIdAndSecContext(CitusExtensionOwner(), SECURITY_LOCAL_USERID_CHANGE);
persistedMaxClock.logical =
DirectFunctionCall1(nextval_oid, ObjectIdGetDatum(DistClockLogicalSequenceId()));
SetUserIdAndSecContext(saveUserId, savedSecurityCtx);
/*
* Sequence 1 indicates no prior clock timestamps on this server, retain
* the wall clock i.e. no adjustment needed.

View File

@ -1,6 +1,6 @@
CREATE OR REPLACE FUNCTION pg_catalog.citus_get_node_clock()
RETURNS pg_catalog.cluster_clock
LANGUAGE C STABLE PARALLEL SAFE STRICT
LANGUAGE C VOLATILE PARALLEL UNSAFE STRICT
AS 'MODULE_PATHNAME',$$citus_get_node_clock$$;
COMMENT ON FUNCTION pg_catalog.citus_get_node_clock()
IS 'Returns monotonically increasing timestamp with logical clock value as close to epoch value (in milli seconds) as possible, and a counter for ticks(maximum of 4 million) within the logical clock';

View File

@ -1,6 +1,6 @@
CREATE OR REPLACE FUNCTION pg_catalog.citus_get_node_clock()
RETURNS pg_catalog.cluster_clock
LANGUAGE C STABLE PARALLEL SAFE STRICT
LANGUAGE C VOLATILE PARALLEL UNSAFE STRICT
AS 'MODULE_PATHNAME',$$citus_get_node_clock$$;
COMMENT ON FUNCTION pg_catalog.citus_get_node_clock()
IS 'Returns monotonically increasing timestamp with logical clock value as close to epoch value (in milli seconds) as possible, and a counter for ticks(maximum of 4 million) within the logical clock';

View File

@ -1,6 +1,6 @@
CREATE OR REPLACE FUNCTION pg_catalog.citus_get_transaction_clock()
RETURNS pg_catalog.cluster_clock
LANGUAGE C STABLE PARALLEL SAFE STRICT
LANGUAGE C VOLATILE PARALLEL UNSAFE STRICT
AS 'MODULE_PATHNAME',$$citus_get_transaction_clock$$;
COMMENT ON FUNCTION pg_catalog.citus_get_transaction_clock()
IS 'Returns a transaction timestamp logical clock';

View File

@ -1,6 +1,6 @@
CREATE OR REPLACE FUNCTION pg_catalog.citus_get_transaction_clock()
RETURNS pg_catalog.cluster_clock
LANGUAGE C STABLE PARALLEL SAFE STRICT
LANGUAGE C VOLATILE PARALLEL UNSAFE STRICT
AS 'MODULE_PATHNAME',$$citus_get_transaction_clock$$;
COMMENT ON FUNCTION pg_catalog.citus_get_transaction_clock()
IS 'Returns a transaction timestamp logical clock';

View File

@ -328,9 +328,51 @@ WARNING: GUC enable_cluster_clock is off
(1 row)
END;
SET citus.enable_cluster_clock to ON;
-- Test if the clock UDFs are volatile, result should never be the same
SELECT citus_get_node_clock() = citus_get_node_clock();
?column?
---------------------------------------------------------------------
f
(1 row)
select citus_get_transaction_clock() = citus_get_transaction_clock();
DEBUG: node xxxx transaction clock xxxxxx
DEBUG: final global transaction clock xxxxxx
DEBUG: node xxxx transaction clock xxxxxx
DEBUG: final global transaction clock xxxxxx
?column?
---------------------------------------------------------------------
f
(1 row)
-- Test if the clock UDFs are usable by non-superusers
CREATE ROLE non_super_user_clock;
SET ROLE non_super_user_clock;
SELECT citus_get_node_clock();
citus_get_node_clock
---------------------------------------------------------------------
(xxxxxxxxxxxxx,x)
(1 row)
BEGIN;
SELECT citus_get_transaction_clock();
DEBUG: node xxxx transaction clock xxxxxx
DEBUG: final global transaction clock xxxxxx
citus_get_transaction_clock
---------------------------------------------------------------------
(xxxxxxxxxxxxx,x)
(1 row)
COMMIT;
-- Test setting the persisted clock value (it must fail)
SELECT setval('pg_dist_clock_logical_seq', 100, true);
ERROR: permission denied for sequence pg_dist_clock_logical_seq
\c
RESET client_min_messages;
RESET citus.enable_cluster_clock;
DROP ROLE non_super_user_clock;
DROP SCHEMA clock CASCADE;
NOTICE: drop cascades to 2 other objects
DETAIL: drop cascades to table clock_test
drop cascades to table cluster_clock_type
DETAIL: drop cascades to table clock.clock_test
drop cascades to table clock.cluster_clock_type

View File

@ -141,6 +141,25 @@ BEGIN;
SELECT citus_get_transaction_clock();
END;
SET citus.enable_cluster_clock to ON;
-- Test if the clock UDFs are volatile, result should never be the same
SELECT citus_get_node_clock() = citus_get_node_clock();
select citus_get_transaction_clock() = citus_get_transaction_clock();
-- Test if the clock UDFs are usable by non-superusers
CREATE ROLE non_super_user_clock;
SET ROLE non_super_user_clock;
SELECT citus_get_node_clock();
BEGIN;
SELECT citus_get_transaction_clock();
COMMIT;
-- Test setting the persisted clock value (it must fail)
SELECT setval('pg_dist_clock_logical_seq', 100, true);
\c
RESET client_min_messages;
RESET citus.enable_cluster_clock;
DROP ROLE non_super_user_clock;
DROP SCHEMA clock CASCADE;