public inbox for [email protected]  
help / color / mirror / Atom feed
pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
27+ messages / 8 participants
[nested] [flat]

* pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2017-07-26 10:09  tushar <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: tushar @ 2017-07-26 10:09 UTC (permalink / raw)
  To: pgsql-hackers

v9.5/9.6

create these objects -
CREATE TABLE constraint_rename_test (a int CONSTRAINT con1 CHECK (a > 
0), b int, c int);
CREATE TABLE constraint_rename_test2 (a int CONSTRAINT con1 CHECK (a > 
0), d int) INHERITS (constraint_rename_test);
ALTER TABLE constraint_rename_test ADD CONSTRAINT con3 PRIMARY KEY (a);

v9.6/v10 - run pg_upgrade

pg_restore: creating COMMENT "SCHEMA "public""
pg_restore: creating TABLE "public.constraint_rename_test"
pg_restore: creating TABLE "public.constraint_rename_test2"
pg_restore: [archiver (db)] Error while PROCESSING TOC:
pg_restore: [archiver (db)] Error from TOC entry 351; 1259 16388 TABLE 
constraint_rename_test2 edb
pg_restore: [archiver (db)] could not execute query: ERROR:  column "a" 
in child table must be marked NOT NULL
     Command was:
-- For binary upgrade, must preserve pg_type oid
SELECT 
pg_catalog.binary_upgrade_set_next_pg_type_oid('16390'::pg_catalog.oid);

manually i am able to create all these objects .

-- 
regards,tushar
EnterpriseDB  https://www.enterprisedb.com/
The Enterprise PostgreSQL Company



-- 
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2017-07-26 12:05  Michael Paquier <[email protected]>
  parent: tushar <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Michael Paquier @ 2017-07-26 12:05 UTC (permalink / raw)
  To: tushar <[email protected]>; +Cc: pgsql-hackers

On Wed, Jul 26, 2017 at 12:09 PM, tushar <[email protected]> wrote:
> v9.5/9.6
>
> create these objects -
> CREATE TABLE constraint_rename_test (a int CONSTRAINT con1 CHECK (a > 0), b
> int, c int);
> CREATE TABLE constraint_rename_test2 (a int CONSTRAINT con1 CHECK (a > 0), d
> int) INHERITS (constraint_rename_test);
> ALTER TABLE constraint_rename_test ADD CONSTRAINT con3 PRIMARY KEY (a);
>
> v9.6/v10 - run pg_upgrade
>
> pg_restore: creating COMMENT "SCHEMA "public""
> pg_restore: creating TABLE "public.constraint_rename_test"
> pg_restore: creating TABLE "public.constraint_rename_test2"
> pg_restore: [archiver (db)] Error while PROCESSING TOC:
> pg_restore: [archiver (db)] Error from TOC entry 351; 1259 16388 TABLE
> constraint_rename_test2 edb
> pg_restore: [archiver (db)] could not execute query: ERROR:  column "a" in
> child table must be marked NOT NULL
>     Command was:
> -- For binary upgrade, must preserve pg_type oid
> SELECT
> pg_catalog.binary_upgrade_set_next_pg_type_oid('16390'::pg_catalog.oid);
>
> manually i am able to create all these objects .

You can go further down to reproduce the failure of your test case. I
can see the same thing with at least 9.3, which is as far as I tested,
for any upgrade combinations up to HEAD. And I would imagine that this
is an old behavior.

The documentation says the following:
https://www.postgresql.org/docs/9.3/static/ddl-inherit.html
All check constraints and not-null constraints on a parent table are
automatically inherited by its children. Other types of constraints
(unique, primary key, and foreign key constraints) are not inherited.

When adding a primary key, the system does first SET NOT NULL on each
column under cover, but this does not get spread to the child tables
as this is a PRIMARY KEY. The question is, do we want to spread the
NOT NULL constraint created by the PRIMARY KEY command to the child
tables, or not? It is easy enough to fix your problem by switching the
second and third commands in your test case, still I think that the
current behavior is non-intuitive, and that we ought to fix this by
applying NOT NULL to the child's columns when a primary key is added
to the parent. Thoughts?
-- 
Michael


-- 
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2017-07-26 14:02  Tom Lane <[email protected]>
  parent: Michael Paquier <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Tom Lane @ 2017-07-26 14:02 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: tushar <[email protected]>; pgsql-hackers

Michael Paquier <[email protected]> writes:
> The documentation says the following:
> https://www.postgresql.org/docs/9.3/static/ddl-inherit.html
> All check constraints and not-null constraints on a parent table are
> automatically inherited by its children. Other types of constraints
> (unique, primary key, and foreign key constraints) are not inherited.

> When adding a primary key, the system does first SET NOT NULL on each
> column under cover, but this does not get spread to the child tables
> as this is a PRIMARY KEY. The question is, do we want to spread the
> NOT NULL constraint created by the PRIMARY KEY command to the child
> tables, or not?

Yeah, I think we really ought to for consistency.  Quite aside from
pg_dump hazards, this allows situations where selecting from an
apparently not-null column can produce nulls (coming from unconstrained
child tables).  That's exactly what the automatic inheritance rules
are intended to prevent.  If we had a way to mark NOT NULL constraints
as not-inherited, it'd be legitimate for ADD PRIMARY KEY to paste on
one of those instead of a regular one ... but we lack that feature,
so it's moot.

Not sure what's involved there code-wise, though, nor whether we'd want
to back-patch.

			regards, tom lane


-- 
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2017-07-26 14:50  Michael Paquier <[email protected]>
  parent: Tom Lane <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Michael Paquier @ 2017-07-26 14:50 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: tushar <[email protected]>; pgsql-hackers

On Wed, Jul 26, 2017 at 4:02 PM, Tom Lane <[email protected]> wrote:
> Not sure what's involved there code-wise, though, nor whether we'd want
> to back-patch.

I'll try to look at the code around that to come up with a clear
conclusion in the next couple of days, likely more as that's a
vacation period. Surely anything invasive would not be backpatched.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2017-07-27 14:58  Michael Paquier <[email protected]>
  parent: Michael Paquier <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Michael Paquier @ 2017-07-27 14:58 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: tushar <[email protected]>; pgsql-hackers

On Wed, Jul 26, 2017 at 4:50 PM, Michael Paquier
<[email protected]> wrote:
> On Wed, Jul 26, 2017 at 4:02 PM, Tom Lane <[email protected]> wrote:
>> Not sure what's involved there code-wise, though, nor whether we'd want
>> to back-patch.
>
> I'll try to look at the code around that to come up with a clear
> conclusion in the next couple of days, likely more as that's a
> vacation period. Surely anything invasive would not be backpatched.

So I think that the attached patch is able to do the legwork. While
looking at the code, I have bumped into index_check_primary_key() that
discarded the case of sending SET NOT NULL to child tables, as
introduced by 88452d5b. But that's clearly an oversight IMO, and the
comment is wrong to begin with because SET NOT NULL is spread to child
tables. Using is_alter_table instead of a plain true in
index_check_primary_key() for the call of AlterTableInternal() is
defensive, but I don't think that we want to impact any modules
relying on this API, so recursing only when ALTER TABLE is used is the
safest course of action to me. With this logic, we rely as well on the
fact that ADD PRIMARY KEY is not recursive in tablecmds.c. Comments
are welcome, I'll park that into the CF app so as it does not get lost
in the void.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Attachments:

  [application/octet-stream] alttab-setnotnull-v1.patch (1.9K, ../../CAB7nPqSdvhZsBbatH-1nS=pKXCorupS3_Xc+=dY9UGQeDsX8jg@mail.gmail.com/2-alttab-setnotnull-v1.patch)
  download | inline diff:
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index d25b39bb54..4ebac27d10 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -185,6 +185,8 @@ relationHasPrimaryKey(Relation rel)
  * created NOT NULL during CREATE TABLE), do an ALTER SET NOT NULL to mark
  * them so --- or fail if they are not in fact nonnull.
  *
+ * For ALTER TABLE, SET NOT NULL is applied as well to child tables.
+ *
  * Caller had better have at least ShareLock on the table, else the not-null
  * checking isn't trustworthy.
  */
