diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index ea985a635d..d22bb77a50 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.324 2007/10/12 18:55:11 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.325 2007/10/29 19:40:39 tgl Exp $ * * * INTERFACE ROUTINES @@ -1722,6 +1722,21 @@ AddRelationRawConstraints(Relation rel, 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. + * + * Note a nonobvious property of this test: if the column is of a + * domain type, what we'll get is not a bare null Const but a + * CoerceToDomain expr, so we will not discard the default. This is + * critical because the column default needs to be retained to + * override any default that the domain might have. + */ + if (expr == NULL || + (IsA(expr, Const) && ((Const *) expr)->constisnull)) + continue; + StoreAttrDefault(rel, colDef->attnum, nodeToString(expr)); cooked = (CookedConstraint *) palloc(sizeof(CookedConstraint)); diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 75a8b7530e..1f58d989f2 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.108 2007/09/29 17:18:58 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.109 2007/10/29 19:40:39 tgl Exp $ * * DESCRIPTION * The "DefineFoo" routines take the parse tree and pick out the @@ -765,20 +765,40 @@ DefineDomain(CreateDomainStmt *stmt) 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). + * If the expression is just a NULL constant, we treat + * it like not having a default. + * + * Note that if the basetype is another domain, we'll see + * a CoerceToDomain expr here and not discard the default. + * This is critical because the domain default needs to be + * retained to override any default that the base domain + * might have. */ - defaultValue = - deparse_expression(defaultExpr, - deparse_context_for(domainName, - InvalidOid), - false, false); - defaultValueBin = nodeToString(defaultExpr); + 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 { - /* DEFAULT NULL is same as not having a default */ + /* No default (can this still happen?) */ defaultValue = NULL; defaultValueBin = NULL; } @@ -1443,7 +1463,7 @@ AlterDomainDefault(List *names, Node *defaultRaw) 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 */ + /* Store the new default into the tuple */ if (defaultRaw) { /* Create a dummy ParseState for transformExpr */ @@ -1459,30 +1479,46 @@ AlterDomainDefault(List *names, Node *defaultRaw) 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). + * If the expression is just a NULL constant, we treat the command + * like ALTER ... DROP DEFAULT. (But see note for same test in + * DefineDomain.) */ - defaultValue = deparse_expression(defaultExpr, + 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))); + /* + * 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, + 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'; + new_record_repl[Anum_pg_type_typdefault - 1] = 'r'; + } } else - /* Default is NULL, drop it */ { + /* 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'; diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 1452fe7ff2..f9f7a49a31 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -11,7 +11,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.603 2007/09/24 01:29:28 adunstan Exp $ + * $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.604 2007/10/29 19:40:39 tgl Exp $ * * HISTORY * AUTHOR DATE MAJOR EVENT @@ -1685,14 +1685,7 @@ alter_rel_cmd: ; alter_column_default: - SET DEFAULT a_expr - { - /* Treat SET DEFAULT NULL the same as DROP DEFAULT */ - if (exprIsNullConstant($3)) - $$ = NULL; - else - $$ = $3; - } + SET DEFAULT a_expr { $$ = $3; } | DROP DEFAULT { $$ = NULL; } ; @@ -2080,15 +2073,7 @@ ColConstraintElem: 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->raw_expr = $2; n->cooked_expr = NULL; n->keys = NULL; n->indexspace = NULL; @@ -9763,23 +9748,6 @@ parser_init(void) 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. * diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 754e18d687..86fddc4a7a 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.221 2007/06/23 22:12:51 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.222 2007/10/29 19:40:40 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -606,6 +606,21 @@ transformParamRef(ParseState *pstate, ParamRef *pref) 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) { diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 54e7dfdb16..287e82ddee 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -19,7 +19,7 @@ * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/backend/parser/parse_utilcmd.c,v 2.3 2007/08/27 03:36:08 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/parser/parse_utilcmd.c,v 2.4 2007/10/29 19:40:40 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -440,7 +440,6 @@ transformColumnDefinition(ParseState *pstate, CreateStmtContext *cxt, (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; diff --git a/src/include/parser/gramparse.h b/src/include/parser/gramparse.h index ee6be87c02..34f47a9bea 100644 --- a/src/include/parser/gramparse.h +++ b/src/include/parser/gramparse.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/parser/gramparse.h,v 1.38 2007/01/05 22:19:56 momjian Exp $ + * $PostgreSQL: pgsql/src/include/parser/gramparse.h,v 1.39 2007/10/29 19:40:40 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -54,6 +54,5 @@ extern void parser_init(void); extern int base_yyparse(void); extern List *SystemFuncName(char *name); extern TypeName *SystemTypeName(char *name); -extern bool exprIsNullConstant(Node *arg); #endif /* GRAMPARSE_H */ diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out index c7d5b50334..b951ce8caa 100644 --- a/src/test/regress/expected/domain.out +++ b/src/test/regress/expected/domain.out @@ -205,7 +205,15 @@ create table defaulttest , col8 ddef5 ); NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "defaulttest_pkey" for table "defaulttest" -insert into defaulttest default values; +insert into defaulttest(col4) values(0); -- fails, col5 defaults to null +ERROR: null value in column "col5" violates not-null constraint +alter table defaulttest alter column col5 drop default; +insert into defaulttest default values; -- succeeds, inserts domain default +-- We used to treat SET DEFAULT NULL as equivalent to DROP DEFAULT; wrong +alter table defaulttest alter column col5 set default null; +insert into defaulttest(col4) values(0); -- fails +ERROR: null value in column "col5" violates not-null constraint +alter table defaulttest alter column col5 drop default; insert into defaulttest default values; insert into defaulttest default values; -- Test defaults with copy diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql index 7da9971991..7a4ec383d2 100644 --- a/src/test/regress/sql/domain.sql +++ b/src/test/regress/sql/domain.sql @@ -168,7 +168,13 @@ create table defaulttest , col7 ddef4 DEFAULT 8000 , col8 ddef5 ); -insert into defaulttest default values; +insert into defaulttest(col4) values(0); -- fails, col5 defaults to null +alter table defaulttest alter column col5 drop default; +insert into defaulttest default values; -- succeeds, inserts domain default +-- We used to treat SET DEFAULT NULL as equivalent to DROP DEFAULT; wrong +alter table defaulttest alter column col5 set default null; +insert into defaulttest(col4) values(0); -- fails +alter table defaulttest alter column col5 drop default; insert into defaulttest default values; insert into defaulttest default values;