Re: [HACKERS] [PATCH] Add citext_pattern_ops to citext contrib module - Mailing list pgsql-hackers

From Jacob Champion
Subject Re: [HACKERS] [PATCH] Add citext_pattern_ops to citext contrib module
Date
Msg-id CABAq_6FRbMSPA8Z+=reZQtmFc2AycXcxztK=C4LJrRSy8mrn5g@mail.gmail.com
Whole thread Raw
In response to [HACKERS] [PATCH] Add citext_pattern_ops to citext contrib module  (Alexey Chernyshov <a.chernyshov@postgrespro.ru>)
List pgsql-hackers
On Tue, Jul 18, 2017 at 5:18 AM, Alexey Chernyshov
<a.chernyshov@postgrespro.ru> wrote:
> Hi all,

Hi Alexey, I took a look at your patch. Builds fine here, and passes
the new tests.

I'm new to this code, so take my review with a grain of salt.

> The attached patch introduces citext_pattern_ops for citext extension type
> like text_pattern_ops for text type. Here are operators ~<~, ~<=~, ~>~, ~>=~
> combined into citext_pattern_ops operator class. These operators simply
> compare underlying citext values as C strings with memcmp() function.

Are there any cases where performing the str_tolower with the default
collation, then comparing byte-by-byte, could backfire? The added test
cases don't make use of any multibyte/accented characters, so it's not
clear to me yet what *should* be happening in those cases.

It also might be a good idea to add some test cases that compare
strings of different lengths, to exercise all the paths in
internal_citext_pattern_cmp().

> +-- test citext_pattern_cmp() function explicetily.

Spelling nitpick in the new SQL: s/explicetily/explicitly .

--Jacob



pgsql-hackers by date:

Previous
From: Jesper Pedersen
Date:
Subject: Re: [HACKERS] Fix performance of generic atomics
Next
From: Tom Lane
Date:
Subject: Re: [HACKERS] why not parallel seq scan for slow functions