Hi, On 2015-08-27 18:59:13 -0400, Tom Lane wrote: > But I do think that letting custom-scan extensions fool about with > the semantics of plan-copying (and plan-serialization for that matter) > is a risky choice that is not justified by some marginal gains in code > readability
To me the likelihood of having bugs in manual (de)serilization via lists et al. is higher than introducing bugs via copyfuncs. Or even out/readfuncs, especially as we could easily add a bunch of verifications to the latter by verifying field names in READ_*. Wether a prepared statement fails because a copyfuncs hook failed, or whether it's in the fdw itself doesn't seem to make that much of a difference. > especially when there are other feasible ways to attack the > readability problem. Lets at least add a Value variant that can store strings that aren't null terminated, that'd already make things a lot better. Right now you need to reuse Const nodes and stuff a bytea in there or such. But even then, that only works if you have a single flat datastructure that doesn't have any pointers in it. copyObject() style copying can easily cope with that, using a binary blob really can't. As far as I can see you right now basically need to write code for custom scans so that between planning and execution you unconditionally do: Planning: 1) create your internal data 2) serialize data, Execution: 3) unserialize data 4) use unserialized data There's no way to stash data between 2 and 3 anywhere, even if there's no need for the whole copying rigamarole because it was just a plain statement. If you look at the various *Scan, *Join etc. nodes we have in core, most of them have a bunch of variable length data structures and pointers in them. All that you can't sanely do for a custom scan node. > copyObject needs to be a pretty self-contained operation with not a > lot of external dependencies Why? > > [ ruminations about how to improve the system's extensibility ] > > Yeah, I've entertained similar thoughts in the past. It's not very clear > how we get there from here, though --- for instance, any fundamental > change in the Node system would break so much code that it's unlikely > anyone would thank us for it, even assuming we ever finished the > conversion project. How about adding a single 'ExtensibleNode' node type (inheriting from Node as normal). Many of the switches over node types would gain one additional entry for that, and do something like case T_ExtensibleNode: handler = LookupHandler((ExtensibleNode *) node)->extended_type, Handler_ExecInitNode); if (handler) { result = ((ExecInitNodeHandler *) handler)(node, estate, eflags); break; } /* fall through to error */ default: elog(ERROR, "unrecognized node type: %d", (int) nodeTag(node)); that should be doable one-by-one in many of these bigger switches. There's obviously some limits where that's doable, but I don't think it'd break much code? How exactly to identify extended_type in a way that makes sense e.g. between restarting pg, backends, primary/standby isn't trivial, but I do think it should be doable. > I'm also less than convinced that "I should be able to install major new > features without touching the core code" is actually a sane or useful goal > to have. Yea, I have some doubts there as well. There's a lot of features though which we really don't want in core, where some of the limitations of the node system really do become problematic. > As a concrete example, you mentioned the lack of extensibility of the > bison parser, which is undoubtedly a blocking issue if you want a new > feature that would require new SQL syntax. While that's a problem, where I really don't have any good answer, I also think in many cases it's primarily DDL, and primarily extending existing DDL. By far the most things I've seen want to add informations to tables and/or columns - and storage options actually kinda give you that on the grammar level. We just don't allow you too sanely hook into it, and there's a bunch of other pointless restrictions. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers