mirrors/postgres
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Remove the hack in the grammar that "optimized away" DEFAULT NULL clauses.

Browse Source 
Instead put in a test to drop a NULL default at the last moment before
storing the catalog entry.  This changes the behavior in a couple of ways:
* Specifying DEFAULT NULL when creating an inheritance child table will
  successfully suppress inheritance of any default expression from the
  parent's column, where formerly it failed to do so.
* Specifying DEFAULT NULL for a column of a domain type will correctly
  override any default belonging to the domain; likewise for a sub-domain.
The latter change happens because by the time the clause is checked,
it won't be a simple null Const but a CoerceToDomain expression.

Personally I think this should be back-patched, but there doesn't seem to
be consensus for that on pgsql-hackers, so refraining.
This commit is contained in:
Tom Lane 2007-10-29 19:40:40 +00:00
parent bf5ccf382c
commit b17b7fae8c
8 changed files with 115 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/backend/catalog/heap.c
View File

@ -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));

88
src/backend/commands/typecmds.c
View File

@ -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';

38
src/backend/parser/gram.y
View File

@ -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.
*

17
src/backend/parser/parse_expr.c
View File

@ -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)
{

3
src/backend/parser/parse_utilcmd.c
View File

@ -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;

3
src/include/parser/gramparse.h
View File

@ -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 */

10
src/test/regress/expected/domain.out
View File

@ -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

8
src/test/regress/sql/domain.sql
View File

@ -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;