On Thu, Apr 5, 2018 at 8:04 AM, Chapman Flack <chap@anastigmatix.net> wrote:
> On 04/01/18 01:19, Tom Lane wrote:
>> Chapman Flack <chap@anastigmatix.net> writes:
>>> If I copy an out-of-line, on-disk TOAST pointer into a memory context
>>> with transaction lifetime, with an eye to detoasting it later in the
>>> same transaction, are there circumstances where it wouldn't work?
>>
>> Should be safe *as long as you hold onto a snapshot that can see the
>> toast value's parent row*. Otherwise, it's not.
>>
>> For a counterexample, see the changes we had to make to avoid depending
>> on out-of-line toast values in the catcaches, commit 08e261cbc.
>
> Thanks. I think I see two approaches happening in that commit: retaining
> a snapshot, if the tuples will be used within the transaction, or eagerly
> detoasting, when persisting a holdable portal so tuples can be used after
> the transaction.
>
> Please bear with me as I check my understanding of snapshot management
> by looking at PersistHoldablePortal(). There's a
> PushActiveSnapshot(queryDesc->snapshot) in there. Is that because:
>
> (a) that snapshot might not be on the active stack at this point, and it
> needs to be be there to keep it alive during this executor re-run, or
>
> (b) it's known to be somewhere on the active stack already, but not
> necessarily on top, and needs to be pushed so it is topmost while
> re-running this executor (does the top snapshot on the active stack
> affect which tuples the executor sees? or is this stack's only purpose
> to keep snapshots alive?), or
>
> (c) it's known to be somewhere on the stack already, but needs to be
> be pushed to make sure some stack-depth invariant is preserved
> (perhaps because of an implied pop in case of an error), or
>
> (d) some other reason I haven't thought of ?
>
> I *think* I'm smart enough to rule out choice (a), because it wouldn't
> make sense to have queryDesc->snapshot refer to a snapshot that isn't
> already on the stack (unless it's also been registered, in which case,
> why would it need to be pushed on the stack too?), as then I would be
> reckless to assume it's still alive to *be* pushed. Am I close?
>
I think it is somewhat close to what you have mentioned in (b).
Basically, it will help executor to use that snapshot for tuple
visibility.
> I haven't yet gone to track down the code that assigns a snapshot to
> queryDesc->snapshot.
>
See CreateQueryDesc().
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com