On 2015/04/28 15:17, Amit Langote wrote:
The code at the beginning of ATSimpleRecursion() looks like -

/*
  * Propagate to children if desired.  Non-table relations never have
  * children, so no need to search in that case.
  */
  if (recurse && rel->rd_rel->relkind == RELKIND_RELATION)

Not sure if it's great idea, but now that foreign tables can also have
children, should above be changed to take that into account?

Yeah, I think we should now allow the recursion for inheritance parents that are foreign tables as well. Attached is a patch for that.

so even foreign inheritance parents expand for
various ALTER TABLE actions like adding a column though that is not a
meaningful operation on foreign tables anyway.

An example,
postgres=# alter foreign table fparent alter a type char;
ALTER FOREIGN TABLE

postgres=# select * from fparent;
ERROR:  attribute "a" of relation "fchild1" does not match parent's type

Above error, AIUI, is hit much before it is determined that fparent is a
foreign table, whereas the following is FDW-specific (waiting to happen) error,

postgres=# alter foreign table fparent add b char;
ALTER FOREIGN TABLE

postgres=# SELECT * FROM fparent;
ERROR:  column "b" does not exist
CONTEXT:  Remote SQL command: SELECT a, b FROM public.parent

Not sure if the first case could be considered s a bug as foreign tables are
pretty lax in these regards anyway.

I think the first case would be a bug caused by ATSimpleRecursion() that doesn't allow the recursion. But I don't think the second case is a bug. As described in the notes in the reference page on ALTER FOREIGN TABLE, I think it should be the user's responsibility to ensure that the foreign table definition matches the reality.

Best regards,
Etsuro Fujita
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 4367,4376 **** ATSimpleRecursion(List **wqueue, Relation rel,
  				  AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode)
  {
  	/*
! 	 * Propagate to children if desired.  Non-table relations never have
! 	 * children, so no need to search in that case.
  	 */
! 	if (recurse && rel->rd_rel->relkind == RELKIND_RELATION)
  	{
  		Oid			relid = RelationGetRelid(rel);
  		ListCell   *child;
--- 4367,4379 ----
  				  AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode)
  {
  	/*
! 	 * Propagate to children if desired.  Relations other than plain tables
! 	 * and foreign tables never have children, so no need to search in that
! 	 * case.
  	 */
! 	if (recurse &&
! 		(rel->rd_rel->relkind == RELKIND_RELATION ||
! 		 rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE))
  	{
  		Oid			relid = RelationGetRelid(rel);
  		ListCell   *child;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to