Skip to content

Commit c607bd6

Browse files
tglsfdctglsfdc
committed
Clean up the usage of canonicalize_qual(): in particular, be consistent
about whether it is applied before or after eval_const_expressions(). I believe there were some corner cases where the system would fail to recognize that a partial index is applicable because of the previous inconsistency. Store normal rather than 'implicit AND' representations of constraints and index predicates in the catalogs. initdb forced due to representation change of constraints/predicates.
1 parent d167fb1 commit c607bd6

File tree

13 files changed

+149
-209
lines changed

13 files changed

+149
-209
lines changed

‎src/backend/catalog/heap.c

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.256 2003/11/29 19:51:42 pgsql Exp $
11+
* $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.257 2003/12/28 21:57:36 tgl Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -1344,10 +1344,9 @@ StoreRelCheck(Relation rel, char *ccname, char *ccbin)
13441344
int16 *attNos;
13451345

13461346
/*
1347-
* Convert condition to a normal boolean expression tree.
1347+
* Convert condition to an expression tree.
13481348
*/
13491349
expr = stringToNode(ccbin);
1350-
expr = (Node *) make_ands_explicit((List *) expr);
13511350

13521351
/*
13531352
* deparse it
@@ -1651,14 +1650,6 @@ AddRelationRawConstraints(Relation rel,
16511650
(errcode(ERRCODE_GROUPING_ERROR),
16521651
errmsg("cannot use aggregate function in check constraint")));
16531652

1654-
/*
1655-
* Constraints are evaluated with execQual, which expects an
1656-
* implicit-AND list, so convert expression to implicit-AND form.
1657-
* (We could go so far as to convert to CNF, but that's probably
1658-
* overkill...)
1659-
*/
1660-
expr = (Node *) make_ands_implicit((Expr *) expr);
1661-
16621653
/*
16631654
* OK, store it.
16641655
*/

