Thread: 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
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
Albe Laurenz <laurenz.albe@wien.gv.at> 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
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
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
Tomas Vondra <tv@fuzzy.cz> 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