From: | Andreas Karlsson <andreas(at)proxel(dot)se> |
---|---|
To: | emre(at)hasegeli(dot)com, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Contains and is contained by operators of inet datatypes |
Date: | 2016-11-12 05:13:42 |
Message-ID: | 5ad4779a-8007-1009-7ad3-91e3c9e623b7@proxel.se |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Review
- Applies and passes the test suite.
- I think this is a good change since it increases the consistency of
the operators. I also like the choice of <<@ and @>> since they feel
intuitive to me.
- I tested it and both old and new operators use the brin and gist indexes.
- The new spgist index does not support the old deprecated operators,
which is intentional. I do not have a strong opinion here either way but
some people may find this surprising.
- I am not convinced that your changes to the descriptions of the
operators necessarily make things clearer. For example "is contained by
and smaller network (subnet)" only mentions subnets and not IP-addresses.
- Maybe change "deprecated and will eventually be removed." to
"deprecated and may be removed in a future release.". I prefer that
latter wording but I am fine with either.
- Won't renaming the functions which implement risk breaking people's
applications? While the new names are a bit nicer I am not sure it is
worth doing.
- The changes to the code look generally good.
Andreas
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2016-11-12 05:49:38 | Re: Hash Indexes |
Previous Message | Amit Kapila | 2016-11-12 04:58:56 | Re: Fix checkpoint skip logic on idle systems by tracking LSN progress |