Обсуждение: Refactor to eliminate cast-away-const in pg_dump object sort comparator

Hi,

On Mon, Dec 29, 2025 at 05:44:22PM +0800, Chao Li wrote:
> On Dec 24, 2025, at 09:58, Chao Li <li.evan.chao@gmail.com> wrote:
> 
> Hi Hacker,
> 
> While reviewing patch [1], I raised a comment about cast-away-const in
> pg_dump_sort.c. However, the comment was not accepted and the argument was
> that the nearby code did the same thing.
> 
> I saw Tom recently had a commit [2] that removed some cast-away-const in
> ecpg, so I am filing this patch to eliminate all cast-away-const problems
> in pg_dump_sort.c.
> 
> [1]
> https://postgr.es/m/CALDaNm2x3rd7C0_HjUpJFbxpAqXgm=QtoKfkEWDVA8h+JFpa_w@mail.gmail.com
> [2]
> https://git.postgresql.org/cgit/postgresql.git/commit/?id=4eda42e8bdf5bd3bf69576d54a45c10e7cbc3b35
> 
> 
> I just noticed this patch does the similar thing as [3] just handling a
> different file. As Peter had a comment on [3], I addressed the comment as
> well in v2.

I think that your v1 was correct but your v2 now contains things like:

@@ -199,8 +199,8 @@ sortDumpableObjectsByTypeName(DumpableObject **objs, int numObjs)
 static int
 DOTypeNameCompare(const void *p1, const void *p2)
 {
-       DumpableObject *obj1 = *(DumpableObject *const *) p1;
-       DumpableObject *obj2 = *(DumpableObject *const *) p2;
+       const DumpableObject *obj1 = p1;
+       const DumpableObject *obj2 = p2;

that don't look correct. Indeed in sortDumpableObjectsByTypeName(),
objs is pointer to pointer. So qsort passes &objs[0] and &objs[1] to the
comparator. So that dereference is still needed in DOTypeNameCompare().

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On 30.12.25 10:03, Chao Li wrote:
> Thanks a lot for pointing out the error. v3 has reverted the changes to 
> be the same as v1 and rebased.

The explanation of this patch doesn't seem right.  The commit message 
says "... eliminate cast-away-const ...", but that is not what is 
happening in the code.  For example, in

  static int
  DOTypeNameCompare(const void *p1, const void *p2)
  {
-    DumpableObject *obj1 = *(DumpableObject *const *) p1;
-    DumpableObject *obj2 = *(DumpableObject *const *) p2;
+    const DumpableObject *obj1 = *(DumpableObject *const *) p1;
+    const DumpableObject *obj2 = *(DumpableObject *const *) p2;

p1 is of type pointer-to-const-void, which is then cast into 
pointer-to-const-pointer-to-DumpableObject (which preserves the 
qualifier, because it's pointer-to-const-xxx on both sides), which is 
then dereferenced to result in type const-pointer-to-DumpableObject 
(type DumpableObject * const, not const DumpableObject *), which is then 
assigned by value to obj1 of type pointer-to-DumpableObject.  This is 
all entirely correct.

Now there is nothing wrong with making the receiving obj1 have an 
additional const qualification, if that's the promise you want to make 
about it for that scope.  But that's separate from preserving the 
qualifier on p1.  And it's incorrect to claim that this is fixing an 
existing cast-away-const issue.

Independent of that, there appears to be some not quite finished code here:

-        AttrDefInfo *adobj1 = *(AttrDefInfo *const *) p1;
-        AttrDefInfo *adobj2 = *(AttrDefInfo *const *) p2;
+        const AttrDefInfo *adobj1 = p1;//*(AttrDefInfo *const *) p1;
+        const AttrDefInfo *adobj2 = *(AttrDefInfo *const *) p2;