leborchuk commented on PR #1744: URL: https://github.com/apache/cloudberry/pull/1744#issuecomment-4458061490
> Thanks for the detailed explanation. I agree that, given the existing sizing rule: > > ``` > norm_query_buflen = query_len + jstate->clocations_count * 10 > ``` > > and the invariant that each constant contributes at least one byte, while each generated placeholder is bounded to at most 11 bytes, the original overflow claim is not supported. I’ll withdraw the “critical security vulnerability” framing. > > The `snprintf` change is only a defensive hardening/style change, not a demonstrated bug fix. I also see your point that this may conflict with the upstream PostgreSQL pg_stat_statements normalisation changes, so this PR probably is not the right vehicle. > > I’m happy to close this, or retitle it as a very small defensive cleanup if the project would still value replacing `sprintf` with bounded formatting. Otherwise, I’ll defer to the upstream PostgreSQL normalisation work. I will suggest sending a fix to the postgres community. I searched the pgsql-hackers mailing list and did not find any patches for using snprintf in pg_stats_statements. It would be really useful to discuss the issue with the postgres community as well. Also, we are based on the postgres kernel and constantly update it (since postgres is constantly developing). Therefore, the main way to fix something in the kernel code is to get a fix from postgres's code (most likely it has already been fixed), or fix something in postgres and then get a patch back. Otherwise, it is quite easy to forget to add a patch during the rebasing process. Sometimes, an issue cannot be fixed in postgres. Well, that's why our project exists. Let's do it in Cloudberry. But it seems to me that your patch is more important for postgres than for Cloudberry. Let us fix it in postgres! And sooner or later, it will be fixed in all projects based on postgres. If you suddenly have any problems with the patch review, let us know, some of the project participants are also active contributors to postgres. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
