Change LogicalTapeSetBlocks() to use nBlocksWritten.

Previously, it was based on nBlocksAllocated to account for tapes with
open write buffers that may not have made it to the BufFile yet.

That was unnecessary, because callers do not need to get the number of
blocks while a tape has an open write buffer; and it also conflicted
with the preallocation logic added for HashAgg.

Reviewed-by: Peter Geoghegan
Discussion: https://postgr.es/m/ce5af05900fdbd0e9185747825a7423c48501964.camel@j-davis.com
Backpatch-through: 13

diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 28802e6588dbfdc131c7958e415371c352e0cd43..75e5bbf209d536613eaaa8411b073346f3dac1fd 100644 (file)
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -2704,8 +2704,8 @@ agg_refill_hash_table(AggState *aggstate)
 
    if (spill_initialized)
    {
-       hash_agg_update_metrics(aggstate, true, spill.npartitions);
        hashagg_spill_finish(aggstate, &spill, batch->setno);
+       hash_agg_update_metrics(aggstate, true, spill.npartitions);
    }
    else
        hash_agg_update_metrics(aggstate, true, 0);

diff --git a/src/backend/utils/sort/logtape.c b/src/backend/utils/sort/logtape.c
index d6d1e1911ea411d64b8cdeaa278675b88d61ab9d..28905124f965a922fa8907aface8da7de2cd080f 100644 (file)
--- a/src/backend/utils/sort/logtape.c
+++ b/src/backend/utils/sort/logtape.c
@@ -1264,9 +1264,19 @@ LogicalTapeTell(LogicalTapeSet *lts, int tapenum,
 
 /*
  * Obtain total disk space currently used by a LogicalTapeSet, in blocks.
+ *
+ * This should not be called while there are open write buffers; otherwise it
+ * may not account for buffered data.
  */
 long
 LogicalTapeSetBlocks(LogicalTapeSet *lts)
 {
-   return lts->nBlocksAllocated - lts->nHoleBlocks;
+#ifdef USE_ASSERT_CHECKING
+   for (int i = 0; i < lts->nTapes; i++)
+   {
+       LogicalTape *lt = &lts->tapes[i];
+       Assert(!lt->writing || lt->buffer == NULL);
+   }
+#endif
+   return lts->nBlocksWritten - lts->nHoleBlocks;
 }