On 2020-Mar-16, Amit Kapila wrote: > 2. > + /* Setup error traceback support for ereport() */ > + update_vacuum_error_cbarg(vacrelstats, VACUUM_ERRCB_PHASE_SCAN_HEAP, > + InvalidBlockNumber, NULL); > + errcallback.callback = vacuum_error_callback; > + errcallback.arg = vacrelstats; > + errcallback.previous = error_context_stack; > + error_context_stack = &errcallback; > .. > .. > + /* Init vacrelstats for use as error callback by parallel worker: */ > + vacrelstats.relnamespace = get_namespace_name(RelationGetNamespace(onerel)); > + vacrelstats.relname = pstrdup(RelationGetRelationName(onerel)); > + vacrelstats.indname = NULL; > + vacrelstats.phase = VACUUM_ERRCB_PHASE_UNKNOWN; /* Not yet processing */ > + > + /* Setup error traceback support for ereport() */ > + errcallback.callback = vacuum_error_callback; > + errcallback.arg = &vacrelstats; > + errcallback.previous = error_context_stack; > + error_context_stack = &errcallback; > + > > I think the code can be bit simplified if we have a function > setup_vacuum_error_ctx which takes necessary parameters and fill the > required vacrelstats params, setup errcallback. Then we can use > update_vacuum_error_cbarg at required places.
Heh, he had that and I took it away -- it looked unnatural. I thought changing error_context_stack inside such a function, then resetting it back to "previous" outside the function, was too leaky an abstraction. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
