Hi, Tom!
On Fri, May 3, 2024 at 2:19 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alexander Korotkov <aekorotkov@gmail.com> writes:
> > I would appreciate your review of this patchset, and review from Andrei as well.
>
> I hate to say this ... but if we're still finding bugs this
> basic in SJE, isn't it time to give up on it for v17?
>
> I might feel better about it if there were any reason to think
> these were the last major bugs. But you have already committed
> around twenty separate fixes for the original SJE patch, and
> now here you come with several more; so it doesn't seem like
> the defect rate has slowed materially. There can be no doubt
> whatever that the original patch was far from commit-ready.
I think if we subtract from the SJE followup commits containing
improvements (extra comments, asserts) and fix for in-place Bitmapset
modification, which was there before, the number of fixes will be
closer to ten. And the number of pending fixes will be two. But I
totally get your concern that we're quite late in the release cycle
and new SJE-related issues continue to arise. This leads to a
significant risk of raising many bugs for end users.
> I think we should revert SJE for v17 and do a thorough design
> review before trying again in v18.
I agree to revert it for v17, but I'm not exactly sure the issue is
design (nevertheless design review is very welcome as any other type
of review). The experience of the bugs arising with the SJE doesn't
show me a particular weak spot in the feature. It looks more like
this patch has to revise awfully a lot planner data structures to
replace one relid with another. And I don't see the design, which
could avoid that. Somewhere in the thread I have proposed a concept
of "alias relids". However, I suspect that could leave us with more
lurking bugs instead of the bug-free code.
I suggest we should give this feature more review and testing, then
commit early v18. That would leave us enough time to fix any other
issues before v18 release.
------
Regards,
Alexander Korotkov
Supabase
