Hi,
On 04/04/2018 09:29 AM, David Rowley wrote:
Thanks for updating. I've made a pass over v49 and I didn't find very
much wrong with it.
The only real bug I found was a missing IsA(rinfo->clause, Const) in
the pseudoconstant check inside
generate_partition_pruning_steps_internal.
Most of the changes are comment fixes with a few stylistic changes
thrown which are pretty much all there just to try to shrink the code
a line or two or reduce indentation.
I feel pretty familiar with this code now and assuming the attached is
included I'm happy for someone else, hopefully, a committer to take a
look at it.
I'll leave the following notes:
1. Still not sure about RelOptInfo->has_default_part. This flag is
only looked at in generate_partition_pruning_steps. The RelOptInfo and
the boundinfo is available to look at, it's just that the
partition_bound_has_default macro is defined in partition.c rather
than partition.h.
2. Don't really like the new isopne variable name. It's not very
simple to decode, perhaps something like is_not_eq is better?
3. The part of the code I'm least familiar with is
get_steps_using_prefix_recurse(). I admit to not having had time to
fully understand that and consider ways to break it.
Marking as ready for committer.
Passes check-world, and CommitFest app has been updated to reflect the
current patch set. Trivial changes attached.
Best regards,
Jesper
>From 82f718579dc8e06ab77d76df4ed72f0f03ed4a4e Mon Sep 17 00:00:00 2001
From: jesperpedersen <jesper.peder...@redhat.com>
Date: Wed, 4 Apr 2018 11:27:59 -0400
Subject: [PATCH] Trivial changes
---
src/backend/catalog/partition.c | 10 +++++-----
src/backend/optimizer/util/partprune.c | 8 ++++----
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index d6bce9f348..7a268e05dc 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -146,7 +146,7 @@ typedef struct PruneStepResult
{
/*
* This contains the offsets of the bounds in a table's boundinfo, each of
- * which is a bound whose corresponding partition is selected by a a given
+ * which is a bound whose corresponding partition is selected by a given
* pruning step.
*/
Bitmapset *bound_offsets;
@@ -2026,7 +2026,7 @@ get_matching_hash_bounds(PartitionPruneContext *context,
int opstrategy, Datum *values, int nvalues,
FmgrInfo *partsupfunc, Bitmapset *nullkeys)
{
- PruneStepResult *result = palloc0(sizeof(PruneStepResult));
+ PruneStepResult *result = (PruneStepResult *) palloc0(sizeof(PruneStepResult));
PartitionBoundInfo boundinfo = context->boundinfo;
int *partindices = boundinfo->indexes;
int partnatts = context->partnatts;
@@ -2093,7 +2093,7 @@ get_matching_list_bounds(PartitionPruneContext *context,
int opstrategy, Datum value, int nvalues,
FmgrInfo *partsupfunc, Bitmapset *nullkeys)
{
- PruneStepResult *result = palloc0(sizeof(PruneStepResult));
+ PruneStepResult *result = (PruneStepResult *) palloc0(sizeof(PruneStepResult));
PartitionBoundInfo boundinfo = context->boundinfo;
int off,
minoff,
@@ -2147,7 +2147,7 @@ get_matching_list_bounds(PartitionPruneContext *context,
return result;
}
- /* Speical case handling of values coming from a <> operator clause. */
+ /* Special case handling of values coming from a <> operator clause. */
if (opstrategy == InvalidStrategy)
{
/*
@@ -2295,7 +2295,7 @@ get_matching_range_bounds(PartitionPruneContext *context,
int opstrategy, Datum *values, int nvalues,
FmgrInfo *partsupfunc, Bitmapset *nullkeys)
{
- PruneStepResult *result = palloc0(sizeof(PruneStepResult));
+ PruneStepResult *result = (PruneStepResult *) palloc0(sizeof(PruneStepResult));
PartitionBoundInfo boundinfo = context->boundinfo;
Oid *partcollation = context->partcollation;
int partnatts = context->partnatts;
diff --git a/src/backend/optimizer/util/partprune.c b/src/backend/optimizer/util/partprune.c
index 75b7232f5d..2d06c1a519 100644
--- a/src/backend/optimizer/util/partprune.c
+++ b/src/backend/optimizer/util/partprune.c
@@ -433,8 +433,8 @@ generate_partition_pruning_steps_internal(RelOptInfo *rel,
}
/*
- * Fall-through for a NOT clause, which if it's a Boolean clause
- * clause, will be handled in match_clause_to_partition_key(). We
+ * Fall-through for a NOT clause, which if it's a Boolean clause,
+ * will be handled in match_clause_to_partition_key(). We
* currently don't perform any pruning for more complex NOT
* clauses.
*/
@@ -665,7 +665,7 @@ match_clause_to_partition_key(RelOptInfo *rel,
*/
if (match_boolean_partition_clause(partopfamily, clause, partkey, &expr))
{
- *pc = palloc(sizeof(PartClauseInfo));
+ *pc = (PartClauseInfo *) palloc(sizeof(PartClauseInfo));
(*pc)->keyno = partkeyidx;
/* Do pruning with the Boolean equality operator. */
(*pc)->opno = BooleanEqualOperator;
@@ -806,7 +806,7 @@ match_clause_to_partition_key(RelOptInfo *rel,
else
cmpfn = part_scheme->partsupfunc[partkeyidx].fn_oid;
- *pc = palloc(sizeof(PartClauseInfo));
+ *pc = (PartClauseInfo *) palloc(sizeof(PartClauseInfo));
(*pc)->keyno = partkeyidx;
/* For <> operator clauses, pass on the negator. */
--
2.13.6
