On Sat, Aug 12, 2017 at 10:31 AM, Haribabu Kommi <kommi.harib...@gmail.com> wrote: >> >> Why do we need to store handler function in TupleDesc? As of now, the >> above patch series has it available in RelationData and >> TupleTableSlot, I am not sure if instead of that keeping it in >> TupleDesc is a good idea. Which all kind of places require TupleDesc >> to contain handler? If those are few places, can we think of passing >> it as a parameter? > > > Till now I am to able to proceed without adding any storage handler > functions to > TupleDesc structure. Sure, I will try the way of passing as a parameter when > there is a need of it. >
Okay, I think it is better if you discuss such locations before directly modifying those. > During the progress of the patch, I am facing problems in designing the > storage API > regarding the Buffer. For example To replace all the HeapTupleSatisfiesMVCC > and > related functions with function pointers, In HeapTuple format, the tuple may > belongs > to one buffer, so the buffer is passed to the HeapTupleSatifisifes*** > functions along > with buffer, But in case of other storage formats, the single buffer may not > contains > the actual data. > Also, it is quite possible that some of the storage Am's don't even want to return bool as a parameter from HeapTupleSatisfies* API's. I guess what we need here is to provide a way so that different storage am's can register their function pointer for an equivalent to satisfies function. So, we need to change SnapshotData.SnapshotSatisfiesFunc in some way so that different handlers can register their function instead of using that directly. I think that should address the problem you are planning to solve by omitting buffer parameter. > This buffer is used to set the Hint bits and mark the > buffer as dirty. > In case if the buffer is not available, the performance may affect for the > following > queries if the hint bits are not set. > I don't think it is advisable to change that for the current heap. > And also the Buffer is used to get from heap_fetch, heap_lock_tuple and > related > functions to check the Tuple visibility, but currently returning a buffer > from the above > heap_** function is not possible for other formats. > Why not? I mean if we consider that all the formats we are worried at this stage have TID (block number, tuple location), then we can get the buffer. We might want to consider passing TID as a parameter to these API's if required to make that possible. You also agreed above [1] that we can first design the API considering storage formats having TID. > And also for the > HeapTuple data, > the tuple data is copied into palloced buffer instead of pointing directly > to the page. > So, returning a Buffer is a valid or not here? > Yeah, but I think for the sake of compatibility and not changing too much in the current API's signature, we should try to avoid it. > Currently I am proceeding to remove the Buffer as parameter in the API and > proceed > further, In case if it affects the performance, we need to find out a > different appraoch > in handling the hint bits. > Leaving aside the performance concern, I am not convinced that it is a good idea to remove Buffer as a parameter from the API's you have mentioned above. Would you mind thinking once again keeping the suggestions provided above in this email to see if we can avoid removing Buffer as a parameter? [1] - https://www.postgresql.org/message-id/CAJrrPGd8%2Bi8sqZCdhfvBhs2d1akEb_kEuBvgRHSPJ9z2Z7VBJw%40mail.gmail.com -- 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
