Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 999b695

Browse files
author
Arseny Kositsyn
committed
[PGPRO-11599] Fix wrong results returned when order_by_attach=TRUE.
With order_by_attach=TRUE, the RUM index arranges the entries in the posting lists (for keys that have additional information) in order of sorting by additional information. For the remaining keys, the records are sorted by tid. The scanGetItemRegular() function assumes that all posting lists have the same sorting, so it was not suitable for sorting in this case. In order to fix this, the appropriate TIDs for keys that are ordered by tid are assembled in advance into a bitmap. For keys ordered in an alternative order, a routine scan is performed, after which the appropriate tid is checked in the bitmap. Tags: rum
1 parent 4c7cf69 commit 999b695

File tree

3 files changed

+221
-4
lines changed

3 files changed

+221
-4
lines changed

‎src/rum.h‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "utils/memutils.h"
2424

2525
#include "rumsort.h"
26+
#include "rumtidbitmap.h"
2627

2728
/* RUM distance strategies */
2829
#define RUM_DISTANCE 20
@@ -758,8 +759,24 @@ typedef struct RumScanOpaqueData
758759
bool willSort; /* is there any columns in ordering */
759760
RumScanType scanType;
760761

761-
ScanDirection naturalOrder;
762-
bool secondPass;
762+
ScanDirection naturalOrder;
763+
bool secondPass;
764+
765+
/*
766+
* scanWithAltOrderKeys = true if there are several keys in the query
767+
* that are sorted in different order (the entryGetItem() function for
768+
* different RumScanEntry will return the results to curItem in
769+
* different order).
770+
*
771+
* This happens when a multi-column RUM index is scanned and one of its
772+
* keys is ordered by additional information.
773+
*
774+
* In this case, all matching tids for keys that are ordered by tid will
775+
* be placed in the tbm. Keys ordered by additional information will be
776+
* scanned as usual.
777+
*/
778+
bool scanWithAltOrderKeys;
779+
RumTIDBitmap *tbm;
763780
} RumScanOpaqueData;
764781

765782
typedef RumScanOpaqueData *RumScanOpaque;

‎src/rumget.c‎

Lines changed: 194 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
#include "postgres.h"
1515
#include "rumsort.h"
16+
#include "rumtidbitmap.h"
1617

1718
#include "access/relscan.h"
1819
#include "storage/predicate.h"
@@ -759,6 +760,53 @@ scan_entry_cmp(const void *p1, const void *p2, void *arg)
759760
return -cmpEntries(arg, e1, e2);
760761
}
761762

