gpdb
gpdb copied to clipboard
greenplum-db
Forward complete QE notice messages
The QE notice message might be truncated because MPPnoticeReceiver() limited message lenght about 800 bytes sent to the client. The client should not see an incomplete notice message.
This commit changes its behaviour, it allows QE messages of any length (include QE identifier) to send to the client.
Here are some reminders before you submit the pull request
- [ ] Add tests for the change
- [ ] Document changes
- [ ] Communicate in the mailing list if needed
- [ ] Pass
make installcheck - [ ] Review a PR in return to support the community
What is the problem you're facing, exactly?
We have some functions/extensions elog messages from QE to the client for debug/dump purposes, e.g.
if (client_min_messages == LOG && Gp_role != GP_ROLE_DISPATCH)
{
ereport(NOTICE, (errmsg("..."));
}
The messages may be long enough, > 1000 bytes. But
strncpy(message, pfield->contents, 800);
message[800] = '\0';
It causes truncation, even occasional cut of multi-byte chars.
@dh-cloud I am continue to review your PR. At first glance, it looks good.
Only one concern is why limit 800 bytes, looks it's existed a long time (can be found in gp4 code). Do you think adding a GUC: to control 800 or non-limit bytes here is a good idea? Thanks.
@dh-cloud I am continue to review your PR. At first glance, it looks good.
Only one concern is why limit 800 bytes, looks it's existed a long time (can be found in gp4 code). Do you think adding a GUC: to control 800 or non-limit bytes here is a good idea? Thanks.
@dh-cloud just a kindle reminder, please take a look.
@dh-cloud I am continue to review your PR. At first glance, it looks good. Only one concern is why limit 800 bytes, looks it's existed a long time (can be found in gp4 code). Do you think adding a GUC: to control 800 or non-limit bytes here is a good idea? Thanks.
@dh-cloud just a kindle reminder, please take a look.
Sorry, the office external network is gone recently, i did not see github for a long time. This pr has two aim: 1) show full log message. 2) prevent multi-byte chars truncation. Is a new GUC really necessary?
Got it, thanks @dh-cloud Considering it's only for master branch, it's ok to remove the 800 bytes limitation (without introduce a trivial GUC). I will verfiy this PR in my test env and give response later.
@dh-cloud I have verified your PR in my env, it works and looks good, thanks for your contributions. Considering this PR has been open for a long time, please rebase the latest master (then trigger the pr-pipeline test again). If all is ok, this PR will be merged soon.
Thanks!