FDW system columns

Started by Thom Brownabout 14 years ago17 messages
#1Thom Brown
thom@linux.com

I notice that there's some weird info coming out of the system columns
on any FDW:

test=# select tableoid, ctid, xmin, xmax, cmin, cmax, * from dict limit 12;
tableoid | ctid | xmin | xmax | cmin | cmax | words
----------+----------------+------+------------+-------+-------+-----------
16428 | (4294967295,0) | 104 | 4294967295 | 16430 | 16430 | A
16428 | (4294967295,0) | 104 | 4294967295 | 16430 | 16430 | a
16428 | (4294967295,0) | 108 | 4294967295 | 16430 | 16430 | aa
16428 | (4294967295,0) | 112 | 4294967295 | 16430 | 16430 | aal
16428 | (4294967295,0) | 120 | 4294967295 | 16430 | 16430 | aalii
16428 | (4294967295,0) | 112 | 4294967295 | 16430 | 16430 | aam
16428 | (4294967295,0) | 116 | 4294967295 | 16430 | 16430 | Aani
16428 | (4294967295,0) | 132 | 4294967295 | 16430 | 16430 | aardvark
16428 | (4294967295,0) | 132 | 4294967295 | 16430 | 16430 | aardwolf
16428 | (4294967295,0) | 120 | 4294967295 | 16430 | 16430 | Aaron
16428 | (4294967295,0) | 128 | 4294967295 | 16430 | 16430 | Aaronic
16428 | (4294967295,0) | 136 | 4294967295 | 16430 | 16430 | Aaronical
(12 rows)

That's file_fdw. On the not-yet-ready pgsql_fdw:

test=# select tableoid, ctid, xmin, xmax, cmin, cmax from cows limit 5;
tableoid | ctid | xmin | xmax | cmin | cmax
----------+----------------+------+------+------+------
16406 | (4294967295,0) | 0 | 0 | 0 | 0
16406 | (4294967295,0) | 0 | 0 | 0 | 0
16406 | (4294967295,0) | 0 | 0 | 0 | 0
16406 | (4294967295,0) | 0 | 0 | 0 | 0
16406 | (4294967295,0) | 0 | 0 | 0 | 0
(5 rows)

So the ctid is always 2^32-1. Bit weird, but probably explainable.
But xmin on the file_fdw result is odd. Why are these all over the
place?