‎src/backend/catalog/index.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.223 2003/11/29 19:51:43 pgsql Exp $
11+
* $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.224 2003/12/28 21:57:36 tgl Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -397,13 +397,14 @@ UpdateIndexRelation(Oid indexoid,
397397
exprsDatum = (Datum) 0;
398398

399399
/*
400-
* Convert the index predicate (if any) to a text datum
400+
* Convert the index predicate (if any) to a text datum. Note we
401+
* convert implicit-AND format to normal explicit-AND for storage.
401402
*/
402403
if (indexInfo->ii_Predicate != NIL)
403404
{
404405
char *predString;
405406

406-
predString = nodeToString(indexInfo->ii_Predicate);
407+
predString = nodeToString(make_ands_explicit(indexInfo->ii_Predicate));
407408
predDatum = DirectFunctionCall1(textin,
408409
CStringGetDatum(predString));
409410
pfree(predString);

‎src/backend/commands/indexcmds.c

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/indexcmds.c,v 1.116 2003/11/29 19:51:47 pgsql Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/indexcmds.c,v 1.117 2003/12/28 21:57:36 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -42,7 +42,7 @@
4242

4343

4444
/* non-export function prototypes */
45-
static void CheckPredicate(List *predList);
45+
static void CheckPredicate(Expr *predicate);
4646
static void ComputeIndexAttrs(IndexInfo *indexInfo, Oid *classOidP,
4747
List *attList,
4848
Oid relId,
@@ -80,7 +80,6 @@ DefineIndex(RangeVar *heapRelation,
8080
Form_pg_am accessMethodForm;
8181
IndexInfo *indexInfo;
8282
int numberOfAttributes;
83-
List *cnfPred = NIL;
8483

8584
/*
8685
* count attributes in index
@@ -172,16 +171,10 @@ DefineIndex(RangeVar *heapRelation,
172171
}
173172

174173
/*
175-
* Convert the partial-index predicate from parsetree form to an
176-
* implicit-AND qual expression, for easier evaluation at runtime.
177-
* While we are at it, we reduce it to a canonical (CNF or DNF) form
178-
* to simplify the task of proving implications.
174+
* Validate predicate, if given
179175
*/
180176
if (predicate)
181-
{
182-
cnfPred = canonicalize_qual((Expr *) copyObject(predicate), true);
183-
CheckPredicate(cnfPred);
184-
}
177+
CheckPredicate(predicate);
185178

186179
/*
187180
* Check that all of the attributes in a primary key are marked as not
@@ -237,13 +230,13 @@ DefineIndex(RangeVar *heapRelation,
237230

238231
/*
239232
* Prepare arguments for index_create, primarily an IndexInfo
240-
* structure
233+
* structure. Note that ii_Predicate must be in implicit-AND format.
241234
*/
242235
indexInfo = makeNode(IndexInfo);
243236
indexInfo->ii_NumIndexAttrs = numberOfAttributes;
244237
indexInfo->ii_Expressions = NIL; /* for now */
245238
indexInfo->ii_ExpressionsState = NIL;
246-
indexInfo->ii_Predicate = cnfPred;
239+
indexInfo->ii_Predicate = make_ands_implicit(predicate);
247240
indexInfo->ii_PredicateState = NIL;
248241
indexInfo->ii_Unique = unique;
249242

@@ -268,7 +261,7 @@ DefineIndex(RangeVar *heapRelation,
268261

269262
/*
270263
* CheckPredicate
271-
* Checks that the given list of partial-index predicates is valid.
264+
* Checks that the given partial-index predicate is valid.
272265
*
273266
* This used to also constrain the form of the predicate to forms that
274267
* indxpath.c could do something with. However, that seems overly
@@ -278,18 +271,18 @@ DefineIndex(RangeVar *heapRelation,
278271
* (except ones requiring a plan), and let indxpath.c fend for itself.
279272
*/
280273
static void
281-
CheckPredicate(List *predList)
274+
CheckPredicate(Expr *predicate)
282275
{
283276
/*
284277
* We don't currently support generation of an actual query plan for a
285278
* predicate, only simple scalar expressions; hence these
286279
* restrictions.
287280
*/
288-
if (contain_subplans((Node *) predList))
281+
if (contain_subplans((Node *) predicate))
289282
ereport(ERROR,
290283
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
291284
errmsg("cannot use subquery in index predicate")));
292-
if (contain_agg_clause((Node *) predList))
285+
if (contain_agg_clause((Node *) predicate))
293286
ereport(ERROR,
294287
(errcode(ERRCODE_GROUPING_ERROR),
295288
errmsg("cannot use aggregate in index predicate")));
@@ -298,7 +291,7 @@ CheckPredicate(List *predList)
298291
* A predicate using mutable functions is probably wrong, for the same
299292
* reasons that we don't allow an index expression to use one.
300293
*/
301-
if (contain_mutable_functions((Node *) predList))
294+
if (contain_mutable_functions((Node *) predicate))
302295
ereport(ERROR,
303296
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
304297
errmsg("functions in index predicate must be marked IMMUTABLE")));

‎src/backend/executor/execMain.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
*
2727
*
2828
* IDENTIFICATION
29-
* $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.223 2003/12/01 22:07:58 momjian Exp $
29+
* $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.224 2003/12/28 21:57:36 tgl Exp $
3030
*
3131
*-------------------------------------------------------------------------
3232
*/
@@ -40,6 +40,7 @@
4040
#include "executor/execdebug.h"
4141
#include "executor/execdefs.h"
4242
#include "miscadmin.h"
43+
#include "optimizer/clauses.h"
4344
#include "optimizer/var.h"
4445
#include "parser/parsetree.h"
4546
#include "utils/acl.h"
@@ -1658,7 +1659,8 @@ ExecRelCheck(ResultRelInfo *resultRelInfo,
16581659
(List **) palloc(ncheck * sizeof(List *));
16591660
for (i = 0; i < ncheck; i++)
16601661
{
1661-
qual = (List *) stringToNode(check[i].ccbin);
1662+
/* ExecQual wants implicit-AND form */
1663+
qual = make_ands_implicit(stringToNode(check[i].ccbin));
16621664
resultRelInfo->ri_ConstraintExprs[i] = (List *)
16631665
ExecPrepareExpr((Expr *) qual, estate);
16641666
}

‎src/backend/optimizer/plan/planner.c

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.162 2003/11/29 19:51:50 pgsql Exp $
11+
* $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.163 2003/12/28 21:57:36 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -377,30 +377,33 @@ preprocess_expression(Query *parse, Node *expr, int kind)
377377
expr = flatten_join_alias_vars(parse, expr);
378378

379379
/*
380-
* Simplify constant expressions.
381-
*
382-
* Note that at this point quals have not yet been converted to
383-
* implicit-AND form, so we can apply eval_const_expressions directly.
384-
*/
385-
expr = eval_const_expressions(expr);
386-
387-
/*
388-
* If it's a qual or havingQual, canonicalize it, and convert it to
389-
* implicit-AND format.
390-
*
391-
* XXX Is there any value in re-applying eval_const_expressions after
392-
* canonicalize_qual?
380+
* If it's a qual or havingQual, canonicalize it. It seems most useful
381+
* to do this before applying eval_const_expressions, since the latter
382+
* can optimize flattened AND/ORs better than unflattened ones.
393383
*/
394384
if (kind == EXPRKIND_QUAL)
395385
{
396-
expr = (Node *) canonicalize_qual((Expr *) expr, true);
386+
expr = (Node *) canonicalize_qual((Expr *) expr);
397387

398388
#ifdef OPTIMIZER_DEBUG
399389
printf("After canonicalize_qual()\n");
400390
pprint(expr);
401391
#endif
402392
}
403393

394+
/*
395+
* Simplify constant expressions.
396+
*/
397+
expr = eval_const_expressions(expr);
398+
399+
/*
400+
* If it's a qual or havingQual, convert it to implicit-AND format.
401+
* (We don't want to do this before eval_const_expressions, since the
402+
* latter would be unable to simplify a top-level AND correctly.)
403+
*/
404+
if (kind == EXPRKIND_QUAL)
405+
expr = (Node *) make_ands_implicit((Expr *) expr);
406+
404407
/* Expand SubLinks to SubPlans */
405408
if (parse->hasSubLinks)
406409
expr = SS_process_sublinks(expr, (kind == EXPRKIND_QUAL));

0 commit comments

Comments
 (0)