Обсуждение: Don't treat virtual generated columns as missing statistics in vacuumdb --missing-stats-only

Hi,

I found that "vacuumdb --missing-stats-only" always performs ANALYZE
on tables with a virtual generated column, since such columns currently
never have statistics. This seems like an obvious waste, so I've attached
a patch to fix it, ensuring that virtual generated columns are not
regarded as missing statistics.

Regards,
Yugo Nagata

-- 
Yugo Nagata <nagata@sraoss.co.jp>

On Wed, Aug 20, 2025 at 10:42 AM Yugo Nagata <nagata@sraoss.co.jp> wrote:
>
> Hi,
>
> I found that "vacuumdb --missing-stats-only" always performs ANALYZE
> on tables with a virtual generated column, since such columns currently
> never have statistics. This seems like an obvious waste, so I've attached
> a patch to fix it, ensuring that virtual generated columns are not
> regarded as missing statistics.

Thanks for the report and patch! This seems to be an oversight from
the commit that added virtual generated columns.

For the patch, shouldn't we also add a regression test for --missing-stats-only
with generated columns, to prevent this issue from happening again?

Regards,

--
Fujii Masao

On Wed, 20 Aug 2025 12:49:14 +0900
Fujii Masao <masao.fujii@gmail.com> wrote:

> On Wed, Aug 20, 2025 at 10:42 AM Yugo Nagata <nagata@sraoss.co.jp> wrote:
> >
> > Hi,
> >
> > I found that "vacuumdb --missing-stats-only" always performs ANALYZE
> > on tables with a virtual generated column, since such columns currently
> > never have statistics. This seems like an obvious waste, so I've attached
> > a patch to fix it, ensuring that virtual generated columns are not
> > regarded as missing statistics.
> 
> Thanks for the report and patch! This seems to be an oversight from
> the commit that added virtual generated columns.
> 
> For the patch, shouldn't we also add a regression test for --missing-stats-only
> with generated columns, to prevent this issue from happening again?

Thank you for reviewing the patch and your suggestion.

I agree that we should add a test, since the behavior may change in the future
when statistics begin to be collected for virtual generated columns, and the test
will serve as a reminder when this behavior changes.

I've attached a updated patch including the test.

Regards,
Yugo Nagata

-- 
Yugo Nagata <nagata@sraoss.co.jp>

On Wed, 20 Aug 2025 13:30:12 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

> On Wed, 20 Aug 2025 12:49:14 +0900
> Fujii Masao <masao.fujii@gmail.com> wrote:
> 
> > On Wed, Aug 20, 2025 at 10:42 AM Yugo Nagata <nagata@sraoss.co.jp> wrote:
> > >
> > > Hi,
> > >
> > > I found that "vacuumdb --missing-stats-only" always performs ANALYZE
> > > on tables with a virtual generated column, since such columns currently
> > > never have statistics. This seems like an obvious waste, so I've attached
> > > a patch to fix it, ensuring that virtual generated columns are not
> > > regarded as missing statistics.
> > 
> > Thanks for the report and patch! This seems to be an oversight from
> > the commit that added virtual generated columns.
> > 
> > For the patch, shouldn't we also add a regression test for --missing-stats-only
> > with generated columns, to prevent this issue from happening again?
> 
> Thank you for reviewing the patch and your suggestion.
> 
> I agree that we should add a test, since the behavior may change in the future
> when statistics begin to be collected for virtual generated columns, and the test
> will serve as a reminder when this behavior changes.
> 
> I've attached a updated patch including the test.

The patch conflicted with the latest commit, so I rebased it.

Regards,
Yugo Nagata

-- 
Yugo Nagata <nagata@sraoss.co.jp>

On Wed, Aug 20, 2025 at 01:53:38PM +0900, Yugo Nagata wrote:
> The patch conflicted with the latest commit, so I rebased it.

Nice find.  I would suggest adding the virtual generated column to
regression_vacuumdb_test when it is first created so that we can just rely
on the existing test cases.  In fact, by doing so, you'll see that we need
a similar change to the "inheritance and regular stats" part of the query.

-- 
nathan

