Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit4da65a2

Browse files
committed
Code review for CREATE OR REPLACE VIEW patch. Do things in a saner order to
result in hopefully-less-confusing error messages when the new definitionisn't compatible with the old; minor other cleanup.
1 parent78b25fd commit4da65a2

File tree

4 files changed

+48
-38
lines changed

4 files changed

+48
-38
lines changed

‎doc/src/sgml/ref/create_view.sgml

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<!--
2-
$PostgreSQL: pgsql/doc/src/sgml/ref/create_view.sgml,v 1.38 2008/12/06 23:22:46 momjian Exp $
2+
$PostgreSQL: pgsql/doc/src/sgml/ref/create_view.sgml,v 1.39 2008/12/15 21:35:31 tgl Exp $
33
PostgreSQL documentation
44
-->
55

@@ -38,9 +38,10 @@ CREATE [ OR REPLACE ] [ TEMP | TEMPORARY ] VIEW <replaceable class="PARAMETER">n
3838
<para>
3939
<command>CREATE OR REPLACE VIEW</command> is similar, but if a view
4040
of the same name already exists, it is replaced. The new query must
41-
generate all of the same columns that were generated by the original query
42-
in the same order and with the same data types, but may add additional
43-
columns to the end of the list.
41+
generate the same columns that were generated by the existing view query
42+
(that is, the same column names in the same order and with the same data
43+
types), but it may add additional columns to the end of the list. The
44+
calculations giving rise to the output columns may be completely different.
4445
</para>
4546

4647
<para>
@@ -77,7 +78,7 @@ CREATE [ OR REPLACE ] [ TEMP | TEMPORARY ] VIEW <replaceable class="PARAMETER">n
7778
</para>
7879
</listitem>
7980
</varlistentry>
80-
81+
8182
<varlistentry>
8283
<term><replaceable class="parameter">name</replaceable></term>
8384
<listitem>
@@ -164,7 +165,7 @@ CREATE VIEW comedies AS
164165
</programlisting>
165166
</para>
166167
</refsect1>
167-
168+
168169
<refsect1>
169170
<title>Compatibility</title>
170171

‎src/backend/commands/tablecmds.c

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.273 2008/12/13 19:13:44 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.274 2008/12/15 21:35:31 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -3459,9 +3459,7 @@ ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
34593459
attrdesc;
34603460
HeapTuplereltup;
34613461
FormData_pg_attributeattribute;
3462-
inti;
3463-
intminattnum,
3464-
maxatts;
3462+
intnewattnum;
34653463
charrelkind;
34663464
HeapTupletypeTuple;
34673465
OidtypeOid;
@@ -3520,6 +3518,7 @@ ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
35203518
0,0,0);
35213519
if (!HeapTupleIsValid(reltup))
35223520
elog(ERROR,"cache lookup failed for relation %u",myrelid);
3521+
relkind= ((Form_pg_class)GETSTRUCT(reltup))->relkind;
35233522

35243523
/*
35253524
* this test is deliberately not attisdropped-aware, since if one tries to
@@ -3534,15 +3533,12 @@ ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
35343533
errmsg("column \"%s\" of relation \"%s\" already exists",
35353534
colDef->colname,RelationGetRelationName(rel))));
35363535

3537-
minattnum= ((Form_pg_class)GETSTRUCT(reltup))->relnatts;
3538-
relkind= ((Form_pg_class)GETSTRUCT(reltup))->relkind;
3539-
maxatts=minattnum+1;
3540-
if (maxatts>MaxHeapAttributeNumber)
3536+
newattnum= ((Form_pg_class)GETSTRUCT(reltup))->relnatts+1;
3537+
if (newattnum>MaxHeapAttributeNumber)
35413538
ereport(ERROR,
35423539
(errcode(ERRCODE_TOO_MANY_COLUMNS),
35433540
errmsg("tables can have at most %d columns",
35443541
MaxHeapAttributeNumber)));
3545-
i=minattnum+1;
35463542

35473543
typeTuple=typenameType(NULL,colDef->typename,&typmod);
35483544
tform= (Form_pg_type)GETSTRUCT(typeTuple);
@@ -3551,14 +3547,15 @@ ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
35513547
/* make sure datatype is legal for a column */
35523548
CheckAttributeType(colDef->colname,typeOid);
35533549

3550+
/* construct new attribute's pg_attribute entry */
35543551
attribute.attrelid=myrelid;
35553552
namestrcpy(&(attribute.attname),colDef->colname);
35563553
attribute.atttypid=typeOid;
35573554
attribute.attstattarget=-1;
35583555
attribute.attlen=tform->typlen;
35593556
attribute.attcacheoff=-1;
35603557
attribute.atttypmod=typmod;
3561-
attribute.attnum=i;
3558+
attribute.attnum=newattnum;
35623559
attribute.attbyval=tform->typbyval;
35633560
attribute.attndims=list_length(colDef->typename->arrayBounds);
35643561
attribute.attstorage=tform->typstorage;
@@ -3578,7 +3575,7 @@ ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
35783575
/*
35793576
* Update number of attributes in pg_class tuple
35803577
*/
3581-
((Form_pg_class)GETSTRUCT(reltup))->relnatts=maxatts;
3578+
((Form_pg_class)GETSTRUCT(reltup))->relnatts=newattnum;
35823579

