Thread: Bug in DefineRange() with multiranges

Hi,

My colleague, Alex Kozhemyakin, stumbled upon a bug in DefineRange().
The problem is here:

     @@ -1707,7 +1707,6 @@ DefineRange(ParseState *pstate, 
CreateRangeStmt *stmt)
         /* Create cast from the range type to its multirange type */
         CastCreate(typoid, multirangeOid, castFuncOid, 'e', 'f', 
DEPENDENCY_INTERNAL);

     -   pfree(multirangeTypeName);
         pfree(multirangeArrayName);

         return address;


Given a query

     create type textrange1 as range(subtype=text, 
multirange_type_name=multirange_of_text, collation="C");

the string "multirange_of_text" in the parse tree is erroneously
pfree'd.  The corrupted parse tree is then passed to event triggers.

There is another branch in DefineRange() that genereates a multirange
type name which is fine to free.

I wonder what is the proper fix.  Just drop pfree() altogether or add
pstrdup() instead?  I see that makeMultirangeTypeName() doesn't bother
freeing its buf.

Here is a gdb session demonstating the bug:

Breakpoint 1, ProcessUtilitySlow (pstate=0x5652e80c7730, 
pstmt=0x5652e80a6a40, queryString=0x5652e80a5790 "create type textrange1 
as range(subtype=text, multirange_type_name=multirange_of_text, 
collation=\"C\");",
     context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, 
qc=0x7ffe835b4be0, dest=<optimized out>) at 
/pgwork/REL_14_STABLE/src/src/backend/tcop/utility.c:1621
1621                    address = DefineRange((CreateRangeStmt *) 
parsetree);
(gdb) p *(Value *)((TypeName *)((DefElem *)((CreateRangeStmt 
*)parsetree)->params->elements[1].ptr_value)->arg)->names->elements[0].ptr_value
$1 = {type = T_String, val = {ival = -401972176, str = 0x5652e80a6430 
"multirange_of_text"}}
(gdb) n
1900            if (!commandCollected)
(gdb) p *(Value *)((TypeName *)((DefElem *)((CreateRangeStmt 
*)parsetree)->params->elements[1].ptr_value)->arg)->names->elements[0].ptr_value
$2 = {type = T_String, val = {ival = -401972176, str = 0x5652e80a6430 
'\177' <repeats 32 times>, "\020"}}


Regards,

-- 
Sergey Shinderuk        https://postgrespro.com/

On 04.10.21 19:09, Sergey Shinderuk wrote:
> I wonder what is the proper fix.  Just drop pfree() altogether or add
> pstrdup() instead?  I see that makeMultirangeTypeName() doesn't bother
> freeing its buf.

I think removing the pfree()s is a correct fix.

On 10.10.2021 20:12, Peter Eisentraut wrote:
> On 04.10.21 19:09, Sergey Shinderuk wrote:
>> I wonder what is the proper fix.  Just drop pfree() altogether or add
>> pstrdup() instead?  I see that makeMultirangeTypeName() doesn't bother
>> freeing its buf.
> 
> I think removing the pfree()s is a correct fix.
> 

Thanks, here is a patch.


-- 
Sergey Shinderuk        https://postgrespro.com/

On Tue, Oct 12, 2021 at 08:52:29AM +0300, Sergey Shinderuk wrote:
> Thanks, here is a patch.

Looks fine seen from here, so I'll apply shortly.  I was initially
tempted to do pstrdup() on the object name returned by
QualifiedNameGetCreationNamespace(), but just removing the pfree() is
simpler.

I got to wonder about similar mistakes from the other callers of
QualifiedNameGetCreationNamespace(), so I have double-checked but
nothing looks wrong.
--
Michael

On 13.10.2021 07:21, Michael Paquier wrote:
> Looks fine seen from here, so I'll apply shortly.

Thank you!

-- 
Sergey Shinderuk        https://postgrespro.com/