From 84c5b6988b58152fb77f3b35023c3e4dbcb6dc34 Mon Sep 17 00:00:00 2001 From: David Kimura Date: Thu, 22 Feb 2018 08:23:35 -0800 Subject: [PATCH] Fix AO/CO visimap first row number integer overflow (#4564) There is a scenario during AO/CO delete or update where the first row number obtained is negative. The error is caused when the first row number in the aovisimap of an AO/CO table exceeds int max. There is currently a bug where the first row number is retrieved from the tuple as an int, but the first row number is an int64. We fixed this by retrieving as an int64. Co-authored-by: Jimmy Yih Co-authored-by: David Kimura --- .../appendonly/appendonly_visimap_entry.c | 4 +- src/backend/access/appendonly/test/Makefile | 5 +- .../test/appendonly_visimap_entry_test.c | 88 +++++++++++++++++++ 3 files changed, 94 insertions(+), 3 deletions(-) create mode 100644 src/backend/access/appendonly/test/appendonly_visimap_entry_test.c diff --git a/src/backend/access/appendonly/appendonly_visimap_entry.c b/src/backend/access/appendonly/appendonly_visimap_entry.c index 4448f6143a..db5619ffc8 100644 --- a/src/backend/access/appendonly/appendonly_visimap_entry.c +++ b/src/backend/access/appendonly/appendonly_visimap_entry.c @@ -390,7 +390,7 @@ AppendOnlyVisimapEntry_CoversTuple( AppendOnlyVisimapEntry *visiMapEntry, AOTupleId *tupleId) { - int rowNum; + int64 rowNum; Assert(visiMapEntry); Assert(tupleId); @@ -419,7 +419,7 @@ AppendOnlyVisimapEntry_GetFirstRowNum( AOTupleId *tupleId) { (void) visiMapEntry; - int rowNum; + int64 rowNum; rowNum = AOTupleIdGet_rowNum(tupleId); return (rowNum / APPENDONLY_VISIMAP_MAX_RANGE) * APPENDONLY_VISIMAP_MAX_RANGE; diff --git a/src/backend/access/appendonly/test/Makefile b/src/backend/access/appendonly/test/Makefile index 1b48cfe0e4..d621889aa3 100644 --- a/src/backend/access/appendonly/test/Makefile +++ b/src/backend/access/appendonly/test/Makefile @@ -2,7 +2,7 @@ subdir=src/backend/access/appendonly top_builddir=../../../../.. include $(top_builddir)/src/Makefile.global -TARGETS=aomd appendonly_visimap appendonlywriter +TARGETS=aomd appendonly_visimap appendonlywriter appendonly_visimap_entry include $(top_builddir)/src/backend/mock.mk @@ -16,3 +16,6 @@ appendonly_visimap.t: \ appendonlywriter.t: \ $(MOCK_DIR)/backend/utils/hash/dynahash_mock.o + +appendonly_visimap_entry.t: + diff --git a/src/backend/access/appendonly/test/appendonly_visimap_entry_test.c b/src/backend/access/appendonly/test/appendonly_visimap_entry_test.c new file mode 100644 index 0000000000..c8fd71b82b --- /dev/null +++ b/src/backend/access/appendonly/test/appendonly_visimap_entry_test.c @@ -0,0 +1,88 @@ +#include +#include +#include +#include "cmockery.h" +#include "access/appendonlytid.h" + +#include "../appendonly_visimap_entry.c" + +void +test__AppendOnlyVisimapEntry_GetFirstRowNum(void **state) +{ + int64 result, expected; + ItemPointerData fake_ctid; + AOTupleId *tupleId = (AOTupleId *) &fake_ctid; + + /* 5 is less than APPENDONLY_VISIMAP_MAX_RANGE so should get 0 */ + AOTupleIdInit_Init(tupleId); + AOTupleIdInit_segmentFileNum(tupleId, 1); + AOTupleIdInit_rowNum(tupleId, 5); + expected = 0; + + result = AppendOnlyVisimapEntry_GetFirstRowNum(NULL, tupleId); + assert_true(result == expected); + + /* test to make sure we can go above INT32_MAX */ + AOTupleIdInit_rowNum(tupleId, 3000000000); + expected = 2999975936; + + result = AppendOnlyVisimapEntry_GetFirstRowNum(NULL, tupleId); + assert_true(result == expected); +} + +void +test__AppendOnlyVisimapEntry_CoversTuple(void **state) +{ + bool result; + ItemPointerData fake_ctid; + AOTupleId *tupleId = (AOTupleId *) &fake_ctid; + + AOTupleIdInit_Init(tupleId); + AOTupleIdInit_segmentFileNum(tupleId, 1); + AOTupleIdInit_rowNum(tupleId, 5); + + AppendOnlyVisimapEntry* visiMapEntry = malloc(sizeof(AppendOnlyVisimapEntry));; + + /* Check to see if we have an invalid AppendOnlyVisimapEntry. */ + visiMapEntry->segmentFileNum = -1; + visiMapEntry->firstRowNum = 32768; + result = AppendOnlyVisimapEntry_CoversTuple(visiMapEntry, tupleId); + assert_false(result); + + /* Check to see if we have mismatched segment file numbers. */ + visiMapEntry->segmentFileNum = 2; + result = AppendOnlyVisimapEntry_CoversTuple(visiMapEntry, tupleId); + assert_false(result); + + /* Tuple not covered by visimap entry. */ + visiMapEntry->segmentFileNum = 1; + result = AppendOnlyVisimapEntry_CoversTuple(visiMapEntry, tupleId); + assert_false(result); + + /* Tuple is covered by visimap entry. */ + visiMapEntry->firstRowNum = 0; + result = AppendOnlyVisimapEntry_CoversTuple(visiMapEntry, tupleId); + assert_true(result); + + /* Tuple is covered by visimap entry above INT32_MAX row number. */ + AOTupleIdInit_rowNum(tupleId, 3000000000); + visiMapEntry->firstRowNum = 2999975936; + result = AppendOnlyVisimapEntry_CoversTuple(visiMapEntry, tupleId); + assert_true(result); +} + + +int +main(int argc, char *argv[]) +{ + cmockery_parse_arguments(argc, argv); + + const UnitTest tests[] = { + unit_test(test__AppendOnlyVisimapEntry_GetFirstRowNum), + unit_test(test__AppendOnlyVisimapEntry_CoversTuple) + }; + + MemoryContextInit(); + + return run_tests(tests); +} -- GitLab