--
Thom Brown
Twitter: @darkixion
IRC (freenode): dark_ixion
Registered Linux user: #516935

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Thom Brown (#1)
Re: FDW system columns

Thom Brown <thom@linux.com> writes:

So the ctid is always 2^32-1. Bit weird, but probably explainable.

See ItemPointerSetInvalid.

But xmin on the file_fdw result is odd. Why are these all over the
place?

heap_form_tuple initializes the t_choice fields as though for a tuple
Datum, and file_fdw doesn't change it.

Just a couple hours ago I was wondering why we create system columns for
foreign tables at all. Is there a reasonable prospect that they'll ever
be useful? I can see potential value in tableoid, but the others seem
pretty dubious --- even if you were fetching from a remote PG server,
the XIDs would not be meaningful within our own environment.

regards, tom lane

#3Thom Brown
thom@linux.com
In reply to: Tom Lane (#2)
Re: FDW system columns

On 13 November 2011 00:38, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Thom Brown <thom@linux.com> writes:

But xmin on the file_fdw result is odd.  Why are these all over the
place?

heap_form_tuple initializes the t_choice fields as though for a tuple
Datum, and file_fdw doesn't change it.

Just a couple hours ago I was wondering why we create system columns for
foreign tables at all.  Is there a reasonable prospect that they'll ever
be useful?  I can see potential value in tableoid, but the others seem
pretty dubious --- even if you were fetching from a remote PG server,
the XIDs would not be meaningful within our own environment.

Yes, that's what I was thinking when curiosity led me to have a look
at what they contain. As far as I see, they serve no useful function.
I didn't bother looking at tableoid as that's generally useful.

Is there a cost to having them there? Could there be tools that might
break if the columns were no longer available?

--
Thom Brown
Twitter: @darkixion
IRC (freenode): dark_ixion
Registered Linux user: #516935

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#4Peter Eisentraut
peter_e@gmx.net
In reply to: Thom Brown (#3)
Re: FDW system columns

On sön, 2011-11-13 at 00:58 +0000, Thom Brown wrote:

Is there a cost to having them there? Could there be tools that might
break if the columns were no longer available?

Doubtful. Views don't have system columns either.

#5Florian Pflug
fgp@phlo.org
In reply to: Tom Lane (#2)
Re: FDW system columns

On Nov13, 2011, at 01:38 , Tom Lane wrote:

Just a couple hours ago I was wondering why we create system columns for
foreign tables at all. Is there a reasonable prospect that they'll ever
be useful? I can see potential value in tableoid, but the others seem
pretty dubious --- even if you were fetching from a remote PG server,
the XIDs would not be meaningful within our own environment.

At least ctid seems useful too. I've used that in the past as a poor man's
surrogate primary key.

Also, people have used ctid and xmin in the past to re-find previously
visited rows and to check whether they've been modified. So there might be
some value in keeping xmin around also (and make the postgres fdw populate it)

best regards,
Florian Pflug

#6Robert Haas
robertmhaas@gmail.com
In reply to: Florian Pflug (#5)
Re: FDW system columns

On Sun, Nov 13, 2011 at 6:57 PM, Florian Pflug <fgp@phlo.org> wrote:

On Nov13, 2011, at 01:38 , Tom Lane wrote:

Just a couple hours ago I was wondering why we create system columns for
foreign tables at all.  Is there a reasonable prospect that they'll ever
be useful?  I can see potential value in tableoid, but the others seem
pretty dubious --- even if you were fetching from a remote PG server,
the XIDs would not be meaningful within our own environment.

At least ctid seems useful too. I've used that in the past as a poor man's
surrogate primary key.

Also, people have used ctid and xmin in the past to re-find previously
visited rows and to check whether they've been modified. So there might be
some value in keeping xmin around also (and make the postgres fdw populate it)

My vote is to nuke 'em all. :-)

I don't think that we want to encourage people to depend on the
existence of system columns any more than they do already.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#7Shigeru Hanada
shigeru.hanada@gmail.com
In reply to: Robert Haas (#6)
1 attachment(s)
Re: FDW system columns

(2011/11/14 11:25), Robert Haas wrote:

My vote is to nuke 'em all. :-)

+1.

IIRC, main purpose of supporting tableoid for foreign tables was to be
basis of foreign table inheritance, which was not included in 9.1, and
we have not supported it yet. Other system columns are essentially
garbage, but they survived at 9.1 development because (maybe) it seemed
little odd to have system columns partially at that time.

So, IMHO removing all system columns from foreign tables seems
reasonable, unless it doesn't break any external tool seriously (Perhaps
there would be few tools which assume that foreign tables have system
columns).

If there seems to be a consensus on removing system column from foreign
tables, I'd like to work on this issue. Attached is a halfway patch,
and ISTM there is no problem so far.

Regards,
--
Shigeru Hanada

Attachments:

remove_syscol_support.patchtext/plain; name=remove_syscol_support.patchDownload
diff --git a/contrib/file_fdw/input/file_fdw.source b/contrib/file_fdw/input/file_fdw.source
index 8e3d553..8ddeb17 100644
*** a/contrib/file_fdw/input/file_fdw.source
--- b/contrib/file_fdw/input/file_fdw.source
*************** EXECUTE st(100);
*** 111,119 ****
  EXECUTE st(100);
  DEALLOCATE st;
  
- -- tableoid
- SELECT tableoid::regclass, b FROM agg_csv;
- 
  -- updates aren't supported
  INSERT INTO agg_csv VALUES(1,2.0);
  UPDATE agg_csv SET a = 1;
--- 111,116 ----
diff --git a/contrib/file_fdw/output/file_fdw.source b/contrib/file_fdw/output/file_fdw.source
index 84f0750..adf03c5 100644
*** a/contrib/file_fdw/output/file_fdw.source
--- b/contrib/file_fdw/output/file_fdw.source
*************** EXECUTE st(100);
*** 174,188 ****
  (1 row)
  
  DEALLOCATE st;
- -- tableoid
- SELECT tableoid::regclass, b FROM agg_csv;
-  tableoid |    b    
- ----------+---------
-  agg_csv  |  99.097
-  agg_csv  | 0.09561
-  agg_csv  |  324.78
- (3 rows)
- 
  -- updates aren't supported
  INSERT INTO agg_csv VALUES(1,2.0);
  ERROR:  cannot change foreign table "agg_csv"
--- 174,179 ----
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index e11d896..33f91d8 100644
*** a/src/backend/catalog/heap.c
--- b/src/backend/catalog/heap.c
*************** CheckAttributeNamesTypes(TupleDesc tupde
*** 399,408 ****
  	/*
  	 * first check for collision with system attribute names
  	 *
! 	 * Skip this for a view or type relation, since those don't have system
! 	 * attributes.
  	 */
! 	if (relkind != RELKIND_VIEW && relkind != RELKIND_COMPOSITE_TYPE)
  	{
  		for (i = 0; i < natts; i++)
  		{
--- 399,409 ----
  	/*
  	 * first check for collision with system attribute names
  	 *
! 	 * Skip this for a view or type relation or foreign table, since those
! 	 * don't have system attributes.
  	 */
! 	if (relkind != RELKIND_VIEW && relkind != RELKIND_COMPOSITE_TYPE &&
! 		relkind != RELKIND_FOREIGN_TABLE)
  	{
  		for (i = 0; i < natts; i++)
  		{
*************** AddNewAttributeTuples(Oid new_rel_oid,
*** 695,704 ****
  
  	/*
  	 * Next we add the system attributes.  Skip OID if rel has no OIDs. Skip
! 	 * all for a view or type relation.  We don't bother with making datatype
! 	 * dependencies here, since presumably all these types are pinned.
  	 */
! 	if (relkind != RELKIND_VIEW && relkind != RELKIND_COMPOSITE_TYPE)
  	{
  		for (i = 0; i < (int) lengthof(SysAtt); i++)
  		{
--- 696,707 ----
  
  	/*
  	 * Next we add the system attributes.  Skip OID if rel has no OIDs. Skip
! 	 * all for a view or type relation or foreign table.  We don't bother with
! 	 * making datatype dependencies here, since presumably all these types are
! 	 * pinned.
  	 */
! 	if (relkind != RELKIND_VIEW && relkind != RELKIND_COMPOSITE_TYPE &&
! 		relkind != RELKIND_FOREIGN_TABLE)
  	{
  		for (i = 0; i < (int) lengthof(SysAtt); i++)
  		{
diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c
index 841ae69..f7b8393 100644
*** a/src/backend/executor/nodeForeignscan.c
--- b/src/backend/executor/nodeForeignscan.c
*************** ForeignNext(ForeignScanState *node)
*** 51,67 ****
  	MemoryContextSwitchTo(oldcontext);
  
  	/*
! 	 * If any system columns are requested, we have to force the tuple into
! 	 * physical-tuple form to avoid "cannot extract system attribute from
! 	 * virtual tuple" errors later.  We also insert a valid value for
! 	 * tableoid, which is the only actually-useful system column.
  	 */
- 	if (plan->fsSystemCol && !TupIsNull(slot))
- 	{
- 		HeapTuple	tup = ExecMaterializeSlot(slot);
- 
- 		tup->t_tableOid = RelationGetRelid(node->ss.ss_currentRelation);
- 	}
  
  	return slot;
  }
--- 51,62 ----
  	MemoryContextSwitchTo(oldcontext);
  
  	/*
! 	 * XXX If we support system columns and any of them are requested, we have
! 	 * to force the tuple into physical-tuple form to avoid "cannot extract
! 	 * system attribute from virtual tuple" errors later.
! 	 *
! 	 * After materializing, we can also set tableoid of the result tuple.
  	 */
  
  	return slot;
  }
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 63958c3..542584b 100644
*** a/src/backend/nodes/copyfuncs.c
--- b/src/backend/nodes/copyfuncs.c
*************** _copyForeignScan(ForeignScan *from)
*** 590,596 ****
  	/*
  	 * copy remainder of node
  	 */
- 	COPY_SCALAR_FIELD(fsSystemCol);
  	COPY_NODE_FIELD(fdwplan);
  
  	return newnode;
--- 590,595 ----
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index f7d39ed..b416bb2 100644
*** a/src/backend/nodes/outfuncs.c
--- b/src/backend/nodes/outfuncs.c
*************** _outForeignScan(StringInfo str, ForeignS
*** 557,563 ****
  
  	_outScanInfo(str, (Scan *) node);
  
- 	WRITE_BOOL_FIELD(fsSystemCol);
  	WRITE_NODE_FIELD(fdwplan);
  }
  
--- 557,562 ----
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 8138b01..c848ee2 100644
*** a/src/backend/optimizer/plan/createplan.c
--- b/src/backend/optimizer/plan/createplan.c
*************** static CteScan *make_ctescan(List *qptli
*** 123,129 ****
  static WorkTableScan *make_worktablescan(List *qptlist, List *qpqual,
  				   Index scanrelid, int wtParam);
  static ForeignScan *make_foreignscan(List *qptlist, List *qpqual,
! 				 Index scanrelid, bool fsSystemCol, FdwPlan *fdwplan);
  static BitmapAnd *make_bitmap_and(List *bitmapplans);
  static BitmapOr *make_bitmap_or(List *bitmapplans);
  static NestLoop *make_nestloop(List *tlist,
--- 123,129 ----
  static WorkTableScan *make_worktablescan(List *qptlist, List *qpqual,
  				   Index scanrelid, int wtParam);
  static ForeignScan *make_foreignscan(List *qptlist, List *qpqual,
! 				 Index scanrelid, FdwPlan *fdwplan);
  static BitmapAnd *make_bitmap_and(List *bitmapplans);
  static BitmapOr *make_bitmap_or(List *bitmapplans);
  static NestLoop *make_nestloop(List *tlist,
*************** create_foreignscan_plan(PlannerInfo *roo
*** 1827,1833 ****
  	RelOptInfo *rel = best_path->path.parent;
  	Index		scan_relid = rel->relid;
  	RangeTblEntry *rte;
- 	bool		fsSystemCol;
  	int			i;
  
  	/* it should be a base rel... */
--- 1827,1832 ----
*************** create_foreignscan_plan(PlannerInfo *roo
*** 1842,1862 ****
  	/* Reduce RestrictInfo list to bare expressions; ignore pseudoconstants */
  	scan_clauses = extract_actual_clauses(scan_clauses, false);
  
- 	/* Detect whether any system columns are requested from rel */
- 	fsSystemCol = false;
- 	for (i = rel->min_attr; i < 0; i++)
- 	{
- 		if (!bms_is_empty(rel->attr_needed[i - rel->min_attr]))
- 		{
- 			fsSystemCol = true;
- 			break;
- 		}
- 	}
- 
  	scan_plan = make_foreignscan(tlist,
  								 scan_clauses,
  								 scan_relid,
- 								 fsSystemCol,
  								 best_path->fdwplan);
  
  	copy_path_costsize(&scan_plan->scan.plan, &best_path->path);
--- 1841,1849 ----
*************** static ForeignScan *
*** 3170,3176 ****
  make_foreignscan(List *qptlist,
  				 List *qpqual,
  				 Index scanrelid,
- 				 bool fsSystemCol,
  				 FdwPlan *fdwplan)
  {
  	ForeignScan *node = makeNode(ForeignScan);
--- 3157,3162 ----
*************** make_foreignscan(List *qptlist,
*** 3182,3188 ****
  	plan->lefttree = NULL;
  	plan->righttree = NULL;
  	node->scan.scanrelid = scanrelid;
- 	node->fsSystemCol = fsSystemCol;
  	node->fdwplan = fdwplan;
  
  	return node;
--- 3168,3173 ----
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 6685864..8f1dfa3 100644
*** a/src/include/nodes/plannodes.h
--- b/src/include/nodes/plannodes.h
*************** typedef struct WorkTableScan
*** 467,473 ****
  typedef struct ForeignScan
  {
  	Scan		scan;
- 	bool		fsSystemCol;	/* true if any "system column" is needed */
  	/* use struct pointer to avoid including fdwapi.h here */
  	struct FdwPlan *fdwplan;
  } ForeignScan;
--- 467,472 ----
diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
index 2687d51..c11574b 100644
*** a/src/test/regress/expected/foreign_data.out
--- b/src/test/regress/expected/foreign_data.out
*************** ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 
*** 699,706 ****
  ERROR:  "ft1" is not a table
  ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE char(10);
  ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 SET DATA TYPE text;
- ALTER FOREIGN TABLE ft1 ALTER COLUMN xmin OPTIONS (ADD p1 'v1'); -- ERROR
- ERROR:  cannot alter system column "xmin"
  ALTER FOREIGN TABLE ft1 ALTER COLUMN c7 OPTIONS (ADD p1 'v1', ADD p2 'v2'),
                          ALTER COLUMN c8 OPTIONS (ADD p1 'v1', ADD p2 'v2');
  ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 OPTIONS (SET p2 'V2', DROP p1);
--- 699,704 ----
diff --git a/src/test/regress/sql/foreign_data.sql b/src/test/regress/sql/foreign_data.sql
index 051dfa3..fe3750e 100644
*** a/src/test/regress/sql/foreign_data.sql
--- b/src/test/regress/sql/foreign_data.sql
*************** ALTER FOREIGN TABLE ft1 ALTER COLUMN c7 
*** 297,303 ****
  ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE char(10) USING '0'; -- ERROR
  ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE char(10);
  ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 SET DATA TYPE text;
- ALTER FOREIGN TABLE ft1 ALTER COLUMN xmin OPTIONS (ADD p1 'v1'); -- ERROR
  ALTER FOREIGN TABLE ft1 ALTER COLUMN c7 OPTIONS (ADD p1 'v1', ADD p2 'v2'),
                          ALTER COLUMN c8 OPTIONS (ADD p1 'v1', ADD p2 'v2');
  ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 OPTIONS (SET p2 'V2', DROP p1);
--- 297,302 ----
#8Thom Brown
thom@linux.com
In reply to: Shigeru Hanada (#7)
Re: FDW system columns

2011/11/14 Shigeru Hanada <shigeru.hanada@gmail.com>

(2011/11/14 11:25), Robert Haas wrote:

My vote is to nuke 'em all. :-)

+1.

IIRC, main purpose of supporting tableoid for foreign tables was to be
basis of foreign table inheritance, which was not included in 9.1, and
we have not supported it yet. Other system columns are essentially
garbage, but they survived at 9.1 development because (maybe) it seemed
little odd to have system columns partially at that time.

So, IMHO removing all system columns from foreign tables seems
reasonable, unless it doesn't break any external tool seriously (Perhaps
there would be few tools which assume that foreign tables have system
columns).

If there seems to be a consensus on removing system column from foreign
tables, I'd like to work on this issue. Attached is a halfway patch,
and ISTM there is no problem so far.

I can say that at least PgAdmin doesn't use these columns.

--
Thom Brown
Twitter: @darkixion
IRC (freenode): dark_ixion
Registered Linux user: #516935

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#9Thom Brown
thom@linux.com
In reply to: Thom Brown (#8)
Re: FDW system columns

On 14 November 2011 13:07, Thom Brown <thom@linux.com> wrote:

2011/11/14 Shigeru Hanada <shigeru.hanada@gmail.com>

(2011/11/14 11:25), Robert Haas wrote:

My vote is to nuke 'em all.  :-)

+1.

IIRC, main purpose of supporting tableoid for foreign tables was to be
basis of foreign table inheritance, which was not included in 9.1, and
we have not supported it yet.  Other system columns are essentially
garbage, but they survived at 9.1 development because (maybe) it seemed
little odd to have system columns partially at that time.

So, IMHO removing all system columns from foreign tables seems
reasonable, unless it doesn't break any external tool seriously (Perhaps
there would be few tools which assume that foreign tables have system
columns).

If there seems to be a consensus on removing system column from foreign
tables, I'd like to work on this issue.  Attached is a halfway patch,
and ISTM there is no problem so far.

I can say that at least PgAdmin doesn't use these columns.

So we still have all of these columns for foreign tables. I've tested
Hanada-san's patch and it removes all of the system columns. Could we
consider applying it, or has a use-case for them since been
discovered?

--
Thom

#10Robert Haas
robertmhaas@gmail.com
In reply to: Thom Brown (#9)
Re: FDW system columns

On Sat, Feb 25, 2012 at 3:56 PM, Thom Brown <thom@linux.com> wrote:

If there seems to be a consensus on removing system column from foreign
tables, I'd like to work on this issue.  Attached is a halfway patch,
and ISTM there is no problem so far.

I can say that at least PgAdmin doesn't use these columns.

So we still have all of these columns for foreign tables.  I've tested
Hanada-san's patch and it removes all of the system columns.  Could we
consider applying it, or has a use-case for them since been
discovered?

Not to my knowledge, but Hanada-san described his patch as a "halfway
patch", implying that it wasn't done.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#11Shigeru Hanada
shigeru.hanada@gmail.com
In reply to: Robert Haas (#10)
Re: FDW system columns

(2012/02/27 12:35), Robert Haas wrote:

On Sat, Feb 25, 2012 at 3:56 PM, Thom Brown<thom@linux.com> wrote:

If there seems to be a consensus on removing system column from foreign
tables, I'd like to work on this issue. Attached is a halfway patch,
and ISTM there is no problem so far.

I can say that at least PgAdmin doesn't use these columns.

So we still have all of these columns for foreign tables. I've tested
Hanada-san's patch and it removes all of the system columns. Could we
consider applying it, or has a use-case for them since been
discovered?

Not to my knowledge, but Hanada-san described his patch as a "halfway
patch", implying that it wasn't done.

Sorry for long absence.

I've used the word "halfway" because I didn't have enough time to
examine that patch at that time. I tested the patch, and now I think
it's OK to apply. One concern is that there is no mention about
unavailable system columns in any document. ddl.sgml has main
description of system columns, but it just says:

<quote>
Every table has several system columns that are implicitly defined by
the system.
</quote>

Since this doesn't mention detailed type of relation, such as VIEW and
COMPOSITE TYPE, IMO we can leave this paragraph as is.

BTW, I still think that tableoid is useful if foreign tables can inherit
other tables. With such feature, tableoid of foreign table is necessary
to determine actual source table. Once we want to support that feature,
IMO we should revive tableoid system column for foreign tables.

--
Shigeru Hanada

#12Thom Brown
thom@linux.com
In reply to: Shigeru Hanada (#11)
Re: FDW system columns

2012/2/28 Shigeru Hanada <shigeru.hanada@gmail.com>:

(2012/02/27 12:35), Robert Haas wrote:

On Sat, Feb 25, 2012 at 3:56 PM, Thom Brown<thom@linux.com>  wrote:

If there seems to be a consensus on removing system column from foreign
tables, I'd like to work on this issue.  Attached is a halfway patch,
and ISTM there is no problem so far.

I can say that at least PgAdmin doesn't use these columns.

So we still have all of these columns for foreign tables.  I've tested
Hanada-san's patch and it removes all of the system columns.  Could we
consider applying it, or has a use-case for them since been
discovered?

Not to my knowledge, but Hanada-san described his patch as a "halfway
patch", implying that it wasn't done.

Sorry for long absence.

I've used the word "halfway" because I didn't have enough time to
examine that patch at that time.  I tested the patch, and now I think
it's OK to apply.  One concern is that there is no mention about
unavailable system columns in any document.  ddl.sgml has main
description of system columns, but it just says:

<quote>
Every table has several system columns that are implicitly defined by
the system.
</quote>

Since this doesn't mention detailed type of relation, such as VIEW and
COMPOSITE TYPE, IMO we can leave this paragraph as is.

BTW, I still think that tableoid is useful if foreign tables can inherit
other tables.  With such feature, tableoid of foreign table is necessary
to determine actual source table.  Once we want to support that feature,
IMO we should revive tableoid system column for foreign tables.

I'm not familiar with foreign table inheritance, or how it would work.
If that's something that will likely be introduced in future, then
surely we'd want to keep the tableoid column rather than removing it
then re-introducing it later?

--
Thom

#13Shigeru Hanada
shigeru.hanada@gmail.com
In reply to: Thom Brown (#12)
Re: FDW system columns

(2012/02/28 18:08), Thom Brown wrote:

If that's something that will likely be introduced in future, then
surely we'd want to keep the tableoid column rather than removing it
then re-introducing it later?

As background knowledge, currently (9.1 and 9.2dev) foreign tables have
all system columns, but they return meaningless values except tableoid.
For instance, a foreign table pgbench_branches with 3 rows will return
results like below ("bid" in the rightmost is user column):

postgres=# select ctid, xmin, cmin, xmax, cmax, tableoid,
postgres-# bid from pgbench_branches;
ctid | xmin | cmin | xmax | cmax | tableoid | bid
----------------+------+------+------+------+----------+-----
(4294967295,0) | 0 | 0 | 0 | 0 | 16400 | 2
(4294967295,0) | 0 | 0 | 0 | 0 | 16400 | 3
(4294967295,0) | 0 | 0 | 0 | 0 | 16400 | 1
(3 rows)

In this example, 16400 is correct oid of pg_class record for relation
pgbench_branches.

I don't have any idea to use system columns other than tableoid of
foreign tables, because it seems difficult to define common meaning for
various FDWs. One possible idea about ctid column is using it for
virtual tuple id (location information of remote data) for update
support, if FDW can pack location information into ItemPointerData area.

We have three options:

a) remove all system columns (posted patch)
b) remove system columns other than tableoid
c) leave all system columns as is (current 9.2dev)

Incidentally, views, which is very similar object type to foreign
tables, have no system columns.

Thoughts?

--
Shigeru Hanada

#14Robert Haas
robertmhaas@gmail.com
In reply to: Shigeru Hanada (#13)
Re: FDW system columns

On Tue, Feb 28, 2012 at 7:00 AM, Shigeru Hanada
<shigeru.hanada@gmail.com> wrote:

We have three options:

a) remove all system columns (posted patch)
b) remove system columns other than tableoid
c) leave all system columns as is (current 9.2dev)

Incidentally, views, which is very similar object type to foreign
tables, have no system columns.

Thoughts?

I vote against (c). I'm not sure which of (a) or (b) is better.
We've talked about allowing foreign tables to inherit from regular
tables and visca versa, and certainly, in that situation, tableoid
would be useful. But I don't think we've made a definitive decision
about that. I stripped that functionality out of the original patch
because it introduced a bunch of warts that we didn't have time to
figure out how to fix, and it's not clear to me that anyone's spent
any time thinking about that since then.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#15Kohei KaiGai
kaigai@kaigai.gr.jp
In reply to: Shigeru Hanada (#13)
Re: FDW system columns

2012年2月28日12:00 Shigeru Hanada <shigeru.hanada@gmail.com>:

(2012/02/28 18:08), Thom Brown wrote:

If that's something that will likely be introduced in future, then
surely we'd want to keep the tableoid column rather than removing it
then re-introducing it later?

As background knowledge, currently (9.1 and 9.2dev) foreign tables have
all system columns, but they return meaningless values except tableoid.
For instance, a foreign table pgbench_branches with 3 rows will return
results like below ("bid" in the rightmost is user column):

postgres=# select ctid, xmin, cmin, xmax, cmax, tableoid,
postgres-# bid from pgbench_branches;
ctid | xmin | cmin | xmax | cmax | tableoid | bid
----------------+------+------+------+------+----------+-----
(4294967295,0) | 0 | 0 | 0 | 0 | 16400 | 2
(4294967295,0) | 0 | 0 | 0 | 0 | 16400 | 3
(4294967295,0) | 0 | 0 | 0 | 0 | 16400 | 1
(3 rows)

In this example, 16400 is correct oid of pg_class record for relation
pgbench_branches.

I don't have any idea to use system columns other than tableoid of
foreign tables, because it seems difficult to define common meaning for
various FDWs. One possible idea about ctid column is using it for
virtual tuple id (location information of remote data) for update
support, if FDW can pack location information into ItemPointerData area.

We have three options:

a) remove all system columns (posted patch)
b) remove system columns other than tableoid
c) leave all system columns as is (current 9.2dev)

Incidentally, views, which is very similar object type to foreign
tables, have no system columns.

Thoughts?

Which is the expected behavior in case of a foreign table
is constructed as a child table of a particular regular table?
In this case, children foreign tables don't have columns
that exist on the parent table?
(Although it is no matter when a regular table is a child of
a foreign table...)

Thanks,
--
KaiGai Kohei <kaigai@kaigai.gr.jp>

#16Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#14)
Re: FDW system columns

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Feb 28, 2012 at 7:00 AM, Shigeru Hanada
<shigeru.hanada@gmail.com> wrote:

We have three options:

a) remove all system columns (posted patch)
b) remove system columns other than tableoid
c) leave all system columns as is (current 9.2dev)

Incidentally, views, which is very similar object type to foreign
tables, have no system columns.

Thoughts?

I vote against (c). I'm not sure which of (a) or (b) is better.
We've talked about allowing foreign tables to inherit from regular
tables and visca versa, and certainly, in that situation, tableoid
would be useful.

I think it is a mistake to imagine that tableoid is only useful in
inheritance contexts. As one counterexample, pg_dump selects tableoid
from quite a lot of system catalogs, just as a convenient and uniform
way of remembering each object's type (of course, the fact that it needs
to match them up against pg_depend entries has something to do with
that). More generally, if we exclude tableoid from foreign tables,
that just introduces an arbitrary behavioral difference between foreign
and regular tables, thus complicating any code that has use for the
feature.

So I believe that (a) is a pretty bad choice. I would hold still for
(b) but I am not convinced that the case has been made for that either.
I think it would be wise to avoid introducing behavioral churn until
after we have designed and implemented update capabilities for foreign
tables. If we end up putting back ctid to support that, we'll look
pretty silly.

In short, (c) looks like the most reasonable choice for now, with the
expectation of revisiting the question after we have foreign update
working.

regards, tom lane

#17Shigeru Hanada
shigeru.hanada@gmail.com
In reply to: Kohei KaiGai (#15)
Re: FDW system columns

(2012/02/28 23:37), Kohei KaiGai wrote:

2012年2月28日12:00 Shigeru Hanada<shigeru.hanada@gmail.com>:

We have three options:

a) remove all system columns (posted patch)
b) remove system columns other than tableoid
c) leave all system columns as is (current 9.2dev)

Incidentally, views, which is very similar object type to foreign
tables, have no system columns.

Thoughts?

Which is the expected behavior in case of a foreign table
is constructed as a child table of a particular regular table?
In this case, children foreign tables don't have columns
that exist on the parent table?
(Although it is no matter when a regular table is a child of
a foreign table...)

If we support table inheritance by foreign tables, foreign tables should
return something for all system columns, because a child table MUST have
all columns held by all parent tables. I'm not sure that foreign tables
should have system columns physically, like the option c).

For now, c) seems most reasonable to me.

--
Shigeru Hanada