On Sat, Aug 31, 2024 at 11:59 AM Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote: > > Hello > > Here I present another attempt at making not-null constraints be > catalogued. This is largely based on the code reverted at 9ce04b50e120, > except that we now have a not-null constraint automatically created for > every column of a primary key, and such constraint cannot be removed > while the PK exists. Thanks to this, a lot of rather ugly code is gone, > both in pg_dump and in backend -- in particular the handling of NO > INHERIT, which was needed for pg_dump. > > Noteworthy psql difference: because there are now even more not-null > constraints than before, the \d+ display would be far too noisy if we > just let it grow. So here I've made it omit any constraints that > underlie the primary key. This should be OK since you can't do much > with those constraints while the PK is still there. If you drop the PK, > the next \d+ will show those constraints. >
hi. my brief review. create table t1(a int, b int, c int not null, primary key(a, b)); \d+ t1 ERROR: operator is not unique: smallint[] <@ smallint[] LINE 8: coalesce(NOT ARRAY[at.attnum] <@ (SELECT conkey FROM pg_cata... ^ HINT: Could not choose a best candidate operator. You might need to add explicit type casts. the regression test still passed, i have no idea why. anyway, the following changes make the above ERROR disappear. also seems more lean. printfPQExpBuffer(&buf, /* FIXME the coalesce trick looks silly. What's a better way? */ "SELECT co.conname, at.attname, co.connoinherit, co.conislocal,\n" "co.coninhcount <> 0\n" "FROM pg_catalog.pg_constraint co JOIN\n" "pg_catalog.pg_attribute at ON\n" "(at.attrelid = co.conrelid AND at.attnum = co.conkey[1])\n" "WHERE co.contype = 'n' AND\n" "co.conrelid = '%s'::pg_catalog.regclass AND\n" "coalesce(NOT ARRAY[at.attnum] <@ (SELECT conkey FROM pg_catalog.pg_constraint\n" " WHERE contype = 'p' AND conrelid = '%s'::regclass), true)\n" "ORDER BY at.attnum", oid, oid); change to printfPQExpBuffer(&buf, "SELECT co.conname, at.attname, co.connoinherit, co.conislocal,\n" "co.coninhcount <> 0\n" "FROM pg_catalog.pg_constraint co JOIN\n" "pg_catalog.pg_attribute at ON\n" "(at.attrelid = co.conrelid AND at.attnum = co.conkey[1])\n" "WHERE co.contype = 'n' AND\n" "co.conrelid = '%s'::pg_catalog.regclass AND\n" "NOT EXISTS (SELECT 1 FROM pg_catalog.pg_constraint co1 where co1.contype = 'p'\n" "AND at.attnum = any(co1.conkey) AND co1.conrelid = '%s'::pg_catalog.regclass)\n" "ORDER BY at.attnum", oid, oid); steal idea from https://stackoverflow.com/a/75614278/15603477 ============ create type comptype as (r float8, i float8); create domain dcomptype1 as comptype not null no inherit; with cte as ( SELECT oid, conrelid::regclass, conname FROM pg_catalog.pg_constraint where contypid in ('dcomptype1'::regtype)) select pg_get_constraintdef(oid) from cte; current output is NOT NULL but it's not the same as CREATE TABLE ATACC1 (a int, not null a no inherit); with cte as ( SELECT oid, conrelid::regclass, conname FROM pg_catalog.pg_constraint where conrelid in ('ATACC1'::regclass)) select pg_get_constraintdef(oid) from cte; NOT NULL a NO INHERIT i don't really sure the meaning of "on inherit" in "create domain dcomptype1 as comptype not null no inherit;" ==================== bold idea. print out the constraint name: violates not-null constraint \"%s\" for the following code: ereport(ERROR, (errcode(ERRCODE_NOT_NULL_VIOLATION), errmsg("null value in column \"%s\" of relation \"%s\" violates not-null constraint", NameStr(att->attname), RelationGetRelationName(orig_rel)), val_desc ? errdetail("Failing row contains %s.", val_desc) : 0, errtablecol(orig_rel, attrChk))); ==================== in extractNotNullColumn we can Assert(colnum > 0); create table t3(a int , b int , c int ,not null a, not null c, not null b, not null tableoid); this should not be allowed? foreach(lc, RelationGetNotNullConstraints(RelationGetRelid(relation), false)) { AlterTableCmd *atsubcmd; atsubcmd = makeNode(AlterTableCmd); atsubcmd->subtype = AT_AddConstraint; atsubcmd->def = (Node *) lfirst_node(Constraint, lc); atsubcmds = lappend(atsubcmds, atsubcmd); } forgive me for being hypocritical. I guess this is not a good coding pattern. one reason would be: if you do: = list *a = RelationGetNotNullConstraints(RelationGetRelid(relation), false); foreach(lc, a) = then you can call pprint(a). + /* + * If INCLUDING INDEXES is not given and a primary key exists, we need to + * add not-null constraints to the columns covered by the PK (except those + * that already have one.) This is required for backwards compatibility. + */ + if ((table_like_clause->options & CREATE_TABLE_LIKE_INDEXES) == 0) + { + Bitmapset *pkcols; + int x = -1; + Bitmapset *donecols = NULL; + ListCell *lc; + + /* + * Obtain a bitmapset of columns on which we'll add not-null + * constraints in expandTableLikeClause, so that we skip this for + * those. + */ + foreach(lc, RelationGetNotNullConstraints(RelationGetRelid(relation), true)) + { + CookedConstraint *cooked = (CookedConstraint *) lfirst(lc); + + donecols = bms_add_member(donecols, cooked->attnum); + } + + pkcols = RelationGetIndexAttrBitmap(relation, + INDEX_ATTR_BITMAP_PRIMARY_KEY); + while ((x = bms_next_member(pkcols, x)) >= 0) + { + Constraint *notnull; + AttrNumber attnum = x + FirstLowInvalidHeapAttributeNumber; + String *colname; + Form_pg_attribute attForm; + + /* ignore if we already have one for this column */ + if (bms_is_member(attnum, donecols)) + continue; + + attForm = TupleDescAttr(tupleDesc, attnum - 1); + colname = makeString(pstrdup(NameStr(attForm->attname))); + notnull = makeNotNullConstraint(colname); + + cxt->nnconstraints = lappend(cxt->nnconstraints, notnull); + } + } this part (" if (bms_is_member(attnum, donecols))" will always be true? donecols is all not-null attnums, pkcols is pk not-null attnums. so pkcols info will always be included in donecols. i placed a "elog(INFO, "%s we are in", __func__);" above "attForm = TupleDescAttr(tupleDesc, attnum - 1);" all regression tests still passed.