Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()
On 2015-07-12 22:45:18 +0200, Andres Freund wrote: I'm right now not really coming up with a better idea how to fix this. So I guess I'll apply something close to this tomorrow. Took a bit longer than that :( I've pushed a version of your patch. I just opted to remove p_is_update instead of allowing both to be set at the same time. To me that seemed simpler. Thanks for the fix! Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()
On 2015-05-28 18:31:56 -0700, Peter Geoghegan wrote:
Subject: [PATCH 3/3] Fix bug in unique index inference
ON CONFLICT unique index inference had a thinko that could affect cases
where the user-supplied inference clause required that an attribute
match a particular (user named) collation and/or opclass.
Firstly, infer_collation_opclass_match() matched on opclass and/or
collation. Secondly, the attribute had to be in the list of attributes
or expressions known to be in the definition of the index under
consideration. The second step wasn't correct though, because having
some match doesn't necessarily mean that the second step found the same
index attribute as the (collation/opclass wise) match from the first
step.
Yes, makes sense.
+ else
+ {
+ Node *nattExpr = list_nth(idxExprs, (natt - 1) -
nplain);
+
+ /*
+ * Note that unlike routines like
match_index_to_operand(), we're
+ * unconcerned about RelabelType. An exact match is
required.
+ */
+ if (equal(elem-expr, nattExpr))
+ return true;
Why is that?
Regads,
Andres
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()
On Fri, Jul 24, 2015 at 2:58 AM, Andres Freund and...@anarazel.de wrote: I've pushed a version of your patch. I just opted to remove p_is_update instead of allowing both to be set at the same time. To me that seemed simpler. I would be hesitant to remove a struct field from a struct that appears as a hook argument. Someone's extension (that uses parser hooks) might have been relying on that. Perhaps not a big deal. Thanks -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()
On July 24, 2015 7:57:43 PM GMT+02:00, Peter Geoghegan p...@heroku.com wrote: On Fri, Jul 24, 2015 at 2:58 AM, Andres Freund and...@anarazel.de wrote: I've pushed a version of your patch. I just opted to remove p_is_update instead of allowing both to be set at the same time. To me that seemed simpler. I would be hesitant to remove a struct field from a struct that appears as a hook argument. Someone's extension (that uses parser hooks) might have been relying on that. Perhaps not a big deal. They'd also be affected by the change in meaning... --- Please excuse brevity and formatting - I am writing this on my mobile phone. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()
On Fri, Jul 24, 2015 at 3:08 AM, Andres Freund and...@anarazel.de wrote:
+ else
+ {
+ Node *nattExpr = list_nth(idxExprs, (natt - 1) -
nplain);
+
+ /*
+ * Note that unlike routines like
match_index_to_operand(), we're
+ * unconcerned about RelabelType. An exact match is
required.
+ */
+ if (equal(elem-expr, nattExpr))
+ return true;
Why is that?
No very strong reason. RelabelType exists to represent a dummy
coercion between two binary-compatible types. I think that a unique
index inference specification (which is novel in some ways) does not
need to do anything special for this case.
Each inference specification attribute that is an expression should
match some attribute in some index's cataloged definition. The
inference specification looks very much like the CREATE UNIQUE INDEX
that created the unique index that is inferred (usually, they'll be
identical). No need to make it any more complicated than that.
In fact, I don't think it's possible to construct a case where it
could even be argued that it matters. I'm not very caffeinated at the
moment, so I'm not sure of that.
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()
On 2015-05-28 14:37:43 -0700, Peter Geoghegan wrote:
To fix, allow ParseState to reflect that an individual statement can be
both p_is_insert and p_is_update at the same time.
/* Process DO UPDATE */
if (onConflictClause-action == ONCONFLICT_UPDATE)
{
+ /* p_is_update must be set here, after INSERT targetlist
processing */
+ pstate-p_is_update = true;
+
It's not particularly pretty that you document in the commit message
that both is_insert and is_update can be set at the same time, and then
it has constraints like the above.
But that's more crummy API's fault than yours.
I'm right now not really coming up with a better idea how to fix
this. So I guess I'll apply something close to this tomorrow.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()
On Sun, Jul 12, 2015 at 1:45 PM, Andres Freund and...@anarazel.de wrote: But that's more crummy API's fault than yours. As you probably noticed, the only reason the p_is_update and p_is_insert fields exist is for transformAssignedExpr() -- in fact, in the master branch, nothing checks the value of p_is_update (although I suppose a hook into a third-party module could see that module test ParseState.p_is_update, so that isn't quite true). I'm right now not really coming up with a better idea how to fix this. So I guess I'll apply something close to this tomorrow. Sounds good. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()
On Sat, May 30, 2015 at 3:12 PM, Peter Geoghegan p...@heroku.com wrote: Debugging this allowed me to come up with a significantly simplified approach. Attached is a new version of the original fix. Details are in commit message -- there is no actual need to have search_indexed_tlist_for_var() care about Vars being resjunk in a special way, which is a good thing. It feels wrong to not have the additional, paranoid IsVar() check within pull_var_targetlist_clause() check added in most recent revision, even though it should not be necessary. Attached delta patch adds this check. I need to stop working on weekends... -- Peter Geoghegan From 8542ff2623d7f2142ecb8c21c572b47d67500231 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan peter.geoghega...@gmail.com Date: Sat, 30 May 2015 15:25:55 -0700 Subject: [PATCH 2/7] Add additional, paranoid check --- src/backend/optimizer/prep/preptlist.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c index 6b7adb7..8c418d2 100644 --- a/src/backend/optimizer/prep/preptlist.c +++ b/src/backend/optimizer/prep/preptlist.c @@ -432,7 +432,7 @@ pull_var_targetlist_clause(OnConflictExpr *onconfl, List* clause, if (tlist_member((Node *) var, onconfl-exclRelTlist)) continue; /* already got it */ - if (var-varattno != InvalidAttrNumber) + if (!IsA(var, Var) || var-varattno != InvalidAttrNumber) elog(ERROR, cannot pull up non-wholerow Var in excluded targetlist); tle = makeTargetEntry((Expr *) var, var-varattno, NULL, true); -- 1.9.1 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()
On Sat, May 30, 2015 at 1:07 AM, Peter Geoghegan p...@heroku.com wrote: My fix for this issue (0001-Fix-bug-with-whole-row-Vars-in-excluded-targetlist.patch) still missed something. There needs to be additional handling in ruleutils.c: Debugging this allowed me to come up with a significantly simplified approach. Attached is a new version of the original fix. Details are in commit message -- there is no actual need to have search_indexed_tlist_for_var() care about Vars being resjunk in a special way, which is a good thing. There is also no need for further ruleutils.c specialization, as I implied before. Some deparsing tests are now included on top of what was already in the first version. -- Peter Geoghegan From 9d2f2b51ed3640a6a89313b7be3365168746ee00 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan peter.geoghega...@gmail.com Date: Mon, 25 May 2015 01:08:30 -0700 Subject: [PATCH 1/6] Fix bug with whole row Vars in excluded targetlist Follow the approach taken with RETURNING clauses containing references to multiple relations; pull up Vars referencing non-target relations (limited here to the ON CONFLICT DO UPDATE excluded pseudo-relation) that appear in ON CONFLICT DO UPDATE, and add the Vars to the excluded relation targetlist as resjunk. setrefs.c is also changed to call build_tlist_index_other_vars() rather than build_tlist_index(); this isn't important for correctness, but it seems preferable to make the code more consistent with the well established set_returning_clause_references() case. In passing, make a few minor stylistic tweaks, and reject Vars referencing the excluded.* pseudo relation in an invalid manner. These system column Vars are never useful or meaningful, since, of course, excluded is not a real table. --- src/backend/executor/execMain.c | 2 + src/backend/executor/nodeModifyTable.c| 9 +++- src/backend/optimizer/plan/planner.c | 14 +++-- src/backend/optimizer/plan/setrefs.c | 18 --- src/backend/optimizer/prep/preptlist.c| 74 +-- src/include/optimizer/prep.h | 4 +- src/test/regress/expected/insert_conflict.out | 50 ++ src/test/regress/expected/rules.out | 18 +++ src/test/regress/sql/insert_conflict.sql | 24 + src/test/regress/sql/rules.sql| 2 +- 10 files changed, 183 insertions(+), 32 deletions(-) diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index a1561ce..f2c0c64 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1246,6 +1246,8 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo, resultRelInfo-ri_ConstraintExprs = NULL; resultRelInfo-ri_junkFilter = NULL; resultRelInfo-ri_projectReturning = NULL; + resultRelInfo-ri_onConflictSetProj = NULL; + resultRelInfo-ri_onConflictSetWhere = NIL; } /* diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 874ca6a..30a092b 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1505,13 +1505,18 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) mtstate-resultRelInfo = estate-es_result_relations + node-resultRelIndex; mtstate-mt_arowmarks = (List **) palloc0(sizeof(List *) * nplans); mtstate-mt_nplans = nplans; - mtstate-mt_onconflict = node-onConflictAction; - mtstate-mt_arbiterindexes = node-arbiterIndexes; /* set up epqstate with dummy subplan data for the moment */ EvalPlanQualInit(mtstate-mt_epqstate, estate, NULL, NIL, node-epqParam); mtstate-fireBSTriggers = true; + /* initialize ON CONFLICT data */ + mtstate-mt_onconflict = node-onConflictAction; + mtstate-mt_arbiterindexes = node-arbiterIndexes; + mtstate-mt_existing = NULL; + mtstate-mt_excludedtlist = NIL; + mtstate-mt_conflproj = NULL; + /* * call ExecInitNode on each of the plans to be executed and save the * results into the array mt_plans. This is also a convenient place to diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 8afde2b..4ce6ee85 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -488,7 +488,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse, EXPRKIND_TARGET); parse-onConflict-onConflictWhere = - preprocess_expression(root, (Node *) parse-onConflict-onConflictWhere, + preprocess_expression(root, parse-onConflict-onConflictWhere, EXPRKIND_QUAL); } @@ -1273,7 +1273,6 @@ grouping_planner(PlannerInfo *root, double tuple_fraction) bool use_hashed_grouping = false; WindowFuncLists *wflists = NULL; List *activeWindows = NIL; - OnConflictExpr *onconfl; int maxref = 0; int *tleref_to_colnum_map; List *rollup_lists = NIL; @@ -1364,12 +1363,11 @@ grouping_planner(PlannerInfo *root, double tuple_fraction) /* Preprocess targetlist */
Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()
On Thu, May 28, 2015 at 2:37 PM, Peter Geoghegan p...@heroku.com wrote:
Attached, revised version incorporates this small fix, while adding an
additional big fix, and a number of small style tweaks.
This is mainly concerned with fixing the bug I was trying to fix when
I spotted the minor pfree() issue:
postgres=# insert into upsert (key, val) values('Foo', 'Bar') on
conflict (key) do update set val = excluded.val where excluded.* is
not null;
ERROR: XX000: variable not found in subplan target lists
LOCATION: fix_join_expr_mutator, setrefs.c:2003
My fix for this issue
(0001-Fix-bug-with-whole-row-Vars-in-excluded-targetlist.patch) still
missed something. There needs to be additional handling in
ruleutils.c:
postgres=# explain insert into upsert as u
values (1, 'fooz') on conflict (key)
do update set val = excluded.val where excluded.* is not null;
ERROR: XX000: bogus varattno for INNER_VAR var: 0
LOCATION: get_variable, ruleutils.c:5904
I'll look for a fix for this additional issue tomorrow.
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()
On Thu, May 28, 2015 at 6:31 PM, Peter Geoghegan p...@heroku.com wrote:
This concerns a thinko in unique index inference. See the commit
message for full details.
It seems I missed a required defensive measure here. Attached patch
adds it, too.
--
Peter Geoghegan
From c5aee669bbdf58f38f1063934a1d93286506de7b Mon Sep 17 00:00:00 2001
From: Peter Geoghegan peter.geoghega...@gmail.com
Date: Fri, 29 May 2015 02:53:41 -0700
Subject: [PATCH 4/4] Additional defensive measure
---
src/backend/optimizer/util/plancat.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 1b81f4c..4f38972 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -758,7 +758,7 @@ infer_collation_opclass_match(InferenceElem *elem, Relation idxRel,
if (((Var *) elem-expr)-varattno == attno)
return true;
}
- else
+ else if (attno == 0)
{
Node *nattExpr = list_nth(idxExprs, (natt - 1) - nplain);
--
1.9.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()
On Sun, May 24, 2015 at 6:17 PM, Peter Geoghegan p...@heroku.com wrote:
Attached patch adds such a pfree() call.
Attached, revised version incorporates this small fix, while adding an
additional big fix, and a number of small style tweaks.
This is mainly concerned with fixing the bug I was trying to fix when
I spotted the minor pfree() issue:
postgres=# insert into upsert (key, val) values('Foo', 'Bar') on
conflict (key) do update set val = excluded.val where excluded.* is
not null;
ERROR: XX000: variable not found in subplan target lists
LOCATION: fix_join_expr_mutator, setrefs.c:2003
postgres=# insert into upsert (key, val) values(('Foo', 'Bar') on
conflict (key) do update set val = excluded.val where excluded.ctid is
not null;
ERROR: XX000: variable not found in subplan target lists
LOCATION: fix_join_expr_mutator, setrefs.c:2003
The first query shown should clearly finish processing by the
optimizer without raising this error message (execution should work
correctly too, of course). The second query shown should fail with a
user visible error message about referencing the excluded
pseudo-relation's ctid column not making sense.
The basic problem is that there wasn't much thought put into how the
excluded pseudo-relation's reltargetlist is generated -- it
currently comes from a call to expandRelAttrs() during parse analysis,
which, on its own, doesn't allow whole row Vars to work.
One approach to fixing this is to follow the example of RETURNING
lists with references to more than one relation:
preprocess_targetlist() handles this by calling pull_var_clause() and
making new TargetEntry entries for Vars found to not be referencing
the target (and not already in the targetlist for some other reason).
Another approach, preferred by Andres, is to have query_planner() do
more. I understand that the idea there is to make excluded.* closer to
a regular table, in that it can be expected to have a baserel, and
within the executor we have something closer to a bona-fide scan
reltargetlist, that we can expect to have all Vars appear in. This
should be enough to make the reltargetlist have the appropriate Vars
more or less in the regular course of planning, including excluded.*
whole row Vars. To make this work we could call
add_vars_to_targetlist(), and call add_base_rels_to_query() and then
build_base_rel_tlists() within query_planner() (while moving a few
other things around).
However, the ordering dependencies within query_planner() seemed quite
complicated to me, and I didn't want to modify such an important
routine just to fix this bug. RETURNING seemed like a perfectly good
precedent to follow, so that's what I did. Now, it might have been
that I misunderstood Andres when we discussed this problem on
Jabber/IM, but ISTM that the second idea doesn't have much advantage
over the first (I'm sure that Andres will be able to explain what he'd
like to do better here -- it was a quick conversation). I did
prototype the second approach, and the code footprint of what I have
here (the first approach) seems lower than it would have to be with
the second. Besides, I didn't see a convenient choke point to reject
system column references with the second approach. Attached patch
fixes the bug using the first approach. Tests were added demonstrating
that the cases above are fixed.
A second attached patch fixes another, largely independent bug. I
noticed array assignment with ON CONFLICT DO UPDATE was broken. See
commit message for full details.
Thoughts?
--
Peter Geoghegan
From 72076f565b142debe39eb1e5a6cac27100b76fdb Mon Sep 17 00:00:00 2001
From: Peter Geoghegan peter.geoghega...@gmail.com
Date: Thu, 28 May 2015 00:18:19 -0700
Subject: [PATCH 2/2] Fix bug with assigned-to expressions with indirection
Handling of assigned-to expressions with indirection (as used with
UPDATE targetlists, where, for example, assigned-to expressions allow
array elements to be directly assigned to) could previously become
confused. The problem was that ParseState was consulted to determine if
an INSERT-appropriate or UPDATE-appropriate behavior should be used, and
so for INSERT statements with ON CONFLICT DO UPDATE, an
INSERT-targetlist-applicable codepath could incorrectly be taken.
To fix, allow ParseState to reflect that an individual statement can be
both p_is_insert and p_is_update at the same time.
---
src/backend/parser/analyze.c | 3 +++
src/backend/parser/parse_target.c| 3 ++-
src/include/parser/parse_node.h | 2 +-
src/test/regress/expected/arrays.out | 21 +
src/test/regress/sql/arrays.sql | 13 +
5 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index fc463fa..be474dc 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -891,6 +891,9 @@ transformOnConflictClause(ParseState *pstate,
/* Process DO UPDATE */
if (onConflictClause-action ==
Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()
On Thu, May 28, 2015 at 2:37 PM, Peter Geoghegan p...@heroku.com wrote:
A second attached patch fixes another, largely independent bug. I
noticed array assignment with ON CONFLICT DO UPDATE was broken. See
commit message for full details.
Finally, here is a third patch, fixing the final bug that I discussed
with you privately. There are now fixes for all bugs that I'm
currently aware of.
This concerns a thinko in unique index inference. See the commit
message for full details.
--
Peter Geoghegan
From 904f46f4d9e7758c588254e2c2bb3ef3ef93f3c9 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan peter.geoghega...@gmail.com
Date: Thu, 28 May 2015 15:45:48 -0700
Subject: [PATCH 3/3] Fix bug in unique index inference
ON CONFLICT unique index inference had a thinko that could affect cases
where the user-supplied inference clause required that an attribute
match a particular (user named) collation and/or opclass.
Firstly, infer_collation_opclass_match() matched on opclass and/or
collation. Secondly, the attribute had to be in the list of attributes
or expressions known to be in the definition of the index under
consideration. The second step wasn't correct though, because having
some match doesn't necessarily mean that the second step found the same
index attribute as the (collation/opclass wise) match from the first
step.
To fix, make infer_collation_opclass_match() more discriminating in its
second step.
---
src/backend/optimizer/util/plancat.c | 45 +--
src/test/regress/expected/insert_conflict.out | 18 +++
src/test/regress/sql/insert_conflict.sql | 12 +++
3 files changed, 59 insertions(+), 16 deletions(-)
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index b04dc2e..1b81f4c 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -52,7 +52,7 @@ get_relation_info_hook_type get_relation_info_hook = NULL;
static bool infer_collation_opclass_match(InferenceElem *elem, Relation idxRel,
- Bitmapset *inferAttrs, List *idxExprs);
+ List *idxExprs);
static int32 get_rel_data_width(Relation rel, int32 *attr_widths);
static List *get_relation_constraints(PlannerInfo *root,
Oid relationObjectId, RelOptInfo *rel,
@@ -615,8 +615,7 @@ infer_arbiter_indexes(PlannerInfo *root)
* this for both expressions and ordinary (non-expression)
* attributes appearing as inference elements.
*/
- if (!infer_collation_opclass_match(elem, idxRel, inferAttrs,
- idxExprs))
+ if (!infer_collation_opclass_match(elem, idxRel, idxExprs))
goto next;
/*
@@ -681,11 +680,10 @@ next:
* infer_collation_opclass_match - ensure infer element opclass/collation match
*
* Given unique index inference element from inference specification, if
- * collation was specified, or if opclass (represented here as opfamily +
- * opcintype) was specified, verify that there is at least one matching
- * indexed attribute (occasionally, there may be more). Skip this in the
- * common case where inference specification does not include collation or
- * opclass (instead matching everything, regardless of cataloged
+ * collation was specified, or if opclass was specified, verify that there is
+ * at least one matching indexed attribute (occasionally, there may be more).
+ * Skip this in the common case where inference specification does not include
+ * collation or opclass (instead matching everything, regardless of cataloged
* collation/opclass of indexed attribute).
*
* At least historically, Postgres has not offered collations or opclasses
@@ -707,11 +705,12 @@ next:
*/
static bool
infer_collation_opclass_match(InferenceElem *elem, Relation idxRel,
- Bitmapset *inferAttrs, List *idxExprs)
+ List *idxExprs)
{
AttrNumber natt;
- Oid inferopfamily = InvalidOid; /* OID of att opfamily */
- Oid inferopcinputtype = InvalidOid; /* OID of att opfamily */
+ Oid inferopfamily = InvalidOid; /* OID of opclass opfamily */
+ Oid inferopcinputtype = InvalidOid; /* OID of opclass input type */
+ int nplain = 0; /* # plain attrs observed */
/*
* If inference specification element lacks collation/opclass, then no
@@ -734,6 +733,10 @@ infer_collation_opclass_match(InferenceElem *elem, Relation idxRel,
Oid opfamily = idxRel-rd_opfamily[natt - 1];
Oid opcinputtype = idxRel-rd_opcintype[natt - 1];
Oid collation = idxRel-rd_indcollation[natt - 1];
+ int attno = idxRel-rd_index-indkey.values[natt - 1];
+
+ if (attno != 0)
+ nplain++;
if (elem-inferopclass != InvalidOid
(inferopfamily != opfamily || inferopcinputtype != opcinputtype))
@@ -749,12 +752,22 @@ infer_collation_opclass_match(InferenceElem *elem, Relation idxRel,
continue;
}
- if ((IsA(elem-expr, Var)
- bms_is_member(((Var *) elem-expr)-varattno, inferAttrs)) ||
- list_member(idxExprs, elem-expr))
+ /* If one matching
