On Thu, Jun 23, 2022 at 2:25 AM Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote: > > On Thu, Jun 23, 2022 at 1:04 AM David G. Johnston <david.g.johnston@gmail.com> wrote: >> >> - WRITE_FLOAT_FIELD(rows, "%.0f"); >> + WRITE_FLOAT_FIELD(rows, "%.2f"); >> >> This one looks suspicious, though I haven't dug into the code to see exactly what all is being touched. That it doesn't have an nloops condition like everything else stands out. >> > I was also thinking about that, but I don't see any harm when we ultimately truncating that decimal > at a latter stage of code in case of loop = 1. >
That change is in the path node which we anyway not going to target as part of this change. We only want to change the display for actual rows in Explain Analyze. So, I can't see how the quoted change can help in any way.
Agreed removed.
Few miscellaneous comments: ======================== * static FullTransactionId XactTopFullTransactionId = {InvalidTransactionId}; -static int nParallelCurrentXids = 0; +static int nParallelCurrentXids = 0;
Removed.
I don't see why this change is required.
* Can you please add a comment explaining why we are making this change for actual rows?
Done
* Can you please write a test case unless there is some existing test that covers the change by displaying actual rows values in decimal but in that case patch should have that changed output test? If you don't think we can reliably write such a test then please let me know the reason?
I think there are tests, and I have updated the results accordingly.