| 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: | Whole Thread | Raw Message | 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 |