Re: dsm_unpin_segment

On Sat, Aug 20, 2016 at 6:13 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Sat, Aug 20, 2016 at 7:37 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> 2.
>> + if (dsm_control->item[seg->control_slot].pinned)
>> + elog(ERROR, "cannot pin a segment that is already pinned");
>>
>> Shouldn't this be a user facing error (which means we should use ereport)?
>
> Uh, certainly not. This can't happen because of SQL the user enters;
> it can only happen because of a C coding error. elog() is the
> appropriate tool for that case.
>

Okay, your point makes sense to me, but in that case why not use an
Assert here? I think elog() could also be used here as well, but it
seems to me elog() is most appropriate for the cases where it is not
straightforward to tell the behaviour of API or value of variable like
when PageAddItem() is not successful.

void
dsm_pin_segment(dsm_segment *seg)

+void
+dsm_unpin_segment(dsm_handle handle)

Another point, I want to highlight here is that pin/unpin API's have
different type of arguments. The comments on top of function
dsm_unpin_segment() explains the need of using dsm_handle which seems
reasonable. I just pointed out to see if anybody else has a different
view.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com