Re: [WIP]Vertical Clustered Index (columnar store extension) - take2

Hi Peter!

> What exactly did Valgrind report? For example, you said the
> VciScanState points beyond allocated memory. Do you have any more
> clues, like where that happened? Did you discover where that (smaller
> than it should be) memory was allocated in the first place?

Doing some experiments I've faced a segfault on a query joining tables
filled with some amount of data. It was flaky so I used Valgrind.
There is a line in vci_scan.c, exec_custom_plan_enabling_vp():
if (!scanstate->first_fetch || (scanstate->pos.num_fetched_rows <=
scanstate->pos.current_row))
Valgrind reported that line as Invalid read of size 1, 4 and 4. So all
three of the values checked in this line are read from some random
memory, possibly allocated and used by other objects already.

When the expression in exec_custom_plan_enabling_vp() randomly
evaluated to true, the following ExecClearTuple() dereferences NULL in
slot->tts_ops.
The memory was originally allocated in nodeHashJoin.c, in hjstate =
makeNode(HashJoinState) line so it is really smaller than VciScanState.

I did not use any table data for reproducer since asserts helps to
catch the original problem. I also simplified the original query for a
reproducer.

> OK. I am not 100% certain about the asserts, but since the existing
> VCI tests are passing, I have merged your patch as-is into v24-0002.
> I
> guess we will find out later if the bug below is due to an old code
> cast problem or a new code assert problem.
>

Thanks for merging asserts. And looks like the problem is related to
VCI join nodes.
There is no VCI hash join or VCI nested loop. There is a code in VCI
planner that still puts VCI Sort or VCI Aggregate nodes on top of
regular join nodes which makes no sense to me. This is the cause of the
problem. VCI Sort and VCI Aggregate then convert outer nodes to VCI
Scan since they know there can't be anything another. This can be fixed
either by implementing VCI joins either by disabling them in a deeper
way. Since we already have developer GUCs for them I would rather set
them to disabled by default instead of removing all useful VCI joins
related code.

Made a patch with a test and a simplest fix (disabling joins in GUCs).

--
Regards,
Timur Magomedov