@@ -253,17 +255,13 @@ index_check_primary_key(Relation heapRel,
 	}
 
 	/*
-	 * XXX: Shouldn't the ALTER TABLE .. SET NOT NULL cascade to child tables?
-	 * Currently, since the PRIMARY KEY itself doesn't cascade, we don't
-	 * cascade the notnull constraint(s) either; but this is pretty debatable.
-	 *
 	 * XXX: possible future improvement: when being called from ALTER TABLE,
 	 * it would be more efficient to merge this with the outer ALTER TABLE, so
 	 * as to avoid two scans.  But that seems to complicate DefineIndex's API
 	 * unduly.
 	 */
 	if (cmds)
-		AlterTableInternal(RelationGetRelid(heapRel), cmds, false);
+		AlterTableInternal(RelationGetRelid(heapRel), cmds, is_alter_table);
 }
 
 /*
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 13d6a4b747..e9fd1aacce 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -328,7 +328,7 @@ Number of child tables: 1 (Use \d+ to list them.)
       Table "public.constraint_rename_test2"
  Column |  Type   | Collation | Nullable | Default 
 --------+---------+-----------+----------+---------
- a      | integer |           |          | 
+ a      | integer |           | not null | 
  b      | integer |           |          | 
  c      | integer |           |          | 
  d      | integer |           |          | 


^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2017-08-04 15:50  Tom Lane <[email protected]>
  parent: Michael Paquier <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Tom Lane @ 2017-08-04 15:50 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: tushar <[email protected]>; pgsql-hackers

Michael Paquier <[email protected]> writes:
> So I think that the attached patch is able to do the legwork.

I've pushed this into HEAD.  It seems like enough of a behavioral
change that we wouldn't want to back-patch, but IMO it's not too late
to be making this type of change in v10.  If we wait for the next CF
then it will take another year for the fix to reach the field.

> While
> looking at the code, I have bumped into index_check_primary_key() that
> discarded the case of sending SET NOT NULL to child tables, as
> introduced by 88452d5b. But that's clearly an oversight IMO, and the
> comment is wrong to begin with because SET NOT NULL is spread to child
> tables. Using is_alter_table instead of a plain true in
> index_check_primary_key() for the call of AlterTableInternal() is
> defensive, but I don't think that we want to impact any modules
> relying on this API, so recursing only when ALTER TABLE is used is the
> safest course of action to me.

I didn't find that persuasive: I think passing "recurse" as just plain
"true" is safer and more readable.  We shouldn't be able to get there
in a CREATE case, as per the comments; and if we did, there shouldn't
be any child tables anyway; but if somehow there were, surely the same
consistency argument would imply that we *must* recurse and fix those
child tables.  So I just made it "true".

			regards, tom lane


-- 
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2017-08-04 16:06  Michael Paquier <[email protected]>
  parent: Tom Lane <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Michael Paquier @ 2017-08-04 16:06 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: tushar <[email protected]>; pgsql-hackers

On Fri, Aug 4, 2017 at 5:50 PM, Tom Lane <[email protected]> wrote:
> Michael Paquier <[email protected]> writes:
>> So I think that the attached patch is able to do the legwork.
>
> I've pushed this into HEAD.  It seems like enough of a behavioral
> change that we wouldn't want to back-patch, but IMO it's not too late
> to be making this type of change in v10.  If we wait for the next CF
> then it will take another year for the fix to reach the field.

Thanks for applying the fix. My intention when adding that in a CF is
not to see things lost.

>> While
>> looking at the code, I have bumped into index_check_primary_key() that
>> discarded the case of sending SET NOT NULL to child tables, as
>> introduced by 88452d5b. But that's clearly an oversight IMO, and the
>> comment is wrong to begin with because SET NOT NULL is spread to child
>> tables. Using is_alter_table instead of a plain true in
>> index_check_primary_key() for the call of AlterTableInternal() is
>> defensive, but I don't think that we want to impact any modules
>> relying on this API, so recursing only when ALTER TABLE is used is the
>> safest course of action to me.
>
> I didn't find that persuasive: I think passing "recurse" as just plain
> "true" is safer and more readable.  We shouldn't be able to get there
> in a CREATE case, as per the comments; and if we did, there shouldn't
> be any child tables anyway; but if somehow there were, surely the same
> consistency argument would imply that we *must* recurse and fix those
> child tables.  So I just made it "true".

Yeah, that matches what I read as well. No complains to switch to a
plain "true" even if it is not reached yet.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2017-12-13 00:22  Ali Akbar <[email protected]>
  parent: Michael Paquier <[email protected]>
  0 siblings, 2 replies; 27+ messages in thread

From: Ali Akbar @ 2017-12-13 00:22 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Tom Lane <[email protected]>; tushar <[email protected]>; pgsql-hackers

Just stumbled across the same issues while upgrading one of my cluster
to Pg 10 with pg_upgrade. Finished the upgrade by fixing the old
database(s) and re-running pg_upgrade.

2017-08-04 23:06 GMT+07:00 Michael Paquier <[email protected]>:
>
> On Fri, Aug 4, 2017 at 5:50 PM, Tom Lane <[email protected]> wrote:
> > Michael Paquier <[email protected]> writes:
> >> So I think that the attached patch is able to do the legwork.
> >
> > I've pushed this into HEAD.  It seems like enough of a behavioral
> > change that we wouldn't want to back-patch, but IMO it's not too late
> > to be making this type of change in v10.  If we wait for the next CF
> > then it will take another year for the fix to reach the field.
>
> Thanks for applying the fix. My intention when adding that in a CF is
> not to see things lost.

Thans for the fix. Just found some issues:

1. My old database schema becomes like that by accidental modification
on the child table, and on HEAD, it still works:

# create table parent (id serial PRIMARY KEY, name VARCHAR(52) NOT NULL);
CREATE TABLE
# create table child () inherits (parent);
CREATE TABLE
# alter table child alter column name drop not null;
ALTER TABLE
# \d parent
                                   Table "public.parent"
 Column |         Type          | Collation | Nullable |
Default
--------+-----------------------+-----------+----------+------------------------------------
 id     | integer               |           | not null |
nextval('parent_id_seq'::regclass)
 name   | character varying(52) |           | not null |
Indexes:
    "parent_pkey" PRIMARY KEY, btree (id)
Number of child tables: 1 (Use \d+ to list them.)

# \d child
                                    Table "public.child"
 Column |         Type          | Collation | Nullable |
Default
--------+-----------------------+-----------+----------+------------------------------------
 id     | integer               |           | not null |
nextval('parent_id_seq'::regclass)
 name   | character varying(52) |           |          |
Inherits: parent


2. If we execute pg_dump manually, it silently corrects the schema:

..... (cut)
--
-- Name: parent; Type: TABLE; Schema: public; Owner: -
--

CREATE TABLE parent (
    id integer NOT NULL,
    name character varying(52) NOT NULL
);


--
-- Name: child; Type: TABLE; Schema: public; Owner: -
--

CREATE TABLE child (
)
INHERITS (parent);

...... (cut)

There is't any DROP NOT NULL there.



For me, it's better to prevent that from happening. So, attempts to
DROP NOT NULL on the child must be rejected. The attached patch does
that.

Unfortunately, pg_class has no "has_parent" attribute, so in this
patch, it hits pg_inherits everytime DROP NOT NULL is attempted.

Notes:
- It looks like we could remove the parent partition checking above?
Because the new check already covers what it does
- If this patch will be applied, i will work on pg_upgrade to check
for this problem before attempting to dump schema. In my case, because
the cluster has many databases, the error arise much late in the
process, it will be much better if pg_upgrade complains while
performing pre-checks.


Best Regards,
Ali Akbar


Attachments:

  [text/x-patch] setnotnull-child-v1.patch (4.1K, ../../CACQjQLrQn8QkEYnURfJ00zVu-CXtcsMBYFUKHadv1H5kdRz9MQ@mail.gmail.com/2-setnotnull-child-v1.patch)
  download | inline diff:
diff --git a/src/backend/catalog/pg_inherits.c b/src/backend/catalog/pg_inherits.c
index 1bd8a58b7f..74903a8f24 100644
--- a/src/backend/catalog/pg_inherits.c
+++ b/src/backend/catalog/pg_inherits.c
@@ -242,6 +242,40 @@ find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents)
 }
 
 
+/*
+ * get_superclasses -
+ *		Returns a list of relation OIDs of direct parents
+ */
+List *
+get_superclasses(Oid relationId)
+{
+	List	   *list = NIL;
+	Relation	catalog;
+	SysScanDesc scan;
+	ScanKeyData skey;
+	HeapTuple	inheritsTuple;
+	Oid			inhparent;
+
+	catalog = heap_open(InheritsRelationId, AccessShareLock);
+	ScanKeyInit(&skey, Anum_pg_inherits_inhrelid, BTEqualStrategyNumber,
+				F_OIDEQ, ObjectIdGetDatum(relationId));
+	scan = systable_beginscan(catalog, InheritsRelidSeqnoIndexId, true,
+							  NULL, 1, &skey);
+
+	while ((inheritsTuple = systable_getnext(scan)) != NULL)
+	{
+		inhparent = ((Form_pg_inherits) GETSTRUCT(inheritsTuple))->inhparent;
+		list = lappend_oid(list, inhparent);
+	}
+
+	systable_endscan(scan);
+
+	heap_close(catalog, AccessShareLock);
+
+	return list;
+}
+
+
 /*
  * has_subclass - does this relation have any children?
  *
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d979ce266d..c76fc3715d 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5683,6 +5683,8 @@ ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode)
 	Relation	attr_rel;
 	List	   *indexoidlist;
 	ListCell   *indexoidscan;
+	List	   *parentlist;
+	ListCell   *parentscan;
 	ObjectAddress address;
 
 	/*
@@ -5773,6 +5775,24 @@ ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode)
 		heap_close(parent, AccessShareLock);
 	}
 
+	/* If rel has parents, shoudn't drop NOT NULL if parent has the same */
+	parentlist = get_superclasses(RelationGetRelid(rel));
+	foreach(parentscan, parentlist) {
+		Oid			parentId = lfirst_oid(parentscan);
+		Relation	parent = heap_open(parentId, AccessShareLock);
+		TupleDesc	tupDesc = RelationGetDescr(parent);
+		AttrNumber	parent_attnum;
+
+		parent_attnum = get_attnum(parentId, colName);
+		if (parent_attnum != InvalidAttrNumber &&
+			TupleDescAttr(tupDesc, parent_attnum - 1)->attnotnull)
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+					 errmsg("column \"%s\" is marked NOT NULL in parent table",
+							colName)));
+		heap_close(parent, AccessShareLock);
+	}
+
 	/*
 	 * Okay, actually perform the catalog change ... if needed
 	 */
