CVE-2017-7484-induced bugs, or, btree cmp functions are not leakproof?
I poked into the odd behavior reported in bug #15251:
/messages/by-id/152967245839.1266.6939666809369185595@wrigleys.postgresql.org
The reason for the poor plan chosen when the caller hasn't got select
privilege on the child table is that statistic_proc_security_check()
decides not to allow use of the stats for the child table. There are
two functions for which it decides that, because they aren't leakproof:
textregexeq() and btint4cmp(). Now, it's probably necessary that we
consider textregexeq() leaky, since it might spit up errors about its
pattern argument. But btint4cmp? It seems pretty silly that the
integer comparison operators are marked leakproof while their underlying
comparison function isn't.
(The reason we're hitting this is that calc_arraycontsel() finds the
datatype's default btree comparison function and tries to use that
to estimate selectivity. While marking btint4cmp leakproof doesn't
completely fix the misestimation, it goes a long way in this example.)
I propose to run through the system operator classes, find any for which
the comparison function isn't marked leakproof but the operators are,
and fix them. This is clearly appropriate for HEAD and maybe it's not
too late to force an initdb for v11 --- thoughts?
Another question that could be raised is why we are refusing to use
stats for a child table when the caller has select on the parent.
It's completely trivial to extract data from a child table if you
have select on the parent, so it seems like we are checking the
wrong table's privileges.
regards, tom lane
On 2018-Jul-10, Tom Lane wrote:
I propose to run through the system operator classes, find any for which
the comparison function isn't marked leakproof but the operators are,
and fix them. This is clearly appropriate for HEAD and maybe it's not
too late to force an initdb for v11 --- thoughts?
on initdb in v11, see
/messages/by-id/CAKJS1f9cqoSKS9JVcBKGa2mdn-24YPWc9XLzFDNsmjJMUpth1w@mail.gmail.com
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
on initdb in v11, see
/messages/by-id/CAKJS1f9cqoSKS9JVcBKGa2mdn-24YPWc9XLzFDNsmjJMUpth1w@mail.gmail.com
[confused...] I recall a thread mentioning that we might need initdb
in v11, but that one doesn't seem to be it?
regards, tom lane
I wrote:
I propose to run through the system operator classes, find any for which
the comparison function isn't marked leakproof but the operators are,
and fix them. This is clearly appropriate for HEAD and maybe it's not
too late to force an initdb for v11 --- thoughts?
I did that for the built-in btree opclasses. I decided that it's probably
not worth forcing an initdb in v11 for, though. In principle, losing
selectivity estimates because of non-leakproof functions should only
happen in queries that are going to fail at runtime anyway. The real
problem that ought to be addressed and perhaps back-patched is this:
Another question that could be raised is why we are refusing to use
stats for a child table when the caller has select on the parent.
It's completely trivial to extract data from a child table if you
have select on the parent, so it seems like we are checking the
wrong table's privileges.
regards, tom lane
On 12.07.18 00:52, Tom Lane wrote:
Another question that could be raised is why we are refusing to use
stats for a child table when the caller has select on the parent.
It's completely trivial to extract data from a child table if you
have select on the parent, so it seems like we are checking the
wrong table's privileges.
That seems like an oversight.
The underlying principle is that we want to allow access to statistics
if the user could read the table, or more accurately the column, anyway.
This could also happen through inheritance, so we should check that as
well, but we need to make sure that the particular column is inherited
and not added locally. Also, for the expression index case, we don't
track the individual columns, so we don't have that information. For
partitioning, we can rely on all the columns being inherited, but not
for plain inheritance. So there are some details to work through, it seems.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
... For
partitioning, we can rely on all the columns being inherited, but not
for plain inheritance.
Uh, what?
regards, tom lane
On Sat, Jul 14, 2018 at 11:29 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
... For
partitioning, we can rely on all the columns being inherited, but not
for plain inheritance.Uh, what?
Maybe he meant that partitioning doesn't allow locally defined columns
in children, but plain inheritance does. Btw, Peter also said this
earlier in the paragraph:
"This could also happen through inheritance, so we should check that as
well, but we need to make sure that the particular column is inherited
and not added locally."
But maybe for the case under question, that's irrelevant, because
we're only interested in access to inherited columns as those are the
only ones that can be accessed in queries via parent.
Thanks,
Amit
Amit Langote <amitlangote09@gmail.com> writes:
On Sat, Jul 14, 2018 at 11:29 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
... For
partitioning, we can rely on all the columns being inherited, but not
for plain inheritance.
Uh, what?
But maybe for the case under question, that's irrelevant, because
we're only interested in access to inherited columns as those are the
only ones that can be accessed in queries via parent.
Yeah, that's what I thought. It seems like it should be possible to base
all stats access decisions off the table actually named in the query,
because only columns appearing in that table could be referenced, and only
that table's permissions actually get checked at runtime.
I guess it's possible that a child table could have, say, an index on
column X (inherited) and column Y (local) and that some aspect of costing
might then be interested in the behavior of column Y, even though the
query could only mention X not Y. But then we could fall back to the
existing behavior.
regards, tom lane
On Mon, Oct 22, 2018 at 7:47 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Amit Langote <amitlangote09@gmail.com> writes:
On Sat, Jul 14, 2018 at 11:29 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
... For
partitioning, we can rely on all the columns being inherited, but not
for plain inheritance.Uh, what?
But maybe for the case under question, that's irrelevant, because
we're only interested in access to inherited columns as those are the
only ones that can be accessed in queries via parent.Yeah, that's what I thought. It seems like it should be possible to base
all stats access decisions off the table actually named in the query,
because only columns appearing in that table could be referenced, and only
that table's permissions actually get checked at runtime.I guess it's possible that a child table could have, say, an index on
column X (inherited) and column Y (local) and that some aspect of costing
might then be interested in the behavior of column Y, even though the
query could only mention X not Y. But then we could fall back to the
existing behavior.
Basically, if the relation is RELOPT_OTHER_MEMBER_REL, we can
recursively fetch its parent until we reach to the base relation
(which is actually named in the query). And, once we have the base
relation we can check ACL on that and set vardata->acl_ok accordingly.
Additionally, for getting the parent RTI we need to traverse
"root->append_rel_list". Another alternative could be that we can add
parent_rti member in RelOptInfo structure.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On 2018/10/25 19:54, Dilip Kumar wrote:
On Mon, Oct 22, 2018 at 7:47 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Amit Langote <amitlangote09@gmail.com> writes:
But maybe for the case under question, that's irrelevant, because
we're only interested in access to inherited columns as those are the
only ones that can be accessed in queries via parent.Yeah, that's what I thought. It seems like it should be possible to base
all stats access decisions off the table actually named in the query,
because only columns appearing in that table could be referenced, and only
that table's permissions actually get checked at runtime.I guess it's possible that a child table could have, say, an index on
column X (inherited) and column Y (local) and that some aspect of costing
might then be interested in the behavior of column Y, even though the
query could only mention X not Y. But then we could fall back to the
existing behavior.Basically, if the relation is RELOPT_OTHER_MEMBER_REL, we can
recursively fetch its parent until we reach to the base relation
(which is actually named in the query). And, once we have the base
relation we can check ACL on that and set vardata->acl_ok accordingly.
Additionally, for getting the parent RTI we need to traverse
"root->append_rel_list". Another alternative could be that we can add
parent_rti member in RelOptInfo structure.
Adding parent_rti would be a better idea [1]I've named it inh_root_parent in one of the patches I'm working on where I needed such a field (https://commitfest.postgresql.org/20/1778/). I think that traversing
append_rel_list every time would be inefficient.
Thanks,
Amit
[1]: I've named it inh_root_parent in one of the patches I'm working on where I needed such a field (https://commitfest.postgresql.org/20/1778/)
where I needed such a field (https://commitfest.postgresql.org/20/1778/)
On Fri, Oct 26, 2018 at 1:12 PM Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
On 2018/10/25 19:54, Dilip Kumar wrote:
On Mon, Oct 22, 2018 at 7:47 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Amit Langote <amitlangote09@gmail.com> writes:
But maybe for the case under question, that's irrelevant, because
we're only interested in access to inherited columns as those are the
only ones that can be accessed in queries via parent.Yeah, that's what I thought. It seems like it should be possible to base
all stats access decisions off the table actually named in the query,
because only columns appearing in that table could be referenced, and only
that table's permissions actually get checked at runtime.I guess it's possible that a child table could have, say, an index on
column X (inherited) and column Y (local) and that some aspect of costing
might then be interested in the behavior of column Y, even though the
query could only mention X not Y. But then we could fall back to the
existing behavior.Basically, if the relation is RELOPT_OTHER_MEMBER_REL, we can
recursively fetch its parent until we reach to the base relation
(which is actually named in the query). And, once we have the base
relation we can check ACL on that and set vardata->acl_ok accordingly.
Additionally, for getting the parent RTI we need to traverse
"root->append_rel_list". Another alternative could be that we can add
parent_rti member in RelOptInfo structure.Adding parent_rti would be a better idea [1]. I think that traversing
append_rel_list every time would be inefficient.
[1] I've named it inh_root_parent in one of the patches I'm working on
where I needed such a field (https://commitfest.postgresql.org/20/1778/)
Ok, Make sense. I have written a patch by adding this variable.
There is still one FIXME in the patch, basically, after getting the
baserel rte I need to convert child varattno to parent varattno
because in case of inheritance that can be different. Do we already
have any mapping from child attno to parent attno or we have to look
up the cache.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
Attachments:
bug_fix_childrel_stat_access_v1.patchapplication/octet-stream; name=bug_fix_childrel_stat_access_v1.patchDownload+52-1
On Sat, Oct 27, 2018 at 10:07 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Fri, Oct 26, 2018 at 1:12 PM Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:On 2018/10/25 19:54, Dilip Kumar wrote:
On Mon, Oct 22, 2018 at 7:47 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Amit Langote <amitlangote09@gmail.com> writes:
But maybe for the case under question, that's irrelevant, because
we're only interested in access to inherited columns as those are the
only ones that can be accessed in queries via parent.Yeah, that's what I thought. It seems like it should be possible to base
all stats access decisions off the table actually named in the query,
because only columns appearing in that table could be referenced, and only
that table's permissions actually get checked at runtime.I guess it's possible that a child table could have, say, an index on
column X (inherited) and column Y (local) and that some aspect of costing
might then be interested in the behavior of column Y, even though the
query could only mention X not Y. But then we could fall back to the
existing behavior.Basically, if the relation is RELOPT_OTHER_MEMBER_REL, we can
recursively fetch its parent until we reach to the base relation
(which is actually named in the query). And, once we have the base
relation we can check ACL on that and set vardata->acl_ok accordingly.
Additionally, for getting the parent RTI we need to traverse
"root->append_rel_list". Another alternative could be that we can add
parent_rti member in RelOptInfo structure.Adding parent_rti would be a better idea [1]. I think that traversing
append_rel_list every time would be inefficient.
[1] I've named it inh_root_parent in one of the patches I'm working on
where I needed such a field (https://commitfest.postgresql.org/20/1778/)Ok, Make sense. I have written a patch by adding this variable.
There is still one FIXME in the patch, basically, after getting the
baserel rte I need to convert child varattno to parent varattno
because in case of inheritance that can be different. Do we already
have any mapping from child attno to parent attno or we have to look
up the cache.
Attached patch handles this issue.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
Attachments:
bug_fix_childrel_stat_access_v2.patchapplication/octet-stream; name=bug_fix_childrel_stat_access_v2.patchDownload+87-3
Thank you for creating the patch.
On 2018/10/28 20:35, Dilip Kumar wrote:
On Sat, Oct 27, 2018 at 10:07 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Fri, Oct 26, 2018 at 1:12 PM Amit Langote wrote:
On 2018/10/25 19:54, Dilip Kumar wrote:
Basically, if the relation is RELOPT_OTHER_MEMBER_REL, we can
recursively fetch its parent until we reach to the base relation
(which is actually named in the query). And, once we have the base
relation we can check ACL on that and set vardata->acl_ok accordingly.
Additionally, for getting the parent RTI we need to traverse
"root->append_rel_list". Another alternative could be that we can add
parent_rti member in RelOptInfo structure.Adding parent_rti would be a better idea [1]. I think that traversing
append_rel_list every time would be inefficient.[1] I've named it inh_root_parent in one of the patches I'm working on
where I needed such a field (https://commitfest.postgresql.org/20/1778/)Ok, Make sense. I have written a patch by adding this variable.
There is still one FIXME in the patch, basically, after getting the
baserel rte I need to convert child varattno to parent varattno
because in case of inheritance that can be different. Do we already
have any mapping from child attno to parent attno or we have to look
up the cache.
Sorry I forgot to cc you, but I'd posted a patch on the "speeding up
planning with partitions" thread, that's extracted from the bigger patch,
which adds inh_root_parent member to RelOptInfo [1]/messages/by-id/f06a398a-40a9-efb4-fab9-784400fecf13@lab.ntt.co.jp. Find it attached
with this email.
One of the differences from your patch is that it makes inh_root_parent
work not just for partitioning, but to inheritance in general. Also, it
codes the changes to build_simple_rel to set inh_root_parent's value a bit
differently than your patch.
Attached patch handles this issue.
I noticed a typo in your patch:
transalate_varattno -> translate_varattno
+static int
+transalate_varattno(Oid oldrelid, Oid newrelid, int old_attno)
+{
+ Relation oldrelation = heap_open(oldrelid, NoLock);It doesn't seem nice that it performs a heap_open on the parent relation.
+ att = TupleDescAttr(old_tupdesc, old_attno - 1);
+ attname = NameStr(att->attname);
+
+ newtup = SearchSysCacheAttName(newrelid, attname);
+ if (!newtup)
+ {
+ heap_close(oldrelation, NoLock);
+ return InvalidAttrNumber;
+ }and
+ varattno = transalate_varattno(relid, rte->relid, var->varattno);
+ if (AttributeNumberIsValid(varattno))
+ relid = rte->relid;
+ else
+ varattno = var->varattno;It's not possible for varattno to be invalid here, because the query on
inheritance parent only allows to select parent's columns, so we'd error
out before getting here if a column not present in the parent were selected.
Anyway, why don't we just use the child table's AppendRelInfo to get the
parent's version of varattno instead of creating a new function? It can
be done as shown in the attached revised version of the portion of the
patch changing selfuncs.c. Please take a look.
[1]: /messages/by-id/f06a398a-40a9-efb4-fab9-784400fecf13@lab.ntt.co.jp
/messages/by-id/f06a398a-40a9-efb4-fab9-784400fecf13@lab.ntt.co.jp
Attachments:
0001-Store-inheritance-root-parent-index-in-otherrel-s-Re.patchtext/plain; charset=UTF-8; name=0001-Store-inheritance-root-parent-index-in-otherrel-s-Re.patchDownload+19-1
bug_fix_childrel_stat_access_v3.patchtext/plain; charset=UTF-8; name=bug_fix_childrel_stat_access_v3.patchDownload+55-3
On Mon, Oct 29, 2018 at 2:53 PM Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
Thank you for creating the patch.
On 2018/10/28 20:35, Dilip Kumar wrote:
On Sat, Oct 27, 2018 at 10:07 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Fri, Oct 26, 2018 at 1:12 PM Amit Langote wrote:
On 2018/10/25 19:54, Dilip Kumar wrote:
Basically, if the relation is RELOPT_OTHER_MEMBER_REL, we can
recursively fetch its parent until we reach to the base relation
(which is actually named in the query). And, once we have the base
relation we can check ACL on that and set vardata->acl_ok accordingly.
Additionally, for getting the parent RTI we need to traverse
"root->append_rel_list". Another alternative could be that we can add
parent_rti member in RelOptInfo structure.Adding parent_rti would be a better idea [1]. I think that traversing
append_rel_list every time would be inefficient.[1] I've named it inh_root_parent in one of the patches I'm working on
where I needed such a field (https://commitfest.postgresql.org/20/1778/)Ok, Make sense. I have written a patch by adding this variable.
There is still one FIXME in the patch, basically, after getting the
baserel rte I need to convert child varattno to parent varattno
because in case of inheritance that can be different. Do we already
have any mapping from child attno to parent attno or we have to look
up the cache.Sorry I forgot to cc you, but I'd posted a patch on the "speeding up
planning with partitions" thread, that's extracted from the bigger patch,
which adds inh_root_parent member to RelOptInfo [1]. Find it attached
with this email.One of the differences from your patch is that it makes inh_root_parent
work not just for partitioning, but to inheritance in general. Also, it
codes the changes to build_simple_rel to set inh_root_parent's value a bit
differently than your patch.Attached patch handles this issue.
I noticed a typo in your patch:
transalate_varattno -> translate_varattno
+static int +transalate_varattno(Oid oldrelid, Oid newrelid, int old_attno) +{ + Relation oldrelation = heap_open(oldrelid, NoLock);It doesn't seem nice that it performs a heap_open on the parent relation.
+ att = TupleDescAttr(old_tupdesc, old_attno - 1); + attname = NameStr(att->attname); + + newtup = SearchSysCacheAttName(newrelid, attname); + if (!newtup) + { + heap_close(oldrelation, NoLock); + return InvalidAttrNumber; + }and
+ varattno = transalate_varattno(relid, rte->relid, var->varattno); + if (AttributeNumberIsValid(varattno)) + relid = rte->relid; + else + varattno = var->varattno;It's not possible for varattno to be invalid here, because the query on
inheritance parent only allows to select parent's columns, so we'd error
out before getting here if a column not present in the parent were selected.
Make sense to me.
Anyway, why don't we just use the child table's AppendRelInfo to get the
parent's version of varattno instead of creating a new function? It can
be done as shown in the attached revised version of the portion of the
patch changing selfuncs.c. Please take a look.
+1
[1]
/messages/by-id/f06a398a-40a9-efb4-fab9-784400fecf13@lab.ntt.co.jp
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On 2018/11/01 20:34, Dilip Kumar wrote:
On Mon, Oct 29, 2018 at 2:53 PM Amit Langote wrote:
Anyway, why don't we just use the child table's AppendRelInfo to get the
parent's version of varattno instead of creating a new function? It can
be done as shown in the attached revised version of the portion of the
patch changing selfuncs.c. Please take a look.+1
Okay, here are two patches:
0001 adds a new RelOptInfo member inh_root_parent that's set for
inheritance child otherrels and contains the RT index of the inheritance
parent table mentioned in the query from which they originated.
0002 is your patch that modifies examine_variable, etc. to use the
permissions granted on parent before reading stats on otherrel inheritance
child tables. I've added your name as the author in the 2nd patch.
Thanks,
Amit
Attachments:
0001-Store-inheritance-root-parent-index-in-otherrel-s-Re.patchtext/plain; charset=UTF-8; name=0001-Store-inheritance-root-parent-index-in-otherrel-s-Re.patchDownload+19-1
0002-Use-permissions-granted-on-parent-to-read-stats-on-o.patchtext/plain; charset=UTF-8; name=0002-Use-permissions-granted-on-parent-to-read-stats-on-o.patchDownload+55-4
On Fri, Nov 2, 2018 at 1:34 PM Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
On 2018/11/01 20:34, Dilip Kumar wrote:
On Mon, Oct 29, 2018 at 2:53 PM Amit Langote wrote:
Anyway, why don't we just use the child table's AppendRelInfo to get the
parent's version of varattno instead of creating a new function? It can
be done as shown in the attached revised version of the portion of the
patch changing selfuncs.c. Please take a look.+1
Okay, here are two patches:
0001 adds a new RelOptInfo member inh_root_parent that's set for
inheritance child otherrels and contains the RT index of the inheritance
parent table mentioned in the query from which they originated.0002 is your patch that modifies examine_variable, etc. to use the
permissions granted on parent before reading stats on otherrel inheritance
child tables. I've added your name as the author in the 2nd patch.
I have looked into the patches and these look fine to me. I have also
added it to the next commitfest.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Wed, Jul 10, 2019 at 9:44 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Fri, Nov 2, 2018 at 1:34 PM Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:On 2018/11/01 20:34, Dilip Kumar wrote:
On Mon, Oct 29, 2018 at 2:53 PM Amit Langote wrote:
Anyway, why don't we just use the child table's AppendRelInfo to get the
parent's version of varattno instead of creating a new function? It can
be done as shown in the attached revised version of the portion of the
patch changing selfuncs.c. Please take a look.+1
Okay, here are two patches:
0001 adds a new RelOptInfo member inh_root_parent that's set for
inheritance child otherrels and contains the RT index of the inheritance
parent table mentioned in the query from which they originated.0002 is your patch that modifies examine_variable, etc. to use the
permissions granted on parent before reading stats on otherrel inheritance
child tables. I've added your name as the author in the 2nd patch.I have looked into the patches and these look fine to me. I have also
added it to the next commitfest.
Hi Amit,
I have reviewed your 0001 patch and I think you have already taken a
look on 0002. So should I move it to "Ready for Committer" or you
want to review it further?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
Hi Dilip,
On Wed, Jul 10, 2019 at 1:29 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Wed, Jul 10, 2019 at 9:44 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Fri, Nov 2, 2018 at 1:34 PM Amit Langote wrote:
Okay, here are two patches:
0001 adds a new RelOptInfo member inh_root_parent that's set for
inheritance child otherrels and contains the RT index of the inheritance
parent table mentioned in the query from which they originated.0002 is your patch that modifies examine_variable, etc. to use the
permissions granted on parent before reading stats on otherrel inheritance
child tables. I've added your name as the author in the 2nd patch.I have looked into the patches and these look fine to me. I have also
added it to the next commitfest.Hi Amit,
I have reviewed your 0001 patch and I think you have already taken a
look on 0002. So should I move it to "Ready for Committer" or you
want to review it further?
Thanks for checking. There has been a lot of churn in the inheritance
planning code since my last email on this thread, so I'd like to
reconsider. I'm busy this week with some things, so I'll try posting
something on next Tuesday.
Thanks,
Amit
On Wed, Jul 10, 2019 at 10:15 AM Amit Langote <amitlangote09@gmail.com> wrote:
Hi Dilip,
On Wed, Jul 10, 2019 at 1:29 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Wed, Jul 10, 2019 at 9:44 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Fri, Nov 2, 2018 at 1:34 PM Amit Langote wrote:
Okay, here are two patches:
0001 adds a new RelOptInfo member inh_root_parent that's set for
inheritance child otherrels and contains the RT index of the inheritance
parent table mentioned in the query from which they originated.0002 is your patch that modifies examine_variable, etc. to use the
permissions granted on parent before reading stats on otherrel inheritance
child tables. I've added your name as the author in the 2nd patch.I have looked into the patches and these look fine to me. I have also
added it to the next commitfest.Hi Amit,
I have reviewed your 0001 patch and I think you have already taken a
look on 0002. So should I move it to "Ready for Committer" or you
want to review it further?Thanks for checking. There has been a lot of churn in the inheritance
planning code since my last email on this thread, so I'd like to
reconsider. I'm busy this week with some things, so I'll try posting
something on next Tuesday.
Sounds good.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Wed, Jul 10, 2019 at 2:43 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Wed, Jul 10, 2019 at 10:15 AM Amit Langote <amitlangote09@gmail.com> wrote:
Thanks for checking. There has been a lot of churn in the inheritance
planning code since my last email on this thread, so I'd like to
reconsider. I'm busy this week with some things, so I'll try posting
something on next Tuesday.Sounds good.
I looked at this today and concluded that the problem and the patches
discussed here are fairly isolated from inheritance planning changes
committed to PG 12.
I've combined the two patches into one. I tried to think up test
cases to go with the code changes, but couldn't come up with one.
Thanks,
Amit