> On Fri, 6 Sept 2024 at 19:08, Ashutosh Bapat > <ashutosh.bapat....@gmail.com> wrote: >> The changes look better. A nitpick though. With their definitions >> changed, I think it's better to change the names of the functions >> since their purpose has changed. Right now they report the storage >> type and size used, respectively, at the time of calling the function. >> With this patch, they report maximum space ever used and the storage >> corresponding to the maximum space. tuplestore_space_used() may be >> changed to tuplestore_maxspace_used(). I am having difficulty with >> tuplestore_storage_type_name(); tuplestore_largest_storage_type_name() >> seems mouthful and yet not doing justice to the functionality. It >> might be better to just have one funciton tuplestore_maxspace_used() >> which returns both the maximum space used as well as the storage type >> when maximum space was used. > > How about just removing tuplestore_storage_type_name() and > tuplestore_space_used() and adding tuplestore_get_stats(). I did take > some inspiration from tuplesort.c for this, so maybe we can defer back > there for further guidance. I'm not so sure it's worth having a stats > struct type like tuplesort.c has. All we need is a char ** and an > int64 * output parameter to pass to the stats function. I don't think > we need to copy the tuplesort_method_name(). It seems fine just to > point the output parameter of the stats function at the statically > allocated constant.
Are you going to push the changes to tuplestore.c anytime soon? I would like to rebase my patch[1] but the patch could be affected by the tuplestore API change. Best reagards, [1] https://www.postgresql.org/message-id/CAApHDvoY8cibGcicLV0fNh%3D9JVx9PANcWvhkdjBnDCc9Quqytg%40mail.gmail.com -- Tatsuo Ishii SRA OSS K.K. English: http://www.sraoss.co.jp/index_en/ Japanese:http://www.sraoss.co.jp
