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] Partition with out-of-order columns constructs incorrect UPDATE payload #25521

Open
1 task done
karthik-ramanathan-3006 opened this issue Jan 7, 2025 · 0 comments
Open
1 task done
Assignees
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue status/awaiting-triage Issue awaiting triage

Comments

@karthik-ramanathan-3006
Copy link
Contributor

karthik-ramanathan-3006 commented Jan 7, 2025

Jira Link: DB-14778

Description

Behavior observed on master (4a9292c / YugabyteDB on PG 15).

Consider the following test adapted from insert_conflict.sql

create table parted_conflict_test (a int unique, b char) partition by list (a);
create table parted_conflict_test_2 (b char, a int unique);
alter table parted_conflict_test attach partition parted_conflict_test_2 for values in (3);

insert into parted_conflict_test values (3, 'a') on conflict (a) do update set b = excluded.b;

-- The following query constructs incorrect UPDATE payload
insert into parted_conflict_test values (3, 'b') on conflict (a) do update set b = excluded.b;

Running the following SELECT query should give:

postgres=# select * from parted_conflict_test order by a;
 a | b
---+---
 3 | b
(1 row)

while it produces:

yugabyte=# select * from parted_conflict_test order by a;
 a | b
---+---
 3 | a
(1 row)

Relevant DocDB logs:

2025-01-08 03:31:07.904 IST [43147] DEBUG:  No cols in category: Columns that are inspected and modified
2025-01-08 03:31:07.904 IST [43147] DEBUG:  No cols in category: Columns that are inspected and unmodified
2025-01-08 03:31:07.904 IST [43147] DEBUG:  Relation: 21319	Columns that are marked for update: 2 (10)
I0107 14:01:07.904209 4055927808 pg_session.cc:289] Session id 2: Buffering operation: { WRITE active: 1 read_time: { read: <invalid> local_limit: <invalid> global_limit: <invalid> in_txn_limit: <invalid> serial_no: 0 } request: client: YQL_CLIENT_PGSQL stmt_id: 4719304992 stmt_type: PGSQL_UPDATE table_id: "000034cb000030008000000000005354" schema_version: 1 hash_code: 14030 ybctid_column_value { value { binary_value: "4736CE539446430562664B6D8908C19A0B53AE9500002121" } } column_new_values { column_id: 2 expr { value { int32_value: 3 } } } column_refs { ids: 2 } col_refs { column_id: 2 attno: 2 } partition_key: "6�" ysql_db_catalog_version: 119 ysql_db_oid: 13515 }

Column 2(10) corresponds to column b in the parent while 1(9) references the same column in the child partition.
However, notice that the update to the table uses column ID 2.

#14985 references a similar issue but was reported on YugabyteDB on PG 11.

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.
@karthik-ramanathan-3006 karthik-ramanathan-3006 added area/ysql Yugabyte SQL (YSQL) status/awaiting-triage Issue awaiting triage labels Jan 7, 2025
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels Jan 7, 2025
@karthik-ramanathan-3006 karthik-ramanathan-3006 removed the status/awaiting-triage Issue awaiting triage label Jan 7, 2025
@yugabyte-ci yugabyte-ci added the status/awaiting-triage Issue awaiting triage label Jan 7, 2025
karthik-ramanathan-3006 added a commit that referenced this issue Jan 22, 2025
…NSERT ... ON CONFLICT DO UPDATE queries

Summary:
## The problem
Postgres allows a table partition to define its columns in an order that is different from the parent (root) table. Consider the following example where the base table defines columns in the order [a, b, c], while the partition defines columns in the order [c, b, a]:

```
CREATE TABLE base (a INT PRIMARY KEY, b TEXT, c FLOAT) PARTITION BY LIST (a);

-- Create a partition
CREATE TABLE part1 (c FLOAT, b TEXT, a INT NOT NULL);
ALTER TABLE base ATTACH PARTITION part1 FOR VALUES IN (1);
```

Further, a query may insert/update a tuple in a partition while referencing the parent table as follows:

```
INSERT INTO p  VALUES (1, 'bar') ON CONFLICT (a) DO UPDATE SET a=1;
```

This implies that the input tuple is received in the layout of the parent table, and needs to be converted into the format of the partition before insert/update.

Vanilla postgres largely takes care of this by maintaining a tuple conversion map, which maintains mapping between columns in the parent to that in the partition. This conversion is applied whenever the tuple is routed, and the tuple is inserted/updated in the correct format and table.

However, in the case of Yugabyte’s implementation of INSERT ON CONFLICT .. DO UPDATE, we rely on a list of modified columns that is passed down from the planner to prepare the update payload. There are cases where the destination partition is not resolved at planning time, which results in the parent table’s columns to be passed down which is then incorrectly used to apply the update on the child partition.

This revision explicitly maps the columns of the root to that of the partition whose tuple is being updated when:
 - The operation being performed is INSERT ... ON CONFLICT DO UPDATE
 - The partitioned table defines its columns in an order that is different from that of its partition root.

Further, this revision also enables vanilla PG's INSERT ... ON CONFLICT tests that were previously commented out because of the above bugs.
Jira: DB-4172, DB-14778

Test Plan:
Run the following tests:
```
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressPgDml#schedule'
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressPartitions#yb_partitions_tests'
```

TBD: More tests will be added to yb_partitions_out_of_order.

Reviewers: aagrawal, amartsinchyk

Reviewed By: aagrawal

Subscribers: steve.varnau, smishra, yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D41110
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 status/awaiting-triage Issue awaiting triage
Projects
None yet
Development

No branches or pull requests

2 participants