Re: "Some tests to cover hash_index"

From: Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Mithun Cy <mithun(dot)cy(at)enterprisedb(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: "Some tests to cover hash_index"
Date: 2016-08-23 06:17:35
Message-ID: CAE9k0PmvYW+NGTa_Q=YatySjsWRK8CToRCHbxBzOj47OYdAcdw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi All,

I have reverified the code coverage for hash index code using the test file
(commit-hash_coverage_test) attached with this mailing list and have found
that some of the code in _hash_squeezebucket() function flow is not being
covered. For this i have added a small testcase on top of 'commit
hash_coverage_test' patch. I have done this mainly to test Amit's WAL for
hash index patch [1].

I have also removed the warning message that we used to get for hash index
like 'WARNING: hash indexes are not WAL-logged and their use is
discouraged' as this message is now no more visible w.r.t hash index after
the WAL patch for hash index. Please have a look and let me know your
thoughts.

[1] -
https://www.postgresql.org/message-id/CAA4eK1JOBX%3DYU33631Qh-XivYXtPSALh514%2BjR8XeD7v%2BK3r_Q%40mail.gmail.com

With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

On Sat, Aug 6, 2016 at 9:41 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:

> On Thu, Aug 4, 2016 at 7:24 PM, Mithun Cy <mithun(dot)cy(at)enterprisedb(dot)com>
> wrote:
>
>> I am attaching the patch to improve some coverage of hash index code [1].
>> I have added some basic tests, which mainly covers overflow pages. It
>> took 2 sec extra time in my machine in parallel schedule.
>>
>>
>>
>>
>> Hit Total Coverage
>> old tests Line Coverage 780 1478 52.7
>>
>> Function Coverage 63 85 74.1
>> improvement after tests Line Coverage 1181 1478 79.9 %
>>
>> Function Coverage 78 85 91.8 %
>>
>>
>
> I think the code coverage improvement for hash index with these tests
> seems to be quite good, however time for tests seems to be slightly on
> higher side. Do anybody have better suggestion for these tests?
>
> diff --git a/src/test/regress/sql/concurrent_hash_index.sql
> b/src/test/regress/sql/concurrent_hash_index.sql
> I wonder why you have included a new file for these tests, why can't be
> these added to existing hash_index.sql.
>
> +--
> +-- Cause some overflow insert and splits.
> +--
> +CREATE TABLE con_hash_index_table (keycol INT);
> +CREATE INDEX con_hash_index on con_hash_index_table USING HASH (keycol);
>
> The relation name con_hash_index* choosen in above tests doesn't seem to
> be appropriate, how about hash_split_heap* or something like that.
>
> Register your patch in latest CF (https://commitfest.postgresql.org/10/)
>
> --
> With Regards,
> Amit Kapila.
> EnterpriseDB: http://www.enterprisedb.com
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Sharma 2016-08-23 06:30:23 Re: "Some tests to cover hash_index"
Previous Message Ryan Murphy 2016-08-23 06:04:19 Re: Patch: initdb: "'" for QUOTE_PATH (non-windows)