diff --git a/src/include/catalog/pg_inherits_fn.h b/src/include/catalog/pg_inherits_fn.h
index 7743388899..291861b846 100644
--- a/src/include/catalog/pg_inherits_fn.h
+++ b/src/include/catalog/pg_inherits_fn.h
@@ -20,6 +20,7 @@
 extern List *find_inheritance_children(Oid parentrelId, LOCKMODE lockmode);
 extern List *find_all_inheritors(Oid parentrelId, LOCKMODE lockmode,
 					List **parents);
+extern List *get_superclasses(Oid relationId);
 extern bool has_subclass(Oid relationId);
 extern bool has_superclass(Oid relationId);
 extern bool typeInheritsFrom(Oid subclassTypeId, Oid superclassTypeId);


^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2017-12-13 00:26  Ali Akbar <[email protected]>
  parent: Ali Akbar <[email protected]>
  1 sibling, 0 replies; 27+ messages in thread

From: Ali Akbar @ 2017-12-13 00:26 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Tom Lane <[email protected]>; tushar <[email protected]>; pgsql-hackers

> For me, it's better to prevent that from happening. So, attempts to
> DROP NOT NULL on the child must be rejected. The attached patch does
> that.

I'm sorry. Accidentaly i "--color"-ed the patch format. Attached the
correct patch.

Best Regards,
Ali Akbar


Attachments:

  [text/x-patch] setnotnull-child-v2.patch (3.1K, ../../CACQjQLrd89RuV7+1xEKmreh7o8u6fn=CYSCMtbERbw5o5t2VPg@mail.gmail.com/2-setnotnull-child-v2.patch)
  download | inline diff:
diff --git a/src/backend/catalog/pg_inherits.c b/src/backend/catalog/pg_inherits.c
index 1bd8a58b7f..74903a8f24 100644
--- a/src/backend/catalog/pg_inherits.c
+++ b/src/backend/catalog/pg_inherits.c
@@ -242,6 +242,40 @@ find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents)
 }
 
 
+/*
+ * get_superclasses -
+ *		Returns a list of relation OIDs of direct parents
+ */
+List *
+get_superclasses(Oid relationId)
+{
+	List	   *list = NIL;
+	Relation	catalog;
+	SysScanDesc scan;
+	ScanKeyData skey;
+	HeapTuple	inheritsTuple;
+	Oid			inhparent;
+
+	catalog = heap_open(InheritsRelationId, AccessShareLock);
+	ScanKeyInit(&skey, Anum_pg_inherits_inhrelid, BTEqualStrategyNumber,
+				F_OIDEQ, ObjectIdGetDatum(relationId));
+	scan = systable_beginscan(catalog, InheritsRelidSeqnoIndexId, true,
+							  NULL, 1, &skey);
+
+	while ((inheritsTuple = systable_getnext(scan)) != NULL)
+	{
+		inhparent = ((Form_pg_inherits) GETSTRUCT(inheritsTuple))->inhparent;
+		list = lappend_oid(list, inhparent);
+	}
+
+	systable_endscan(scan);
+
+	heap_close(catalog, AccessShareLock);
+
+	return list;
+}
+
+
 /*
  * has_subclass - does this relation have any children?
  *
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d979ce266d..c76fc3715d 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5683,6 +5683,8 @@ ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode)
 	Relation	attr_rel;
 	List	   *indexoidlist;
 	ListCell   *indexoidscan;
+	List	   *parentlist;
+	ListCell   *parentscan;
 	ObjectAddress address;
 
 	/*
@@ -5773,6 +5775,24 @@ ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode)
 		heap_close(parent, AccessShareLock);
 	}
 
+	/* If rel has parents, shoudn't drop NOT NULL if parent has the same */
+	parentlist = get_superclasses(RelationGetRelid(rel));
+	foreach(parentscan, parentlist) {
+		Oid			parentId = lfirst_oid(parentscan);
+		Relation	parent = heap_open(parentId, AccessShareLock);
+		TupleDesc	tupDesc = RelationGetDescr(parent);
+		AttrNumber	parent_attnum;
+
+		parent_attnum = get_attnum(parentId, colName);
+		if (parent_attnum != InvalidAttrNumber &&
+			TupleDescAttr(tupDesc, parent_attnum - 1)->attnotnull)
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+					 errmsg("column \"%s\" is marked NOT NULL in parent table",
+							colName)));
+		heap_close(parent, AccessShareLock);
+	}
+
 	/*
 	 * Okay, actually perform the catalog change ... if needed
 	 */
diff --git a/src/include/catalog/pg_inherits_fn.h b/src/include/catalog/pg_inherits_fn.h
index 7743388899..291861b846 100644
--- a/src/include/catalog/pg_inherits_fn.h
+++ b/src/include/catalog/pg_inherits_fn.h
@@ -20,6 +20,7 @@
 extern List *find_inheritance_children(Oid parentrelId, LOCKMODE lockmode);
 extern List *find_all_inheritors(Oid parentrelId, LOCKMODE lockmode,
 					List **parents);
+extern List *get_superclasses(Oid relationId);
 extern bool has_subclass(Oid relationId);
 extern bool has_superclass(Oid relationId);
 extern bool typeInheritsFrom(Oid subclassTypeId, Oid superclassTypeId);


^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2017-12-13 01:41  Amit Langote <[email protected]>
  parent: Ali Akbar <[email protected]>
  1 sibling, 1 reply; 27+ messages in thread

From: Amit Langote @ 2017-12-13 01:41 UTC (permalink / raw)
  To: Ali Akbar <[email protected]>; Michael Paquier <[email protected]>; +Cc: Tom Lane <[email protected]>; tushar <[email protected]>; pgsql-hackers

Hi.

On 2017/12/13 9:22, Ali Akbar wrote:
> For me, it's better to prevent that from happening. So, attempts to
> DROP NOT NULL on the child must be rejected. The attached patch does
> that.
> 
> Unfortunately, pg_class has no "has_parent" attribute, so in this
> patch, it hits pg_inherits everytime DROP NOT NULL is attempted.
> 
> Notes:
> - It looks like we could remove the parent partition checking above?
> Because the new check already covers what it does

