Re: check for null value before looking up the hash function - Mailing list pgsql-hackers

From Tomas Vondra
Subject Re: check for null value before looking up the hash function
Date
Msg-id 722844cb-b578-fd93-b5a5-c696a7e58b90@enterprisedb.com
Whole thread Raw
In response to Re: check for null value before looking up the hash function  (Ranier Vilela <ranier.vf@gmail.com>)
Responses Re: check for null value before looking up the hash function  (Ranier Vilela <ranier.vf@gmail.com>)
List pgsql-hackers

On 5/21/22 15:06, Ranier Vilela wrote:
>>Zhihong Yu <zyu(at)yugabyte(dot)com> writes:
>>> I was looking at the code in hash_record()
>>> of src/backend/utils/adt/rowtypes.c
>>> It seems if nulls[i] is true, we don't need to look up the hash function.
> 
>>I don't think this is worth changing. It complicates the logic,
>>rendering it unlike quite a few other functions written in the same
>>style. In cases where the performance actually matters, the hash
>>function is cached across multiple calls anyway. You might save
>>something if you have many calls in a query and not one of them
>>receives a non-null input, but how likely is that?
> 
> I disagree.
> I think that is worth changing. The fact of complicating the logic
> is irrelevant.

That's a quite bold claim, and yet you haven't supported it by any
argument whatsoever. Trade-offs between complexity and efficiency are a
crucial development task, so complicating the logic clearly does matter.

It might be out-weighted by efficiency benefits, but as Tom suggested
the cases that might benefit from this are extremely unlikely (data with
just NULL values). And even for those cases no benchmark quantifying the
difference was presented.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



pgsql-hackers by date:

Previous
From: Bharath Rupireddy
Date:
Subject: Enforce "max_wal_size/ min_wal_size must be at least twice wal_segment_size" limit while setting GUCs
Next
From: Ranier Vilela
Date:
Subject: Re: check for null value before looking up the hash function