Movatterモバイル変換

[0]ホーム
URL:

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

Commitdced55d

Browse files
committed
More code review for get_qual_for_list().
Avoid trashing the input PartitionBoundSpec; while that might be safe forcurrent callers, it's certainly trouble waiting to happen. In the samevein, make sure that all of the result data structure is freshly palloc'd,rather than some of it being pointers into the input data structures(which we don't know the lifespans of).Simplify the logic for tacking on IS NULL or IS NOT NULL conditions somemore; commit85c2b9a left a lot on the table there. And rearrange theconstruction of the nodes into (what seems to me) a more logical order.In passing, make sure that get_qual_for_range() also returns a freshlypalloc'd structure, since there's no value in having that guarantee foronly one kind of partitioning. And improve some comments there.Jeevan Ladhe, with further tweaking by meDiscussion:https://postgr.es/m/CAOgcT0MAcYoMs93W80iTUf_dP36=1mZQzeUk+nnwY_-qWDrCfw@mail.gmail.com
1 parent917d912 commitdced55d

File tree

1 file changed

+61
-65
lines changed

1 file changed

+61
-65
lines changed

‎src/backend/catalog/partition.c

Lines changed: 61 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1296,23 +1296,22 @@ make_partition_op_expr(PartitionKey key, int keynum,
12961296
/*
12971297
* get_qual_for_list
12981298
*
1299-
* Returns a list of expressions to use as a list partition's constraint.
1299+
* Returns an implicit-AND list of expressions to use as a list partition's
1300+
* constraint, given the partition key and bound structures.
13001301
*/
13011302
staticList*
13021303
get_qual_for_list(PartitionKeykey,PartitionBoundSpec*spec)
13031304
{
13041305
List*result;
1306+
Expr*keyCol;
13051307
ArrayExpr*arr;
13061308
Expr*opexpr;
1307-
ListCell*cell,
1308-
*prev,
1309-
*next;
1310-
Expr*keyCol;
1309+
NullTest*nulltest;
1310+
ListCell*cell;
1311+
List*arrelems=NIL;
13111312
boollist_has_null= false;
1312-
NullTest*nulltest1=NULL,
1313-
*nulltest2=NULL;
13141313

1315-
/*Left operand is either a simple Var or arbitrary expression */
1314+
/*Construct Var or expression representing the partition column */
13161315
if (key->partattrs[0]!=0)
13171316
keyCol= (Expr*)makeVar(1,
13181317
key->partattrs[0],
@@ -1323,74 +1322,62 @@ get_qual_for_list(PartitionKey key, PartitionBoundSpec *spec)
13231322
else
13241323
keyCol= (Expr*)copyObject(linitial(key->partexprs));
13251324

1326-
/*
1327-
* We must remove any NULL value in the list; we handle it separately
1328-
* below.
1329-
*/
1330-
prev=NULL;
1331-
for (cell=list_head(spec->listdatums);cell;cell=next)
1325+
/* Create list of Consts for the allowed values, excluding any nulls */
1326+
foreach(cell,spec->listdatums)
13321327
{
13331328
Const*val=castNode(Const,lfirst(cell));
13341329

1335-
next=lnext(cell);
1336-
13371330
if (val->constisnull)
1338-
{
13391331
list_has_null= true;
1340-
spec->listdatums=list_delete_cell(spec->listdatums,
1341-
cell,prev);
1342-
}
13431332
else
1344-
prev=cell;
1333+
arrelems=lappend(arrelems,copyObject(val));
13451334
}
13461335

1347-
if (!list_has_null)
1348-
{
1349-
/*
1350-
* Gin up a col IS NOT NULL test that will be AND'd with other
1351-
* expressions
1352-
*/
1353-
nulltest1=makeNode(NullTest);
1354-
nulltest1->arg=keyCol;
1355-
nulltest1->nulltesttype=IS_NOT_NULL;
1356-
nulltest1->argisrow= false;
1357-
nulltest1->location=-1;
1358-
}
1359-
else
1360-
{
1361-
/*
1362-
* Gin up a col IS NULL test that will be OR'd with other expressions
1363-
*/
1364-
nulltest2=makeNode(NullTest);
1365-
nulltest2->arg=keyCol;
1366-
nulltest2->nulltesttype=IS_NULL;
1367-
nulltest2->argisrow= false;
1368-
nulltest2->location=-1;
1369-
}
1370-
1371-
/* Right operand is an ArrayExpr containing this partition's values */
1336+
/* Construct an ArrayExpr for the non-null partition values */
13721337
arr=makeNode(ArrayExpr);
13731338
arr->array_typeid= !type_is_array(key->parttypid[0])
13741339
?get_array_type(key->parttypid[0])
13751340
:key->parttypid[0];
13761341
arr->array_collid=key->parttypcoll[0];
13771342
arr->element_typeid=key->parttypid[0];
1378-
arr->elements=spec->listdatums;
1343+
arr->elements=arrelems;
13791344
arr->multidims= false;
13801345
arr->location=-1;
13811346

13821347
/* Generate the main expression, i.e., keyCol = ANY (arr) */
13831348
opexpr=make_partition_op_expr(key,0,BTEqualStrategyNumber,
13841349
keyCol, (Expr*)arr);
13851350

1386-
if (nulltest1)
1387-
result=list_make2(nulltest1,opexpr);
1351+
if (!list_has_null)
1352+
{
1353+
/*
1354+
* Gin up a "col IS NOT NULL" test that will be AND'd with the main
1355+
* expression. This might seem redundant, but the partition routing
1356+
* machinery needs it.
1357+
*/
1358+
nulltest=makeNode(NullTest);
1359+
nulltest->arg=keyCol;
1360+
nulltest->nulltesttype=IS_NOT_NULL;
1361+
nulltest->argisrow= false;
1362+
nulltest->location=-1;
1363+
1364+
result=list_make2(nulltest,opexpr);
1365+
}
13881366
else
13891367
{
1368+
/*
1369+
* Gin up a "col IS NULL" test that will be OR'd with the main
1370+
* expression.
1371+
*/
13901372
Expr*or;
13911373

1392-
Assert(nulltest2!=NULL);
1393-
or=makeBoolExpr(OR_EXPR,list_make2(nulltest2,opexpr),-1);
1374+
nulltest=makeNode(NullTest);
1375+
nulltest->arg=keyCol;
1376+
nulltest->nulltesttype=IS_NULL;
1377+
nulltest->argisrow= false;
1378+
nulltest->location=-1;
1379+
1380+
or=makeBoolExpr(OR_EXPR,list_make2(nulltest,opexpr),-1);
13941381
result=list_make1(or);
13951382
}
13961383

@@ -1401,8 +1388,16 @@ get_qual_for_list(PartitionKey key, PartitionBoundSpec *spec)
14011388
* get_range_key_properties
14021389
*Returns range partition key information for a given column
14031390
*
1404-
* On return, *partexprs_item points to the cell containing the next
1405-
* expression in the key->partexprs list, or NULL.
1391+
* This is a subroutine for get_qual_for_range, and its API is pretty
1392+
* specialized to that caller.
1393+
*
1394+
* Constructs an Expr for the key column (returned in *keyCol) and Consts
1395+
* for the lower and upper range limits (returned in *lower_val and
1396+
* *upper_val). For UNBOUNDED limits, NULL is returned instead of a Const.
1397+
* All of these structures are freshly palloc'd.
1398+
*
1399+
* *partexprs_item points to the cell containing the next expression in
1400+
* the key->partexprs list, or NULL. It may be advanced upon return.
14061401
*/
14071402
staticvoid
14081403
get_range_key_properties(PartitionKeykey,intkeynum,
@@ -1412,7 +1407,7 @@ get_range_key_properties(PartitionKey key, int keynum,
14121407
Expr**keyCol,
14131408
Const**lower_val,Const**upper_val)
14141409
{
1415-
/*Partition key expression for this column */
1410+
/*Get partition key expression for this column */
14161411
if (key->partattrs[keynum]!=0)
14171412
{
14181413
*keyCol= (Expr*)makeVar(1,
@@ -1424,27 +1419,29 @@ get_range_key_properties(PartitionKey key, int keynum,
14241419
}
14251420
else
14261421
{
1422+
if (*partexprs_item==NULL)
1423+
elog(ERROR,"wrong number of partition key expressions");
14271424
*keyCol=copyObject(lfirst(*partexprs_item));
14281425
*partexprs_item=lnext(*partexprs_item);
14291426
}
14301427

1428+
/* Get appropriate Const nodes for the bounds */
14311429
if (!ldatum->infinite)
1432-
*lower_val=castNode(Const,ldatum->value);
1430+
*lower_val=castNode(Const,copyObject(ldatum->value));
14331431
else
14341432
*lower_val=NULL;
14351433

14361434
if (!udatum->infinite)
1437-
*upper_val=castNode(Const,udatum->value);
1435+
*upper_val=castNode(Const,copyObject(udatum->value));
14381436
else
14391437
*upper_val=NULL;
14401438
}
14411439

14421440
/*
14431441
* get_qual_for_range
14441442
*
1445-
* Get a list of expressions to use as a range partition's constraint.
1446-
* If there are multiple expressions, they are to be considered implicitly
1447-
* ANDed.
1443+
* Returns an implicit-AND list of expressions to use as a range partition's
1444+
* constraint, given the partition key and bound structures.
14481445
*
14491446
* For a multi-column range partition key, say (a, b, c), with (al, bl, cl)
14501447
* as the lower bound tuple and (au, bu, cu) as the upper bound tuple, we
@@ -1480,9 +1477,8 @@ get_range_key_properties(PartitionKey key, int keynum,
14801477
* does not really have a constraint, except the IS NOT NULL constraint for
14811478
* partition keys.
14821479
*
1483-
* If we end up with an empty result list, we append return a single-member
1484-
* list containing a constant-true expression in that case, because callers
1485-
* expect a non-empty list.
1480+
* If we end up with an empty result list, we return a single-member list
1481+
* containing a constant TRUE, because callers expect a non-empty list.
14861482
*/
14871483
staticList*
14881484
get_qual_for_range(PartitionKeykey,PartitionBoundSpec*spec)
@@ -1573,7 +1569,7 @@ get_qual_for_range(PartitionKey key, PartitionBoundSpec *spec)
15731569
/*
15741570
* Since get_range_key_properties() modifies partexprs_item, and we
15751571
* might need to start over from the previous expression in the later
1576-
* part of thisfunctiom, save away the current value.
1572+
* part of thisfunction, save away the current value.
15771573
*/
15781574
partexprs_item_saved=partexprs_item;
15791575

@@ -1638,6 +1634,7 @@ get_qual_for_range(PartitionKey key, PartitionBoundSpec *spec)
16381634
List*lower_or_arm_args=NIL,
16391635
*upper_or_arm_args=NIL;
16401636

1637+
/* Restart scan of columns from the i'th one */
16411638
j=i;
16421639
partexprs_item=partexprs_item_saved;
16431640

@@ -1702,7 +1699,6 @@ get_qual_for_range(PartitionKey key, PartitionBoundSpec *spec)
17021699
strategy,
17031700
keyCol,
17041701
(Expr*)upper_val));
1705-
17061702
}
17071703

17081704
/*
@@ -1759,7 +1755,7 @@ get_qual_for_range(PartitionKey key, PartitionBoundSpec *spec)
17591755
:linitial(upper_or_arms));
17601756

17611757
/* As noted above, caller expects the list to be non-empty. */
1762-
if (result==NULL)
1758+
if (result==NIL)
17631759
result=list_make1(makeBoolConst(true, false));
17641760

17651761
returnresult;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp