On Tue, Aug 9, 2022 at 4:37 AM Nathan Bossart <nathandboss...@gmail.com> wrote: > > On Mon, Aug 08, 2022 at 12:56:28PM +0530, Bharath Rupireddy wrote: > > 1) pg_lfind32 - why just uint32? If it's not possible to define > > functions for char, unsigned char, int16, uint16, int32, int64, uint64 > > and so on, can we add a few comments around that? Also, the comments > > can talk about if the base type or element data type of array or data > > type of key matters to use pg_lfind32. > > I figured that we'd add functions for other types when needed. I > considered making the new function generic by adding an argument for the > element size. Then, we could branch to optimized routines based on the > element size (e.g., pg_lfind() would call pg_lfind32() if the element size > was 4 bytes). However, that seemed like more complexity than is required, > and it's probably nice to avoid the extra branching. > > > 3) Can pg_lfind32 return the index of the key found, for instance to > > use it for setting/resetting the found element in the array? > > As discussed upthread, only returning whether the element is present in the > array is slightly faster. If we ever needed a version that returned the > address of the matching element, we could reevaluate whether this small > boost was worth creating a separate function or if we should just modify > pg_lfind32() to be a tad slower. I don't think we need to address that > now, though.
Isn't it a good idea to capture the above two points as comments in port/pg_lfind.h just to not lose track of it? I know these are present in the hackers thread, but having them in the form of comments helps developers who attempt to change or use the new function. -- Bharath Rupireddy RDS Open Source Databases: https://aws.amazon.com/rds/postgresql/
