citusdata / citusdata/citus

Improve TRY/CATCH blocks

Open
#3,620 0 comments 0 reactions 0 assignees View on GitHub
bug refactor
Dominant language
C
Stars
12.8k
Forks
794
Avg merge
2d 14h
Merged PRs (30d)
31

Description

We recently had some internal discussions around `TRY/CATCH` blocks. One specific example we've considered was `TryCreateDistributedPlannedStmt `:

https://github.com/citusdata/citus/blob/99c5b0add788e0454b0b59c4339d2747e96e4447/src/backend/distributed/planner/distributed_planner.c#L836-L866

Andres mentioned this e-mail thread on Postgres hackers list: https://www.postgresql.org/message-id/CAPpHfdusVmROP2PepYs2ZV6six9hQ2mzqDpKP9jWX2vi7aSU-Q%40mail.gmail.com

And, Marco noted that: `Generating subtransactions in the Citus planner is probably a bad idea (e.g. we'll send savepoints...). Better to use DeferredError.`

So, for specifically `TryCreateDistributedPlannedStmt` make sure that we use `DeferredErrors` instead of `TRY/CATCH`. The planner has already support for `DeferredErrors` very commonly, there are very few cases the planner throws hard errors. We should go over them and make sure that all errors are deferred, so that we don't need `TRY/CATCH`.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.