763+
/*
764+
* The entryGetItem() function from rumget.c write the results to curItem
765+
* in a specific order. The functions below allow you to understand the order
766+
* in which the results will be returned. This is important in a multi-column
767+
* RUM index, when the results will be returned in different order for
768+
* different subquery conditions.
769+
*/
770+
static bool
771+
isEntryOrderedByAddInfo(RumState *rumstate, RumScanEntry entry)
772+
{
773+
if (rumstate->useAlternativeOrder)
774+
return (rumstate->attrnAddToColumn == entry->attnumOrig);
775+
else
776+
return false;
777+
}
778+
779+
static bool
780+
isKeyOrderedByAddInfo(RumState *rumstate, RumScanKey key)
781+
{
782+
if (rumstate->useAlternativeOrder)
783+
return (rumstate->attrnAddToColumn == key->attnumOrig);
784+
else
785+
return false;
786+
}
787+
788+
static bool
789+
isScanWithAltOrderKeys(RumScanOpaque so)
790+
{
791+
RumState *rumstate = &so->rumstate;
792+
793+
bool withAltKeys = false;
794+
bool withUsualKeys = false;
795+
796+
if (so->nkeys == 1)
797+
return false;
798+
799+
for (int i = 0; i < so->nkeys; i++)
800+
{
801+
if (isKeyOrderedByAddInfo(rumstate, so->keys[i]))
802+
withAltKeys = true;
803+
else
804+
withUsualKeys = true;
805+
}
806+
807+
return (withUsualKeys && withAltKeys);
808+
}
809+
762810
static void
763811
startScan(IndexScanDesc scan)
764812
{
@@ -806,6 +854,17 @@ startScan(IndexScanDesc scan)
806854
for (i = 0; i < so->nkeys; i++)
807855
startScanKey(rumstate, so->keys[i]);
808856

857+
/*
858+
* If there are several keys in the scan and its are ordered differently,
859+
* need to set the scanWithAltOrderKeys flag and create a bitmap for
860+
* TIDs that are suitable for regular keys (which are ordered by tid).
861+
*/
862+
if (isScanWithAltOrderKeys(so))
863+
{
864+
so->scanWithAltOrderKeys= true;
865+
so->tbm = rum_tbm_create(work_mem * 1024L, NULL);
866+
}
867+
809868
/*
810869
* Check if we can use a fast scan.
811870
* Use fast scan iff all keys have preConsistent method. But we can stop
@@ -1361,6 +1420,16 @@ compareCurRumItemScanDirection(RumState *rumstate, RumScanEntry entry,
13611420
&entry->curItem, minItem);
13621421
}
13631422

1423+
static int
1424+
rumCompareItemPointersScanDirection(ScanDirection scanDirection,
1425+
const ItemPointerData *a,
1426+
const ItemPointerData *b)
1427+
{
1428+
int res = rumCompareItemPointers(a, b);
1429+
1430+
return (ScanDirectionIsForward(scanDirection)) ? res : -res;
1431+
}
1432+
13641433
static void
13651434
keyGetItem(RumState * rumstate, MemoryContext tempCtx, RumScanKey key)
13661435
{
@@ -1445,6 +1514,63 @@ keyGetItem(RumState * rumstate, MemoryContext tempCtx, RumScanKey key)
14451514
MemoryContextReset(tempCtx);
14461515
}
14471516

1517+
/*
1518+
* The function puts all the suitable tids for
1519+
* the passed RumScanKey into the RumTIDBitmap.
1520+
*/
1521+
static void
1522+
collectAllCurItemsToBitmap(RumScanKey key, RumTIDBitmap *tbm,
1523+
IndexScanDesc scan)
1524+
{
1525+
RumScanOpaque so = (RumScanOpaque) scan->opaque;
1526+
RumState *rumstate = &so->rumstate;
1527+
ItemPointerData advancePast;
1528+
1529+
ItemPointerSetInvalid(&advancePast);
1530+
1531+
for (;;)
1532+
{
1533+
/*
1534+
* Set the curItem for all the RumScanEntry
1535+
* of our RumScanKey after advancePast.
1536+
*/
1537+
for (int i = 0; i < key->nentries; i++)
1538+
{
1539+
RumScanEntry entry = key->scanEntry[i];
1540+
1541+
while (entry->isFinished == false &&
1542+
(!ItemPointerIsValid(&advancePast) ||
1543+
rumCompareItemPointersScanDirection(entry->scanDirection,
1544+
&entry->curItem.iptr, &advancePast) <= 0))
1545+
{
1546+
entryGetItem(rumstate, entry, NULL, scan->xs_snapshot);
1547+
1548+
/*
1549+
* On the first iteration of the loop,
1550+
* RumScanEntry->curItem is the first available.
1551+
*/
1552+
if (!ItemPointerIsValid(&advancePast))
1553+
break;
1554+
}
1555+
}
1556+
1557+
/*
1558+
* For RumScanKey, we set the minimum
1559+
* curItem from its RumScanEntry.
1560+
*/
1561+
keyGetItem(rumstate, so->tempCtx, key);
1562+
1563+
if (key->isFinished)
1564+
return;
1565+
1566+
/* If curItem is suitable, put it in the RumTIDBitmap */
1567+
if (key->curItemMatches)
1568+
rum_tbm_add_tuples(tbm, &key->curItem.iptr, 1, false);
1569+
1570+
advancePast = key->curItem.iptr;
1571+
}
1572+
}
1573+
14481574
/*
14491575
* Get next heap item pointer (after advancePast) from scan.
14501576
* Returns true if anything found.
@@ -1453,6 +1579,11 @@ keyGetItem(RumState * rumstate, MemoryContext tempCtx, RumScanKey key)
14531579
* Note: this is very nearly the same logic as in keyGetItem(), except
14541580
* that we know the keys are to be combined with AND logic, whereas in
14551581
* keyGetItem() the combination logic is known only to the consistentFn.
1582+
*
1583+
* Note: In the case of a scan that contains several keys ordered in different
1584+
* ways, you need to prepare. To do this, for all keys ordered by tid,
1585+
* the appropriate TIDs are placed in the RumTIDBitmap. After that, all
1586+
* these bitmaps intersect (because the RumScanKeys are connected via AND).
14561587
*/
14571588
static bool
14581589
scanGetItemRegular(IndexScanDesc scan, RumItem *advancePast,
@@ -1465,6 +1596,43 @@ scanGetItemRegular(IndexScanDesc scan, RumItem *advancePast,
14651596
bool allFinished;
14661597
bool match, itemSet;
14671598

1599+
RumTIDBitmap *cur_key_tbm = NULL;
1600+
bool soBitmapSet = false;
1601+
bool bitmapRecheck = false;
1602+
1603+
/*
1604+
* Preparation to scan with altOrderKeys. This preparation
1605+
* needs to be done only in the first call to scanGetItemRegular().
1606+
*/
1607+
if (so->scanWithAltOrderKeys &&
1608+
ItemPointerIsValid(&advancePast->iptr) == false)
1609+
{
1610+
/* For each key, put all the appropriate tids into the bitmap */
1611+
for (int i = 0; i < so->nkeys; i++)
1612+
{
1613+
if (so->keys[i]->orderBy ||
1614+
isKeyOrderedByAddInfo(rumstate, so->keys[i]))
1615+
continue;
1616+
1617+
cur_key_tbm = rum_tbm_create(work_mem * 1024L, NULL);
1618+
collectAllCurItemsToBitmap(so->keys[i], cur_key_tbm, scan);
1619+
1620+
/*
1621+
* so->tbm contains matching tids for
1622+
* all keys that are ordered by tid.
1623+
*/
1624+
if (soBitmapSet == false)
1625+
{
1626+
soBitmapSet = true;
1627+
rum_tbm_union(so->tbm, cur_key_tbm);
1628+
}
1629+
else
1630+
rum_tbm_intersect(so->tbm, cur_key_tbm);
1631+
1632+
rum_tbm_free(cur_key_tbm);
1633+
}
1634+
}
1635+
14681636
for (;;)
14691637
{
14701638
/*
@@ -1478,6 +1646,11 @@ scanGetItemRegular(IndexScanDesc scan, RumItem *advancePast,
14781646
{
14791647
RumScanEntry entry = so->entries[i];
14801648

1649+
/* In the case of scanning with altOrderKeys, skip the usual keys */
1650+
if (so->scanWithAltOrderKeys &&
1651+
isEntryOrderedByAddInfo(rumstate, entry) == false)
1652+
continue;
1653+
14811654
while (entry->isFinished == false &&
14821655
(!ItemPointerIsValid(&myAdvancePast.iptr) ||
14831656
compareCurRumItemScanDirection(rumstate, entry,
@@ -1512,7 +1685,9 @@ scanGetItemRegular(IndexScanDesc scan, RumItem *advancePast,
15121685
RumScanKey key = so->keys[i];
15131686
int cmp;
15141687

1515-
if (key->orderBy)
1688+
/* In the case of scanning with altOrderKeys, skip the usual keys */
1689+
if (key->orderBy || (so->scanWithAltOrderKeys &&
1690+
isKeyOrderedByAddInfo(rumstate, key) == false))
15161691
continue;
15171692

15181693
keyGetItem(&so->rumstate, so->tempCtx, key);
@@ -1541,7 +1716,9 @@ scanGetItemRegular(IndexScanDesc scan, RumItem *advancePast,
15411716
{
15421717
RumScanKey key = so->keys[i];
15431718

1544-
if (key->orderBy)
1719+
/* In the case of scanning with altOrderKeys, skip the usual keys */
1720+
if (key->orderBy || (so->scanWithAltOrderKeys &&
1721+
isKeyOrderedByAddInfo(rumstate, key) == false))
15451722
continue;
15461723

15471724
if (key->curItemMatches)
@@ -1553,6 +1730,17 @@ scanGetItemRegular(IndexScanDesc scan, RumItem *advancePast,
15531730
break;
15541731
}
15551732

1733+
/*
1734+
* Now (if scan with altOrderKeys), if an *item is suitable for all
1735+
* altOrderKeys, you need to find it in the bitmap. If it's there,
1736+
* then it fits.
1737+
*
1738+
* Due to the fact that there may be lossy pages in the bitmap,
1739+
* you may need to recheck the *item.
1740+
*/
1741+
if (so->scanWithAltOrderKeys && match)
1742+
match = rum_tbm_contains_tid(so->tbm, &item->iptr, &bitmapRecheck);
1743+
15561744
if (match)
15571745
break;
15581746

@@ -1595,6 +1783,10 @@ scanGetItemRegular(IndexScanDesc scan, RumItem *advancePast,
15951783
}
15961784
}
15971785

1786+
/* If there was a lossy page in the bitmap, need to recheck */
1787+
if (so->scanWithAltOrderKeys && bitmapRecheck)
1788+
*recheck = true;
1789+
15981790
return true;
15991791
}
16001792

‎src/rumscan.c‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ rumbeginscan(Relation rel, int nkeys, int norderbys)
3838
"Rum scan temporary context");
3939
so->keyCtx = RumContextCreate(CurrentMemoryContext,
4040
"Rum scan key context");
41+
so->scanWithAltOrderKeys = false;
42+
so->tbm = NULL;
4143

4244
initRumState(&so->rumstate, scan->indexRelation);
4345

@@ -760,6 +762,12 @@ rumendscan(IndexScanDesc scan)
760762

761763
freeScanKeys(so);
762764

765+
if (so->tbm)
766+
{
767+
rum_tbm_free(so->tbm);
768+
so->tbm = NULL;
769+
}
770+
763771
MemoryContextDelete(so->tempCtx);
764772
MemoryContextDelete(so->keyCtx);
765773

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /