Skip to content

Commit

Permalink
[Bug Fix] : writing incorrect data to target Merge repartition Command (
Browse files Browse the repository at this point in the history
#7659)

We were writing incorrect data to target collection in some cases of merge command. In case of repartition when source query is RELATION. We were referring to incorrect attribute number that was resulting into
this incorrect behavior.

Example :

![image](https://github.com/user-attachments/assets/a101cb36-7976-459c-befb-96a55a5b3dc1)

![image](https://github.com/user-attachments/assets/e5c83b7b-5b8e-4d79-a927-95684dc9ba49)

I have added fixed tests as part of this PR , Thanks.
  • Loading branch information
paragikjain authored Sep 13, 2024
1 parent 4775715 commit 5bad6c6
Show file tree
Hide file tree
Showing 5 changed files with 194 additions and 4 deletions.
2 changes: 1 addition & 1 deletion src/backend/distributed/planner/merge_planner.c
Original file line number Diff line number Diff line change
Expand Up @@ -858,7 +858,7 @@ ConvertRelationRTEIntoSubquery(Query *mergeQuery, RangeTblEntry *sourceRte,
newRangeTableRef->rtindex = SINGLE_RTE_INDEX;
sourceResultsQuery->jointree = makeFromExpr(list_make1(newRangeTableRef), NULL);
sourceResultsQuery->targetList =
CreateAllTargetListForRelation(sourceRte->relid, requiredAttributes);
CreateFilteredTargetListForRelation(sourceRte->relid, requiredAttributes);
List *restrictionList =
GetRestrictInfoListForRelation(sourceRte, plannerRestrictionContext);
List *copyRestrictionList = copyObject(restrictionList);
Expand Down
4 changes: 1 addition & 3 deletions src/backend/distributed/planner/query_colocation_checker.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@
static RangeTblEntry * AnchorRte(Query *subquery);
static List * UnionRelationRestrictionLists(List *firstRelationList,
List *secondRelationList);
static List * CreateFilteredTargetListForRelation(Oid relationId,
List *requiredAttributes);
static List * CreateDummyTargetList(Oid relationId, List *requiredAttributes);
static TargetEntry * CreateTargetEntryForColumn(Form_pg_attribute attributeTuple, Index
rteIndex,
Expand Down Expand Up @@ -378,7 +376,7 @@ CreateAllTargetListForRelation(Oid relationId, List *requiredAttributes)
* only the required columns of the given relation. If there is not required
* columns then a dummy NULL column is put as the only entry.
*/
static List *
List *
CreateFilteredTargetListForRelation(Oid relationId, List *requiredAttributes)
{
Relation relation = relation_open(relationId, AccessShareLock);
Expand Down
2 changes: 2 additions & 0 deletions src/include/distributed/query_colocation_checker.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,7 @@ extern Query * WrapRteRelationIntoSubquery(RangeTblEntry *rteRelation,
List *requiredAttributes,
RTEPermissionInfo *perminfo);
extern List * CreateAllTargetListForRelation(Oid relationId, List *requiredAttributes);
extern List * CreateFilteredTargetListForRelation(Oid relationId,
List *requiredAttributes);

#endif /* QUERY_COLOCATION_CHECKER_H */
106 changes: 106 additions & 0 deletions src/test/regress/expected/merge_vcore.out
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,112 @@ WHEN MATCHED THEN DO NOTHING;
Filter: ('2'::bigint = id)
(11 rows)

DROP TABLE IF EXISTS source;
DROP TABLE IF EXISTS target;
-- Bug Fix Test as part of this PR
-- Test 1
CREATE TABLE source (
id int,
age int,
salary int
);
CREATE TABLE target (
id int,
age int,
salary int
);
SELECT create_distributed_table('source', 'id', colocate_with=>'none');
create_distributed_table
---------------------------------------------------------------------

(1 row)

SELECT create_distributed_table('target', 'id', colocate_with=>'none');
create_distributed_table
---------------------------------------------------------------------

(1 row)

INSERT INTO source (id, age, salary) VALUES (1,30, 100000);
MERGE INTO ONLY target USING source ON (source.id = target.id)
WHEN NOT MATCHED THEN
INSERT (id, salary) VALUES (source.id, source.salary);
SELECT * FROM TARGET;
id | age | salary
---------------------------------------------------------------------
1 | | 100000
(1 row)

DROP TABLE IF EXISTS source;
DROP TABLE IF EXISTS target;
-- Test 2
CREATE TABLE source (
id int,
age int,
salary int
);
CREATE TABLE target (
id int,
age int,
salary int
);
SELECT create_distributed_table('source', 'id', colocate_with=>'none');
create_distributed_table
---------------------------------------------------------------------

(1 row)

SELECT create_distributed_table('target', 'id', colocate_with=>'none');
create_distributed_table
---------------------------------------------------------------------

(1 row)

INSERT INTO source (id, age, salary) VALUES (1,30, 100000);
MERGE INTO ONLY target USING source ON (source.id = target.id)
WHEN NOT MATCHED THEN
INSERT (salary, id) VALUES (source.salary, source.id);
SELECT * FROM TARGET;
id | age | salary
---------------------------------------------------------------------
1 | | 100000
(1 row)

DROP TABLE IF EXISTS source;
DROP TABLE IF EXISTS target;
-- Test 3
CREATE TABLE source (
id int,
age int,
salary int
);
CREATE TABLE target (
id int,
age int,
salary int
);
SELECT create_distributed_table('source', 'id', colocate_with=>'none');
create_distributed_table
---------------------------------------------------------------------

(1 row)

SELECT create_distributed_table('target', 'id', colocate_with=>'none');
create_distributed_table
---------------------------------------------------------------------

(1 row)

INSERT INTO source (id, age, salary) VALUES (1,30, 100000);
MERGE INTO ONLY target USING source ON (source.id = target.id)
WHEN NOT MATCHED THEN
INSERT (salary, id, age) VALUES (source.age, source.id, source.salary);
SELECT * FROM TARGET;
id | age | salary
---------------------------------------------------------------------
1 | 100000 | 30
(1 row)

DROP TABLE IF EXISTS source;
DROP TABLE IF EXISTS target;
DROP SCHEMA IF EXISTS merge_vcore_schema CASCADE;
84 changes: 84 additions & 0 deletions src/test/regress/sql/merge_vcore.sql
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,90 @@ WHEN MATCHED THEN DO NOTHING;
DROP TABLE IF EXISTS source;
DROP TABLE IF EXISTS target;


-- Bug Fix Test as part of this PR
-- Test 1
CREATE TABLE source (
id int,
age int,
salary int
);

CREATE TABLE target (
id int,
age int,
salary int
);

SELECT create_distributed_table('source', 'id', colocate_with=>'none');
SELECT create_distributed_table('target', 'id', colocate_with=>'none');

INSERT INTO source (id, age, salary) VALUES (1,30, 100000);

MERGE INTO ONLY target USING source ON (source.id = target.id)
WHEN NOT MATCHED THEN
INSERT (id, salary) VALUES (source.id, source.salary);

SELECT * FROM TARGET;
DROP TABLE IF EXISTS source;
DROP TABLE IF EXISTS target;


-- Test 2
CREATE TABLE source (
id int,
age int,
salary int
);

CREATE TABLE target (
id int,
age int,
salary int
);

SELECT create_distributed_table('source', 'id', colocate_with=>'none');
SELECT create_distributed_table('target', 'id', colocate_with=>'none');

INSERT INTO source (id, age, salary) VALUES (1,30, 100000);

MERGE INTO ONLY target USING source ON (source.id = target.id)
WHEN NOT MATCHED THEN
INSERT (salary, id) VALUES (source.salary, source.id);

SELECT * FROM TARGET;
DROP TABLE IF EXISTS source;
DROP TABLE IF EXISTS target;


-- Test 3
CREATE TABLE source (
id int,
age int,
salary int
);

CREATE TABLE target (
id int,
age int,
salary int
);

SELECT create_distributed_table('source', 'id', colocate_with=>'none');
SELECT create_distributed_table('target', 'id', colocate_with=>'none');

INSERT INTO source (id, age, salary) VALUES (1,30, 100000);

MERGE INTO ONLY target USING source ON (source.id = target.id)
WHEN NOT MATCHED THEN
INSERT (salary, id, age) VALUES (source.age, source.id, source.salary);

SELECT * FROM TARGET;
DROP TABLE IF EXISTS source;
DROP TABLE IF EXISTS target;



DROP SCHEMA IF EXISTS merge_vcore_schema CASCADE;


Expand Down

0 comments on commit 5bad6c6

Please sign in to comment.