Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[YSQL] Insert with ON CONFLICT on a temporary table fails #19909

Open
1 task done
arpang opened this issue Nov 9, 2023 · 0 comments
Open
1 task done

[YSQL] Insert with ON CONFLICT on a temporary table fails #19909

arpang opened this issue Nov 9, 2023 · 0 comments
Assignees
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue

Comments

@arpang
Copy link
Contributor

arpang commented Nov 9, 2023

Jira Link: DB-8854

Description

Steps to reproduce:

create temporary table mytmp (id int primary key, name text, count int);
insert into mytmp values (1, 'foo', 0);
insert into mytmp values (1, 'foo') on conflict ON CONSTRAINT mytmp_pkey do update set id = mytmp.id+1;

throws:

ERROR:  invalid memory alloc request size 2139062175

Issue Type

kind/bug

Warning: Please confirm that this issue does not contain any sensitive information

  • I confirm this issue does not contain any sensitive information.
@arpang arpang added area/ysql Yugabyte SQL (YSQL) status/awaiting-triage Issue awaiting triage labels Nov 9, 2023
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels Nov 9, 2023
@arpang arpang changed the title [YSQL] Insert with ON CONFLICT on a temporary table fails. [YSQL] Insert with ON CONFLICT on a temporary table fails Nov 9, 2023
@sushantrmishra sushantrmishra removed the status/awaiting-triage Issue awaiting triage label Nov 9, 2023
arpang added a commit that referenced this issue Nov 14, 2023
Summary:
Fixes the following issues related to ON CONFLICT DO UPDATE clause:

- Issue 1: The commands:
```
create temporary table mytmp (id int primary key, name text, count int);
insert into mytmp values (1, 'foo', 0);
insert into mytmp values (1, 'foo') on conflict ON CONSTRAINT mytmp_pkey do update set id = mytmp.id+1;
```
throw
```
TRAP: FailedAssertion("ItemPointerIsValid(otid)", File: "../../../../../../../src/postgres/src/backend/access/heap/heapam.c", Line: 3216, PID: 44939)
0   postgres                            0x000000010112977c ExceptionalCondition + 280
1   postgres                            0x000000010077e868 heap_update + 316
2   postgres                            0x0000000100794c4c heapam_tuple_update + 168
3   postgres                            0x0000000100bb3580 table_tuple_update + 140
4   postgres                            0x0000000100bb2648 ExecUpdateAct + 620
5   postgres                            0x0000000100baee94 ExecUpdate + 744
6   postgres                            0x0000000100bb4a88 ExecOnConflictUpdate + 2680
```
This is because the merge commit D27766 incorrectly set `ybTid` to NULL  in `ExecOnConflictUpdate` for temporary relations. Bring back the old behavior for temporary relations.

- Issue 2: The above fix exposed another issue: insertion with ON CONFLICT clause into a temp table that has BEFORE ROW UPDATE triggers set fails at assertion `Assert(HeapTupleIsValid(fdw_trigtuple) ^ ItemPointerIsValid(tupleid))` in `ExecBRUpdateTriggers`. This is because in the pre-existing execution flow of function `ExecOnConflictUpdate`, `ybOldTuple ` is set for temporary relations. With the above-mentioned change, `ybTid` field is also set for such relations. But the assertion expects only one of them to be set. To fix the issue, do not set `ybOldTuple ` for temporary relations (the same behaviour as upstream PG). This issue should exist in the master branch as well, but I cannot verify it because inserting into a temp table with ON CONFLICT fails with `invalid memory alloc request size` there (GH #19909).

Also, do the following refactors:

- refactor 1: In function `ExecOnConflictUpdate`, replace `IsYBBackedRelation` with `IsYBRelation`. This does not have any run time implication because `IsYBBackedRelation` (in addition to when `IsYBRelation` is true) is true for views on non-temporary relations. For an insertion with ON CONFLICT clause on such a view, `ExecOnConflictUpdate` will be called for the underlying base relation. This change just makes the YB relation check consistent throughout the function.

- refactor 2: Add 'yb' prefix to some YB-specific variables.

Test Plan: Jenkins: rebase: pg15, test regex: org.yb.pgsql.TestPgRegress.*

Reviewers: jason, tnayak

Reviewed By: jason

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D30062
@github-project-automation github-project-automation bot moved this to Backlog in YSQL Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue
Projects
Status: Backlog
Development

No branches or pull requests

4 participants