Re: progress report for ANALYZE

From: Tatsuro Yamada <tatsuro(dot)yamada(dot)tf(at)nttcom(dot)co(dot)jp>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Pg Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, Anthony Nowocien <anowocien(at)gmail(dot)com>
Subject: Re: progress report for ANALYZE
Date: 2019-09-06 02:21:33
Message-ID: 81d058ba-857f-ceb2-5017-2a38a409a2b3@nttcom.co.jp_1
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Alvaro,

> There were some minor problems in v5 -- bogus Docbook as well as
> outdated rules.out, small "git diff --check" complaint about whitespace.
> This v6 (on today's master) fixes those, no other changes.


Thanks for fixing that. :)
I'll test it later.

I think we have to address the following comment from Robert. Right?
Do you have any ideas?

>> I'm not a fan of the way you set the scan-table phase and inh flag in
>> one place, and then slightly later set the relation OID and block
>> count. That creates a race during which users could see the first bit
>> of data set and the second not set. I don't see any reason not to set
>> all four fields together.
>
>
> Hmm... I understand but it's little difficult because if there are
> child rels, acquire_inherited_sample_rows() calls acquire_sample_rows()
> (See below). So, it is able to set all four fields together if inh flag
> is given as a parameter of those functions, I suppose. But I'm not sure
> whether it's okay to add the parameter to both functions or not.
> Do you have any ideas?
>
>
> # do_analyze_rel()
> ...
> if (inh)
> numrows = acquire_inherited_sample_rows(onerel, elevel,
> rows, targrows,
> &totalrows, &totaldeadrows);
> else
> numrows = (*acquirefunc) (onerel, elevel,
> rows, targrows,
> &totalrows, &totaldeadrows);
>
>
> # acquire_inherited_sample_rows()
> ...
> foreach(lc, tableOIDs)
> {
> ...
> /* Check table type (MATVIEW can't happen, but might as well allow) */
> if (childrel->rd_rel->relkind == RELKIND_RELATION ||
> childrel->rd_rel->relkind == RELKIND_MATVIEW)
> {
> /* Regular table, so use the regular row acquisition function */
> acquirefunc = acquire_sample_rows;
> ...
> /* OK, we'll process this child */
> has_child = true;
> rels[nrels] = childrel;
> acquirefuncs[nrels] = acquirefunc;
> ...
> }
> ...
> for (i = 0; i < nrels; i++)
> {
> ...
> AcquireSampleRowsFunc acquirefunc = acquirefuncs[i];
> ...
> if (childtargrows > 0)
> {
> ...
> /* Fetch a random sample of the child's rows */
> childrows = (*acquirefunc) (childrel, elevel,
> rows + numrows, childtargrows,
> &trows, &tdrows)

Thanks,
Tatsuro Yamada

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2019-09-06 03:18:58 Re: enhance SPI to support EXECUTE commands
Previous Message Quan Zongliang 2019-09-06 01:35:51 Re: enhance SPI to support EXECUTE commands