Re: Not deleted mentions of the cleared path

Hi! Thank you for the interest to this issue.

On 31.10.2023 06:25, Richard Guo wrote:

> On Mon, Oct 30, 2023 at 7:31 PM Alena Rybakina <a(dot)rybakina(at)postgrespro(dot)ru> wrote:
>
> I have already written about the problem of InvalidPath [0]
> appearing. I investigated this and found an error in the add_path()
> function, when we reject a path, we free up the memory of the path,
> but do not delete various mentions of it (for example, in the
> ancestor of relation, as in the example below).
>
> I agree that what you observed is true - add_path() may free a path
> while it's still referenced from some lower rels.  For instance, when
> creating ordered paths, we may use the input path unchanged without
> copying if it's already well ordered, and it might be freed afterwards
> if it fails when competing in add_path().
> But this doesn't seem to be a problem in practice.  We will not access
> these references from the lower rels.
> I'm not sure if this is an issue that we need to fix, or we need to live
> with.  But I do think it deserves some explanation in the comment of
> add_path().

I agree that the code looks like an error, but without a real request,
it is still difficult to identify it as a bug. I'll try to reproduce it.
And yes, at least a comment is required here, and to be honest, I have
already faced this problem myself.

On 30.10.2023 17:36, Ashutosh Bapat wrote:
> On Mon, Oct 30, 2023 at 5:01 PM Alena Rybakina
> <a(dot)rybakina(at)postgrespro(dot)ru> wrote:
>> Hi, hackers!
>>
>> I have already written about the problem of InvalidPath [0] appearing. I investigated this and found an error in the add_path() function, when we reject a path, we free up the memory of the path, but do not delete various mentions of it (for example, in the ancestor of relation, as in the example below).
>>
>> Thus, we may face the problem of accessing the freed memory.
>>
>> I demonstrated this below using gdb when I call a query after running a test in test/regression/sql/create_misc.sql:
>>
>> Query:
>>
>> :-- That ALTER TABLE should have added TOAST tables.
>>
>> SELECT relname, reltoastrelid <> 0 AS has_toast_table
>> FROM pg_class
>> WHERE oid::regclass IN ('a_star', 'c_star')
>> ORDER BY 1;
>>
>> --UPDATE b_star*
>> -- SET a = text 'gazpacho'
>> -- WHERE aa > 4;
>>
>> SELECT class, aa, a FROM a_star*;
>>
>>
>> gdb:
>>
>> 0x00007ff3f7325fda in epoll_wait (epfd=5, events=0x55bf9ee75c38, maxevents=1, timeout=-1) at ../sysdeps/unix/sysv/linux/epoll_wait.c:30
>> 30 ../sysdeps/unix/sysv/linux/epoll_wait.c: No such file or directory.
>> (gdb) b /home/alena/postgrespro_3/src/backend/optimizer/util/pathnode.c:620
>> Breakpoint 1 at 0x55bf9cfe4c65: file pathnode.c, line 621.
>> (gdb) c
>> Continuing.
>>
>> Breakpoint 1, add_path (parent_rel=0x55bf9ef7f5c0, new_path=0x55bf9ef7f4e0) at pathnode.c:621
>> 621 if (!IsA(new_path, IndexPath))
>> (gdb) n
>> 622 pfree(new_path);
>> (gdb) n
>> 624 }
>> (gdb) p *new_path
>> $1 = {type = T_Invalid, pathtype = T_Invalid, parent = 0x7f7f7f7f7f7f7f7f, pathtarget = 0x7f7f7f7f7f7f7f7f,
>> param_info = 0x7f7f7f7f7f7f7f7f, parallel_aware = 127, parallel_safe = 127, parallel_workers = 2139062143,
>> rows = 1.3824172084878715e+306, startup_cost = 1.3824172084878715e+306, total_cost = 1.3824172084878715e+306,
>> pathkeys = 0x7f7f7f7f7f7f7f7f}
>> (gdb) p new_path
>> $2 = (Path *) 0x55bf9ef7f4e0
> At this point the new_path has not been added to the parent_rel. We do
> not set the cheapest* paths while paths are being added. The stack
> trace will give you an idea where this is happening.
>> (gdb) p ((ProjectionPath *)((SortPath*)parent_rel->pathlist->elements [0].ptr_value)->subpath)->path->parent->cheapest_startup_path
>> $20 = (struct Path *) 0x55bf9ef7f4e0
> This looks familiar though. There was some nearby thread where Tom
> Lane, if my memory serves well, provided a case where a path from
> lower rel was added to an upper rel without copying or changing its
> parent. This very much looks like that case.
>
Thank you, I think this might help me to find a query to reproduce it.