Thread

Commits

  1. Fix minor memory leak in pg_dump

  2. pg_dump: Fix handling of ALTER DEFAULT PRIVILEGES

  1. [patch] some PQExpBuffer are not destroyed in pg_dump

    Zhang, Jie <zhangjie2@cn.fujitsu.com> — 2020-04-07T02:42:40Z

    Hi al
    
    In getDefaultACLs function, some PQExpBuffer are not destroy
    
    File: src/bin/pg_dump/pg_dump.c
    DefaultACLInfo *
    getDefaultACLs(Archive *fout, int *numDefaultACLs)
    {
    ......
    	if (fout->remoteVersion >= 90600)
    	{
    		PQExpBuffer acl_subquery = createPQExpBuffer();       // *** acl_subquery not destroyed ***
    		PQExpBuffer racl_subquery = createPQExpBuffer();      // *** racl_subquery not destroyed ***
    		PQExpBuffer initacl_subquery = createPQExpBuffer();   // *** initacl_subquery not destroyed ***
    		PQExpBuffer initracl_subquery = createPQExpBuffer();  // *** initracl_subquery not destroyed ***
    
    		buildACLQueries(acl_subquery, racl_subquery, initacl_subquery,
    						initracl_subquery, "defaclacl", "defaclrole",
    						"CASE WHEN defaclobjtype = 'S' THEN 's' ELSE defaclobjtype END::\"char\"",
    						dopt->binary_upgrade);
    
    		appendPQExpBuffer(query, "SELECT d.oid, d.tableoid, "
    						  "(%s d.defaclrole) AS defaclrole, "
    						  "d.defaclnamespace, "
    						  "d.defaclobjtype, "
    						  "%s AS defaclacl, "
    						  "%s AS rdefaclacl, "
    						  "%s AS initdefaclacl, "
    						  "%s AS initrdefaclacl "
    						  "FROM pg_default_acl d "
    						  "LEFT JOIN pg_init_privs pip ON "
    						  "(d.oid = pip.objoid "
    						  "AND pip.classoid = 'pg_default_acl'::regclass "
    						  "AND pip.objsubid = 0) ",
    						  username_subquery,
    						  acl_subquery->data,
    						  racl_subquery->data,
    						  initacl_subquery->data,
    						  initracl_subquery->data);
    	}
    ......
    
    Here is a patch.
    
    Best Regards!
    
    
    
    
  2. Re: [patch] some PQExpBuffer are not destroyed in pg_dump

    Masahiko Sawada <masahiko.sawada@2ndquadrant.com> — 2020-04-13T07:51:06Z

    On Tue, 7 Apr 2020 at 11:42, Zhang, Jie <zhangjie2@cn.fujitsu.com> wrote:
    >
    > Hi al
    >
    > In getDefaultACLs function, some PQExpBuffer are not destroy
    >
    
    Yes, it looks like an oversight. It's related to the commit
    e2090d9d20d809 which is back-patched to 9.6.
    
    The patch looks good to me.
    
    Regards,
    
    -- 
    Masahiko Sawada            http://www.2ndQuadrant.com/
    PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
    
    
    
    
  3. Re: [patch] some PQExpBuffer are not destroyed in pg_dump

    Michael Paquier <michael@paquier.xyz> — 2020-04-14T01:11:56Z

    On Mon, Apr 13, 2020 at 04:51:06PM +0900, Masahiko Sawada wrote:
    > On Tue, 7 Apr 2020 at 11:42, Zhang, Jie <zhangjie2@cn.fujitsu.com> wrote:
    >> In getDefaultACLs function, some PQExpBuffer are not destroy
    > 
    > Yes, it looks like an oversight. It's related to the commit
    > e2090d9d20d809 which is back-patched to 9.6.
    > 
    > The patch looks good to me.
    
    Indeed.  Any code path of pg_dump calling buildACLQueries() clears up
    things, and I think that it is a better practice to clean up properly
    PQExpBuffer stuff even if there is always the argument that pg_dump
    is a tool running in a "short"-term context.  So I will backpatch that
    unless there are any objections from others.
    
    The part I am actually rather amazed of here is that I don't recall
    seeing Coverity complaining about leaks after this commit.  Perhaps it
    just got lost.
    --
    Michael
    
  4. Re: [patch] some PQExpBuffer are not destroyed in pg_dump

    Michael Paquier <michael@paquier.xyz> — 2020-04-15T07:05:10Z

    On Tue, Apr 14, 2020 at 10:11:56AM +0900, Michael Paquier wrote:
    > Indeed.  Any code path of pg_dump calling buildACLQueries() clears up
    > things, and I think that it is a better practice to clean up properly
    > PQExpBuffer stuff even if there is always the argument that pg_dump
    > is a tool running in a "short"-term context.  So I will backpatch that
    > unless there are any objections from others.
    
    And done as of 8f4ee44.
    --
    Michael