35833580
simple_heap_update(pgclass,&reltup->t_self,reltup);
35843581

@@ -3635,9 +3632,13 @@ ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
36353632
* returned by AddRelationNewConstraints, so that the right thing happens
36363633
* when a datatype's default applies.
36373634
*
3638-
* We skip this logic completely for views.
3635+
* We skip this step completely for views. For a view, we can only get
3636+
* here from CREATE OR REPLACE VIEW, which historically doesn't set up
3637+
* defaults, not even for domain-typed columns. And in any case we mustn't
3638+
* invoke Phase 3 on a view, since it has no storage.
36393639
*/
3640-
if (relkind!=RELKIND_VIEW) {
3640+
if (relkind!=RELKIND_VIEW)
3641+
{
36413642
defval= (Expr*)build_column_default(rel,attribute.attnum);
36423643

36433644
if (!defval&&GetDomainConstraints(typeOid)!=NIL)
@@ -3680,7 +3681,7 @@ ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
36803681
/*
36813682
* Add needed dependency entries for the new column.
36823683
*/
3683-
add_column_datatype_dependency(myrelid,i,attribute.atttypid);
3684+
add_column_datatype_dependency(myrelid,newattnum,attribute.atttypid);
36843685
}
36853686

36863687
/*

‎src/backend/commands/view.c

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/view.c,v 1.108 2008/12/06 23:22:46 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/view.c,v 1.109 2008/12/15 21:35:31 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -165,6 +165,9 @@ DefineVirtualRelation(const RangeVar *relation, List *tlist, bool replace)
165165
aclcheck_error(ACLCHECK_NOT_OWNER,ACL_KIND_CLASS,
166166
RelationGetRelationName(rel));
167167

168+
/* Also check it's not in use already */
169+
CheckTableNotInUse(rel,"CREATE OR REPLACE VIEW");
170+
168171
/*
169172
* Due to the namespace visibility rules for temporary objects, we
170173
* should only end up replacing a temporary view with another
@@ -173,37 +176,41 @@ DefineVirtualRelation(const RangeVar *relation, List *tlist, bool replace)
173176
Assert(relation->istemp==rel->rd_istemp);
174177

175178
/*
176-
* If new attributes have been added, we must modify the pre-existing
177-
* view.
178-
*/
179-
if (list_length(attrList)>rel->rd_att->natts) {
179+
* Create a tuple descriptor to compare against the existing view, and
180+
* verify that the old column list is an initial prefix of the new
181+
* column list.
182+
*/
183+
descriptor=BuildDescForRelation(attrList);
184+
checkViewTupleDesc(descriptor,rel->rd_att);
185+
186+
/*
187+
* If new attributes have been added, we must add pg_attribute entries
188+
* for them. It is convenient (although overkill) to use the ALTER
189+
* TABLE ADD COLUMN infrastructure for this.
190+
*/
191+
if (list_length(attrList)>rel->rd_att->natts)
192+
{
180193
List*atcmds=NIL;
181194
ListCell*c;
182195
intskip=rel->rd_att->natts;
183196

184-
foreach(c,attrList) {
197+
foreach(c,attrList)
198+
{
185199
AlterTableCmd*atcmd;
186200

187-
if (skip>0) {
188-
--skip;
201+
if (skip>0)
202+
{
203+
skip--;
189204
continue;
190205
}
191206
atcmd=makeNode(AlterTableCmd);
192207
atcmd->subtype=AT_AddColumnToView;
193-
atcmd->def=lfirst(c);
208+
atcmd->def=(Node*)lfirst(c);
194209
atcmds=lappend(atcmds,atcmd);
195210
}
196211
AlterTableInternal(viewOid,atcmds, true);
197212
}
198213

199-
/*
200-
* Create a tuple descriptor to compare against the existing view, and
201-
* verify that the old column list is an initial prefix of the new
202-
* column list.
203-
*/
204-
descriptor=BuildDescForRelation(attrList);
205-
checkViewTupleDesc(descriptor,rel->rd_att);
206-
207214
/*
208215
* Seems okay, so return the OID of the pre-existing view.
209216
*/
@@ -238,6 +245,7 @@ DefineVirtualRelation(const RangeVar *relation, List *tlist, bool replace)
238245
* Verify that tupledesc associated with proposed new view definition
239246
* matches tupledesc of old view. This is basically a cut-down version
240247
* of equalTupleDescs(), with code added to generate specific complaints.
248+
* Also, we allow the new tupledesc to have more columns than the old.
241249
*/
242250
staticvoid
243251
checkViewTupleDesc(TupleDescnewdesc,TupleDescolddesc)

‎src/test/regress/expected/create_view.out

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ ERROR: cannot drop columns from view
5353
-- should fail
5454
CREATE OR REPLACE VIEW viewtest AS
5555
SELECT 1, * FROM viewtest_tbl;
56-
ERROR:column "b"ofrelation "viewtest" already exists
56+
ERROR:cannot change nameofview column "a"
5757
-- should fail
5858
CREATE OR REPLACE VIEW viewtest AS
5959
SELECT a, b::numeric FROM viewtest_tbl;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp