I wrote:
> ... I propose stripping out gram.y's special
> hack for this, and preserving the efficiency of the case by
> inserting a test very much later to see if the expression is just
> a NULL constant. Maybe AddRelationRawConstraints is the right place.
I did this (see attached patch) and got an interesting failure in the
domain regression test. That test has
create domain ddef1 int4 DEFAULT 3;
create table defaulttest
...
, col5 ddef1 NOT NULL DEFAULT NULL
...
insert into defaulttest default values;
and the 'expected' result is that this succeeds and inserts 3; meaning
that the domain default overrides the explicit per-column specification.
But with the patch it fails with "null value in column "col5" violates
not-null constraint".
AFAICS this is a flat-out bug too, since the per-column default should
override the domain's default; that's certainly how it works for any
non-null column default value. But I wasn't expecting any regression
failures with this patch. Is it OK to change this behavior? Should I
back-patch, or not?
(BTW, the reason this is exposed is that when a domain is involved, the
apparently plain NULL is really a CoerceToDomain operation, which the
new test sees as not being the same as a plain null constant; correctly
so, if you ask me, since CoerceToDomain might or might not reject a
null.)
regards, tom lane
Index: src/backend/catalog/heap.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/catalog/heap.c,v
retrieving revision 1.324
diff -c -r1.324 heap.c
*** src/backend/catalog/heap.c 12 Oct 2007 18:55:11 -0000 1.324
--- src/backend/catalog/heap.c 28 Oct 2007 21:04:59 -0000
***************
*** 1722,1727 ****
--- 1722,1736 ----
atp->atttypid,
atp->atttypmod,
NameStr(atp->attname));
+ /*
+ * If the expression is just a NULL constant, we do not bother
+ * to make an explicit pg_attrdef entry, since the default
behavior
+ * is equivalent.
+ */
+ if (expr == NULL ||
+ (IsA(expr, Const) && ((Const *) expr)->constisnull))
+ continue;
+
StoreAttrDefault(rel, colDef->attnum, nodeToString(expr));
cooked = (CookedConstraint *) palloc(sizeof(CookedConstraint));
Index: src/backend/commands/typecmds.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/typecmds.c,v
retrieving revision 1.108
diff -c -r1.108 typecmds.c
*** src/backend/commands/typecmds.c 29 Sep 2007 17:18:58 -0000 1.108
--- src/backend/commands/typecmds.c 28 Oct 2007 21:04:59 -0000
***************
*** 765,784 ****
domainName);
/*
! * Expression must be stored as a
nodeToString result, but
! * we also require a valid textual
representation (mainly
! * to make life easier for pg_dump).
*/
! defaultValue =
! deparse_expression(defaultExpr,
!
deparse_context_for(domainName,
!
InvalidOid),
!
false, false);
! defaultValueBin =
nodeToString(defaultExpr);
}
else
{
! /* DEFAULT NULL is same as not having a
default */
defaultValue = NULL;
defaultValueBin = NULL;
}
--- 765,798 ----
domainName);
/*
! * If the expression is just a NULL
constant, we treat
! * it like not having a default.
*/
! if (defaultExpr == NULL ||
! (IsA(defaultExpr, Const) &&
! ((Const *)
defaultExpr)->constisnull))
! {
! defaultValue = NULL;
! defaultValueBin = NULL;
! }
! else
! {
! /*
! * Expression must be stored as
a nodeToString result,
! * but we also require a valid
textual representation
! * (mainly to make life easier
for pg_dump).
! */
! defaultValue =
!
deparse_expression(defaultExpr,
!
deparse_context_for(domainName,
!
InvalidOid),
!
false, false);
! defaultValueBin =
nodeToString(defaultExpr);
! }
}
else
{
! /* No default (can this still happen?)
*/
defaultValue = NULL;
defaultValueBin = NULL;
}
***************
*** 1443,1449 ****
MemSet(new_record_nulls, ' ', sizeof(new_record_nulls));
MemSet(new_record_repl, ' ', sizeof(new_record_repl));
! /* Store the new default, if null then skip this step */
if (defaultRaw)
{
/* Create a dummy ParseState for transformExpr */
--- 1457,1463 ----
MemSet(new_record_nulls, ' ', sizeof(new_record_nulls));
MemSet(new_record_repl, ' ', sizeof(new_record_repl));
! /* Store the new default into the tuple */
if (defaultRaw)
{
/* Create a dummy ParseState for transformExpr */
***************
*** 1459,1488 ****
NameStr(typTup->typname));
/*
! * Expression must be stored as a nodeToString result, but we
also
! * require a valid textual representation (mainly to make life
easier
! * for pg_dump).
*/
! defaultValue = deparse_expression(defaultExpr,
deparse_context_for(NameStr(typTup->typname),
InvalidOid),
false, false);
! /*
! * Form an updated tuple with the new default and write it back.
! */
! new_record[Anum_pg_type_typdefaultbin - 1] =
DirectFunctionCall1(textin,
!
CStringGetDatum(
!
nodeToString(defaultExpr)));
! new_record_repl[Anum_pg_type_typdefaultbin - 1] = 'r';
! new_record[Anum_pg_type_typdefault - 1] =
DirectFunctionCall1(textin,
CStringGetDatum(defaultValue));
! new_record_repl[Anum_pg_type_typdefault - 1] = 'r';
}
else
- /* Default is NULL, drop it */
{
new_record_nulls[Anum_pg_type_typdefaultbin - 1] = 'n';
new_record_repl[Anum_pg_type_typdefaultbin - 1] = 'r';
new_record_nulls[Anum_pg_type_typdefault - 1] = 'n';
--- 1473,1517 ----
NameStr(typTup->typname));
/*
! * If the expression is just a NULL constant, we treat the
command
! * like ALTER ... DROP DEFAULT.
*/
! if (defaultExpr == NULL ||
! (IsA(defaultExpr, Const) && ((Const *)
defaultExpr)->constisnull))
! {
! /* Default is NULL, drop it */
! new_record_nulls[Anum_pg_type_typdefaultbin - 1] = 'n';
! new_record_repl[Anum_pg_type_typdefaultbin - 1] = 'r';
! new_record_nulls[Anum_pg_type_typdefault - 1] = 'n';
! new_record_repl[Anum_pg_type_typdefault - 1] = 'r';
! }
! else
! {
! /*
! * Expression must be stored as a nodeToString result,
but we also
! * require a valid textual representation (mainly to
make life
! * easier for pg_dump).
! */
! defaultValue = deparse_expression(defaultExpr,
deparse_context_for(NameStr(typTup->typname),
InvalidOid),
false, false);
! /*
! * Form an updated tuple with the new default and write
it back.
! */
! new_record[Anum_pg_type_typdefaultbin - 1] =
DirectFunctionCall1(textin,
!
CStringGetDatum(nodeToString(defaultExpr)));
! new_record_repl[Anum_pg_type_typdefaultbin - 1] = 'r';
! new_record[Anum_pg_type_typdefault - 1] =
DirectFunctionCall1(textin,
CStringGetDatum(defaultValue));
! new_record_repl[Anum_pg_type_typdefault - 1] = 'r';
! }
}
else
{
+ /* ALTER ... DROP DEFAULT */
new_record_nulls[Anum_pg_type_typdefaultbin - 1] = 'n';
new_record_repl[Anum_pg_type_typdefaultbin - 1] = 'r';
new_record_nulls[Anum_pg_type_typdefault - 1] = 'n';
Index: src/backend/parser/gram.y
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.603
diff -c -r2.603 gram.y
*** src/backend/parser/gram.y 24 Sep 2007 01:29:28 -0000 2.603
--- src/backend/parser/gram.y 28 Oct 2007 21:05:00 -0000
***************
*** 1685,1698 ****
;
alter_column_default:
! SET DEFAULT a_expr
! {
! /* Treat SET DEFAULT NULL the same as
DROP DEFAULT */
! if (exprIsNullConstant($3))
! $$ = NULL;
! else
! $$ = $3;
! }
| DROP DEFAULT { $$ = NULL; }
;
--- 1685,1691 ----
;
alter_column_default:
! SET DEFAULT a_expr { $$ = $3; }
| DROP DEFAULT { $$ = NULL; }
;
***************
*** 2080,2094 ****
Constraint *n = makeNode(Constraint);
n->contype = CONSTR_DEFAULT;
n->name = NULL;
! if (exprIsNullConstant($2))
! {
! /* DEFAULT NULL should be
reported as empty expr */
! n->raw_expr = NULL;
! }
! else
! {
! n->raw_expr = $2;
! }
n->cooked_expr = NULL;
n->keys = NULL;
n->indexspace = NULL;
--- 2073,2079 ----
Constraint *n = makeNode(Constraint);
n->contype = CONSTR_DEFAULT;
n->name = NULL;
! n->raw_expr = $2;
n->cooked_expr = NULL;
n->keys = NULL;
n->indexspace = NULL;
***************
*** 9763,9785 ****
QueryIsRule = FALSE;
}
- /* exprIsNullConstant()
- * Test whether an a_expr is a plain NULL constant or not.
- */
- bool
- exprIsNullConstant(Node *arg)
- {
- if (arg && IsA(arg, A_Const))
- {
- A_Const *con = (A_Const *) arg;
-
- if (con->val.type == T_Null &&
- con->typename == NULL)
- return TRUE;
- }
- return FALSE;
- }
-
/* doNegate()
* Handle negation of a numeric constant.
*
--- 9748,9753 ----
Index: src/backend/parser/parse_expr.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/parse_expr.c,v
retrieving revision 1.221
diff -c -r1.221 parse_expr.c
*** src/backend/parser/parse_expr.c 23 Jun 2007 22:12:51 -0000 1.221
--- src/backend/parser/parse_expr.c 28 Oct 2007 21:05:00 -0000
***************
*** 606,611 ****
--- 606,626 ----
return (Node *) param;
}
+ /* Test whether an a_expr is a plain NULL constant or not */
+ static bool
+ exprIsNullConstant(Node *arg)
+ {
+ if (arg && IsA(arg, A_Const))
+ {
+ A_Const *con = (A_Const *) arg;
+
+ if (con->val.type == T_Null &&
+ con->typename == NULL)
+ return true;
+ }
+ return false;
+ }
+
static Node *
transformAExprOp(ParseState *pstate, A_Expr *a)
{
Index: src/backend/parser/parse_utilcmd.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/parse_utilcmd.c,v
retrieving revision 2.3
diff -c -r2.3 parse_utilcmd.c
*** src/backend/parser/parse_utilcmd.c 27 Aug 2007 03:36:08 -0000 2.3
--- src/backend/parser/parse_utilcmd.c 28 Oct 2007 21:05:00 -0000
***************
*** 440,446 ****
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("multiple
default values specified for column \"%s\" of table \"%s\"",
column->colname, cxt->relation->relname)));
- /* Note: DEFAULT NULL maps to
constraint->raw_expr == NULL */
column->raw_default = constraint->raw_expr;
Assert(constraint->cooked_expr == NULL);
saw_default = true;
--- 440,445 ----
Index: src/include/parser/gramparse.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/parser/gramparse.h,v
retrieving revision 1.38
diff -c -r1.38 gramparse.h
*** src/include/parser/gramparse.h 5 Jan 2007 22:19:56 -0000 1.38
--- src/include/parser/gramparse.h 28 Oct 2007 21:05:00 -0000
***************
*** 54,59 ****
extern int base_yyparse(void);
extern List *SystemFuncName(char *name);
extern TypeName *SystemTypeName(char *name);
- extern bool exprIsNullConstant(Node *arg);
#endif /* GRAMPARSE_H */
--- 54,58 ----
---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?
http://www.postgresql.org/docs/faq
