Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1vGuZL-007jq7-V6 for pgsql-hackers@arkaria.postgresql.org; Thu, 06 Nov 2025 07:38:07 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1vGuZJ-006Ebq-Gk for pgsql-hackers@arkaria.postgresql.org; Thu, 06 Nov 2025 07:38:04 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1vGuZJ-006Ebh-2j for pgsql-hackers@lists.postgresql.org; Thu, 06 Nov 2025 07:38:04 +0000 Received: from mail-lj1-x22e.google.com ([2a00:1450:4864:20::22e]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.96) (envelope-from ) id 1vGuZG-005gOS-0e for pgsql-hackers@lists.postgresql.org; Thu, 06 Nov 2025 07:38:03 +0000 Received: by mail-lj1-x22e.google.com with SMTP id 38308e7fff4ca-378e8d10494so7711681fa.2 for ; Wed, 05 Nov 2025 23:38:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1762414680; x=1763019480; darn=lists.postgresql.org; h=in-reply-to:from:content-language:references:cc:to:subject :user-agent:mime-version:date:message-id:from:to:cc:subject:date :message-id:reply-to; bh=LwH4AnsV4nvs0GFtNfHddMvhvHlmsPDRaurb3Q3AiMw=; b=MC+zUs0DjccqmlZAuovm1ZB7MVwJDNoGO0C4o1vk1qlQ6X3V1CakMf1ugBNcAEb9fK /KVDYrylSWg/76k2KakKTuOHw64Ds5H5OAC10Eh08V1E+PPNOwdrVWsLTfnuU5lzEVzs L5cdGMzmh60VUjCRogOcxW3in5/87yC27MUfdtQALk41YtBOcBwUtKSyxKXiKwm9XCzw LFAso4Hpekd9rkOi/72reTXI2XDqXWAJWtS25QquT2nrtOtStsMhqRRPnCX9lVmmt61K xjvlrlLTiKxtm5JiFptF6YAfK1UHwI1OTJE5qL11rthPTuHB0G5W1A7W8Q9F9qYo3o2y DpKQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1762414680; x=1763019480; h=in-reply-to:from:content-language:references:cc:to:subject :user-agent:mime-version:date:message-id:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=LwH4AnsV4nvs0GFtNfHddMvhvHlmsPDRaurb3Q3AiMw=; b=Iv3hQHCPwJmwDAVw9NZ2MnCuQrusSSZINh7SutTnha5Z6Q5RW6mmSZjfzJJ488X/IL 9kxOtQPFz7uioPzT2NZ/wG8CTGxa7YKrdW6M+EgLvXvOTFzszoDpNJDvl02rrtqNuDnY hPWiqEbEzkRbPd8Tz+Fjab+SwqlPyN75LwmOuFuqNBXp8/hrldq657dBo1Tbp+fZdIrH y14CPlJRRGJDURCucThcKoxdffsOgAI4DASfcC2y6jebwom3BrF3FEG48XJGtAgi8jGp ddB611rJ9WHuHUZA6ocxDC2SuuF5x+AgW+MogWb/MA1SwYjNLxXHJ0/STLd6UK6V1gwB T3AQ== X-Gm-Message-State: AOJu0YyP5RCiavoSKP8e/wY8e+/6/Tov61V6EfQ3SZ4OZykQYiyBTv+/ /A9hhKdbc9zmG3VbezDsliEJJlhaQlE1BThqk00O7F+shh2f/IKIsCNm X-Gm-Gg: ASbGnctE6xfRZic9I7bJZ81gvYmHNVVU3ywfuSjWiErBs3dHEut0BdXo/5PskWb19Ng U8IctfMcOGIv65iWfXTMGVV6RfdDt0nP3EkxfBsy85I/h+D6COzDS3DmRZILceOcvUb6Vv/BLdO xAH48rnvugSQ7yXIf6tI537mdvEy/khtwfIqw4udXXfotuUE3FCV+zfDIAKdEGbY8duf1Vm3s82 jxhqSzhvSDjGgaZG7o6Bvlmww/VsOeFB0jSpYFg3k0C6fJwK9cHHplKhjBCgH0gQLEEpOH3N1IR 4FEFX/NrCG2efn7VjZ2OZPjMJYPpJ6YD9wPUzXHgwlsugSOLK1tmYh/W6N1TR1rdNNsAcVUZO6r xDGihuIotr8/f50gj6Qhh/61XL73OedWU45DU2tNehE/wmBeW9GTfgCjb6h/8UlkYA4tlX2HOXP lgBuLvvY3cWdbDZHXtXbQZyEZUiHBOfaG47Ws5pcEiYQ8s9Kv9Cp65+Yf8FRp2ew== X-Google-Smtp-Source: AGHT+IGCiPIzHHDsP7W/8NpxUhNAWZd6/8/S1NErsDhumEyEfFsfWdUWq4/kGzpVsOr0T5YCKiLcKA== X-Received: by 2002:a05:6512:108e:b0:591:eb9d:ac01 with SMTP id 2adb3069b0e04-5943d7e5762mr1857114e87.41.1762414680135; Wed, 05 Nov 2025 23:38:00 -0800 (PST) Received: from ?IPV6:2a02:aa1:114a:fa1c:dc3a:c8ca:288d:f528? ([2a02:aa1:114a:fa1c:dc3a:c8ca:288d:f528]) by smtp.gmail.com with ESMTPSA id 2adb3069b0e04-5944a593a0esm491694e87.95.2025.11.05.23.37.59 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 05 Nov 2025 23:37:59 -0800 (PST) Content-Type: multipart/alternative; boundary="------------qyGUQkJbrBorD5CkZiBl2HWm" Message-ID: <71b9d87a-aa3f-4d5e-bc40-e9e4e60e1c82@gmail.com> Date: Thu, 6 Nov 2025 08:37:58 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: Use stack-allocated StringInfoData To: David Rowley Cc: pgsql-hackers@lists.postgresql.org References: <4379aac8-26f1-42f2-a356-ff0e886228d3@gmail.com> <1865ee50-8290-4062-b062-5f5ab17364f2@gmail.com> <04a6a8b3-7572-4021-aaf8-5d3643bc9dac@gmail.com> Content-Language: en-US From: Mats Kindahl In-Reply-To: List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk This is a multi-part message in MIME format. --------------qyGUQkJbrBorD5CkZiBl2HWm Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 11/5/25 12:01, David Rowley wrote: > On Tue, 4 Nov 2025 at 21:25, Mats Kindahl wrote: >> Here is an updated patch that I checked manually for unnecessary >> whitespace changes. > Thanks for updating the patch. > >> There is one such case affected by this patch, and there the buffer >> pointer is copied to another variable and then returned. This can >> probably be improved by just returning the buffer pointer directly >> without intermediate assignment to a variable, but I avoided this to >> make reviewing the code easier. It is easy to add this change either now >> or later and should not affect the generated code except at very low >> optimization levels. > Yeah, I think you must mean in build_backup_content(). I noticed that too. Yes, that is the one I meant. >> I also added fixes manually inside check_publications_origin_tables, >> check_publications, and fetch_relation_list. In >> check_publication_origin_table three StringInfo was dynamically >> allocated but not released after. In check_publications and >> fetch_relation_list there were extra cases of using a dynamically >> allocated StringInfo that was not necessary. > It's not your fault, but check_publications_origin_tables() looks like > a mess. It seems like excessive use of additional code just to avoid > two separate ereport() calls. Might be worth a follow-up patch to > clean that up. Yeah, I have noted that too, but I wanted to avoid making reviews more complicated than necessary. I thought it might be better to do as a separate effort. I'll take a look at it. > The patch looks good to me. The only thing I'd adjust is to get rid of > the "data" variable from build_backup_content(), which I think you > mentioned above. I can take care of that one. Thanks David. Best wishes, Mats Kindahl --------------qyGUQkJbrBorD5CkZiBl2HWm Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit
On 11/5/25 12:01, David Rowley wrote:
On Tue, 4 Nov 2025 at 21:25, Mats Kindahl <mats.kindahl@gmail.com> wrote:

Here is an updated patch that I checked manually for unnecessary
whitespace changes.

Thanks for updating the patch.


There is one such case affected by this patch, and there the buffer
pointer is copied to another variable and then returned. This can
probably be improved by just returning the buffer pointer directly
without intermediate assignment to a variable, but I avoided this to
make reviewing the code easier. It is easy to add this change either now
or later and should not affect the generated code except at very low
optimization levels.

Yeah, I think you must mean in build_backup_content(). I noticed that too.


Yes, that is the one I meant.  

I also added fixes manually inside check_publications_origin_tables,
check_publications, and fetch_relation_list. In
check_publication_origin_table three StringInfo was dynamically
allocated but not released after. In check_publications and
fetch_relation_list there were extra cases of using a dynamically
allocated StringInfo that was not necessary.

It's not your fault, but check_publications_origin_tables() looks like
a mess. It seems like excessive use of additional code just to avoid
two separate ereport() calls. Might be worth a follow-up patch to
clean that up.


Yeah, I have noted that too, but I wanted to avoid making reviews more complicated than necessary. I thought it might be better to do as a separate effort.

I'll take a look at it.


The patch looks good to me. The only thing I'd adjust is to get rid of
the "data" variable from build_backup_content(), which I think you
mentioned above. I can take care of that one.

Thanks David.

Best wishes,
Mats Kindahl

--------------qyGUQkJbrBorD5CkZiBl2HWm--