Horton

Thread

Commits

  1. Make memset() use sizeof() rather than re-compute size

  2. Implement multivariate n-distinct coefficients

  1. Code cleanup patch submission for extended_stats.c

    Mark Dilger <hornschnorter@gmail.com> — 2017-11-25T19:57:08Z

    Hackers, Alvaro,

In src/backend/statistics/extended_stats.c, in statext_store, there is a section:

    Datum       values[Natts_pg_statistic_ext];
    bool        nulls[Natts_pg_statistic_ext];
    bool        replaces[Natts_pg_statistic_ext];

    memset(nulls, 1, Natts_pg_statistic_ext * sizeof(bool));
    memset(replaces, 0, Natts_pg_statistic_ext * sizeof(bool));
    memset(values, 0, Natts_pg_statistic_ext * sizeof(Datum));

It looks to me like Alvaro introduced this in the original version of the file which 
was created in commit 7b504eb282ca2f5104b5c00b4f05a3ef6bb1385b.  Grep'ing
through the code base, it seems the following would be more consistent with
how these initializations are handled elsewhere:

    Datum       values[Natts_pg_statistic_ext];
    bool        nulls[Natts_pg_statistic_ext];
    bool        replaces[Natts_pg_statistic_ext];

    memset(nulls, 1, sizeof(nulls));
    memset(replaces, 0, sizeof(replaces));
    memset(values, 0, sizeof(values));

Patch attached as 0001_extended_stats_sizeof.patch.1


mark

  2. Re: Code cleanup patch submission for extended_stats.c

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-11-25T22:05:39Z

    Mark Dilger <hornschnorter@gmail.com> writes:
> It looks to me like Alvaro introduced this in the original version of the file which 
> was created in commit 7b504eb282ca2f5104b5c00b4f05a3ef6bb1385b.  Grep'ing
> through the code base, it seems the following would be more consistent with
> how these initializations are handled elsewhere:

>     memset(nulls, 1, sizeof(nulls));
>     memset(replaces, 0, sizeof(replaces));
>     memset(values, 0, sizeof(values));

+1.  I'd be inclined to use "false" and "true" for the init values of
the boolean arrays, too.

			regards, tom lane

  3. Re: Code cleanup patch submission for extended_stats.c

    Mark Dilger <hornschnorter@gmail.com> — 2017-11-26T02:07:14Z

    > On Nov 25, 2017, at 2:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> Mark Dilger <hornschnorter@gmail.com> writes:
>> It looks to me like Alvaro introduced this in the original version of the file which 
>> was created in commit 7b504eb282ca2f5104b5c00b4f05a3ef6bb1385b.  Grep'ing
>> through the code base, it seems the following would be more consistent with
>> how these initializations are handled elsewhere:
> 
>>    memset(nulls, 1, sizeof(nulls));
>>    memset(replaces, 0, sizeof(replaces));
>>    memset(values, 0, sizeof(values));
> 
> +1.  I'd be inclined to use "false" and "true" for the init values of
> the boolean arrays, too.

Done.

mark

  4. Re: Code cleanup patch submission for extended_stats.c

    Michael Paquier <michael.paquier@gmail.com> — 2017-11-26T07:40:16Z

    On Sun, Nov 26, 2017 at 11:07 AM, Mark Dilger <hornschnorter@gmail.com> wrote:
>
>> On Nov 25, 2017, at 2:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>
>> Mark Dilger <hornschnorter@gmail.com> writes:
>>> It looks to me like Alvaro introduced this in the original version of the file which
>>> was created in commit 7b504eb282ca2f5104b5c00b4f05a3ef6bb1385b.  Grep'ing
>>> through the code base, it seems the following would be more consistent with
>>> how these initializations are handled elsewhere:
>>
>>>    memset(nulls, 1, sizeof(nulls));
>>>    memset(replaces, 0, sizeof(replaces));
>>>    memset(values, 0, sizeof(values));
>>
>> +1.  I'd be inclined to use "false" and "true" for the init values of
>> the boolean arrays, too.
>
> Done.

