From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> |
Cc: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Missing CHECK_FOR_INTERRUPTS in hash joins |
Date: | 2017-02-15 21:03:06 |
Message-ID: | 6969.1487192586@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> writes:
> On Wed, Feb 15, 2017 at 2:22 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Adding a C.F.I. inside this loop is the most straightforward fix, but
>> I am leaning towards adding one in ExecHashJoinGetSavedTuple instead,
> Would it also make sense to put one in the loop in
> ExecHashIncreaseNumBatches (or perhaps
> ExecHashJoinSaveTuple for symmetry with the above)? Otherwise you
> might have to wait for a few hundred MB of tuples to be written out
> which could be slow if IO is somehow overloaded.
Mmm, good point. I think in that case the C.F.I. had better be in
the loop in ExecHashIncreaseNumBatches, because if you were unlucky
the loop might not take the ExecHashJoinSaveTuple path for a long time.
Looking around at other callers of ExecHashJoinSaveTuple, the only one
that seems to be in need of a C.F.I. is the loop in
ExecHashRemoveNextSkewBucket, and there again there's a code path
whereby the loop doesn't call ExecHashJoinSaveTuple.
Will CFI-ify all three places.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2017-02-15 21:03:38 | Re: Patch to implement pg_current_logfile() function |
Previous Message | Karl O. Pinc | 2017-02-15 20:53:34 | Re: Patch to implement pg_current_logfile() function |