Restrict yb_read_time to session-only configuration by hari90 · Pull Request #30962 · yugabyte/yugabyte-db

hari90 · 2026-04-03T02:23:57Z

Summary

This change restricts the yb_read_time parameter to only be set within a session or transaction, preventing it from being persisted via ALTER DATABASE SET or ALTER ROLE SET commands.

Key Changes

Changed yb_read_time GUC category from PGC_SUSET to PGC_USERSET to allow non-superusers to set it within sessions
Added GUC_DISALLOW_IN_FILE flag to prevent the parameter from being set in configuration files
Added validation logic in check_yb_read_time() to reject non-default values coming from database, role, global, or test sources
Updated the parameter documentation to clarify that it can only be set within a session

Implementation Details

The validation check ensures that yb_read_time cannot be set to a non-zero value through:

PGC_S_DATABASE - ALTER DATABASE SET
PGC_S_USER - ALTER ROLE SET
PGC_S_DATABASE_USER - ALTER DATABASE/ROLE SET for specific user
PGC_S_GLOBAL - Global configuration
PGC_S_TEST - Test/validation sources

This prevents accidental persistence of read-time settings that should only apply to individual sessions or transactions.

https://claude.ai/code/session_01RtyT4u9XyfxAyv91cqAKNL

Phorge: D51783

…N_FILE yb_read_time should only be settable per-session or within a transaction, not persisted across sessions via ysql_pg_conf_csv or ALTER SYSTEM. - Change context from PGC_SUSET to PGC_USERSET so any user can set it - Add GUC_DISALLOW_IN_FILE flag to prevent setting via config files Fixes yugabyte#30930 https://claude.ai/code/session_01RtyT4u9XyfxAyv91cqAKNL

Add a new GUC flag GUC_YB_DISALLOW_IN_DB_ROLE_SETTING that prevents a parameter from being persisted via ALTER DATABASE SET or ALTER ROLE SET. Apply it to yb_read_time so it can only be set within a session. The check is enforced in AlterSetting() which is the common path for both ALTER DATABASE SET and ALTER ROLE SET commands. https://claude.ai/code/session_01RtyT4u9XyfxAyv91cqAKNL

Instead of a new GUC_YB_DISALLOW_IN_DB_ROLE_SETTING flag and AlterSetting check, use the existing check_yb_read_time hook to reject non-default values when the source indicates persistence (PGC_S_DATABASE, PGC_S_USER, PGC_S_DATABASE_USER, PGC_S_GLOBAL, PGC_S_TEST). PGC_S_TEST is included because ALTER DATABASE/ROLE SET uses it for validation via validate_option_array_item, so this makes the ALTER command itself fail with a clear error. https://claude.ai/code/session_01RtyT4u9XyfxAyv91cqAKNL

netlify · 2026-04-03T02:25:30Z

✅ Deploy Preview for infallible-bardeen-164bc9 ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`5f87633`
🔍 Latest deploy log	https://app.netlify.com/projects/infallible-bardeen-164bc9/deploys/69cf24c07a1acd0008dff6dd
😎 Deploy Preview	https://deploy-preview-30962--infallible-bardeen-164bc9.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

gemini-code-assist

Code Review

This pull request modifies the yb_read_time configuration parameter to allow non-superuser access while restricting it to session-level usage. It adds the GUC_DISALLOW_IN_FILE flag and implements validation logic to prevent the parameter from being persisted via ALTER DATABASE or ALTER ROLE commands. A suggestion was made to simplify the source validation logic in check_yb_read_time by using a helper function or a more concise condition.

gemini-code-assist · 2026-04-03T02:26:23Z

+		(source == PGC_S_DATABASE ||
+		 source == PGC_S_USER ||
+		 source == PGC_S_DATABASE_USER ||
+		 source == PGC_S_GLOBAL ||
+		 source == PGC_S_TEST))


The check for GucSource can be simplified by checking if the source is one of the persistent sources. Consider using a helper function or a more concise condition if this logic is reused elsewhere, though this is acceptable for now.

@claude fix this

Tests verify: - SET/SET LOCAL work in a session (PGC_USERSET) - Non-superuser can SET yb_read_time - ALTER DATABASE SET yb_read_time fails with non-zero value - ALTER ROLE SET yb_read_time fails with non-zero value - ALTER DATABASE SET yb_read_time TO '0' and RESET still succeed https://claude.ai/code/session_01RtyT4u9XyfxAyv91cqAKNL

CLAassistant · 2026-04-03T02:26:47Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

hari90 · 2026-04-03T02:32:29Z

+  ASSERT_NOK(status);
+  ASSERT_STR_CONTAINS(status.ToString(), "yb_read_time can only be set within a session");


Assert_nok_str_contains

hari90 · 2026-04-03T02:32:39Z

+  // ALTER ROLE SET yb_read_time should fail.
+  status = conn.Execute("ALTER ROLE ALL SET yb_read_time TO '1000'");
+  ASSERT_NOK(status);
+  ASSERT_STR_CONTAINS(status.ToString(), "yb_read_time can only be set within a session");


Replace separate ASSERT_NOK + ASSERT_STR_CONTAINS calls with the more concise ASSERT_NOK_STR_CONTAINS macro as suggested in review. https://claude.ai/code/session_01RtyT4u9XyfxAyv91cqAKNL

The persistent sources (PGC_S_GLOBAL through PGC_S_DATABASE_USER) are contiguous in the GucSource enum, so use a range check instead of listing each value individually. https://claude.ai/code/session_01RtyT4u9XyfxAyv91cqAKNL

