Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return error code instead of throwing error in plan_tree_mutator #2

Open
zhangh43 opened this issue Dec 9, 2019 · 0 comments
Open

Comments

@zhangh43
Copy link
Owner

zhangh43 commented Dec 9, 2019

Throwing errors, i.e. elog(ERROR,...), is not safe in Postgres and may lead to resource leak.
Better to return error code in plan_tree_mutator and propagate this error upstairs

Konstantin wrote:
5. Throwing and catching exception for queries which can not be vectorized seems to be not the safest and most efficient way of handling such cases.
May be it is better to return error code in plan_tree_mutator and propagate this error upstairs? 
 
Hubert wrote:
Yes, as for efficiency, another way is to enable some plan node to be vectorized and leave other nodes not vectorized and add batch/unbatch layer between them(Is this what you said "propagate this error upstairs"). As you mentioned, this could introduce additional overhead. Is there any other good approaches?
What do you mean by not safest?
PG catch will receive the ERROR, and fallback to the original non-vectorized plan.

Konstantin wrote:
The problem with catching and ignoring exception was many times discussed in hackers.
Unfortunately Postgres PG_TRY/PG_CATCH mechanism is not analog of exception mechanism in more high level languages, like C++, Java...
It doesn't perform stack unwind. If some resources (files, locks, memory,...) were obtained before throwing error, then them are not reclaimed.
Only rollback of transaction is guaranteed to release all resources. And it actually happen in case of normal error processing.
But if you catch and ignore exception , trying to continue execution, then it can cause many problems.

May be in your case it is not a problem, because you know for sure where error can happen: it is thrown by plan_tree_mutator
and looks like there are no resources obtained by this function.  But in any case overhead of setjmp is much higher than of explicit checks of return code.
So checking return codes will not actually add some noticeable overhead except code complication by adding extra checks.
But in can be hidden in macros which are used in any case (like MUTATE).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant