Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

[YSQL] Match Pg semantics for volatile functions in READ COMMITTED isolation level #11640

Closed
pkj415 opened this issue Mar 3, 2022 · 0 comments
Closed

[YSQL] Match Pg semantics for volatile functions in READ COMMITTED isolation level #11640

pkj415 opened this issue Mar 3, 2022 · 0 comments
Assignees
@pkj415
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug pg-compatibility Label for issues that result in differences b/w YSQL and Pg semantics priority/medium Medium priority issue

Comments

@pkj415
Copy link
Contributor

pkj415 commented Mar 3, 2022 •
edited
Loading

Jira Link: DB-506

This issue is to ensure YSQL matches Pg semantics for volatile functions in READ COMMITTED isolation level. Quoting from Pg docs (https://www.postgresql.org/docs/current/xfunc-volatility.html) -

STABLE and IMMUTABLE functions use a snapshot established as of the start of the calling query, whereas VOLATILE functions obtain a fresh snapshot at the start of each query they execute.
@pkj415 pkj415 added the area/ysql Yugabyte SQL (YSQL) label Mar 3, 2022
@pkj415 pkj415 self-assigned this Mar 3, 2022
@pkj415 pkj415 changed the title [YSQL] Pick new snapshot for each statement even in volatile procedures if READ COMMITTED isolation [YSQL] Match Pg semantics for volatile functions in READ COMMITTED isolation level Mar 3, 2022
@pkj415 pkj415 mentioned this issue Mar 3, 2022
Open
@pkj415 pkj415 added the pg-compatibility Label for issues that result in differences b/w YSQL and Pg semantics label May 24, 2022
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels May 24, 2022
pkj415 added a commit to pkj415/yugabyte-db that referenced this issue Jun 17, 2022
@pkj415
[yugabyte#12254] [yugabyte#11640] [YSQL] Support read committed seman…
7de0e16 
…tics for all non-DDL work

Summary:
Till now, we supported READ COMMITTED semantics only for SELECT/ INSERT/ UPDATE
and DELETE statements. This is done by restarting the statement after rolling
back work done by the failed execution of the statement.

With this diff, the logic is allowed for all statements except DDLs.

Test Plan: ./yb_build.sh --java-test org.yb.pgsql.TestPgIsolationRegress#isolationRegress

Reviewers: alex, mtakahara

Subscribers: yql

Differential Revision: https://phabricator.dev.yugabyte.com/D17440
pkj415 added a commit that referenced this issue Jul 11, 2022
@pkj415
[#12254, #11640] YSQL: Support read committed semantics for all non-D…
a224db2 
…DL work

Summary:
Till now, we supported READ COMMITTED semantics only for SELECT/ INSERT/ UPDATE
and DELETE statements. This is done by restarting the statement after rolling
back work done by the failed execution of the statement.

With this diff, the logic is allowed for all other statements except DDLs.

TODOs for a later diff:
(1) Move read committed retries to a finer level in functions and procedures i.e., per query (#12958).
This is required for multiple reasons:

(i) Performance - in read committed isolation we can retry just the statement that faced a
kConflict or kReadRestart in the function/ procedure. We don't have to restart the whole
procedure or function.

(ii) We should not be redoing non-transactional side-effects in functions and procedures.
This will be resolved automatically once we perform retries at a statement level in the
func/ proc.

(2) Allow saving a snapshot, switching the another and reusing the saved snapshot in YSQL
Detailed use case in #12959

Test Plan:
./yb_build.sh --java-test org.yb.pgsql.TestPgIsolationRegress#isolationRegress
./yb_build.sh --java-test org.yb.pgsql.TestPgReadCommittedVolatileFuncs#testFunctionSemantics

Reviewers: alex, mtakahara

Reviewed By: mtakahara

Subscribers: lnguyen, bogdan, zyu, yql

Differential Revision: https://phabricator.dev.yugabyte.com/D17440
pkj415 added a commit that referenced this issue Jul 13, 2022
@pkj415
[BACKPORT 2.14][#12254, #11640] YSQL: Support read committed semantic…
fd49967 
…s for all non-DDL work

Summary:
Till now, we supported READ COMMITTED semantics only for SELECT/ INSERT/ UPDATE
and DELETE statements. This is done by restarting the statement after rolling
back work done by the failed execution of the statement.

With this diff, the logic is allowed for all other statements except DDLs.

TODOs for a later diff:
(1) Move read committed retries to a finer level in functions and procedures i.e., per query (#12958).
This is required for multiple reasons:

(i) Performance - in read committed isolation we can retry just the statement that faced a
kConflict or kReadRestart in the function/ procedure. We don't have to restart the whole
procedure or function.

(ii) We should not be redoing non-transactional side-effects in functions and procedures.
This will be resolved automatically once we perform retries at a statement level in the
func/ proc.

(2) Allow saving a snapshot, switching the another and reusing the saved snapshot in YSQL
Detailed use case in #12959

Original commit: a224db2 / D17440

Test Plan:
./yb_build.sh --java-test org.yb.pgsql.TestPgIsolationRegress#isolationRegress
./yb_build.sh --java-test org.yb.pgsql.TestPgReadCommittedVolatileFuncs#testFunctionSemantics

Reviewers: alex, mtakahara

Reviewed By: mtakahara

Subscribers: yql, zyu, bogdan, lnguyen

Differential Revision: https://phabricator.dev.yugabyte.com/D18252
@pkj415 pkj415 closed this as completed Aug 9, 2022
@pkj415 pkj415 changed the title [YSQL] Match Pg semantics for volatile functions in READ COMMITTED isolation level [YSQL] Match Pg semantics for other non-DDL statements in READ COMMITTED isolation level Aug 9, 2022
@pkj415 pkj415 changed the title [YSQL] Match Pg semantics for other non-DDL statements in READ COMMITTED isolation level [YSQL] Match Pg semantics for volatile functions in READ COMMITTED isolation level Aug 9, 2022
@pkj415 pkj415 mentioned this issue Aug 9, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pkj415 pkj415

Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug pg-compatibility Label for issues that result in differences b/w YSQL and Pg semantics priority/medium Medium priority issue
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@pkj415 @yugabyte-ci