Hi,
On Monday, February 4, 2019 2:15 AM +0000, Michael Paquier wrote:
> On Tue, Dec 04, 2018 at 04:07:34AM +0000, Ideriha, Takeshi wrote:
> > Sure. I didn't have a strong opinion about it, so it's ok.
> From what I can see this is waiting input from a native English speaker, so for now I have moved this entry to next
CF.
Although I also use English in daily life,
I am not from a native English-speaking country.
But I'm giving it a try, since the current patch is not that complex to check.
> HINT: In a moment you should be able to reconnect to the
> database and restart your transaction.
I think this error hint message makes sense for idle-in-transaction timeout.
It's grammatically correct and gives a clearer hint for that.
I agree that "reconnect to database" implies "restart session",
so it may not be the best word for errhint message.
Now the next point raised by Ideriha-san is whether the
other places in the code should also be changed.
I also checked the source code where the other errhints appear,
besides idle-in-transaction.
(ERRCODE_CRASH_SHUTDOWN and ERRCODE_T_R_SERIALIZATION_FAILURE)
Those errcodes use "repeat your command" for the hint.
> errhint("In a moment you should be able to reconnect to the"
> " database and repeat your command.")));
(1) (errcode(ERRCODE_CRASH_SHUTDOWN)
As per its errdetail, the transaction is rollbacked,
so "restart transaction" also makes sense.
(2) ERRCODE_T_R_SERIALIZATION_FAILURE
The second one is under the clause below...
> if (RecoveryConflictPending && DoingCommandRead)
...so using "repeat your command" makes sense.
I'm fine with retaining the other two hint messages as they are.
Regardless if we want to change the other similarly written
errhint messages or add errhint messages for other errcodes
that don't have it, I think the latest patch that provides errhint
message for idle-in-transaction timeout may be changed to
"Ready for Committer", as it still applies and the tests
successfully passed.
Regards,
Kirk Jamison