On 11 Feb 2026 at 12:34 +0100, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>, wrote:
> > Thanks for sorting all these changes Dean, all look good to me. The only
> > thing that gave me pause is this change in analyse.c lines 1238-1247:
> >
> > So in v.24 we set p_is_insert = false for DO SELECT as well.
> > If I’m understanding the code correctly, this is only used when using indirection
> > on target columns for updates, like setting a value inside an array, like for example:
> > update example set arr[2] = 10;
> > So changing this should have any effect, as the DO SELECT doesn’t have any SET clause.
> > Logically it makes sense that DO SELECT is p_is_insert = false.
> > But maybe the comment should be updated to explain this?
>
> Yes, that's right. I was just trying to reduce code churn. However, if
> we need to change the comment to explain why it's OK for DO SELECT,
> the code churn argument doesn't work, and it's probably better to only
> set it for DO UPDATE. So I've reverted it back to how it was in v23,
> with slightly updated comments.
Looking more at this, I’m quite sure that the p_is_insert field can just be removed?
See 0002. If you’d rather focus on reducing code churn, or you believe this is risky,
I’m also happy for you to just commit 0001.
> Re-reading the INSERT docs, I still wasn't entirely happy with the
> description of the conflict_action parameter, so I had a go at
> rewriting it. I also pulled out the description of the locking
> options, describing them separately and linking to the description of
> the locking clause on the SELECT page.
Had a look through it and yes, the docs are better with your changes.