On Thu, 18 Dec 2025 at 14:05, Chao Li <[email protected]> wrote: > > > > > On Dec 18, 2025, at 03:51, Noah Misch <[email protected]> wrote: > > > > On Wed, Dec 17, 2025 at 10:11:58AM +0530, vignesh C wrote: > >> The attached v3 version patch has the changes for the same. > > > > The "tag" variable needed a change to compensate for the subrinfo->dobj.name > > change. I plan to push the attached version. > > <DO_SUBSCRIPTION_REL-v4.patch> > > v4 looks solid. A couple of nitpicks: > > 1 > ``` > + SubRelInfo *srobj1 = *(SubRelInfo *const *) p1; > + SubRelInfo *srobj2 = *(SubRelInfo *const *) p2; > ``` > > These two temp pointers can be const, like: > ``` > const SubRelInfo *srobj1 = *(SubRelInfo *const *) p1; > const SubRelInfo *srobj2 = *(SubRelInfo *const *) p2; > ```
I felt the way it is handled in the patch is ok and consistent with the other variables used in this function. > 2 > ``` > + /* Sort by subscription name, since (namespace, name) match > the rel */ > ``` > > This comment is correct, but sounds a little insider-ish. Maybe: > > /* Tiebreak by subscription name; (namespace, name) already identify the > table */ Similarly here too, it is inline with similar comments of other enums in this function. Regards, Vignesh
