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. David
