Re: [HACKERS] writable FDWs / update targets confusion
Tomas Vondra writes: > I think that we should make the documentation more explicit about this > limitation, because the current wording in fdw-callbacks documentation > seems to suggest it's possible to add such hidden columns. At least > that's how I read it before running into this. You can add hidden columns if you've got 'em ;-). What's missing is the ability to create any hidden columns other than the ones in standard PG tables. What we most likely need is the ability for an FDW to override the type assigned to the CTID column at foreign table creation. (We'd then also need to think about where such a column could be shoehorned into a tuple, but the catalog support has to come first.) Alternatively, it might work to append "junk" columns in the user column numbering domain, which would only exist in runtime tuple descriptors and not in the catalogs. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] writable FDWs / update targets confusion
On 18.11.2013 09:28, Albe Laurenz wrote: > Tom Lane wrote: >>> Tom, could you show us a rope if there is one? >> >> What is it you actually need to fetch? >> >> IIRC, the idea was that most FDWs would do the equivalent of fetching the >> primary-key columns to use in an update. If that's what you need, then >> AddForeignUpdateTargets should identify those columns and generate Vars >> for them. postgres_fdw is probably not a good model since it's using >> ctid (a non-portable thing) and piggybacking on the existence of a tuple >> header field to put that in. >> >> If you're dealing with some sort of hidden tuple identity column that >> works like CTID but doesn't fit in six bytes, there may not be any good >> solution in the current state of the FDW support. As I mentioned, we'd >> batted around the idea of letting FDWs define a system column with some >> other datatype besides TID, but we'd not figured out all the nitty >> gritty details in time for 9.3. > > I was hoping for the latter (a hidden column). > > But I'll have to settle for primary keys, which is also ok. I was hoping for the latter too, and I can't really switch to primary key columns. I can live with 6B passed in the ctid field for now, but it'd be nice to be able to use at least 8B. I think that we should make the documentation more explicit about this limitation, because the current wording in fdw-callbacks documentation seems to suggest it's possible to add such hidden columns. At least that's how I read it before running into this. regards Tomas -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] writable FDWs / update targets confusion
Tom Lane wrote: >> Tom, could you show us a rope if there is one? > > What is it you actually need to fetch? > > IIRC, the idea was that most FDWs would do the equivalent of fetching the > primary-key columns to use in an update. If that's what you need, then > AddForeignUpdateTargets should identify those columns and generate Vars > for them. postgres_fdw is probably not a good model since it's using > ctid (a non-portable thing) and piggybacking on the existence of a tuple > header field to put that in. > > If you're dealing with some sort of hidden tuple identity column that > works like CTID but doesn't fit in six bytes, there may not be any good > solution in the current state of the FDW support. As I mentioned, we'd > batted around the idea of letting FDWs define a system column with some > other datatype besides TID, but we'd not figured out all the nitty > gritty details in time for 9.3. I was hoping for the latter (a hidden column). But I'll have to settle for primary keys, which is also ok. Thanks for taking the time. Yours, Laurenz Albe -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] writable FDWs / update targets confusion
Albe Laurenz writes: > Tom, could you show us a rope if there is one? What is it you actually need to fetch? IIRC, the idea was that most FDWs would do the equivalent of fetching the primary-key columns to use in an update. If that's what you need, then AddForeignUpdateTargets should identify those columns and generate Vars for them. postgres_fdw is probably not a good model since it's using ctid (a non-portable thing) and piggybacking on the existence of a tuple header field to put that in. If you're dealing with some sort of hidden tuple identity column that works like CTID but doesn't fit in six bytes, there may not be any good solution in the current state of the FDW support. As I mentioned, we'd batted around the idea of letting FDWs define a system column with some other datatype besides TID, but we'd not figured out all the nitty gritty details in time for 9.3. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] writable FDWs / update targets confusion
Tomas Vondra wrote: > I'm working on adding write support to one of my FDWs. Adding INSERT went > pretty fine, but when adding DELETE/UPDATE I got really confused about how > the update targets are supposed to work. > > My understanding of how it's supposed to work is this: > > (1) AddForeignUpdateTargets adds columns that serve as ID of the record > (e.g. postgres_fdw adds 'ctid') > > (2) planning the inner foreign scan handles the new column appropriately > (e.g. scans system columns, as in case of 'ctid' etc.) > > (3) IterateForeignScan will see the column in the tuple descriptor, will > set it just like any other column, etc. > > (4) ExecForeignDelete will fetch the new column and do something with it > > However no matter what I do, I can't get the steps (3) and (4) working this > way. I have no idea either. > And looking at postgres_fdw it seems to me it does not really set the ctid > into the tuple as a column, but just does this: > > if (ctid) > tuple->t_self = *ctid; > > which I can't really do because I need to use INT8 and not TID. But even > if I do this, What exactly did you do? Did you try tuple->t_self = myInt8; That would write 8 bytes into a 6-byte variable, thus scribbling past the end, right? > Interestingly, if I do this in ExecForeignDelete > > static TupleTableSlot * > myExecForeignDelete(EState *estate, > ResultRelInfo *resultRelInfo, > TupleTableSlot *slot, > TupleTableSlot *planSlot) > { > > bool isNull; > MyModifyState state = (MyModifyState)resultRelInfo->ri_FdwState; > int64 ctid; > > Datum datum = ExecGetJunkAttribute(planSlot, >state->ctidAttno, &isNull); > > ctid = DatumGetInt64(datum); > > elog(WARNING, "ID = %ld", ctid); > > if (isNull) > elog(ERROR, "ctid is NULL"); > > /* FIXME not yet implemented */ > return NULL; > } > > I do get (isNull=FALSE) but the ctid evaluates to some random number, e.g. > > WARNING: ID = 44384788 (44384788) > WARNING: ID = 44392980 (44392980) > > and so on. Maybe that's the effect of writing past the end of the variable. > So what did I get wrong? Is it possible to use arbitrary hidden column as > "junk" columns (documentation seems to suggest that)? What is the right > way to do that / whad did I get wrong? I would like to know an answer to this as well. I don't think that assigning to tuple->t_self will work, and I know too little about the executor to know if there's any way to fit a ctid that is *not* an ItemPointerData into a TupleTableSlot so that it will show up as resjunk TargerEntry in ExecForeignUpdate. Tom, could you show us a rope if there is one? Yours, Laurenz Albe -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] writable FDWs / update targets confusion
Hi, I'm working on adding write support to one of my FDWs. Adding INSERT went pretty fine, but when adding DELETE/UPDATE I got really confused about how the update targets are supposed to work. My understanding of how it's supposed to work is this: (1) AddForeignUpdateTargets adds columns that serve as ID of the record (e.g. postgres_fdw adds 'ctid') (2) planning the inner foreign scan handles the new column appropriately (e.g. scans system columns, as in case of 'ctid' etc.) (3) IterateForeignScan will see the column in the tuple descriptor, will set it just like any other column, etc. (4) ExecForeignDelete will fetch the new column and do something with it However no matter what I do, I can't get the steps (3) and (4) working this way. This is what I do in AddForeignUpdateTargets (pretty much a 1:1 copy from postgres_fdw, except that I'm using INT8OID instead of TIDOID): static void myAddForeignUpdateTargets(Query *parsetree, RangeTblEntry *target_rte, Relation target_relation) { Var *var; const char *attrname; TargetEntry *tle; /* Make a Var representing the desired value */ var = makeVar(parsetree->resultRelation, SelfItemPointerAttributeNumber, INT8OID, -1, InvalidOid, 0); /* Wrap it in a resjunk TLE with the right name ... */ attrname = "ctid"; tle = makeTargetEntry((Expr *) var, list_length(parsetree->targetList) + 1, pstrdup(attrname), true); /* ... and add it to the query's targetlist */ parsetree->targetList = lappend(parsetree->targetList, tle); } Then in GetForeignPlan I collect all the attnums (including the new one), and in BeginForeignScan decide which columns to actually fetch. So if any attnum is (attnum < 0) I know I need to fetch the new 'ctid' column. However in IterateForeignScan, the tuple descriptor does not change. It still has only the columns of the foreign table, so I have no place to set the ctid to. So even though ExecFindJunkAttributeInTlist(subplan->targetlist, "ctid"); return "1" I can't really set any of the column to the CTID, because the column might be used in a WHERE condition. And looking at postgres_fdw it seems to me it does not really set the ctid into the tuple as a column, but just does this: if (ctid) tuple->t_self = *ctid; which I can't really do because I need to use INT8 and not TID. But even if I do this, Interestingly, if I do this in ExecForeignDelete static TupleTableSlot * myExecForeignDelete(EState *estate, ResultRelInfo *resultRelInfo, TupleTableSlot *slot, TupleTableSlot *planSlot) { bool isNull; MyModifyState state = (MyModifyState)resultRelInfo->ri_FdwState; int64 ctid; Datum datum = ExecGetJunkAttribute(planSlot, state->ctidAttno, &isNull); ctid = DatumGetInt64(datum); elog(WARNING, "ID = %ld", ctid); if (isNull) elog(ERROR, "ctid is NULL"); /* FIXME not yet implemented */ return NULL; } I do get (isNull=FALSE) but the ctid evaluates to some random number, e.g. WARNING: ID = 44384788 (44384788) WARNING: ID = 44392980 (44392980) and so on. So what did I get wrong? Is it possible to use arbitrary hidden column as "junk" columns (documentation seems to suggest that)? What is the right way to do that / whad did I get wrong? regards Tomas -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers