From: | "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com> |
---|---|
To: | PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Subject: | Add null termination to string received in parallel apply worker |
Date: | 2023-10-11 06:48:44 |
Message-ID: | OS0PR01MB5716FAB36CB02B0A4C58F5C594CCA@OS0PR01MB5716.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
The parallel apply worker didn't add null termination to the string received
from the leader apply worker via the shared memory queue. This action doesn't
bring bugs as it's binary data but violates the rule established in StringInfo,
which guarantees the presence of a terminating '\0' at the end of the string.
Although the original string in leader already includes the null termination,
but we cannot just send all string including the null termination to the
parallel worker, because that would increase the length while at the receiver
there is still no null termination at data[length] which actually the length +
1 position.
And we also cannot directly modify the received string like data[len] = '\0',
because the data still points a shared buffer maintained by shared memory
queue, so we'd better not modify the data outside of the string length.
So, here is patch to use the standard StringInfo API to store the string while
ensuring the addition of null termination. This can also resolve the Assert
issue raised by another patch[1] currently under discussion.
Thanks to Amit for offlist discussion regarding the analysis and
fix.
Best Regards,
Hou Zhijie
Attachment | Content-Type | Size |
---|---|---|
0001-Add-null-termination-to-string-received-in-parallel-.patch | application/octet-stream | 2.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2023-10-11 06:51:27 | Re: Pre-proposal: unicode normalized text |
Previous Message | Drouvot, Bertrand | 2023-10-11 06:48:04 | Re: Add a new BGWORKER_BYPASS_ROLELOGINCHECK flag |