Range comparison on enum values is fragile if the enum is reordered. Use explicit comparisons instead. https://claude.ai/code/session_01RtyT4u9XyfxAyv91cqAKNL

hari90 · 2026-04-03T05:26:22Z

Trigger Jenkins

hari90 · 2026-04-03T05:26:30Z

Phorge diff synced: D51783
Commit: 54cc497b

Jenkins build has been triggered. Results will be posted here once it completes.

JenkinsBot

hari90 · 2026-04-03T09:28:29Z

❌ Jenkins build for commit 54cc497b: Fail

Exceptions:

'Multi-Build Test Failures' failed. CSI-Checks (http://csiweb.dev.yugabyte.com/D51783/#CSI-Checks)
Passed: 11

🚫 Failed Landing Requirements

'Multi-Build Test Failures' failed

PgMiniTestBase - YbReadTimeSessionOnly

🔨 DB Build/Test Job Summary

Build	Total	Passed	Failed	Failed After Retries
D51783-arm-alma8-clang21-release	11131	10699	14	10
D51783-alma8-clang21-tsan	11661	10022	8	6
D51783-mac14-clang-release	2	2	0	0
D51783-alma9-clang21-asan	11749	11012	15	13
D51783-alma8-clang21-release	11133	10703	14	7
D51783-arm-mac14-clang-release	17	17	0	0
D51783-ubuntu22.04-clang21-debug	2	2	0	0
D51783-alma8-gcc12-fastdebug	11872	11408	7	4

Full status

JenkinsBot

The test was setting yb_read_time to a timestamp captured before CREATE ROLE test_user, causing cache refresh to fail resolving the role OID at the historical read time. https://claude.ai/code/session_01RtyT4u9XyfxAyv91cqAKNL

Replace repeated inline SQL for fetching the current timestamp in microseconds with a shared helper function. https://claude.ai/code/session_01RtyT4u9XyfxAyv91cqAKNL

hari90 · 2026-04-03T17:54:40Z

Trigger Jenkins

hari90 · 2026-04-03T17:54:59Z

Phorge diff synced: D51783
Commit: 17ea1244

Jenkins build has been triggered. Results will be posted here once it completes.

JenkinsBot

hari90 · 2026-04-03T20:55:46Z

✅ Jenkins build for commit 17ea1244: Pass

Passed: 12

🔨 DB Build/Test Job Summary

Build	Total	Passed	Failed	Failed After Retries
D51783-alma8-gcc12-fastdebug	11872	11409	5	3
D51783-arm-alma8-clang21-release	11131	10700	12	9
D51783-alma8-clang21-tsan	11661	10022	10	3
D51783-alma9-clang21-asan	11749	11013	15	10
D51783-mac14-clang-release	2	2	0	0
D51783-alma8-clang21-release	11133	10704	7	4
D51783-arm-mac14-clang-release	17	17	0	0
D51783-ubuntu22.04-clang21-debug	2	2	0	0

Full status

JenkinsBot

yamen-haddad · 2026-04-07T10:28:37Z

I am already tracking this work in https://phorge.dev.yugabyte.com/D51724 and LRT team is already reviewing the diff.

claude added 3 commits April 2, 2026 19:08

hari90 requested a review from yamen-haddad April 3, 2026 02:23

gemini-code-assist Bot reviewed Apr 3, 2026

View reviewed changes

hari90 commented Apr 3, 2026

View reviewed changes

claude added 3 commits April 3, 2026 04:16

Address PR review: use ASSERT_NOK_STR_CONTAINS for test assertions

04e3cd9

Replace separate ASSERT_NOK + ASSERT_STR_CONTAINS calls with the more concise ASSERT_NOK_STR_CONTAINS macro as suggested in review. https://claude.ai/code/session_01RtyT4u9XyfxAyv91cqAKNL

Revert GucSource range check to explicit enum listing

54cc497

Range comparison on enum values is fragile if the enum is reordered. Use explicit comparisons instead. https://claude.ai/code/session_01RtyT4u9XyfxAyv91cqAKNL

claude added 2 commits April 3, 2026 15:57

Extract FetchCurrentTimestampMicros helper in pg_read_time-test

17ea124

Replace repeated inline SQL for fetching the current timestamp in microseconds with a shared helper function. https://claude.ai/code/session_01RtyT4u9XyfxAyv91cqAKNL

hari90 closed this Apr 15, 2026

hari90 deleted the claude/fix-session-persistence-51wP4 branch April 15, 2026 17:39

		ASSERT_NOK(status);
		ASSERT_STR_CONTAINS(status.ToString(), "yb_read_time can only be set within a session");

Conversation

hari90 commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Key Changes

Implementation Details

Uh oh!

netlify Bot commented Apr 3, 2026

✅ Deploy Preview for infallible-bardeen-164bc9 ready!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

hari90 Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

CLAassistant commented Apr 3, 2026

Uh oh!

hari90 Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

hari90 Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

hari90 commented Apr 3, 2026

Uh oh!

hari90 commented Apr 3, 2026

Uh oh!

hari90 commented Apr 3, 2026

🚫 Failed Landing Requirements

🔨 DB Build/Test Job Summary

Uh oh!

hari90 commented Apr 3, 2026

Uh oh!

hari90 commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hari90 commented Apr 3, 2026

🔨 DB Build/Test Job Summary

Uh oh!

yamen-haddad commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hari90 commented Apr 3, 2026 •

edited

Loading

hari90 commented Apr 3, 2026 •

edited

Loading