Re: [PATCH] Do not flush BufFile for regular temp files

Hi,

Maybe my test case is wrong and possibly also my understanding of BufFile.

There is some ambiguity  about what interXact=true really means.

Current code documentations for bufFileCreateTemp says:

* If interXact is true, the temp file will not be automatically deleted
* at end of transaction.
*
* Note: if interXact is true, the caller had better be calling us in a
* memory context, and with a resource owner, that will survive across
* transaction boundaries.

buffile.c headers also say:

* BufFile also supports temporary files that can be used by the single
backend
* when the corresponding files need to be survived across the
transaction and
* need to be opened and closed multiple times. Such files need to be created
* as a member of a FileSet.

I don't know what this means for a BufFile created with interXact=True and is not a FileSet.

The only use case I have found in in tuplestore_puttuple_common:

/*
* We switch out of the state->context as this is a generation
* context, which isn't ideal for allocations relating to the
* BufFile.
*/
oldcxt = MemoryContextSwitchTo(state->context->parent);

state->myfile = BufFileCreateTemp(state->interXact);

MemoryContextSwitchTo(oldcxt);

CurrentResourceOwner = oldowner;

This seems to be used by cursors declared with HOLD.

I agree with your patch because as far as I understand interXact=true keeps temp file open
and does not mean bufFileClose is called several time for the same temp file.

Le 17/08/2026 à 08:01, Sergey Soloviev a écrit :
> Hi!
>
> Thanks for response.
>
> I do not know whether this is correct to throw error when we can not
> do it.
> 'temp_file_limit' is enforced for actually created files and not on
> BufFile, so
> if we can create one without it's associated file, then I see here no
> problems.
>
> And this must be true for both normal and inter-xact, because in that
> case
> owner backend is the only who can work with the file, so there is no need
> to forcibly create disk file.
>
>> Hello
>>
>> I think there is an issue with this patch: you are not taking into
>> account a inter-transaction BufFile.
>>
>> If a BufFile is created with isInterXact set to true, BufFileClose
>> does not flush data and this looks wrong.
>>
>> You can test this with the following extension
>> https://github.com/pierreforstmann/buffile .
>>
>> With PostgreSQL 20devdel, I get:
>> --
>> -- Test BufFileClose() flushing behavior.
>> --
>> CREATE EXTENSION buffile;
>> -- A regular temporary BufFile should not be flushed by BufFileClose().
>> --
>> -- With temp_file_limit = 0, a physical write must fail. Therefore
>> -- false means that BufFileClose() did not attempt the write.
>> SET temp_file_limit = 0;
>> SELECT test_buffile_temp(false);
>> INFO:  BufFileClose has not flushed data
>>   test_buffile_temp
>> -------------------
>>   f
>> (1 row)
>>
>> -- An inter-transaction BufFile must still be flushed by BufFileClose().
>> --
>> -- Therefore the physical write should fail with temp_file_limit = 0.
>> SELECT test_buffile_temp(true);
>> INFO:  BufFileClose has not flushed data
>>   test_buffile_temp
>> -------------------
>>   f
>> (1 row)
>>
>> RESET temp_file_limit;
>> -- A FileSet BufFile must also be flushed by BufFileClose(), because
>> -- another opener must be able to see its contents.
>> SELECT test_buffile_fileset();
>> INFO:  BufFileClose has flushed data
>>   test_buffile_fileset
>> ----------------------
>>   t
>> (1 row)
>>
>>
>> Regards
>> Pierre Forstmann
>
> ---
> Sergey Soloviev
>
> TantorLabs: https://tantorlabs.com
>
>