Re: bumping HASH_VERSION to 3

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Noah Misch <noah(at)leadboat(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: bumping HASH_VERSION to 3
Date: 2017-05-16 11:44:57
Message-ID: CA+TgmobaWMMDTY_t-XR-3fcK5Zf0awj+f-PY7P-YLDXR3e0hFA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, May 16, 2017 at 7:31 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> + snprintf(output_path, sizeof(output_path), "reindex_hash.sql");
>>
>> This looks suspiciously pointless. The contents of output_path will
>> always be precisely "reindex_hash.sql"; you could just char
>> *output_path = "reindex_hash.sql" and get the same effect.
>
> Sure, but the same code pattern is used in all other similar functions
> in version.c, refer new_9_0_populate_pg_largeobject_metadata. Both
> the ways will serve the purpose, do you think it makes sense to write
> this differently?

Yes. It's silly to copy a constant string into a new buffer just for
the heck of it. Perhaps the old instances should also be cleaned up
at some point, but even if we don't bother, copying absolutely
pointless coding into new places isn't getting us anywhere.

> Hmm, "./" is required for non-windows, but as mentioned above, this is
> not required for our case.

OK.

> I will send an updated patch once we agree on above points.

Sounds good.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ildus Kurbangaliev 2017-05-16 12:05:16 Re: Bug in ExecModifyTable function and trigger issues for foreign tables
Previous Message Amit Kapila 2017-05-16 11:31:11 Re: bumping HASH_VERSION to 3