On Mon, Mar 20, 2017 at 8:58 PM, Mithun Cy <mithun...@enterprisedb.com> wrote: > Hi Amit, Thanks for the review, > > On Mon, Mar 20, 2017 at 5:17 PM, Amit Kapila <amit.kapil...@gmail.com> wrote: >> idea could be to make hashm_spares a two-dimensional array >> hashm_spares[32][4] where the first dimension will indicate the split >> point and second will indicate the sub-split number. I am not sure >> whether it will be simpler or complex than the method used in the >> proposed patch, but I think we should think a bit more to see if we >> can come up with some simple technique to solve this problem. > > I think making it a 2-dimensional array will not be any useful in fact > we really treat the given array 2-dimensional elements now. >
Sure, I was telling you based on that. If you are implicitly treating it as 2-dimensional array, it might be easier to compute the array offsets. > The main concern of yours I think is the calculation steps to find the > phase of the splitpoint group the bucket belongs to. > + tbuckets = (1 << (splitpoint_group + 2)); > + phases_beyond_bucket = > + (tbuckets - num_bucket) / (1 << (splitpoint_group - 1)); > + return (((splitpoint_group + 1) << 2) - phases_beyond_bucket) - 1; > It is not only about above calculation, but also what the patch is doing in function _hash_get_tbuckets(). By the way function name also seems unclear (mainly *tbuckets* in the name). -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers