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
inherited stats, it should not affect the estimates
+SELECT * FROM check_estimated_rows('SELECT * FROM stxdinh* GROUP BY 1,2');
+DROP TABLE stxdinh, stxdinh1;
+
-- basic test for statistics on expressions
CREATE TABLE ab1 (a INTEGER, b INTEGER, c TIMESTAMP, d TIMESTAMPTZ);
--
2.31.1
F
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
timates
SELECT * FROM check_estimated_rows('SELECT * FROM stxdinh* GROUP BY 1,2');
estimated | actual
---+
- 1000 | 1008
+ 1008 | 1008
+(1 row)
+
+-- Ensure correct (non-inherited) stats are applied to inherited query
+SELECT * FROM check_estimated_rows(&
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
> 0002 - Build inherited extended stats on partitioned tables
>
> Those are mostly just Justin's patches, with more detailed comments and
> updated commit message. I've considered moving the rel->inh check to
> statext_clauselist_selectivity, and then removing the check
gt; 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
torings 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 patches, with more detailed comments and
>
> 0002 - Build inherited extended stats on partitioned tables
>
> Those are mostly just Justin's patches, with more detailed comments and
> updated commit message. I've considered moving the rel->inh check to
> statext_clauselist_selectivity, and then removing the check
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&#
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
rited stats, it should not affect the estimates
+SELECT * FROM check_estimated_rows('SELECT * FROM stxdinh* GROUP BY 1,2');
+DROP TABLE stxdinh, stxdinh1;
+
-- basic test for statistics on expressions
CREATE TABLE ab1 (a INTEGER, b INTEGER, c TIMESTAMP, d TIMESTAMPTZ);
--
2.17.0
>From 3462
ffect the estimates
+SELECT * FROM check_estimated_rows('SELECT * FROM stxdinh* GROUP BY 1,2');
+DROP TABLE stxdinh, stxdinh1;
+
-- basic test for statistics on expressions
CREATE TABLE ab1 (a INTEGER, b INTEGER, c TIMESTAMP, d TIMESTAMPTZ);
--
2.17.0
>From 435b154ca177053f6ddc54f7
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
1,2');
+DROP TABLE stxdinh, stxdinh1;
+
-- basic test for statistics on expressions
CREATE TABLE ab1 (a INTEGER, b INTEGER, c TIMESTAMP, d TIMESTAMPTZ);
--
2.17.0
>From b5bcff5331d052ea753d25ec7f443dc1d807fb13 Mon Sep 17 00:00:00 2001
From: Tomas Vondra
Date: Sat, 25 Sep 2021 23:01:21 +
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.
>
rited stats, it should not affect the estimates
+SELECT * FROM check_estimated_rows('SELECT * FROM stxdinh* GROUP BY 1,2');
+DROP TABLE stxdinh, stxdinh1;
+
-- basic test for statistics on expressions
CREATE TABLE ab1 (a INTEGER, b INTEGER, c TIMESTAMP, d TIMESTAMPTZ);
--
2.17.0
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
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 populated - pg_statistic_ext(_data) is empty
37 matches
Mail list logo
