On Fri, Oct 24, 2014 at 4:46 PM, Jim Nasby <jim.na...@bluetreble.com> wrote: > On 10/24/14, 12:21 PM, Robert Haas wrote: >> - What should we call dsm_unkeep_mapping, if not that? > > Only option I can think of beyond unkeep would be > dsm_(un)register_keep_mapping. Dunno that it's worth it.
Hmm, we could rename dsm_keep_mapping() to dsm_unregister_mapping(), since it's arranging to keep it by unregistering it from the resource owner. And then we could call the new function dsm_register_mapping(). That has the appeal that "unregister" is a word, whereas "unkeep" isn't, but it's a little confusing otherwise, because the sense is reversed vs. the current naming. Or we could just leave dsm_keep_mapping() alone and call the new function dsm_register_mapping(). A little non-orthogonal, but I think it'd be OK. >> - Does anyone have a tangible suggestion for how to reduce the code >> duplication in patch #6? > > Between execute_sql_string() and tcop/exec_simple_query()? Is there stuff in > exec_simple that's not safe for bgwriter? I'm not seeing why we can't use > exec_simple. :/ There are some differences if you compare them closely. > BTW, it does occur to me that we could do something to combine > AllocSetContextCreate() followed by oldcontext=MemoryContextSwitchTo(). Meh. > pg_background_result() > + dsm_unkeep_mapping(info->seg); > + > + /* Set up tuple-descriptor based on colum definition list. > */ > + if (get_call_result_type(fcinfo, NULL, &tupdesc) != > TYPEFUNC_COMPOSITE) > + ereport(ERROR, > Is there a reason we can't check the result type before unkeeping? Seems > silly to throw the results away just because someone flubbed a function > call... Hmm, yeah, I see no reason why we couldn't move that up higher in the function. It's probably a pretty common failure mode, too, so I can see the convenience factor there. > + default: > + elog(WARNING, "unknown message type: %c (%zu > bytes)", > + msg.data[0], nbytes); > It'd be useful to also provide DEBUG output with the message itself... I think that's more work than is justified. We'd have to hexdump it or something because it might not be valid in the client encoding, and it's a case that shouldn't happen anyway. (Hmm, that reminds me that I haven't thought of a solution to the problem that the background worker might try to SET client_encoding, which would be bad. Not sure what the best fix for that is, off-hand. I'd rather not prohibit ALL GUC changes in the background worker, as there's no good reason for such a draconian restriction.) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