I noticed that too.

So, if we're going to prevent DROP NOT NULL on *all* child table (not just
partitions), we don't need the code that now sits there to prevent that
only for partitions.

Minor comments on the patch:

+	/* If rel has parents, shoudn't drop NOT NULL if parent has the same */

How about: ".. if some parent has the same"

+		heap_close(parent, AccessShareLock);

Maybe, we shouldn't be dropping the lock so soon.

Thanks,
Amit





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2017-12-13 02:10  Michael Paquier <[email protected]>
  parent: Amit Langote <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Michael Paquier @ 2017-12-13 02:10 UTC (permalink / raw)
  To: Amit Langote <[email protected]>; +Cc: Ali Akbar <[email protected]>; Tom Lane <[email protected]>; tushar <[email protected]>; pgsql-hackers

On Wed, Dec 13, 2017 at 10:41 AM, Amit Langote
<[email protected]> wrote:
> On 2017/12/13 9:22, Ali Akbar wrote:
>> For me, it's better to prevent that from happening. So, attempts to
>> DROP NOT NULL on the child must be rejected. The attached patch does
>> that.
>>
>> Unfortunately, pg_class has no "has_parent" attribute, so in this
>> patch, it hits pg_inherits everytime DROP NOT NULL is attempted.
>>
>> Notes:
>> - It looks like we could remove the parent partition checking above?
>> Because the new check already covers what it does
>
> I noticed that too.
>
> So, if we're going to prevent DROP NOT NULL on *all* child table (not just
> partitions), we don't need the code that now sits there to prevent that
> only for partitions.

It is not the first time that this topic shows up. See for example
this thread from myself's version of last year:
https://www.postgresql.org/message-id/[email protected]...
Or this one:
http://www.postgresql.org/message-id/[email protected]

And the conclusion is that we don't want to do what you are doing
here, and that it would be better to store NOT NULL constraints in a
way similar to CHECK constraints.

> How about: ".. if some parent has the same"
>
> +               heap_close(parent, AccessShareLock);
>
> Maybe, we shouldn't be dropping the lock so soon.

Yes, such things usually need to be kept until the end of the
transaction, and usually you need to be careful about potential lock
upgrades that could cause deadlocks. This patch looks wrong for both
of those things.
-- 
Michael




^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2017-12-13 06:59  Ali Akbar <[email protected]>
  parent: Michael Paquier <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Ali Akbar @ 2017-12-13 06:59 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Amit Langote <[email protected]>; Tom Lane <[email protected]>; tushar <[email protected]>; pgsql-hackers

2017-12-13 9:10 GMT+07:00 Michael Paquier <[email protected]>:

>
> It is not the first time that this topic shows up. See for example
> this thread from myself's version of last year:
> https://www.postgresql.org/message-id/CAB7nPqTPXgX9HiyhhtAgpW7jbA1is
> [email protected]
> Or this one:
> http://www.postgresql.org/message-id/[email protected]
>
> And the conclusion is that we don't want to do what you are doing
> here, and that it would be better to store NOT NULL constraints in a
> way similar to CHECK constraints.
>

Thanks for the link to those thread.

Judging from the discussion there, it will be a long way to prevent DROP
NOT NULL. So for this problem (pg_upgrade failing because of it), i propose
that we only add a check in pg_upgrade, so anyone using pg_upgrade can know
and fix the issue before the error?

If it's OK, i can write the patch.


> > How about: ".. if some parent has the same"
> >
> > +               heap_close(parent, AccessShareLock);
> >
> > Maybe, we shouldn't be dropping the lock so soon.
>
> Yes, such things usually need to be kept until the end of the
> transaction, and usually you need to be careful about potential lock
> upgrades that could cause deadlocks. This patch looks wrong for both
> of those things.


Thanks. Judging from above, it's better that we continue the DROP NOT NULL
problem in another patch (and another thread)

Best Regards,
Ali Akbar


^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2017-12-13 08:37  Amit Langote <[email protected]>
  parent: Ali Akbar <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Amit Langote @ 2017-12-13 08:37 UTC (permalink / raw)
  To: Ali Akbar <[email protected]>; Michael Paquier <[email protected]>; +Cc: Tom Lane <[email protected]>; tushar <[email protected]>; pgsql-hackers

On 2017/12/13 15:59, Ali Akbar wrote:
> 2017-12-13 9:10 GMT+07:00 Michael Paquier <[email protected]>:
> 
>>
>> It is not the first time that this topic shows up. See for example
>> this thread from myself's version of last year:
>> https://www.postgresql.org/message-id/CAB7nPqTPXgX9HiyhhtAgpW7jbA1is
>> [email protected]
>> Or this one:
>> http://www.postgresql.org/message-id/[email protected]
>>
>> And the conclusion is that we don't want to do what you are doing
>> here, and that it would be better to store NOT NULL constraints in a
>> way similar to CHECK constraints.
>>
> 
> Thanks for the link to those thread.
> 
> Judging from the discussion there, it will be a long way to prevent DROP
> NOT NULL.

Yeah, I remembered that discussion when writing my email, but was for some
reason convinced that everything's fine even without the elaborate
book-keeping of inheritance information for NOT NULL constraints.  Thanks
Michael for reminding.

Thanks,
Amit





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2017-12-14 01:51  Ali Akbar <[email protected]>
  parent: Amit Langote <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Ali Akbar @ 2017-12-14 01:51 UTC (permalink / raw)
  To: Amit Langote <[email protected]>; +Cc: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; tushar <[email protected]>; pgsql-hackers

2017-12-13 15:37 GMT+07:00 Amit Langote <[email protected]>:

> On 2017/12/13 15:59, Ali Akbar wrote:
> >
> > Thanks for the link to those thread.
> >
> > Judging from the discussion there, it will be a long way to prevent DROP
> > NOT NULL.
>
> Yeah, I remembered that discussion when writing my email, but was for some
> reason convinced that everything's fine even without the elaborate
> book-keeping of inheritance information for NOT NULL constraints.  Thanks
> Michael for reminding.
>

Patch for adding check in pg_upgrade. Going through git history, the check
for inconsistency in NOT NULL constraint has ben there since a long time
ago. In this patch the check will be applied for all old cluster version.
I'm not sure in which version was the release of table inheritance.

Thanks,
Ali Akbar


Attachments:

  [text/x-patch] pg_upgrade_check_not_null-v1.patch (4.0K, ../../CACQjQLoMsE+1pyLe98pi0KvPG2jQQ94LWJ+PTiLgVRK4B=i_jg@mail.gmail.com/3-pg_upgrade_check_not_null-v1.patch)
  download | inline diff:
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 1b9083597c..29bafdff74 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -26,6 +26,7 @@ static void check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster);
 static void check_for_reg_data_type_usage(ClusterInfo *cluster);
 static void check_for_jsonb_9_4_usage(ClusterInfo *cluster);
 static void check_for_pg_role_prefix(ClusterInfo *cluster);
+static void check_for_not_null_inheritance(ClusterInfo *cluster);
 static char *get_canonical_locale_name(int category, const char *locale);
 
 
@@ -99,6 +100,7 @@ check_and_dump_old_cluster(bool live_check)
 	check_for_prepared_transactions(&old_cluster);
 	check_for_reg_data_type_usage(&old_cluster);
 	check_for_isn_and_int8_passing_mismatch(&old_cluster);