That's better practice. More places deserve the same treatment if you
grep for them:
$ git grep memset | grep nulls | grep "[1|0]"
contrib/dblink/dblink.c:        memset(nulls, 0, sizeof(nulls));
contrib/pageinspect/ginfuncs.c:    memset(nulls, 0, sizeof(nulls));
contrib/pageinspect/ginfuncs.c:    memset(nulls, 0, sizeof(nulls));
contrib/pageinspect/ginfuncs.c:        memset(nulls, 0, sizeof(nulls));
contrib/pageinspect/heapfuncs.c:        memset(nulls, 0, sizeof(nulls));
contrib/pageinspect/rawpage.c:    memset(nulls, 0, sizeof(nulls));
contrib/pg_stat_statements/pg_stat_statements.c:        memset(nulls,
0, sizeof(nulls));
contrib/pgstattuple/pgstatapprox.c:    memset(nulls, 0, sizeof(nulls));
src/backend/access/heap/heapam.c:    memset(nulls, 1, sizeof(nulls));
src/backend/catalog/pg_collation.c:    memset(nulls, 0, sizeof(nulls));
src/backend/catalog/pg_range.c:    memset(nulls, 0, sizeof(nulls));
src/backend/commands/extension.c:    memset(nulls, 0, sizeof(nulls));
src/backend/commands/extension.c:            memset(nulls, 0, sizeof(nulls));
src/backend/commands/extension.c:        memset(nulls, 0, sizeof(nulls));
src/backend/commands/extension.c:            memset(nulls, 0, sizeof(nulls));
src/backend/commands/extension.c:        memset(nulls, 0, sizeof(nulls));
src/backend/commands/sequence.c:    memset(pgs_nulls, 0, sizeof(pgs_nulls));
src/backend/commands/tablecmds.c:    memset(nulls, 0, sizeof(nulls));
src/backend/libpq/hba.c:    memset(nulls, 0, sizeof(nulls));
src/backend/libpq/hba.c:        memset(&nulls[1], true,
(NUM_PG_HBA_FILE_RULES_ATTS - 2) * sizeof(bool));
src/backend/replication/logical/logicalfuncs.c:    memset(nulls, 0,
sizeof(nulls));
src/backend/replication/logical/origin.c:            memset(&nulls, 0,
sizeof(nulls));
src/backend/replication/logical/origin.c:        memset(nulls, 1,
sizeof(nulls));
src/backend/replication/slotfuncs.c:    memset(nulls, 0, sizeof(nulls));
src/backend/replication/slotfuncs.c:        memset(nulls, 0, sizeof(nulls));
src/backend/replication/walsender.c:        memset(nulls, 0, sizeof(nulls));
src/backend/statistics/extended_stats.c:    memset(nulls, 1,
Natts_pg_statistic_ext * sizeof(bool));
src/backend/utils/adt/genfile.c:        memset(nulls, 0, sizeof(nulls));
src/backend/utils/misc/guc.c:        memset(nulls, 0, sizeof(nulls));
-- 
Michael

  5. Re: Code cleanup patch submission for extended_stats.c

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2017-11-29T03:20:39Z

    Mark Dilger wrote:
> 
> > On Nov 25, 2017, at 2:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > 
> > Mark Dilger <hornschnorter@gmail.com> writes:
> >> It looks to me like Alvaro introduced this in the original version of the file which 
> >> was created in commit 7b504eb282ca2f5104b5c00b4f05a3ef6bb1385b.  Grep'ing
> >> through the code base, it seems the following would be more consistent with
> >> how these initializations are handled elsewhere:
> > 
> >>    memset(nulls, 1, sizeof(nulls));
> >>    memset(replaces, 0, sizeof(replaces));
> >>    memset(values, 0, sizeof(values));
> > 
> > +1.  I'd be inclined to use "false" and "true" for the init values of
> > the boolean arrays, too.
> 
> Done.

Pushed, thanks.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services