I've pushed the part adding stxdinherit flag to the catalog, so that on
master we build statistics both with and without data from the child
relations.
I'm not going to push the ACL refactoring. We have similar code on
various other places (not addressed by the proposed patch), and it'd
make
On 1/15/22 19:35, Tomas Vondra wrote:
On 1/15/22 06:11, Justin Pryzby wrote:
On Mon, Dec 13, 2021 at 09:40:09PM +0100, Tomas Vondra wrote:
1) If the table is a separate relation (not part of an inheritance
tree), this should make no difference. -> OK
2) If the table is using "old" inheritan
On 1/15/22 06:11, Justin Pryzby wrote:
On Mon, Dec 13, 2021 at 09:40:09PM +0100, Tomas Vondra wrote:
1) If the table is a separate relation (not part of an inheritance
tree), this should make no difference. -> OK
2) If the table is using "old" inheritance, this reverts back to
pre-regression be
On Mon, Dec 13, 2021 at 09:40:09PM +0100, Tomas Vondra wrote:
> 1) If the table is a separate relation (not part of an inheritance
> tree), this should make no difference. -> OK
>
> 2) If the table is using "old" inheritance, this reverts back to
> pre-regression behavior. So people will keep usin
On 12/13/21 14:53, Justin Pryzby wrote:
> On Sun, Dec 12, 2021 at 11:23:19PM +0100, Tomas Vondra wrote:
>> On 12/12/21 22:32, Justin Pryzby wrote:
>>> On Sun, Dec 12, 2021 at 05:17:10AM +0100, Tomas Vondra wrote:
The one thing bugging me a bit is that the regression test checks only a
GRO
On 12/13/21 14:48, Justin Pryzby wrote:
> On Sun, Dec 12, 2021 at 10:29:39PM +0100, Tomas Vondra wrote:
>>> In your 0003 patch, the "if inh: break" isn't removed from
>>> examine_variable(),
>>> but the corresponding thing is removed everywhere else.
>>
>> Ah, you're right. And it wasn't updated i
On Sun, Dec 12, 2021 at 11:23:19PM +0100, Tomas Vondra wrote:
> On 12/12/21 22:32, Justin Pryzby wrote:
> > On Sun, Dec 12, 2021 at 05:17:10AM +0100, Tomas Vondra wrote:
> > > The one thing bugging me a bit is that the regression test checks only a
> > > GROUP BY query. It'd be nice to add queries
On Sun, Dec 12, 2021 at 10:29:39PM +0100, Tomas Vondra wrote:
> > In your 0003 patch, the "if inh: break" isn't removed from
> > examine_variable(),
> > but the corresponding thing is removed everywhere else.
>
> Ah, you're right. And it wasn't updated in the 0002 patch either - it
> should do th
On 12/12/21 22:32, Justin Pryzby wrote:
On Sun, Dec 12, 2021 at 05:17:10AM +0100, Tomas Vondra wrote:
The one thing bugging me a bit is that the regression test checks only a
GROUP BY query. It'd be nice to add queries testing MCV/dependencies
too, but that seems tricky because most queries w
On Sun, Dec 12, 2021 at 10:29:39PM +0100, Tomas Vondra wrote:
> On 12/12/21 18:52, Justin Pryzby wrote:
> That's mostly a conscious choice, so that I don't have to include
> parsetree.h. But maybe that'd be better ...
>
> > The regression tests changed as a result of not populating stx_data; I thi
On Sun, Dec 12, 2021 at 05:17:10AM +0100, Tomas Vondra wrote:
> The one thing bugging me a bit is that the regression test checks only a
> GROUP BY query. It'd be nice to add queries testing MCV/dependencies
> too, but that seems tricky because most queries will use per-partitions
> stats.
You mea
On 12/12/21 18:52, Justin Pryzby wrote:
> + * XXX Can't we simply look at rte->inh?
> + */
> + inh = root->append_rel_array == NULL ? false :
> + root->append_rel_array[onerel->relid]->parent_relid != 0;
>
> I think so. That's what I came up with while tr
Tomas Vondra writes:
> On 12/12/21 16:37, Zhihong Yu wrote:
>> Since the rte (RangeTblEntry*) doesn't seem to be used beyond checking
>> inh, I think it would be better if the above style of checking is used
>> throughout the patch (without introducing rte variable).
> It's mostly a matter of p
+ * XXX Can't we simply look at rte->inh?
+ */
+ inh = root->append_rel_array == NULL ? false :
+ root->append_rel_array[onerel->relid]->parent_relid != 0;
I think so. That's what I came up with while trying to figured this out, and
it's no great surprise
On 12/12/21 16:37, Zhihong Yu wrote:
Hi,
For patch 1, minor comment:
+ if (planner_rt_fetch(onerel->relid, root)->inh)
Since the rte (RangeTblEntry*) doesn't seem to be used beyond checking
inh, I think it would be better if the above style of checking is used
throughout the patch
On 12/12/21 14:47, Zhihong Yu wrote:
On Sat, Dec 11, 2021 at 9:14 PM Tomas Vondra
mailto:tomas.von...@enterprisedb.com>>
wrote:
>
...
> + /* skip statistics with mismatching stxdinherit value */
> + if (stat->inherit != rte->inh)
>
> Should a log be added fo
On Sat, Dec 11, 2021 at 8:17 PM Tomas Vondra
wrote:
> Hi,
>
> Attached is a rebased and cleaned-up version of these patches, with more
> comments, refactorings etc. Justin and Zhihong, can you take a look?
>
>
> 0001 - Ignore extended statistics for inheritance trees
>
> 0002 - Build inherited ex
On Sat, Dec 11, 2021 at 9:14 PM Tomas Vondra
wrote:
>
>
> On 12/12/21 05:38, Zhihong Yu wrote:
> >
> >
> > On Sat, Dec 11, 2021 at 8:17 PM Tomas Vondra
> > mailto:tomas.von...@enterprisedb.com>>
> > wrote:
> >
> > Hi,
> >
> > Attached is a rebased and cleaned-up version of these patches,
On 12/12/21 05:38, Zhihong Yu wrote:
>
>
> On Sat, Dec 11, 2021 at 8:17 PM Tomas Vondra
> mailto:tomas.von...@enterprisedb.com>>
> wrote:
>
> Hi,
>
> Attached is a rebased and cleaned-up version of these patches, with more
> comments, refactorings etc. Justin and Zhihong, can you
On Sat, Dec 11, 2021 at 8:17 PM Tomas Vondra
wrote:
> Hi,
>
> Attached is a rebased and cleaned-up version of these patches, with more
> comments, refactorings etc. Justin and Zhihong, can you take a look?
>
>
> 0001 - Ignore extended statistics for inheritance trees
>
> 0002 - Build inherited ex
Hi,
Attached is a rebased and cleaned-up version of these patches, with more
comments, refactorings etc. Justin and Zhihong, can you take a look?
0001 - Ignore extended statistics for inheritance trees
0002 - Build inherited extended stats on partitioned tables
Those are mostly just Justin's p
On Thu, Dec 2, 2021 at 9:24 PM Justin Pryzby wrote:
> On Thu, Nov 04, 2021 at 12:44:45AM +0100, Tomas Vondra wrote:
> > >> And I'm not sure we do the right thing after removing children, for
> example
> > >> (that should drop the inheritance stats, I guess).
>
> > > Do you mean for inheritance on
On Thu, Nov 04, 2021 at 12:44:45AM +0100, Tomas Vondra wrote:
> >> And I'm not sure we do the right thing after removing children, for example
> >> (that should drop the inheritance stats, I guess).
> > Do you mean for inheritance only ? Or partitions too ?
> > I think for partitions, the stats s
On Thu, Nov 04, 2021 at 12:44:45AM +0100, Tomas Vondra wrote:
> > I probably did this to make the code change small, to avoid indentin the
> > whole
> > block.
>
> But indenting the block is not necessary. It's possible to do something
> like this:
>
> if (!rel->inh)
> return 1.0;
>
On 11/4/21 12:19 AM, Justin Pryzby wrote:
> On Wed, Nov 03, 2021 at 11:48:44PM +0100, Tomas Vondra wrote:
>> On 10/8/21 12:45 AM, Justin Pryzby wrote:
>>> On Thu, Oct 07, 2021 at 03:26:46PM -0500, Jaime Casanova wrote:
On Sun, Sep 26, 2021 at 03:25:50PM -0500, Justin Pryzby wrote:
> On
On Wed, Nov 03, 2021 at 11:48:44PM +0100, Tomas Vondra wrote:
> On 10/8/21 12:45 AM, Justin Pryzby wrote:
> > On Thu, Oct 07, 2021 at 03:26:46PM -0500, Jaime Casanova wrote:
> >> On Sun, Sep 26, 2021 at 03:25:50PM -0500, Justin Pryzby wrote:
> >>> On Sat, Sep 25, 2021 at 05:31:52PM -0500, Justin Pr
On 10/8/21 12:45 AM, Justin Pryzby wrote:
> On Thu, Oct 07, 2021 at 03:26:46PM -0500, Jaime Casanova wrote:
>> On Sun, Sep 26, 2021 at 03:25:50PM -0500, Justin Pryzby wrote:
>>> On Sat, Sep 25, 2021 at 05:31:52PM -0500, Justin Pryzby wrote:
It seems like your patch should also check "inh" in e
On Thu, Oct 07, 2021 at 03:26:46PM -0500, Jaime Casanova wrote:
> On Sun, Sep 26, 2021 at 03:25:50PM -0500, Justin Pryzby wrote:
> > On Sat, Sep 25, 2021 at 05:31:52PM -0500, Justin Pryzby wrote:
> > > It seems like your patch should also check "inh" in examine_variable and
> > > statext_expression
On Sun, Sep 26, 2021 at 03:25:50PM -0500, Justin Pryzby wrote:
> On Sat, Sep 25, 2021 at 05:31:52PM -0500, Justin Pryzby wrote:
> > It seems like your patch should also check "inh" in examine_variable and
> > statext_expressions_load.
>
> I tried adding that - I mostly kept my patches separate.
>
On Sat, Sep 25, 2021 at 05:31:52PM -0500, Justin Pryzby wrote:
> It seems like your patch should also check "inh" in examine_variable and
> statext_expressions_load.
I tried adding that - I mostly kept my patches separate.
Hopefully this is more helpful than a complication.
I added at: https://com
On 9/25/21 11:46 PM, Justin Pryzby wrote:
> On Sat, Sep 25, 2021 at 11:01:21PM +0200, Tomas Vondra wrote:
>>> Do you think it's possible to backpatch a fix to handle partitioned tables
>>> specifically ?
>>>
>>> The "tuple already updated" error which I reported and which was fixed by
>>> 859b3003
(Resending with -hackers)
It seems like your patch should also check "inh" in examine_variable and
statext_expressions_load.
Which leads to another issue in stable branches:
ANALYZE builds only non-inherited stats, but they're incorrectly used for
inherited queries - the rowcount estimate is wor
On Sat, Sep 25, 2021 at 11:01:21PM +0200, Tomas Vondra wrote:
> > Do you think it's possible to backpatch a fix to handle partitioned tables
> > specifically ?
> >
> > The "tuple already updated" error which I reported and which was fixed by
> > 859b3003 involved inheritence children. Since parti
On 9/25/21 9:53 PM, Justin Pryzby wrote:
On Sat, Sep 25, 2021 at 09:27:10PM +0200, Tomas Vondra wrote:
On 9/23/21 11:26 PM, Justin Pryzby wrote:
extended stats objects are allowed on partitioned tables since v10.
https://www.postgresql.org/message-id/flat/CAKJS1f-BmGo410bh5RSPZUvOO0LhmHL2NYmd
On Sat, Sep 25, 2021 at 09:27:10PM +0200, Tomas Vondra wrote:
> On 9/23/21 11:26 PM, Justin Pryzby wrote:
> > extended stats objects are allowed on partitioned tables since v10.
> > https://www.postgresql.org/message-id/flat/CAKJS1f-BmGo410bh5RSPZUvOO0LhmHL2NYmdrC_Jm8pk_FfyCA%40mail.gmail.com
> > 8
On 9/23/21 11:26 PM, Justin Pryzby wrote:
extended stats objects are allowed on partitioned tables since v10.
https://www.postgresql.org/message-id/flat/CAKJS1f-BmGo410bh5RSPZUvOO0LhmHL2NYmdrC_Jm8pk_FfyCA%40mail.gmail.com
8c5cdb7f4f6e1d6a6104cb58ce4f23453891651b
But since 859b3003de they're not
36 matches
Mail list logo
