On Thu, Oct 6, 2016 at 12:00 AM, Heikki Linnakangas <hlinn...@iki.fi> wrote: > This is related to earlier the discussion with Peter G, on whether we should > change state->maxTapes to reflect the actual number of tape that were used, > when that's less than maxTapes. I think his confusion about the loop in > init_tape_buffers(), in > CAM3SWZQ7=fcy1iuz6jnzuunnktag6uitc1i-donxscp-9zs...@mail.gmail.com would > also have been avoided, if we had done that. So I think we should reconsider > that.
-1 on that from me. I don't think that you should modify a variable that is directly linkable to Knuth's description of polyphase merge -- doing so seems like a bad idea. state->maxTapes (Knuth's T) really is something that is established pretty early on, and doesn't change. While the fix you pushed was probably a good idea anyway, I still think you should not use state->maxTapes to exhaustively call LogicalTapeAssignReadBufferSize() on every tape, even non-active tapes. That's the confusing part. It's not as if your need for the number of input tapes (the number of maybe-active tapes) is long lived; you just need to instruct logtape.c on buffer sizing once, at the start of mergeruns(). Besides, what I propose to do is really exactly the same as what you also want to do, except it avoids actually changing state->maxTapes. We'd just pass down what you propose to assign to state->maxTapes directly, which differs (and not just in the common case where there are inactive tapes -- it's always at least off-by-one). Right? -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
