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








Reply via email to