+	check_for_not_null_inheritance(&old_cluster);
 
 	/*
 	 * Pre-PG 10 allowed tables with 'unknown' type columns and non WAL logged
@@ -1096,6 +1098,105 @@ check_for_pg_role_prefix(ClusterInfo *cluster)
 	check_ok();
 }
 
+/*
+ * check_for_not_null_inheritance()
+ *
+ *  Currently pg_upgrade will fail if there are any inconsistencies in NOT NULL
+ *  constraint inheritance: In Postgres version 9.5, 9.6, 10 we can have a column
+ *  that is NOT NULL in parent, but nullabe in its children. But during schema
+ *  restore, that will cause error.
+ */
+static void
+check_for_not_null_inheritance(ClusterInfo *cluster)
+{
+	int			dbnum;
+	FILE	   *script = NULL;
+	bool		found = false;
+	char		output_path[MAXPGPATH];
+
+	prep_status("Checking for NOT NULL inconsistencies in inheritance");
+
+	snprintf(output_path, sizeof(output_path), "not_null_inconsistent_columns.txt");
+
+	for (dbnum = 0; dbnum < cluster->dbarr.ndbs; dbnum++)
+	{
+		PGresult   *res;
+		bool		db_used = false;
+		int			ntups;
+		int			rowno;
+		int			i_nspname,
+					i_relname,
+					i_attname;
+		DbInfo	   *active_db = &cluster->dbarr.dbs[dbnum];
+		PGconn	   *conn = connectToServer(cluster, active_db->db_name);
+
+		res = executeQueryOrDie(conn,
+								"WITH RECURSIVE parents AS ( "
+								"	SELECT	i.inhrelid, i.inhparent "
+								"	FROM	pg_catalog.pg_inherits i "
+								"	UNION ALL "
+								"	SELECT	p.inhrelid, i.inhparent "
+								"   FROM	parents p "
+								"	JOIN	pg_catalog.pg_inherits i "
+								"		ON	i.inhrelid = p.inhparent "
+								") "
+								"SELECT	n.nspname, c.relname, ac.attname  "
+								"FROM	parents p, "
+								"		pg_catalog.pg_attribute ac, "
+								"		pg_catalog.pg_attribute ap, "
+								"		pg_catalog.pg_class c, "
+								"		pg_catalog.pg_namespace n "
+								"WHERE	NOT ac.attnotnull AND "
+								"		ac.attrelid = p.inhrelid AND "
+								"		ap.attrelid = p.inhparent AND "
+								"		ac.attname = ap.attname AND "
+								"		ap.attnotnull AND "
+								"		c.oid = ac.attrelid AND "
+								"		c.relnamespace = n.oid");
+
+		ntups = PQntuples(res);
+		i_nspname = PQfnumber(res, "nspname");
+		i_relname = PQfnumber(res, "relname");
+		i_attname = PQfnumber(res, "attname");
+		for (rowno = 0; rowno < ntups; rowno++)
+		{
+			found = true;
+			if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
+				pg_fatal("could not open file \"%s\": %s\n", output_path,
+						 strerror(errno));
+			if (!db_used)
+			{
+				fprintf(script, "Database: %s\n", active_db->db_name);
+				db_used = true;
+			}
+			fprintf(script, "  %s.%s.%s\n",
+					PQgetvalue(res, rowno, i_nspname),
+					PQgetvalue(res, rowno, i_relname),
+					PQgetvalue(res, rowno, i_attname));
+		}
+
+		PQclear(res);
+
+		PQfinish(conn);
+	}
+
+	if (script)
+		fclose(script);
+
+	if (found)
+	{
+		pg_log(PG_REPORT, "fatal\n");
+		pg_fatal("Your installation contains has inconsistencies in NOT NULL\n"
+				 "constraint inheritance: child column is not market NOT NULL\n"
+				 "while parent column has NOT NULL constraint. You can fix the\n"
+				 "inconsistency with adding NOT NULL constraint and restart the\n"
+				 "upgrade.\n"
+				 "A list of the problem columns is in the file:\n"
+				 "    %s\n\n", output_path);
+	}
+	else
+		check_ok();
+}
 
 /*
  * get_canonical_locale_name


^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2017-12-14 05:49  Justin Pryzby <[email protected]>
  parent: Ali Akbar <[email protected]>
  0 siblings, 2 replies; 27+ messages in thread

From: Justin Pryzby @ 2017-12-14 05:49 UTC (permalink / raw)
  To: Ali Akbar <[email protected]>; +Cc: Amit Langote <[email protected]>; Michael Paquier <[email protected]>; Tom Lane <[email protected]>; tushar <[email protected]>; pgsql-hackers

On Thu, Dec 14, 2017 at 08:51:06AM +0700, Ali Akbar wrote:
> 2017-12-13 15:37 GMT+07:00 Amit Langote <[email protected]>:
> 
> > On 2017/12/13 15:59, Ali Akbar wrote:
> > >
> > > Thanks for the link to those thread.
> > >
> > > Judging from the discussion there, it will be a long way to prevent DROP
> > > NOT NULL.
> >
> > Yeah, I remembered that discussion when writing my email, but was for some
> > reason convinced that everything's fine even without the elaborate
> > book-keeping of inheritance information for NOT NULL constraints.  Thanks
> > Michael for reminding.
> >
> 
> Patch for adding check in pg_upgrade. Going through git history, the check
> for inconsistency in NOT NULL constraint has ben there since a long time
> ago. In this patch the check will be applied for all old cluster version.
> I'm not sure in which version was the release of table inheritance.

Here are some spelling and grammar fixes to that patch:

but nullabe in its children: nullable
child column is not market: marked
with adding: by adding
and restart: and restarting
the problem columns: the problematic columns
9.5, 9.6, 10: 9.5, 9.6, and 10
restore, that will cause error.: restore phase of pg_upgrade, that would cause an error.

Justin




^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2017-12-14 08:08  Michael Paquier <[email protected]>
  parent: Justin Pryzby <[email protected]>
  1 sibling, 1 reply; 27+ messages in thread

From: Michael Paquier @ 2017-12-14 08:08 UTC (permalink / raw)
  To: Justin Pryzby <[email protected]>; +Cc: Ali Akbar <[email protected]>; Amit Langote <[email protected]>; Tom Lane <[email protected]>; tushar <[email protected]>; pgsql-hackers

On Thu, Dec 14, 2017 at 2:49 PM, Justin Pryzby <[email protected]> wrote:
> On Thu, Dec 14, 2017 at 08:51:06AM +0700, Ali Akbar wrote:
>> 2017-12-13 15:37 GMT+07:00 Amit Langote <[email protected]>:
>> Patch for adding check in pg_upgrade. Going through git history, the check
>> for inconsistency in NOT NULL constraint has ben there since a long time
>> ago. In this patch the check will be applied for all old cluster version.
>> I'm not sure in which version was the release of table inheritance.
>
> Here are some spelling and grammar fixes to that patch:
>
> but nullabe in its children: nullable
> child column is not market: marked
> with adding: by adding
> and restart: and restarting
> the problem columns: the problematic columns
> 9.5, 9.6, 10: 9.5, 9.6, and 10
> restore, that will cause error.: restore phase of pg_upgrade, that would cause an error.

I agree that we should do better in pg_upgrade for HEAD and
REL_10_STABLE as it is not cool to fail late in the upgrade process
especially if you are using the --link mode.

+ *  Currently pg_upgrade will fail if there are any inconsistencies in NOT NULL
+ *  constraint inheritance: In Postgres version 9.5, 9.6, 10 we can
have a column
+ *  that is NOT NULL in parent, but nullabe in its children. But during schema
+ *  restore, that will cause error.
On top of the multiple typos here, there is no need to mention
anything other than "PostgreSQL 9.6 and older versions did not add NOT
NULL constraints when a primary key was added to to a parent, so check
for inconsistent NOT NULL constraints in inheritance trees".

You seem to take a path similar to what is done with the jsonb check.
Why not. I would find cleaner to tell also the user about using ALTER
TABLE to add the proper constraints.

Note that I have not checked or tested the query in details, but
reading through the thing looks basically correct as it handles
inheritance chains with WITH RECURSIVE.

@@ -99,6 +100,7 @@ check_and_dump_old_cluster(bool live_check)
     check_for_prepared_transactions(&old_cluster);
     check_for_reg_data_type_usage(&old_cluster);
     check_for_isn_and_int8_passing_mismatch(&old_cluster);
+    check_for_not_null_inheritance(&old_cluster);
The check needs indeed to happen in check_and_dump_old_cluster(), but
there is no point to check for it in servers with version 10 and
upwards, hence you should make it conditional with GET_MAJOR_VERSION()
<= 906.
-- 
Michael




^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2017-12-14 12:18  Ali Akbar <[email protected]>
  parent: Michael Paquier <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Ali Akbar @ 2017-12-14 12:18 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Amit Langote <[email protected]>; tushar <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers

2017-12-14 15:08 GMT+07:00 Michael Paquier <[email protected]>:
>
> On Thu, Dec 14, 2017 at 2:49 PM, Justin Pryzby <[email protected]> wrote:
> > On Thu, Dec 14, 2017 at 08:51:06AM +0700, Ali Akbar wrote:
> >> 2017-12-13 15:37 GMT+07:00 Amit Langote <[email protected]>:
> >> Patch for adding check in pg_upgrade. Going through git history, the check
> >> for inconsistency in NOT NULL constraint has ben there since a long time
> >> ago. In this patch the check will be applied for all old cluster version.
> >> I'm not sure in which version was the release of table inheritance.
> >
> > Here are some spelling and grammar fixes to that patch:
> >
> > but nullabe in its children: nullable
> > child column is not market: marked
> > with adding: by adding
> > and restart: and restarting
> > the problem columns: the problematic columns
> > 9.5, 9.6, 10: 9.5, 9.6, and 10
> > restore, that will cause error.: restore phase of pg_upgrade, that would cause an error.


Thanks. Typo fixed.

>
> I agree that we should do better in pg_upgrade for HEAD and
> REL_10_STABLE as it is not cool to fail late in the upgrade process
> especially if you are using the --link mode.
>
> + *  Currently pg_upgrade will fail if there are any inconsistencies in NOT NULL
> + *  constraint inheritance: In Postgres version 9.5, 9.6, 10 we can
> have a column
> + *  that is NOT NULL in parent, but nullabe in its children. But during schema
> + *  restore, that will cause error.
> On top of the multiple typos here, there is no need to mention
> anything other than "PostgreSQL 9.6 and older versions did not add NOT
> NULL constraints when a primary key was added to to a parent, so check
> for inconsistent NOT NULL constraints in inheritance trees".


In my case, my database becomes like that because accidental ALTER
COLUMN x DROP NOT NULL, and no, not the Primary Key.

Something like this:

CREATE TABLE parent (id SERIAL PRIMARY KEY, name VARCHAR(52) NOT NULL);
CREATE TABLE child () INHERITS (parent);
ALTER TABLE child ALTER COLUMN name DROP NOT NULL;

And yes, in current version 10 (and HEAD), we can still do that.

Because both version currently accepts that inconsistency, ideally
pg_upgrade must be able to upgrade the cluster without problem. Hence
the comment: "Currently pg_upgrade will fail if there are any
inconsistencies in NOT NULL constraint inheritance".

> You seem to take a path similar to what is done with the jsonb check.
> Why not. I would find cleaner to tell also the user about using ALTER
> TABLE to add the proper constraints.
>
> Note that I have not checked or tested the query in details, but
> reading through the thing looks basically correct as it handles
> inheritance chains with WITH RECURSIVE.

Any suggestion to make it more readable? Maybe by separating column
query with schema & table name by using another CTE?

> @@ -99,6 +100,7 @@ check_and_dump_old_cluster(bool live_check)
>      check_for_prepared_transactions(&old_cluster);
>      check_for_reg_data_type_usage(&old_cluster);
>      check_for_isn_and_int8_passing_mismatch(&old_cluster);
> +    check_for_not_null_inheritance(&old_cluster);
> The check needs indeed to happen in check_and_dump_old_cluster(), but
> there is no point to check for it in servers with version 10 and
> upwards, hence you should make it conditional with GET_MAJOR_VERSION()
> <= 906.

No, currently it can happen again in version 10 (and above) because of
the problem above.

By the way, should i add this patch to the current commitfest?

Thanks,
Ali Akbar


Attachments:

  [text/x-patch] pg_upgrade_check_not_null-v2.patch (4.0K, ../../CACQjQLqoXTzCV4+-s1XOT62tY8ggkZkH4kY03gm2aQYOMXE+SA@mail.gmail.com/2-pg_upgrade_check_not_null-v2.patch)
  download | inline diff:
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 1b9083597c..0bd2cd0ed1 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -26,6 +26,7 @@ static void check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster);
 static void check_for_reg_data_type_usage(ClusterInfo *cluster);
 static void check_for_jsonb_9_4_usage(ClusterInfo *cluster);
 static void check_for_pg_role_prefix(ClusterInfo *cluster);
+static void check_for_not_null_inheritance(ClusterInfo *cluster);
 static char *get_canonical_locale_name(int category, const char *locale);
 
 
@@ -99,6 +100,7 @@ check_and_dump_old_cluster(bool live_check)
 	check_for_prepared_transactions(&old_cluster);
 	check_for_reg_data_type_usage(&old_cluster);
 	check_for_isn_and_int8_passing_mismatch(&old_cluster);
+	check_for_not_null_inheritance(&old_cluster);
 
 	/*
 	 * Pre-PG 10 allowed tables with 'unknown' type columns and non WAL logged
@@ -1096,6 +1098,105 @@ check_for_pg_role_prefix(ClusterInfo *cluster)
 	check_ok();
 }
 
+/*
+ * check_for_not_null_inheritance()
+ *
+ *  Currently pg_upgrade will fail if there are any inconsistencies in NOT NULL
+ *  constraint inheritance: In PostgreSQL, we can have a column that is NOT NULL
+ *  in parent, but nullable in its children. So check for inconsistent NOT NULL
+ *  constraints in inheritance tree.
+ */
+static void
+check_for_not_null_inheritance(ClusterInfo *cluster)
+{
+	int			dbnum;
+	FILE	   *script = NULL;
+	bool		found = false;
+	char		output_path[MAXPGPATH];
+
+	prep_status("Checking for NOT NULL inconsistencies in inheritance");
+
+	snprintf(output_path, sizeof(output_path), "not_null_inconsistent_columns.txt");
+
+	for (dbnum = 0; dbnum < cluster->dbarr.ndbs; dbnum++)
+	{
+		PGresult   *res;
+		bool		db_used = false;
+		int			ntups;
+		int			rowno;
+		int			i_nspname,
+					i_relname,
+					i_attname;
+		DbInfo	   *active_db = &cluster->dbarr.dbs[dbnum];
+		PGconn	   *conn = connectToServer(cluster, active_db->db_name);
+
+		res = executeQueryOrDie(conn,
+								"WITH RECURSIVE parents AS ( "
+								"	SELECT	i.inhrelid, i.inhparent "
+								"	FROM	pg_catalog.pg_inherits i "
+								"	UNION ALL "
+								"	SELECT	p.inhrelid, i.inhparent "
+								"   FROM	parents p "
+								"	JOIN	pg_catalog.pg_inherits i "
+								"		ON	i.inhrelid = p.inhparent "
+								") "
+								"SELECT	n.nspname, c.relname, ac.attname  "
+								"FROM	parents p, "
+								"		pg_catalog.pg_attribute ac, "
+								"		pg_catalog.pg_attribute ap, "
+								"		pg_catalog.pg_class c, "
+								"		pg_catalog.pg_namespace n "
+								"WHERE	NOT ac.attnotnull AND "
+								"		ac.attrelid = p.inhrelid AND "
+								"		ap.attrelid = p.inhparent AND "
+								"		ac.attname = ap.attname AND "
+								"		ap.attnotnull AND "
+								"		c.oid = ac.attrelid AND "
+								"		c.relnamespace = n.oid");
+
+		ntups = PQntuples(res);
+		i_nspname = PQfnumber(res, "nspname");
+		i_relname = PQfnumber(res, "relname");
+		i_attname = PQfnumber(res, "attname");
+		for (rowno = 0; rowno < ntups; rowno++)
+		{
+			found = true;
+			if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
+				pg_fatal("could not open file \"%s\": %s\n", output_path,
+						 strerror(errno));
+			if (!db_used)
+			{
+				fprintf(script, "Database: %s\n", active_db->db_name);
+				db_used = true;
+			}
+			fprintf(script, "  %s.%s.%s\n",
+					PQgetvalue(res, rowno, i_nspname),
+					PQgetvalue(res, rowno, i_relname),
+					PQgetvalue(res, rowno, i_attname));
+		}
+
+		PQclear(res);
+
+		PQfinish(conn);
+	}
+
+	if (script)
+		fclose(script);
+
+	if (found)
+	{
+		pg_log(PG_REPORT, "fatal\n");
+		pg_fatal("Your installation contains has inconsistencies in NOT NULL\n"
+				 "constraint inheritance: child column is not marked NOT NULL\n"
+				 "while parent column has NOT NULL constraint. You can fix the\n"
+				 "inconsistency by adding NOT NULL constraint and restarting the\n"
+				 "upgrade.\n"
+				 "A list of the problematic columns is in the file:\n"
+				 "    %s\n\n", output_path);
+	}
+	else
+		check_ok();
+}
 
 /*
  * get_canonical_locale_name


^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2025-07-03 18:53  Alvaro Herrera <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Alvaro Herrera @ 2025-07-03 18:53 UTC (permalink / raw)
  To: Justin Pryzby <[email protected]>; +Cc: Michael Paquier <[email protected]>; Ali Akbar <[email protected]>; Michael Paquier <[email protected]>; Amit Langote <[email protected]>; tushar <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers; [email protected]

Hello,

We seem to have forgotten about this issue.  I think this is even more
pressing with the changes to 18 for not-null constraints, though
strictly speaking it's always been a problem.  Here's an updated patch.
I simplified the query (no need for a recursive CTE as far as I can
tell) and updated to use the pg_upgrade "task" infrastructure.

I think from 18 on, the problem can no longer be recreated, since
removing the attnotnull bit from a child table is not possible.

I should add some testing too before pushing, of course.  I only tested
manually.

I'd like to hear from RMT about getting this in 18.  Actually I think we
should consider backporting to all live versions ... thoughts?

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Investigación es lo que hago cuando no sé lo que estoy haciendo"
(Wernher von Braun)


^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2025-07-03 18:58  Justin Pryzby <[email protected]>
  parent: Alvaro Herrera <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Justin Pryzby @ 2025-07-03 18:58 UTC (permalink / raw)
  To: Alvaro Herrera <[email protected]>; +Cc: Michael Paquier <[email protected]>; Ali Akbar <[email protected]>; Michael Paquier <[email protected]>; Amit Langote <[email protected]>; tushar <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers; [email protected]

> I think from 18 on, the problem can no longer be recreated,

> Actually I think we should consider backporting to all live versions

If you don't backpatch it, there's no point.





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2025-07-03 19:59  Alvaro Herrera <[email protected]>
  parent: Justin Pryzby <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Alvaro Herrera @ 2025-07-03 19:59 UTC (permalink / raw)
  To: Justin Pryzby <[email protected]>; +Cc: Michael Paquier <[email protected]>; Ali Akbar <[email protected]>; Michael Paquier <[email protected]>; Amit Langote <[email protected]>; tushar <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers; [email protected]

On 2025-Jul-03, Justin Pryzby wrote:

> > I think from 18 on, the problem can no longer be recreated,
> 
> > Actually I think we should consider backporting to all live versions
> 
> If you don't backpatch it, there's no point.

Oh yeah, you're absolutely right.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"[PostgreSQL] is a great group; in my opinion it is THE best open source
development communities in existence anywhere."                (Lamar Owen)





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2025-07-03 20:07  Tom Lane <[email protected]>
  parent: Alvaro Herrera <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Tom Lane @ 2025-07-03 20:07 UTC (permalink / raw)
  To: Alvaro Herrera <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Michael Paquier <[email protected]>; Ali Akbar <[email protected]>; Michael Paquier <[email protected]>; Amit Langote <[email protected]>; tushar <[email protected]>; pgsql-hackers; [email protected]

Alvaro Herrera <[email protected]> writes:
> On 2025-Jul-03, Justin Pryzby wrote:
>>> Actually I think we should consider backporting to all live versions

>> If you don't backpatch it, there's no point.

> Oh yeah, you're absolutely right.

No, it'd still be useful for handling upgrades from $busted_version
to current.  But it seems also useful in the back branches, so
+1 for back-patch.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2025-07-04 16:20  Álvaro Herrera <[email protected]>
  parent: Justin Pryzby <[email protected]>
  1 sibling, 0 replies; 27+ messages in thread

From: Álvaro Herrera @ 2025-07-04 16:20 UTC (permalink / raw)
  To: Justin Pryzby <[email protected]>; +Cc: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers; Ali Akbar <[email protected]>

Two more minor things on this:

I found it a bit silly to mention the ALTER command in the error message
and then output only the three-part-qualified name in the output file
(and no specific command to run) -- so for an instant I was tempted to
write the full ALTER TABLE command in the output file instead.  However
I then noticed that check.c does not use fmtId() or fmtQualifiedId()
anywhere, so that code would look out of place in check.c and perhaps
even be a problem for backpatching.  Do people thing it'd be an
improvement to change it that way?

On 2017-Dec-13, Justin Pryzby wrote:

> Here are some spelling and grammar fixes to that patch:
> 
> but nullabe in its children: nullable
> child column is not market: marked
> with adding: by adding
> and restart: and restarting
> the problem columns: the problematic columns
> 9.5, 9.6, 10: 9.5, 9.6, and 10
> restore, that will cause error.: restore phase of pg_upgrade, that would cause an error.

So the phrase "the problem columns" appears in multiple places in
pg_upgrade/check.c.  Should we fix them all?  It turns out that I
rewrote the message as submitted into completely different wording
(without realizing that this had been reported), so these words don't
appear anymore in the new commit.  But we could fix these old ones while
we're here -- yes?  (See below)

My rewrite is this:
       pg_fatal("Your installation contains inconsistent NOT NULL constraints.\n"
                "If the parent column(s) are NOT NULL, then the child column must\n"
                "also be marked NOT NULL, or the upgrade will fail.\n"
                "You can fix this by running\n"
                "  ALTER TABLE tablename ALTER column SET NOT NULL;\n"
                "on each column listed in the file:\n"
                "    %s", report.path);


Here's what a patch looks like to change "problem columns" to
"problematic columns".

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 2106869b999..ad80f00c055 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -119,7 +119,7 @@ static DataTypesUsageChecks data_types_usage_checks[] =
 		gettext_noop("Your installation contains system-defined composite types in user tables.\n"
 					 "These type OIDs are not stable across PostgreSQL versions,\n"
 					 "so this cluster cannot currently be upgraded.  You can drop the\n"
-					 "problem columns and restart the upgrade.\n"),
+					 "problematic columns and restart the upgrade.\n"),
 		.threshold_version = ALL_VERSIONS
 	},
 
@@ -139,7 +139,7 @@ static DataTypesUsageChecks data_types_usage_checks[] =
 					 "This data type changed its internal and input/output format\n"
 					 "between your old and new versions so this\n"
 					 "cluster cannot currently be upgraded.  You can\n"
-					 "drop the problem columns and restart the upgrade.\n"),
+					 "drop the problematic columns and restart the upgrade.\n"),
 		.threshold_version = 903
 	},
 
@@ -183,7 +183,7 @@ static DataTypesUsageChecks data_types_usage_checks[] =
 		gettext_noop("Your installation contains one of the reg* data types in user tables.\n"
 					 "These data types reference system OIDs that are not preserved by\n"
 					 "pg_upgrade, so this cluster cannot currently be upgraded.  You can\n"
-					 "drop the problem columns and restart the upgrade.\n"),
+					 "drop the problematic columns and restart the upgrade.\n"),
 		.threshold_version = ALL_VERSIONS
 	},
 
@@ -200,7 +200,7 @@ static DataTypesUsageChecks data_types_usage_checks[] =
 		gettext_noop("Your installation contains the \"aclitem\" data type in user tables.\n"
 					 "The internal format of \"aclitem\" changed in PostgreSQL version 16\n"
 					 "so this cluster cannot currently be upgraded.  You can drop the\n"
-					 "problem columns and restart the upgrade.\n"),
+					 "problematic columns and restart the upgrade.\n"),
 		.threshold_version = 1500
 	},
 
@@ -223,7 +223,7 @@ static DataTypesUsageChecks data_types_usage_checks[] =
 		.report_text =
 		gettext_noop("Your installation contains the \"unknown\" data type in user tables.\n"
 					 "This data type is no longer allowed in tables, so this cluster\n"
-					 "cannot currently be upgraded.  You can drop the problem columns\n"
+					 "cannot currently be upgraded.  You can drop the problematic columns\n"
 					 "and restart the upgrade.\n"),
 		.threshold_version = 906
 	},
@@ -279,7 +279,7 @@ static DataTypesUsageChecks data_types_usage_checks[] =
 		gettext_noop("Your installation contains the \"abstime\" data type in user tables.\n"
 					 "The \"abstime\" type has been removed in PostgreSQL version 12,\n"
 					 "so this cluster cannot currently be upgraded.  You can drop the\n"
-					 "problem columns, or change them to another data type, and restart\n"
+					 "problematic columns, or change them to another data type, and restart\n"
 					 "the upgrade.\n"),
 		.threshold_version = 1100
 	},
@@ -292,7 +292,7 @@ static DataTypesUsageChecks data_types_usage_checks[] =
 		gettext_noop("Your installation contains the \"reltime\" data type in user tables.\n"
 					 "The \"reltime\" type has been removed in PostgreSQL version 12,\n"
 					 "so this cluster cannot currently be upgraded.  You can drop the\n"
-					 "problem columns, or change them to another data type, and restart\n"
+					 "problematic columns, or change them to another data type, and restart\n"
 					 "the upgrade.\n"),
 		.threshold_version = 1100
 	},
@@ -305,7 +305,7 @@ static DataTypesUsageChecks data_types_usage_checks[] =
 		gettext_noop("Your installation contains the \"tinterval\" data type in user tables.\n"
 					 "The \"tinterval\" type has been removed in PostgreSQL version 12,\n"
 					 "so this cluster cannot currently be upgraded.  You can drop the\n"
-					 "problem columns, or change them to another data type, and restart\n"
+					 "problematic columns, or change them to another data type, and restart\n"
 					 "the upgrade.\n"),
 		.threshold_version = 1100
 	},
@@ -423,7 +423,7 @@ process_data_type_check(DbInfo *dbinfo, PGresult *res, void *arg)
 		pg_log(PG_REPORT, "failed check: %s", _(state->check->status));
 		appendPQExpBuffer(*state->report, "\n%s\n%s    %s\n",
 						  _(state->check->report_text),
-						  _("A list of the problem columns is in the file:"),
+						  _("A list of the problematic columns is in the file:"),
 						  output_path);
 	}
 	state->result = true;

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Ed is the standard text editor."
      http://groups.google.com/group/alt.religion.emacs/msg/8d94ddab6a9b0ad3





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2025-07-04 16:21  Alvaro Herrera <[email protected]>
  parent: Tom Lane <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Alvaro Herrera @ 2025-07-04 16:21 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Michael Paquier <[email protected]>; Ali Akbar <[email protected]>; Michael Paquier <[email protected]>; Amit Langote <[email protected]>; tushar <[email protected]>; pgsql-hackers; [email protected]

On 2025-Jul-03, Tom Lane wrote:

> Alvaro Herrera <[email protected]> writes:
> > On 2025-Jul-03, Justin Pryzby wrote:
> >>> Actually I think we should consider backporting to all live versions
> 
> >> If you don't backpatch it, there's no point.
> 
> > Oh yeah, you're absolutely right.
> 
> No, it'd still be useful for handling upgrades from $busted_version
> to current.  But it seems also useful in the back branches, so
> +1 for back-patch.

Pushed to all branches.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"El miedo atento y previsor es la madre de la seguridad" (E. Burke)





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2025-07-04 16:45  Alvaro Herrera <[email protected]>
  parent: Alvaro Herrera <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Alvaro Herrera @ 2025-07-04 16:45 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Michael Paquier <[email protected]>; Ali Akbar <[email protected]>; Michael Paquier <[email protected]>; tushar <[email protected]>; pgsql-hackers; [email protected]

Hmm, crake doesn't like the fact that there's no regnamespace in 9.4.
Apparently in version 13 we claim to support back to 9.0.  I only tried
with an old server version 12.  Should be an easy fix ...

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2025-07-04 17:19  Alvaro Herrera <[email protected]>
  parent: Alvaro Herrera <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Alvaro Herrera @ 2025-07-04 17:19 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Michael Paquier <[email protected]>; Ali Akbar <[email protected]>; Michael Paquier <[email protected]>; tushar <[email protected]>; pgsql-hackers; [email protected]

On 2025-Jul-04, Alvaro Herrera wrote:

> Hmm, crake doesn't like the fact that there's no regnamespace in 9.4.
> Apparently in version 13 we claim to support back to 9.0.  I only tried
> with an old server version 12.  Should be an easy fix ...

It goes like this.  It's tested with 9.2 only though, as 9.1 doesn't
compile from Git (lack of Flex) and I didn't want to try a tarball.

(This is for master.  17 and back require a small change for lack of
task.c)

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"I apologize for the confusion in my previous responses.
 There appears to be an error."                   (ChatGPT)


^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2025-07-04 17:37  Tom Lane <[email protected]>
  parent: Alvaro Herrera <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Tom Lane @ 2025-07-04 17:37 UTC (permalink / raw)
  To: Alvaro Herrera <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Michael Paquier <[email protected]>; Ali Akbar <[email protected]>; Michael Paquier <[email protected]>; tushar <[email protected]>; pgsql-hackers; [email protected]

Alvaro Herrera <[email protected]> writes:
> On 2025-Jul-04, Alvaro Herrera wrote:
>> Hmm, crake doesn't like the fact that there's no regnamespace in 9.4.
>> Apparently in version 13 we claim to support back to 9.0.  I only tried
>> with an old server version 12.  Should be an easy fix ...

> It goes like this.  It's tested with 9.2 only though, as 9.1 doesn't
> compile from Git (lack of Flex) and I didn't want to try a tarball.

Query works, or at least throws no error, back to 8.4 (the oldest I
have at hand).

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
@ 2025-07-05 08:07  Alvaro Herrera <[email protected]>
  parent: Tom Lane <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Alvaro Herrera @ 2025-07-05 08:07 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Michael Paquier <[email protected]>; Ali Akbar <[email protected]>; Michael Paquier <[email protected]>; tushar <[email protected]>; pgsql-hackers; [email protected]

On 2025-Jul-04, Tom Lane wrote:

> Alvaro Herrera <[email protected]> writes:
> > On 2025-Jul-04, Alvaro Herrera wrote:
> >> Hmm, crake doesn't like the fact that there's no regnamespace in 9.4.
> >> Apparently in version 13 we claim to support back to 9.0.  I only tried
> >> with an old server version 12.  Should be an easy fix ...
> 
> > It goes like this.  It's tested with 9.2 only though, as 9.1 doesn't
> > compile from Git (lack of Flex) and I didn't want to try a tarball.
> 
> Query works, or at least throws no error, back to 8.4 (the oldest I
> have at hand).

Thanks for checking!  Pushed late yesterday, crake is happy now.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"Ninguna manada de bestias tiene una voz tan horrible como la humana" (Orual)





^ permalink  raw  reply  [nested|flat] 27+ messages in thread


end of thread, other threads:[~2025-07-05 08:07 UTC | newest]

Thread overview: 27+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-07-26 10:09 pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL tushar <[email protected]>
2017-07-26 12:05 ` Michael Paquier <[email protected]>
2017-07-26 14:02   ` Tom Lane <[email protected]>
2017-07-26 14:50     ` Michael Paquier <[email protected]>
2017-07-27 14:58       ` Michael Paquier <[email protected]>
2017-08-04 15:50         ` Tom Lane <[email protected]>
2017-08-04 16:06           ` Michael Paquier <[email protected]>
2017-12-13 00:22             ` Ali Akbar <[email protected]>
2017-12-13 00:26               ` Ali Akbar <[email protected]>
2017-12-13 01:41               ` Amit Langote <[email protected]>
2017-12-13 02:10                 ` Michael Paquier <[email protected]>
2017-12-13 06:59                   ` Ali Akbar <[email protected]>
2017-12-13 08:37                     ` Amit Langote <[email protected]>
2017-12-14 01:51                       ` Ali Akbar <[email protected]>
2017-12-14 05:49                         ` Justin Pryzby <[email protected]>
2017-12-14 08:08                           ` Michael Paquier <[email protected]>
2017-12-14 12:18                             ` Ali Akbar <[email protected]>
2025-07-04 16:20                           ` Álvaro Herrera <[email protected]>
2025-07-03 18:53 Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL Alvaro Herrera <[email protected]>
2025-07-03 18:58 ` Justin Pryzby <[email protected]>
2025-07-03 19:59   ` Alvaro Herrera <[email protected]>
2025-07-03 20:07     ` Tom Lane <[email protected]>
2025-07-04 16:21       ` Alvaro Herrera <[email protected]>
2025-07-04 16:45         ` Alvaro Herrera <[email protected]>
2025-07-04 17:19           ` Alvaro Herrera <[email protected]>
2025-07-04 17:37             ` Tom Lane <[email protected]>
2025-07-05 08:07               ` Alvaro Herrera <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox