Hi Pavel,
Hi
Hi Daniel,
Thank you for your follow up, and attaching a new patch which addresses Pavel's comments.
Let me know If I miss anything here.
> On 9 Sep 2021, at 08:23, Dinesh Chemuduru <dinesh.kumar@migops.com> wrote:
> Let me try to fix the suggested case(I tried to fix this case in the past, but this time I will try to spend more time on this), and will submit a new patch.
This CF entry is marked Waiting on Author, have you had the chance to prepare a
new version of the patch addressing the comments from Pavel?
I think the functionality is correct. I am not sure about implementation
Thank you for your time in validating this patch.
1. Is it necessary to introduce a new field "current_query"? Cannot be used "internalquery" instead? Introducing a new field opens some questions - what is difference between internalquery and current_query, and where and when have to be used first and when second? ErrorData is a fundamental generic structure for Postgres, and can be confusing to enhance it by one field just for one purpose, that is not used across Postgres.
Internalquery is the one, which was generated by another command.
For example, "DROP <OBJECT> CASCADE"(current_query) will generate many internal query statements for each of the dependent objects.
At this moment, we do save the current_query in PG_CONTEXT.
As we know, PG_CONTEXT returns the whole statements as stacktrace and it's hard to fetch the required SQL from it.
I failed to see another way to access the current_query apart from the PG_CONTEXT.
Hence, we have introduced the new member "current_query" to the "ErrorData" object.
We tested the internalquery for this purpose, but there are few(10+ unit test) test cases that failed.
This is also another reason we added this new member to the "ErrorData", and with the current patch all test cases are successful,
and we are not disturbing the existing functionality.
2. The name set_current_err_query is not consistent with names in elog.c - probably something like errquery or set_errquery or set_errcurrent_query can be more consistent with other names.
Updated as per your suggestion
Please find the new patch version.