Re: TupleTableSlot abstraction

On Fri, Jul 13, 2018 at 3:45 PM, Ashutosh Bapat
<ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
>
>
>>>
>>> Next steps
>>> 1. Address TODO in the code. I have listed some of those above.
>>
>> There are still a handful of TODOs in the patches. I will work on those next.
>
> The number of TODOs has reduced, but there are still some that I am working on.

The attached patch set has all the TODOs fixed.

>
>>
>>>
>>> 2. Right now we are using TupleTableSlotType, an enum, to create slots
>>> of required type. But extensions which want to add their own slot
>>> types won't be able to add a type in that enum. So, they will need to
>>> write their own MakeTupleTableSlot. That function uses the
>>> TupleTableSlotType to set TupleTableSlotOps and calculate the minimum
>>> size of slot. We could change the function to accept TupleTableSlotOps
>>> and the minimum size and it just works for all the extensions. Or it
>>> could just accept TupleTableSlotOps and there's a callback to
>>> calculate minimum memory required for the slot (say based on the tuple
>>> descriptor available).
>
> This is still TODO.

Done.

>
>>
>>>
>>> 3. compile with LLVM and fix any compilation and regression errors.
>>
>> When I compiled server with just 0003 applied with LLVM, the
>> compilation went well, but there was a server crash. That patch
>> changes type of tts_nvalid from int32 to AttrNumber. I tried debugging
>> the crash with a debug LLVM build, but couldn't complete the work.
>> Attached patch attrnumber_llvm_type.patch is my incomplete attempt to
>> fix that crash. I think, we should make it easy to change the data
>> types of the members in structures shared by JIT and non-JIT code, may
>> be automatically create both copies of the code somehow. I will get
>> back to this after addressing other TODOs.
>>
>
> This is still a TODO

Still a TODO.

>
>>
>>>
>>> 6. ExecCopySlot can be a thin wrapper if we add a callback copyslot()
>>> and invoked on the destination slot type.
>
> This is still a TODO

Still a TODO.

>
>>
>>>
>>> 7. slot_attisnull() deforms a heap/minimal tuple if that status for
>>> given attribute is not available tts_isnull. Should we instead add a
>>> callback attisnull() which can use something like heap_isnull()?
>>
>
> This is still a TODO.

Done. I also noticed that slot_getattr() optimizes the cases when the
requested attributes is NULL or is missing from a tuple. Given that a
custom TupleTableSlot type can have its own optimizations for the
same, added a new call back getattr() to obtain value of a given
attribute from slot. The callback is called from slot_getattr().

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company