On Thu, Dec 17, 2015 at 11:43 PM, Peter Geoghegan <p...@heroku.com> wrote:
> On Wed, Dec 16, 2015 at 12:04 PM, Peter Geoghegan <p...@heroku.com> wrote:
>>> What kind of state is that?  Can't we define this in terms of what it
>>> is rather than how it gets that way?
>>
>> It's zeroed.
>>
>> I guess we can update everything, including existing comments, to reflect 
>> that.

Thanks, this looks much easier to understand from here.

> Attached revision updates both the main commit (the optimization), and
> the backpatch commit (updated the contract).

-       /* abbreviation is possible here only for by-reference types */
+       /*
+        * Abbreviation is possible here only for by-reference types.
Note that a
+        * pass-by-value representation for abbreviated values is forbidden, but
+        * that's a distinct, generic restriction imposed by the SortSupport
+        * contract.

I think that you have not written what you meant to write here.  I
think what you mean is "Note that a pass-by-REFERENCE representation
for abbreviated values is forbidden...".

+       /*
+        * If we produced only one initial run (quite likely if the total data
+        * volume is between 1X and 2X workMem), we can just use that
tape as the
+        * finished output, rather than doing a useless merge.  (This obvious
+        * optimization is not in Knuth's algorithm.)
+        */
+       if (state->currentRun == 1)
+       {
+               state->result_tape = state->tp_tapenum[state->destTape];
+               /* must freeze and rewind the finished output tape */
+               LogicalTapeFreeze(state->tapeset, state->result_tape);
+               state->status = TSS_SORTEDONTAPE;
+               return;
+       }

I don't understand the point of moving this code.  If there's some
reason to do this after rewinding the tapes rather than beforehand, I
think we should articulate that reason in the comment block.

The last hunk in your 0001 patch properly belongs in 0002.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to