On Mon, Apr 21, 2025 at 4:02 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote: > > > > On 2025/04/21 11:30, jian he wrote: > > hi. > > While trying to make the virtual generated column be part of the partition key, > > I found this bug.
I noticed that the check for a generated column in the partition key expression already exists a few lines below. Had that check placed before the if .. else ... block, we wouldn't have had this problem.
I admit that pull_varattnos() + loop through bms_next_member() is a bit wasteful when the expression is just a Var. But probably it's worth taking that hit for the sake of avoiding code duplication. Further, I believe that the two loops: one for system attribute check and the other for generated attribute check can be combined, something like attached. Then it's not as wasteful as it looks and we probably save a loop.
While looking at this I realised that a generated column may end up being part of the partition key if the partition key expression contains a whole row reference. Attached patch also has a fix and a testcase for the same. PARTITION BY RANGE ((gtest_part_key is not null)) expression in the test is kinda silly, but it tests the whole-row reference as part of an expression. I haven't looked for more sensible expressions.
I have included your original tests, but ended up rewriting code. Please let me know what do you think.
