Hi Nathan,
Thanks for your comment.
On Sep 26, 2025, at 22:30, Nathan Bossart <nathandbossart@gmail.com> wrote:
I have mixed feelings about this patch. I have no concrete objections to
the technical content, but some questions come to mind. For example, why
are we only fixing Datum parameters? Are we going to fix other types
later? Also, I'm a bit worried that this will lead to several rounds of
similar patches, which IMHO is arguably unnecessary churn that could lead
to back-patching pain. I personally would be more likely to fix these
sorts of things in passing as part of another patch that touches the same
area of code.
My initial motivation of creating this patch was from when I read the SPI code, for example:
/* Execute a previously prepared plan */
int
SPI_execute_plan(SPIPlanPtr plan, Datum *Values, const char *Nulls,
bool read_only, long tcount)
{
SPIExecuteOptions options;
int res;
This function define Nulls as const, but Values, which led me an impression that Values is an output parameter. But after reading through the function, I realized that Values should also be const. I believe other code readers might get the same confusing.
I always raise comments while reviewing patches when a pointer parameter can be const but not. In the meantime, I wanted to start an effort to cleanup the confusion. Making pointer parameter “const” when possible not only makes the code safer, but also enhances code readability. As definitely we cannot update all code in a single commit, and as “Datum *” is the most typical one, I started with “Datum *”.
As the change only touches function definitions, I guess this patch itself doesn’t need to be back ported. When other patches are back ported to branches, and hitting the change from this patch, adding a few “const” to function parameters won’t be a big overhead.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