On Thu, Aug 21, 2025 at 4:19 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> On Wed, Aug 20, 2025 at 01:53:38PM +0900, Yugo Nagata wrote:
> > The patch conflicted with the latest commit, so I rebased it.
>
> Nice find.  I would suggest adding the virtual generated column to
> regression_vacuumdb_test when it is first created so that we can just rely
> on the existing test cases.  In fact, by doing so, you'll see that we need
> a similar change to the "inheritance and regular stats" part of the query.

+1

Regards,

--
Fujii Masao

On Thu, Aug 21, 2025 at 11:13:44AM +0900, Fujii Masao wrote:
> On Thu, Aug 21, 2025 at 4:19 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
>> Nice find.  I would suggest adding the virtual generated column to
>> regression_vacuumdb_test when it is first created so that we can just rely
>> on the existing test cases.  In fact, by doing so, you'll see that we need
>> a similar change to the "inheritance and regular stats" part of the query.
> 
> +1

Since we're running out of time for v18, I went ahead and updated the
patch.  I've also added an open item for this.

-- 
nathan

On Thu, Aug 21, 2025 at 11:29:56AM -0400, Corey Huinker wrote:
> On Thu, Aug 21, 2025 at 9:56 AM Nathan Bossart <nathandbossart@gmail.com>
> wrote:
>> Since we're running out of time for v18, I went ahead and updated the
>> patch.  I've also added an open item for this.
> 
> +1 on the fix, works for me.

Thanks.  I'll plan on committing this in roughly 24 hours (barring
additional feedback).

-- 
nathan

On Thu, 21 Aug 2025 10:37:10 -0500
Nathan Bossart <nathandbossart@gmail.com> wrote:

> On Thu, Aug 21, 2025 at 11:29:56AM -0400, Corey Huinker wrote:
> > On Thu, Aug 21, 2025 at 9:56 AM Nathan Bossart <nathandbossart@gmail.com>
> > wrote:
> >> Since we're running out of time for v18, I went ahead and updated the
> >> patch.  I've also added an open item for this.
> > 
> > +1 on the fix, works for me.
> 
> Thanks.  I'll plan on committing this in roughly 24 hours (barring
> additional feedback).

Thank you for updating the patch.
I'm fine with your fixes, both in the test and in the additional change
for inheritance tables. I had overlooked the latter one.

Regards,
Yugo Nagata

-- 
Yugo Nagata <nagata@sraoss.co.jp>

On Fri, Aug 22, 2025 at 2:23 AM Yugo Nagata <nagata@sraoss.co.jp> wrote:
>
> On Thu, 21 Aug 2025 10:37:10 -0500
> Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> > On Thu, Aug 21, 2025 at 11:29:56AM -0400, Corey Huinker wrote:
> > > On Thu, Aug 21, 2025 at 9:56 AM Nathan Bossart <nathandbossart@gmail.com>
> > > wrote:
> > >> Since we're running out of time for v18, I went ahead and updated the
> > >> patch.  I've also added an open item for this.
> > >
> > > +1 on the fix, works for me.
> >
> > Thanks.  I'll plan on committing this in roughly 24 hours (barring
> > additional feedback).

Thanks a lot!


> Thank you for updating the patch.
> I'm fine with your fixes, both in the test and in the additional change
> for inheritance tables. I had overlooked the latter one.

The patch looks good to me, too.

At first, I just wondered whether vacuumdb --missing-stats-only would work
on older servers, since the patch adds access to the attgenerated column,
which might not exist there. But that's not an issue, because
--missing-stats-only is supported only on servers version 15 or later,
where attgenerated is available.

Regards,

--
Fujii Masao

On Fri, Aug 22, 2025 at 10:19:32AM +0900, Fujii Masao wrote:
> At first, I just wondered whether vacuumdb --missing-stats-only would work
> on older servers, since the patch adds access to the attgenerated column,
> which might not exist there. But that's not an issue, because
> --missing-stats-only is supported only on servers version 15 or later,
> where attgenerated is available.

Yeah, this crossed my mind, too, and I reached the same conclusion.

-- 
nathan

Committed.

-- 
nathan