Robert Haas wrote:
> On Wed, Mar 18, 2015 at 7:10 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> >> + /*
> >> + * For now, parallel operations are required to be strictly read-only.
> >> + * Unlike heap_update() and heap_delete(), an insert should never create
> >> + * a combo CID, so it might be possible to relax this restrction, but
> >> + * not without more thought and testing.
> >> + */
> >> + if (IsInParallelMode())
> >> + ereport(ERROR,
> >> + (errcode(ERRCODE_INVALID_TRANSACTION_STATE),
> >> + errmsg("cannot insert tuples during a parallel operation")));
> >> +
> >
> > Minor nitpick: Should we perhaps move the ereport to a separate
> > function? Akin to PreventTransactionChain()? Seems a bit nicer to not
> > have the same message everywhere.
>
> Well, it's not actually the same message. They're all a bit
> different. Or mostly all of them. And the variable part is not a
> command name, as in the PreventTransactionChain() case, so it would
> affect translatability if we did that.
Three things that vary are 1) the fact that some check IsParallelWorker()
and others check IsInParallelMode(), and 2) that some of them are
ereports() while others are elog(), and 3) that some are ERROR and
others are FATAL. Is there a reason for these differences?
(Note typo "restrction" in quoted paragraph above.)
Maybe something similar to what commit f88d4cfc did: have a function
where all possible messages are together, and one is selected with some
enum. That way, it's easier to maintain consistency if more cases are
added in the future.
> I don't actually think this is a big deal.
Yeah.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
