Allow CREATE OR REPLACE VIEW to rename the columns
Hi,
Currently CREATE OR REPLACE VIEW command fails if the column names
are changed. For example,
=# CREATE VIEW test AS SELECT 0 AS a;
=# CREATE OR REPLACE VIEW test AS SELECT 0 AS x;
ERROR: cannot change name of view column "a" to "x"
I'd like to propose the attached patch that allows CREATE OR REPLACE VIEW
to rename the view columns. Thought?
Regards,
--
Fujii Masao
Attachments:
rename_view_column_v1.patchapplication/octet-stream; name=rename_view_column_v1.patchDownload
diff --git a/doc/src/sgml/ref/create_view.sgml b/doc/src/sgml/ref/create_view.sgml
index e7a7e9fae2..fcde4a0581 100644
*** a/doc/src/sgml/ref/create_view.sgml
--- b/doc/src/sgml/ref/create_view.sgml
***************
*** 41,47 **** CREATE [ OR REPLACE ] [ TEMP | TEMPORARY ] [ RECURSIVE ] VIEW <replaceable class
<command>CREATE OR REPLACE VIEW</command> is similar, but if a view
of the same name already exists, it is replaced. The new query must
generate the same columns that were generated by the existing view query
! (that is, the same column names in the same order and with the same data
types), but it may add additional columns to the end of the list. The
calculations giving rise to the output columns may be completely different.
</para>
--- 41,47 ----
<command>CREATE OR REPLACE VIEW</command> is similar, but if a view
of the same name already exists, it is replaced. The new query must
generate the same columns that were generated by the existing view query
! (that is, the columns in the same order and with the same data
types), but it may add additional columns to the end of the list. The
calculations giving rise to the output columns may be completely different.
</para>
diff --git a/src/backend/commands/tablecindex 8d25d14772..0bdea166b6 100644
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 2910,2916 **** renameatt_check(Oid myrelid, Form_pg_class classform, bool recursing)
*
* Return value is the attribute number in the 'myrelid' relation.
*/
! static AttrNumber
renameatt_internal(Oid myrelid,
const char *oldattname,
const char *newattname,
--- 2910,2916 ----
*
* Return value is the attribute number in the 'myrelid' relation.
*/
! AttrNumber
renameatt_internal(Oid myrelid,
const char *oldattname,
const char *newattname,
diff --git a/src/backend/commands/view.index bea890f177..3708cee692 100644
*** a/src/backend/commands/view.c
--- b/src/backend/commands/view.c
***************
*** 109,114 **** DefineVirtualRelation(RangeVar *relation, List *tlist, bool replace,
--- 109,115 ----
List *atcmds = NIL;
AlterTableCmd *atcmd;
ObjectAddress address;
+ int i;
/* Relation is already locked, but we must build a relcache entry. */
rel = relation_open(viewOid, NoLock);
***************
*** 172,177 **** DefineVirtualRelation(RangeVar *relation, List *tlist, bool replace,
--- 173,209 ----
CommandCounterIncrement();
}
+ /*
+ * If attributes have been renamed, we must update pg_attribute
+ * entries for them.
+ *
+ * Note that we must do this before updating the query for the view,
+ * since the rules system requires that the correct view columns be in
+ * place when defining the new rules.
+ */
+ for (i = 0; i < rel->rd_att->natts; i++)
+ {
+ char *newattname;
+ char *oldattname;
+
+ newattname = NameStr(TupleDescAttr(descriptor, i)->attname);
+ oldattname = NameStr(TupleDescAttr(rel->rd_att, i)->attname);
+
+ if (strcmp(newattname, oldattname) != 0)
+ {
+ renameatt_internal(viewOid,
+ oldattname,
+ newattname,
+ false,
+ false,
+ 0,
+ DROP_RESTRICT);
+
+ /* Make the new name of view column visible */
+ CommandCounterIncrement();
+ }
+ }
+
/*
* Update the query for the view.
*
***************
*** 272,283 **** checkViewTupleDesc(TupleDesc newdesc, TupleDesc olddesc)
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
errmsg("cannot drop columns from view")));
- if (strcmp(NameStr(newattr->attname), NameStr(oldattr->attname)) != 0)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
- errmsg("cannot change name of view column \"%s\" to \"%s\"",
- NameStr(oldattr->attname),
- NameStr(newattr->attname))));
/* XXX would it be safe to allow atttypmod to change? Not sure */
if (newattr->atttypid != oldattr->atttypid ||
newattr->atttypmod != oldattr->atttypmod)
--- 304,309 ----
diff --git a/src/include/commands/index 9c25a805f2..c7d1f54e82 100644
*** a/src/include/commands/tablecmds.h
--- b/src/include/commands/tablecmds.h
***************
*** 60,65 **** extern void SetRelationHasSubclass(Oid relationId, bool relhassubclass);
--- 60,70 ----
extern ObjectAddress renameatt(RenameStmt *stmt);
+ extern AttrNumber renameatt_internal(Oid myrelid, const char *oldattname,
+ const char *newattname, bool recurse,
+ bool recursing, int expected_parents,
+ DropBehavior behavior);
+
extern ObjectAddress RenameConstraint(RenameStmt *stmt);
extern ObjectAddress RenameRelation(RenameStmt *stmt);
diff --git a/src/test/regress/expected/index 2fd36ca9a1..4a4fc95fd9 100644
*** a/src/test/regress/expected/create_view.out
--- b/src/test/regress/expected/create_view.out
***************
*** 63,74 **** ERROR: cannot drop columns from view
-- should fail
CREATE OR REPLACE VIEW viewtest AS
SELECT 1, * FROM viewtest_tbl;
! ERROR: cannot change name of view column "a" to "?column?"
-- should fail
CREATE OR REPLACE VIEW viewtest AS
SELECT a, b::numeric FROM viewtest_tbl;
ERROR: cannot change data type of view column "b" from integer to numeric
-- should work
CREATE OR REPLACE VIEW viewtest AS
SELECT a, b, 0 AS c FROM viewtest_tbl;
DROP VIEW viewtest;
--- 63,77 ----
-- should fail
CREATE OR REPLACE VIEW viewtest AS
SELECT 1, * FROM viewtest_tbl;
! ERROR: column "b" of relation "viewtest" already exists
-- should fail
CREATE OR REPLACE VIEW viewtest AS
SELECT a, b::numeric FROM viewtest_tbl;
ERROR: cannot change data type of view column "b" from integer to numeric
-- should work
+ CREATE OR REPLACE VIEW viewtest AS
+ SELECT a AS x, b AS y FROM viewtest_tbl;
+ -- should work
CREATE OR REPLACE VIEW viewtest AS
SELECT a, b, 0 AS c FROM viewtest_tbl;
DROP VIEW viewtest;
diff --git a/src/test/regress/sql/create_view.sqindex 8c0f45cc52..7d89b5ee3a 100644
*** a/src/test/regress/sql/create_view.sql
--- b/src/test/regress/sql/create_view.sql
***************
*** 73,78 **** CREATE OR REPLACE VIEW viewtest AS
--- 73,82 ----
CREATE OR REPLACE VIEW viewtest AS
SELECT a, b::numeric FROM viewtest_tbl;
+ -- should work
+ CREATE OR REPLACE VIEW viewtest AS
+ SELECT a AS x, b AS y FROM viewtest_tbl;
+
-- should work
CREATE OR REPLACE VIEW viewtest AS
SELECT a, b, 0 AS c FROM viewtest_tbl;
Fujii Masao <masao.fujii@gmail.com> writes:
Currently CREATE OR REPLACE VIEW command fails if the column names
are changed.
That is, I believe, intentional. It's an effective aid to catching
mistakes in view redefinitions, such as misaligning the new set of
columns relative to the old. That's particularly important given
that we allow you to add columns during CREATE OR REPLACE VIEW.
Consider the oversimplified case where you start with
CREATE VIEW v AS SELECT 1 AS x, 2 AS y;
and you want to add a column z, and you get sloppy and write
CREATE OR REPLACE VIEW v AS SELECT 1 AS x, 3 AS z, 2 AS y;
If we did not throw an error on this, references that formerly
pointed to column y would now point to z (as that's still attnum 2),
which is highly unlikely to be what you wanted.
The right way to handle a column rename in a view is to do a separate
ALTER VIEW RENAME COLUMN, making it totally clear what you intend to
happen. (Right now, we make you spell that "ALTER TABLE RENAME COLUMN",
but it'd be reasonable to add that syntax to ALTER VIEW too.) I don't
think this functionality should be folded into redefinition of the content
of the view. It'd add more opportunity for mistakes than anything else.
regards, tom lane
On Thu, Oct 31, 2019 at 1:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fujii Masao <masao.fujii@gmail.com> writes:
Currently CREATE OR REPLACE VIEW command fails if the column names
are changed.That is, I believe, intentional. It's an effective aid to catching
mistakes in view redefinitions, such as misaligning the new set of
columns relative to the old. That's particularly important given
that we allow you to add columns during CREATE OR REPLACE VIEW.
Consider the oversimplified case where you start withCREATE VIEW v AS SELECT 1 AS x, 2 AS y;
and you want to add a column z, and you get sloppy and write
CREATE OR REPLACE VIEW v AS SELECT 1 AS x, 3 AS z, 2 AS y;
If we did not throw an error on this, references that formerly
pointed to column y would now point to z (as that's still attnum 2),
which is highly unlikely to be what you wanted.
This example makes me wonder if the addtion of column by
CREATE OR REPLACE VIEW also has the same (or even worse) issue.
That is, it may increase the oppotunity for users' mistake.
I'm thinking the case where users mistakenly added new column
into the view when replacing the view definition. This mistake can
happen because CREATE OR REPLACE VIEW allows new column to
be added. But what's the worse is that, currently there is no way to
drop the column from the view, except recreation of the view.
Neither CREATE OR REPLACE VIEW nor ALTER TABLE support
the drop of the column from the view. So, to fix the mistake,
users would need to drop the view itself and recreate it. If there are
some objects depending the view, they also might need to be recreated.
This looks not good. Since the feature has been supported,
it's too late to say that, though...
At least, the support for ALTER VIEW DROP COLUMN might be
necessary to alleviate that situation.
Regards,
--
Fujii Masao
On Thu, Oct 31, 2019 at 12:32 PM Fujii Masao <masao.fujii@gmail.com> wrote:
On Thu, Oct 31, 2019 at 1:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fujii Masao <masao.fujii@gmail.com> writes:
Currently CREATE OR REPLACE VIEW command fails if the column names
are changed.That is, I believe, intentional. It's an effective aid to catching
mistakes in view redefinitions, such as misaligning the new set of
columns relative to the old. That's particularly important given
that we allow you to add columns during CREATE OR REPLACE VIEW.
Consider the oversimplified case where you start withCREATE VIEW v AS SELECT 1 AS x, 2 AS y;
and you want to add a column z, and you get sloppy and write
CREATE OR REPLACE VIEW v AS SELECT 1 AS x, 3 AS z, 2 AS y;
If we did not throw an error on this, references that formerly
pointed to column y would now point to z (as that's still attnum 2),
which is highly unlikely to be what you wanted.This example makes me wonder if the addtion of column by
CREATE OR REPLACE VIEW also has the same (or even worse) issue.
That is, it may increase the oppotunity for users' mistake.
I'm thinking the case where users mistakenly added new column
into the view when replacing the view definition. This mistake can
happen because CREATE OR REPLACE VIEW allows new column to
be added. But what's the worse is that, currently there is no way to
drop the column from the view, except recreation of the view.
Neither CREATE OR REPLACE VIEW nor ALTER TABLE support
the drop of the column from the view. So, to fix the mistake,
users would need to drop the view itself and recreate it. If there are
some objects depending the view, they also might need to be recreated.
This looks not good. Since the feature has been supported,
it's too late to say that, though...At least, the support for ALTER VIEW DROP COLUMN might be
necessary to alleviate that situation.
- Is this intentional not implemented the "RENAME COLUMN" statement for
VIEW because it is implemented for Materialized View? I have made just a
similar
change to view and it works.
ALTER VIEW v RENAME COLUMN d to e;
- For "DROP COLUMN" for VIEW is throwing error.
postgres=# alter view v drop column e;
ERROR: "v" is not a table, composite type, or foreign table
Regards,
--
Fujii Masao
--
Ibrar Ahmed
Attachments:
001_alter_view_rename_column_ibrar_v1.patchapplication/octet-stream; name=001_alter_view_rename_column_ibrar_v1.patchDownload
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 3f67aaf30e..77f69566d9 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8775,6 +8775,28 @@ RenameStmt: ALTER AGGREGATE aggregate_with_argtypes RENAME TO name
n->missing_ok = true;
$$ = (Node *)n;
}
+ | ALTER VIEW qualified_name RENAME opt_column name TO name
+ {
+ RenameStmt *n = makeNode(RenameStmt);
+ n->renameType = OBJECT_COLUMN;
+ n->relationType = OBJECT_MATVIEW;
+ n->relation = $3;
+ n->subname = $6;
+ n->newname = $8;
+ n->missing_ok = false;
+ $$ = (Node *)n;
+ }
+ | ALTER VIEW IF_P EXISTS qualified_name RENAME opt_column name TO name
+ {
+ RenameStmt *n = makeNode(RenameStmt);
+ n->renameType = OBJECT_COLUMN;
+ n->relationType = OBJECT_VIEW;
+ n->relation = $5;
+ n->subname = $8;
+ n->newname = $10;
+ n->missing_ok = true;
+ $$ = (Node *)n;
+ }
| ALTER MATERIALIZED VIEW qualified_name RENAME opt_column name TO name
{
RenameStmt *n = makeNode(RenameStmt);
On Thu, Oct 31, 2019 at 7:59 PM Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote:
On Thu, Oct 31, 2019 at 12:32 PM Fujii Masao <masao.fujii@gmail.com> wrote:
On Thu, Oct 31, 2019 at 1:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fujii Masao <masao.fujii@gmail.com> writes:
Currently CREATE OR REPLACE VIEW command fails if the column names
are changed.That is, I believe, intentional. It's an effective aid to catching
mistakes in view redefinitions, such as misaligning the new set of
columns relative to the old. That's particularly important given
that we allow you to add columns during CREATE OR REPLACE VIEW.
Consider the oversimplified case where you start withCREATE VIEW v AS SELECT 1 AS x, 2 AS y;
and you want to add a column z, and you get sloppy and write
CREATE OR REPLACE VIEW v AS SELECT 1 AS x, 3 AS z, 2 AS y;
If we did not throw an error on this, references that formerly
pointed to column y would now point to z (as that's still attnum 2),
which is highly unlikely to be what you wanted.This example makes me wonder if the addtion of column by
CREATE OR REPLACE VIEW also has the same (or even worse) issue.
That is, it may increase the oppotunity for users' mistake.
I'm thinking the case where users mistakenly added new column
into the view when replacing the view definition. This mistake can
happen because CREATE OR REPLACE VIEW allows new column to
be added. But what's the worse is that, currently there is no way to
drop the column from the view, except recreation of the view.
Neither CREATE OR REPLACE VIEW nor ALTER TABLE support
the drop of the column from the view. So, to fix the mistake,
users would need to drop the view itself and recreate it. If there are
some objects depending the view, they also might need to be recreated.
This looks not good. Since the feature has been supported,
it's too late to say that, though...At least, the support for ALTER VIEW DROP COLUMN might be
necessary to alleviate that situation.- Is this intentional not implemented the "RENAME COLUMN" statement for
VIEW because it is implemented for Materialized View?
Not sure that, but Tom's suggestion to support ALTER VIEW RENAME COLUMN
sounds reasonable whether we support the rename of columns when replacing
the view definition, or not. Attached is the patch that adds support for
ALTER VIEW RENAME COLUMN command.
I have made just a similar
change to view and it works.
Yeah, ISTM that we made the same patch at the same time. You changed gram.y,
but I added the following changes additionally.
- Update the doc
- Add HINT message emit when CRAETE OR REPLACE VIEW fails to rename the columns
- Update tab-complete.c
- Add regression test
One issue I've not addressed yet is about the command tag of
"ALTER VIEW RENAME COLUMN". Currently "ALTER TABLE" is returned as the tag
like ALTER MATERIALIZED VIEW RENAME COLUMN, but ISTM that "ALTER VIEW"
is better. I started the discussion about the command tag of
"ALTER MATERIALIZED VIEW" at another thread. I will update the patch according
to the result of that discussion.
/messages/by-id/CAHGQGwGUaC03FFdTFoHsCuDrrNvFvNVQ6xyd40==P25WvuBJjg@mail.gmail.com
Regards,
--
Fujii Masao
Attachments:
alter_view_rename_column_v1.patchapplication/octet-stream; name=alter_view_rename_column_v1.patchDownload
diff --git a/doc/src/sgml/ref/alter_view.sgml b/doc/src/sgml/ref/alter_view.sgml
index 2e9edc1975..b66160bfb8 100644
*** a/doc/src/sgml/ref/alter_view.sgml
--- b/doc/src/sgml/ref/alter_view.sgml
***************
*** 24,29 **** PostgreSQL documentation
--- 24,30 ----
ALTER VIEW [ IF EXISTS ] <replaceable class="parameter">name</replaceable> ALTER [ COLUMN ] <replaceable class="parameter">column_name</replaceable> SET DEFAULT <replaceable class="parameter">expression</replaceable>
ALTER VIEW [ IF EXISTS ] <replaceable class="parameter">name</replaceable> ALTER [ COLUMN ] <replaceable class="parameter">column_name</replaceable> DROP DEFAULT
ALTER VIEW [ IF EXISTS ] <replaceable class="parameter">name</replaceable> OWNER TO { <replaceable class="parameter">new_owner</replaceable> | CURRENT_USER | SESSION_USER }
+ ALTER VIEW [ IF EXISTS ] <replaceable class="parameter">name</replaceable> RENAME [ COLUMN ] <replaceable class="parameter">column_name</replaceable> TO <replaceable class="parameter">new_column_name</replaceable>
ALTER VIEW [ IF EXISTS ] <replaceable class="parameter">name</replaceable> RENAME TO <replaceable class="parameter">new_name</replaceable>
ALTER VIEW [ IF EXISTS ] <replaceable class="parameter">name</replaceable> SET SCHEMA <replaceable class="parameter">new_schema</replaceable>
ALTER VIEW [ IF EXISTS ] <replaceable class="parameter">name</replaceable> SET ( <replaceable class="parameter">view_option_name</replaceable> [= <replaceable class="parameter">view_option_value</replaceable>] [, ... ] )
***************
*** 65,70 **** ALTER VIEW [ IF EXISTS ] <replaceable class="parameter">name</replaceable> RESET
--- 66,89 ----
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><replaceable class="parameter">column_name</replaceable></term>
+ <listitem>
+ <para>
+ Name of an existing column.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><replaceable class="parameter">new_column_name</replaceable></term>
+ <listitem>
+ <para>
+ New name for an existing column.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal>IF EXISTS</literal></term>
<listitem>
diff --git a/src/backend/commands/view.index bea890f177..1b4d66dcd9 100644
*** a/src/backend/commands/view.c
--- b/src/backend/commands/view.c
***************
*** 277,283 **** checkViewTupleDesc(TupleDesc newdesc, TupleDesc olddesc)
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
errmsg("cannot change name of view column \"%s\" to \"%s\"",
NameStr(oldattr->attname),
! NameStr(newattr->attname))));
/* XXX would it be safe to allow atttypmod to change? Not sure */
if (newattr->atttypid != oldattr->atttypid ||
newattr->atttypmod != oldattr->atttypmod)
--- 277,284 ----
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
errmsg("cannot change name of view column \"%s\" to \"%s\"",
NameStr(oldattr->attname),
! NameStr(newattr->attname)),
! errhint("Use ALTER VIEW ... RENAME COLUMN ... to change name of view column instead.")));
/* XXX would it be safe to allow atttypmod to change? Not sure */
if (newattr->atttypid != oldattr->atttypid ||
newattr->atttypmod != oldattr->atttypmod)
diff --git a/src/backend/parser/grindex 3f67aaf30e..18e4d7636a 100644
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 8775,8780 **** RenameStmt: ALTER AGGREGATE aggregate_with_argtypes RENAME TO name
--- 8775,8802 ----
n->missing_ok = true;
$$ = (Node *)n;
}
+ | ALTER VIEW qualified_name RENAME opt_column name TO name
+ {
+ RenameStmt *n = makeNode(RenameStmt);
+ n->renameType = OBJECT_COLUMN;
+ n->relationType = OBJECT_VIEW;
+ n->relation = $3;
+ n->subname = $6;
+ n->newname = $8;
+ n->missing_ok = false;
+ $$ = (Node *)n;
+ }
+ | ALTER VIEW IF_P EXISTS qualified_name RENAME opt_column name TO name
+ {
+ RenameStmt *n = makeNode(RenameStmt);
+ n->renameType = OBJECT_COLUMN;
+ n->relationType = OBJECT_VIEW;
+ n->relation = $5;
+ n->subname = $8;
+ n->newname = $10;
+ n->missing_ok = true;
+ $$ = (Node *)n;
+ }
| ALTER MATERIALIZED VIEW qualified_name RENAME opt_column name TO name
{
RenameStmt *n = makeNode(RenameStmt);
diff --git a/src/bin/psql/tab-coindex 2b1e3cda4a..2837939e93 100644
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
***************
*** 1797,1804 **** psql_completion(const char *text, int start, int end)
COMPLETE_WITH("TO");
/* ALTER VIEW <name> */
else if (Matches("ALTER", "VIEW", MatchAny))
! COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME TO",
"SET SCHEMA");
/* ALTER MATERIALIZED VIEW <name> */
else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny))
COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME TO",
--- 1797,1816 ----
COMPLETE_WITH("TO");
/* ALTER VIEW <name> */
else if (Matches("ALTER", "VIEW", MatchAny))
! COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
"SET SCHEMA");
+ /* ALTER VIEW xxx RENAME */
+ else if (Matches("ALTER", "VIEW", MatchAny, "RENAME"))
+ COMPLETE_WITH_ATTR(prev2_wd, " UNION SELECT 'COLUMN' UNION SELECT 'TO'");
+ else if (Matches("ALTER", "VIEW", MatchAny, "ALTER|RENAME", "COLUMN"))
+ COMPLETE_WITH_ATTR(prev3_wd, "");
+ /* ALTER VIEW xxx RENAME yyy */
+ else if (Matches("ALTER", "VIEW", MatchAny, "RENAME", MatchAnyExcept("TO")))
+ COMPLETE_WITH("TO");
+ /* ALTER VIEW xxx RENAME COLUMN yyy */
+ else if (Matches("ALTER", "VIEW", MatchAny, "RENAME", "COLUMN", MatchAnyExcept("TO")))
+ COMPLETE_WITH("TO");
+
/* ALTER MATERIALIZED VIEW <name> */
else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny))
COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME TO",
diff --git a/src/test/regress/expeindex 2fd36ca9a1..9a92629fd7 100644
*** a/src/test/regress/expected/create_view.out
--- b/src/test/regress/expected/create_view.out
***************
*** 64,69 **** ERROR: cannot drop columns from view
--- 64,70 ----
CREATE OR REPLACE VIEW viewtest AS
SELECT 1, * FROM viewtest_tbl;
ERROR: cannot change name of view column "a" to "?column?"
+ HINT: Use ALTER VIEW ... RENAME COLUMN ... to change name of view column instead.
-- should fail
CREATE OR REPLACE VIEW viewtest AS
SELECT a, b::numeric FROM viewtest_tbl;
***************
*** 1189,1194 **** select pg_get_viewdef('vv1', true);
--- 1190,1218 ----
CROSS JOIN tt6) j(aa, bb, cc_1, cc, dd);
(1 row)
+ create view v4 as select * from v1;
+ alter view v1 rename column a to x;
+ select pg_get_viewdef('v1', true);
+ pg_get_viewdef
+ ---------------------------------------------------
+ SELECT tt2.b, +
+ tt3.c, +
+ tt2.a AS x, +
+ tt3.ax +
+ FROM tt2 +
+ JOIN tt3 tt3(ax, b, c, c_1, e) USING (b, c);
+ (1 row)
+
+ select pg_get_viewdef('v4', true);
+ pg_get_viewdef
+ ----------------
+ SELECT v1.b, +
+ v1.c, +
+ v1.x AS a,+
+ v1.ax +
+ FROM v1;
+ (1 row)
+
-- Unnamed FULL JOIN USING is lots of fun too
create table tt7 (x int, xx int, y int);
alter table tt7 drop column xx;
***************
*** 1782,1788 **** drop cascades to view aliased_view_2
drop cascades to view aliased_view_3
drop cascades to view aliased_view_4
DROP SCHEMA testviewschm2 CASCADE;
! NOTICE: drop cascades to 63 other objects
DETAIL: drop cascades to table t1
drop cascades to view temporal1
drop cascades to view temporal2
--- 1806,1812 ----
drop cascades to view aliased_view_3
drop cascades to view aliased_view_4
DROP SCHEMA testviewschm2 CASCADE;
! NOTICE: drop cascades to 64 other objects
DETAIL: drop cascades to table t1
drop cascades to view temporal1
drop cascades to view temporal2
***************
*** 1818,1823 **** drop cascades to view v3
--- 1842,1848 ----
drop cascades to table tt5
drop cascades to table tt6
drop cascades to view vv1
+ drop cascades to view v4
drop cascades to table tt7
drop cascades to table tt8
drop cascades to view vv2
diff --git a/src/test/regress/sql/create_view.sqindex 8c0f45cc52..be5d90727a 100644
*** a/src/test/regress/sql/create_view.sql
--- b/src/test/regress/sql/create_view.sql
***************
*** 391,396 **** select pg_get_viewdef('vv1', true);
--- 391,402 ----
alter table tt5 drop column c;
select pg_get_viewdef('vv1', true);
+ create view v4 as select * from v1;
+ alter view v1 rename column a to x;
+ select pg_get_viewdef('v1', true);
+ select pg_get_viewdef('v4', true);
+
+
-- Unnamed FULL JOIN USING is lots of fun too
create table tt7 (x int, xx int, y int);
On Thu, Oct 31, 2019 at 5:01 PM Fujii Masao <masao.fujii@gmail.com> wrote:
On Thu, Oct 31, 2019 at 7:59 PM Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote:
On Thu, Oct 31, 2019 at 12:32 PM Fujii Masao <masao.fujii@gmail.com>
wrote:
On Thu, Oct 31, 2019 at 1:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fujii Masao <masao.fujii@gmail.com> writes:
Currently CREATE OR REPLACE VIEW command fails if the column names
are changed.That is, I believe, intentional. It's an effective aid to catching
mistakes in view redefinitions, such as misaligning the new set of
columns relative to the old. That's particularly important given
that we allow you to add columns during CREATE OR REPLACE VIEW.
Consider the oversimplified case where you start withCREATE VIEW v AS SELECT 1 AS x, 2 AS y;
and you want to add a column z, and you get sloppy and write
CREATE OR REPLACE VIEW v AS SELECT 1 AS x, 3 AS z, 2 AS y;
If we did not throw an error on this, references that formerly
pointed to column y would now point to z (as that's still attnum 2),
which is highly unlikely to be what you wanted.This example makes me wonder if the addtion of column by
CREATE OR REPLACE VIEW also has the same (or even worse) issue.
That is, it may increase the oppotunity for users' mistake.
I'm thinking the case where users mistakenly added new column
into the view when replacing the view definition. This mistake can
happen because CREATE OR REPLACE VIEW allows new column to
be added. But what's the worse is that, currently there is no way to
drop the column from the view, except recreation of the view.
Neither CREATE OR REPLACE VIEW nor ALTER TABLE support
the drop of the column from the view. So, to fix the mistake,
users would need to drop the view itself and recreate it. If there are
some objects depending the view, they also might need to be recreated.
This looks not good. Since the feature has been supported,
it's too late to say that, though...At least, the support for ALTER VIEW DROP COLUMN might be
necessary to alleviate that situation.- Is this intentional not implemented the "RENAME COLUMN" statement for
VIEW because it is implemented for Materialized View?Not sure that, but Tom's suggestion to support ALTER VIEW RENAME COLUMN
sounds reasonable whether we support the rename of columns when replacing
the view definition, or not. Attached is the patch that adds support for
ALTER VIEW RENAME COLUMN command.I have made just a similar
change to view and it works.Yeah, ISTM that we made the same patch at the same time. You changed
gram.y,
but I added the following changes additionally.- Update the doc
- Add HINT message emit when CRAETE OR REPLACE VIEW fails to rename the
columns
- Update tab-complete.c
- Add regression test
Oh, I just sent the patch to ask, good you do that in all the places.
One issue I've not addressed yet is about the command tag of
"ALTER VIEW RENAME COLUMN". Currently "ALTER TABLE" is returned as the tag
like ALTER MATERIALIZED VIEW RENAME COLUMN, but ISTM that "ALTER VIEW"
is better. I started the discussion about the command tag of
"ALTER MATERIALIZED VIEW" at another thread. I will update the patch
according
to the result of that discussion./messages/by-id/CAHGQGwGUaC03FFdTFoHsCuDrrNvFvNVQ6xyd40==P25WvuBJjg@mail.gmail.com
Attached patch contain small change for ALTER MATERIALIZED VIEW.
Regards,
--
Fujii Masao
--
Ibrar Ahmed
Attachments:
alter_view_rename_column_v2.patchapplication/octet-stream; name=alter_view_rename_column_v2.patchDownload
diff --git a/doc/src/sgml/ref/alter_view.sgml b/doc/src/sgml/ref/alter_view.sgml
index 2e9edc1975..b66160bfb8 100644
--- a/doc/src/sgml/ref/alter_view.sgml
+++ b/doc/src/sgml/ref/alter_view.sgml
@@ -24,6 +24,7 @@ PostgreSQL documentation
ALTER VIEW [ IF EXISTS ] <replaceable class="parameter">name</replaceable> ALTER [ COLUMN ] <replaceable class="parameter">column_name</replaceable> SET DEFAULT <replaceable class="parameter">expression</replaceable>
ALTER VIEW [ IF EXISTS ] <replaceable class="parameter">name</replaceable> ALTER [ COLUMN ] <replaceable class="parameter">column_name</replaceable> DROP DEFAULT
ALTER VIEW [ IF EXISTS ] <replaceable class="parameter">name</replaceable> OWNER TO { <replaceable class="parameter">new_owner</replaceable> | CURRENT_USER | SESSION_USER }
+ALTER VIEW [ IF EXISTS ] <replaceable class="parameter">name</replaceable> RENAME [ COLUMN ] <replaceable class="parameter">column_name</replaceable> TO <replaceable class="parameter">new_column_name</replaceable>
ALTER VIEW [ IF EXISTS ] <replaceable class="parameter">name</replaceable> RENAME TO <replaceable class="parameter">new_name</replaceable>
ALTER VIEW [ IF EXISTS ] <replaceable class="parameter">name</replaceable> SET SCHEMA <replaceable class="parameter">new_schema</replaceable>
ALTER VIEW [ IF EXISTS ] <replaceable class="parameter">name</replaceable> SET ( <replaceable class="parameter">view_option_name</replaceable> [= <replaceable class="parameter">view_option_value</replaceable>] [, ... ] )
@@ -65,6 +66,24 @@ ALTER VIEW [ IF EXISTS ] <replaceable class="parameter">name</replaceable> RESET
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><replaceable class="parameter">column_name</replaceable></term>
+ <listitem>
+ <para>
+ Name of an existing column.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><replaceable class="parameter">new_column_name</replaceable></term>
+ <listitem>
+ <para>
+ New name for an existing column.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal>IF EXISTS</literal></term>
<listitem>
diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c
index bea890f177..1b4d66dcd9 100644
--- a/src/backend/commands/view.c
+++ b/src/backend/commands/view.c
@@ -277,7 +277,8 @@ checkViewTupleDesc(TupleDesc newdesc, TupleDesc olddesc)
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
errmsg("cannot change name of view column \"%s\" to \"%s\"",
NameStr(oldattr->attname),
- NameStr(newattr->attname))));
+ NameStr(newattr->attname)),
+ errhint("Use ALTER VIEW ... RENAME COLUMN ... to change name of view column instead.")));
/* XXX would it be safe to allow atttypmod to change? Not sure */
if (newattr->atttypid != oldattr->atttypid ||
newattr->atttypmod != oldattr->atttypmod)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 3f67aaf30e..18e4d7636a 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8775,6 +8775,28 @@ RenameStmt: ALTER AGGREGATE aggregate_with_argtypes RENAME TO name
n->missing_ok = true;
$$ = (Node *)n;
}
+ | ALTER VIEW qualified_name RENAME opt_column name TO name
+ {
+ RenameStmt *n = makeNode(RenameStmt);
+ n->renameType = OBJECT_COLUMN;
+ n->relationType = OBJECT_VIEW;
+ n->relation = $3;
+ n->subname = $6;
+ n->newname = $8;
+ n->missing_ok = false;
+ $$ = (Node *)n;
+ }
+ | ALTER VIEW IF_P EXISTS qualified_name RENAME opt_column name TO name
+ {
+ RenameStmt *n = makeNode(RenameStmt);
+ n->renameType = OBJECT_COLUMN;
+ n->relationType = OBJECT_VIEW;
+ n->relation = $5;
+ n->subname = $8;
+ n->newname = $10;
+ n->missing_ok = true;
+ $$ = (Node *)n;
+ }
| ALTER MATERIALIZED VIEW qualified_name RENAME opt_column name TO name
{
RenameStmt *n = makeNode(RenameStmt);
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c6faa6619d..f834577bb3 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -2405,7 +2405,7 @@ CreateCommandTag(Node *parsetree)
break;
case T_RenameStmt:
- tag = AlterObjectTypeCommandTag(((RenameStmt *) parsetree)->renameType);
+ tag = AlterObjectTypeCommandTag(((RenameStmt *) parsetree)->relationType);
break;
case T_AlterObjectDependsStmt:
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 2b1e3cda4a..2837939e93 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1797,8 +1797,20 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH("TO");
/* ALTER VIEW <name> */
else if (Matches("ALTER", "VIEW", MatchAny))
- COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME TO",
+ COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
"SET SCHEMA");
+ /* ALTER VIEW xxx RENAME */
+ else if (Matches("ALTER", "VIEW", MatchAny, "RENAME"))
+ COMPLETE_WITH_ATTR(prev2_wd, " UNION SELECT 'COLUMN' UNION SELECT 'TO'");
+ else if (Matches("ALTER", "VIEW", MatchAny, "ALTER|RENAME", "COLUMN"))
+ COMPLETE_WITH_ATTR(prev3_wd, "");
+ /* ALTER VIEW xxx RENAME yyy */
+ else if (Matches("ALTER", "VIEW", MatchAny, "RENAME", MatchAnyExcept("TO")))
+ COMPLETE_WITH("TO");
+ /* ALTER VIEW xxx RENAME COLUMN yyy */
+ else if (Matches("ALTER", "VIEW", MatchAny, "RENAME", "COLUMN", MatchAnyExcept("TO")))
+ COMPLETE_WITH("TO");
+
/* ALTER MATERIALIZED VIEW <name> */
else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny))
COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME TO",
diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out
index 2fd36ca9a1..9a92629fd7 100644
--- a/src/test/regress/expected/create_view.out
+++ b/src/test/regress/expected/create_view.out
@@ -64,6 +64,7 @@ ERROR: cannot drop columns from view
CREATE OR REPLACE VIEW viewtest AS
SELECT 1, * FROM viewtest_tbl;
ERROR: cannot change name of view column "a" to "?column?"
+HINT: Use ALTER VIEW ... RENAME COLUMN ... to change name of view column instead.
-- should fail
CREATE OR REPLACE VIEW viewtest AS
SELECT a, b::numeric FROM viewtest_tbl;
@@ -1189,6 +1190,29 @@ select pg_get_viewdef('vv1', true);
CROSS JOIN tt6) j(aa, bb, cc_1, cc, dd);
(1 row)
+create view v4 as select * from v1;
+alter view v1 rename column a to x;
+select pg_get_viewdef('v1', true);
+ pg_get_viewdef
+---------------------------------------------------
+ SELECT tt2.b, +
+ tt3.c, +
+ tt2.a AS x, +
+ tt3.ax +
+ FROM tt2 +
+ JOIN tt3 tt3(ax, b, c, c_1, e) USING (b, c);
+(1 row)
+
+select pg_get_viewdef('v4', true);
+ pg_get_viewdef
+----------------
+ SELECT v1.b, +
+ v1.c, +
+ v1.x AS a,+
+ v1.ax +
+ FROM v1;
+(1 row)
+
-- Unnamed FULL JOIN USING is lots of fun too
create table tt7 (x int, xx int, y int);
alter table tt7 drop column xx;
@@ -1782,7 +1806,7 @@ drop cascades to view aliased_view_2
drop cascades to view aliased_view_3
drop cascades to view aliased_view_4
DROP SCHEMA testviewschm2 CASCADE;
-NOTICE: drop cascades to 63 other objects
+NOTICE: drop cascades to 64 other objects
DETAIL: drop cascades to table t1
drop cascades to view temporal1
drop cascades to view temporal2
@@ -1818,6 +1842,7 @@ drop cascades to view v3
drop cascades to table tt5
drop cascades to table tt6
drop cascades to view vv1
+drop cascades to view v4
drop cascades to table tt7
drop cascades to table tt8
drop cascades to view vv2
diff --git a/src/test/regress/sql/create_view.sql b/src/test/regress/sql/create_view.sql
index 8c0f45cc52..be5d90727a 100644
--- a/src/test/regress/sql/create_view.sql
+++ b/src/test/regress/sql/create_view.sql
@@ -391,6 +391,12 @@ select pg_get_viewdef('vv1', true);
alter table tt5 drop column c;
select pg_get_viewdef('vv1', true);
+create view v4 as select * from v1;
+alter view v1 rename column a to x;
+select pg_get_viewdef('v1', true);
+select pg_get_viewdef('v4', true);
+
+
-- Unnamed FULL JOIN USING is lots of fun too
create table tt7 (x int, xx int, y int);
On Thu, Oct 31, 2019 at 5:11 PM Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote:
On Thu, Oct 31, 2019 at 5:01 PM Fujii Masao <masao.fujii@gmail.com> wrote:
On Thu, Oct 31, 2019 at 7:59 PM Ibrar Ahmed <ibrar.ahmad@gmail.com>
wrote:On Thu, Oct 31, 2019 at 12:32 PM Fujii Masao <masao.fujii@gmail.com>
wrote:
On Thu, Oct 31, 2019 at 1:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fujii Masao <masao.fujii@gmail.com> writes:
Currently CREATE OR REPLACE VIEW command fails if the column names
are changed.That is, I believe, intentional. It's an effective aid to catching
mistakes in view redefinitions, such as misaligning the new set of
columns relative to the old. That's particularly important given
that we allow you to add columns during CREATE OR REPLACE VIEW.
Consider the oversimplified case where you start withCREATE VIEW v AS SELECT 1 AS x, 2 AS y;
and you want to add a column z, and you get sloppy and write
CREATE OR REPLACE VIEW v AS SELECT 1 AS x, 3 AS z, 2 AS y;
If we did not throw an error on this, references that formerly
pointed to column y would now point to z (as that's still attnum 2),
which is highly unlikely to be what you wanted.This example makes me wonder if the addtion of column by
CREATE OR REPLACE VIEW also has the same (or even worse) issue.
That is, it may increase the oppotunity for users' mistake.
I'm thinking the case where users mistakenly added new column
into the view when replacing the view definition. This mistake can
happen because CREATE OR REPLACE VIEW allows new column to
be added. But what's the worse is that, currently there is no way to
drop the column from the view, except recreation of the view.
Neither CREATE OR REPLACE VIEW nor ALTER TABLE support
the drop of the column from the view. So, to fix the mistake,
users would need to drop the view itself and recreate it. If there are
some objects depending the view, they also might need to be recreated.
This looks not good. Since the feature has been supported,
it's too late to say that, though...At least, the support for ALTER VIEW DROP COLUMN might be
necessary to alleviate that situation.- Is this intentional not implemented the "RENAME COLUMN" statement for
VIEW because it is implemented for Materialized View?Not sure that, but Tom's suggestion to support ALTER VIEW RENAME COLUMN
sounds reasonable whether we support the rename of columns when replacing
the view definition, or not. Attached is the patch that adds support for
ALTER VIEW RENAME COLUMN command.I have made just a similar
change to view and it works.Yeah, ISTM that we made the same patch at the same time. You changed
gram.y,
but I added the following changes additionally.- Update the doc
- Add HINT message emit when CRAETE OR REPLACE VIEW fails to rename the
columns
- Update tab-complete.c
- Add regression testOh, I just sent the patch to ask, good you do that in all the places.
One issue I've not addressed yet is about the command tag of
"ALTER VIEW RENAME COLUMN". Currently "ALTER TABLE" is returned as the tag
like ALTER MATERIALIZED VIEW RENAME COLUMN, but ISTM that "ALTER VIEW"
is better. I started the discussion about the command tag of
"ALTER MATERIALIZED VIEW" at another thread. I will update the patch
according
to the result of that discussion./messages/by-id/CAHGQGwGUaC03FFdTFoHsCuDrrNvFvNVQ6xyd40==P25WvuBJjg@mail.gmail.com
Attached patch contain small change for ALTER MATERIALIZED VIEW.
Hmm, my small change of "ALTER MATERIALIZED VIEW" does not work in some
cases need more work on that.
Regards,
--
Fujii Masao--
Ibrar Ahmed
--
Ibrar Ahmed
On Thu, Oct 31, 2019 at 5:28 PM Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote:
On Thu, Oct 31, 2019 at 5:11 PM Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote:
On Thu, Oct 31, 2019 at 5:01 PM Fujii Masao <masao.fujii@gmail.com>
wrote:On Thu, Oct 31, 2019 at 7:59 PM Ibrar Ahmed <ibrar.ahmad@gmail.com>
wrote:On Thu, Oct 31, 2019 at 12:32 PM Fujii Masao <masao.fujii@gmail.com>
wrote:
On Thu, Oct 31, 2019 at 1:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fujii Masao <masao.fujii@gmail.com> writes:
Currently CREATE OR REPLACE VIEW command fails if the column names
are changed.That is, I believe, intentional. It's an effective aid to catching
mistakes in view redefinitions, such as misaligning the new set of
columns relative to the old. That's particularly important given
that we allow you to add columns during CREATE OR REPLACE VIEW.
Consider the oversimplified case where you start withCREATE VIEW v AS SELECT 1 AS x, 2 AS y;
and you want to add a column z, and you get sloppy and write
CREATE OR REPLACE VIEW v AS SELECT 1 AS x, 3 AS z, 2 AS y;
If we did not throw an error on this, references that formerly
pointed to column y would now point to z (as that's still attnum 2),
which is highly unlikely to be what you wanted.This example makes me wonder if the addtion of column by
CREATE OR REPLACE VIEW also has the same (or even worse) issue.
That is, it may increase the oppotunity for users' mistake.
I'm thinking the case where users mistakenly added new column
into the view when replacing the view definition. This mistake can
happen because CREATE OR REPLACE VIEW allows new column to
be added. But what's the worse is that, currently there is no way to
drop the column from the view, except recreation of the view.
Neither CREATE OR REPLACE VIEW nor ALTER TABLE support
the drop of the column from the view. So, to fix the mistake,
users would need to drop the view itself and recreate it. If there are
some objects depending the view, they also might need to be recreated.
This looks not good. Since the feature has been supported,
it's too late to say that, though...At least, the support for ALTER VIEW DROP COLUMN might be
necessary to alleviate that situation.- Is this intentional not implemented the "RENAME COLUMN" statement for
VIEW because it is implemented for Materialized View?Not sure that, but Tom's suggestion to support ALTER VIEW RENAME COLUMN
sounds reasonable whether we support the rename of columns when replacing
the view definition, or not. Attached is the patch that adds support for
ALTER VIEW RENAME COLUMN command.I have made just a similar
change to view and it works.Yeah, ISTM that we made the same patch at the same time. You changed
gram.y,
but I added the following changes additionally.- Update the doc
- Add HINT message emit when CRAETE OR REPLACE VIEW fails to rename the
columns
- Update tab-complete.c
- Add regression testOh, I just sent the patch to ask, good you do that in all the places.
One issue I've not addressed yet is about the command tag of
"ALTER VIEW RENAME COLUMN". Currently "ALTER TABLE" is returned as the
tag
like ALTER MATERIALIZED VIEW RENAME COLUMN, but ISTM that "ALTER VIEW"
is better. I started the discussion about the command tag of
"ALTER MATERIALIZED VIEW" at another thread. I will update the patch
according
to the result of that discussion./messages/by-id/CAHGQGwGUaC03FFdTFoHsCuDrrNvFvNVQ6xyd40==P25WvuBJjg@mail.gmail.com
Attached patch contain small change for ALTER MATERIALIZED VIEW.
Hmm, my small change of "ALTER MATERIALIZED VIEW" does not work in some
cases need more work on that.
The AlterObjectTypeCommandTag function just take one parameter, but to
show "ALTER MATERIALIZED VIEW" instead of ALTER TABLE we need to
pass "relationType = OBJECT_MATVIEW" along with "renameType = OBJECT_COLUMN"
and handle that in the function. The "AlterObjectTypeCommandTag" function
has many
calls. Do you think just for the command tag, we should do all this change?
Regards,
--
Fujii Masao--
Ibrar Ahmed--
Ibrar Ahmed
--
Ibrar Ahmed
Fujii Masao <masao.fujii@gmail.com> writes:
On Thu, Oct 31, 2019 at 1:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fujii Masao <masao.fujii@gmail.com> writes:
Currently CREATE OR REPLACE VIEW command fails if the column names
are changed.
That is, I believe, intentional. It's an effective aid to catching
mistakes in view redefinitions, such as misaligning the new set of
columns relative to the old. [example]
This example makes me wonder if the addtion of column by
CREATE OR REPLACE VIEW also has the same (or even worse) issue.
That is, it may increase the oppotunity for users' mistake.
The idea in CREATE OR REPLACE VIEW is to allow addition of new
columns at the end, the same as you can do with tables. Checking
the column name matchups is a way to ensure that you actually do
add at the end, rather than insert, which wouldn't act as you
expect. Admittedly it's only heuristic.
We could, perhaps, have insisted that adding a column also requires
special syntax, but we didn't. Consider for example a case where
the new column needs to come from an additionally joined table;
then you have to be able to edit the underlying view definition along
with adding the column. So that seems like kind of a pain in the
neck to insist on.
But what's the worse is that, currently there is no way to
drop the column from the view, except recreation of the view.
I think this has been discussed, as well. It's not necessarily
simple to drop a view output column. For example, if the view
uses SELECT DISTINCT, removing an output column would have
semantic effects on the set of rows that can be returned, since
distinct-ness would now mean something else than it did before.
It's conceivable that we could enumerate all such effects and
then allow DROP COLUMN (probably replacing the output column
with a null constant) if none of them apply, but I can't get
terribly excited about it. The field demand for such a feature
has not been high. I'd be a bit worried about bugs arising
from failures to check attisdropped for views, too; so that
the cost of getting this working might be greater than it seems
on the surface.
regards, tom lane
On Thu, Oct 31, 2019 at 10:54 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fujii Masao <masao.fujii@gmail.com> writes:
On Thu, Oct 31, 2019 at 1:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fujii Masao <masao.fujii@gmail.com> writes:
Currently CREATE OR REPLACE VIEW command fails if the column names
are changed.That is, I believe, intentional. It's an effective aid to catching
mistakes in view redefinitions, such as misaligning the new set of
columns relative to the old. [example]This example makes me wonder if the addtion of column by
CREATE OR REPLACE VIEW also has the same (or even worse) issue.
That is, it may increase the oppotunity for users' mistake.The idea in CREATE OR REPLACE VIEW is to allow addition of new
columns at the end, the same as you can do with tables. Checking
the column name matchups is a way to ensure that you actually do
add at the end, rather than insert, which wouldn't act as you
expect. Admittedly it's only heuristic.We could, perhaps, have insisted that adding a column also requires
special syntax, but we didn't. Consider for example a case where
the new column needs to come from an additionally joined table;
then you have to be able to edit the underlying view definition along
with adding the column. So that seems like kind of a pain in the
neck to insist on.But what's the worse is that, currently there is no way to
drop the column from the view, except recreation of the view.I think this has been discussed, as well. It's not necessarily
simple to drop a view output column. For example, if the view
uses SELECT DISTINCT, removing an output column would have
semantic effects on the set of rows that can be returned, since
distinct-ness would now mean something else than it did before.It's conceivable that we could enumerate all such effects and
then allow DROP COLUMN (probably replacing the output column
with a null constant) if none of them apply, but I can't get
terribly excited about it. The field demand for such a feature
has not been high. I'd be a bit worried about bugs arising
from failures to check attisdropped for views, too; so that
the cost of getting this working might be greater than it seems
on the surface.
Thanks for the explanation! Understood.
Regards,
--
Fujii Masao
2019-10-31 21:01 に Fujii Masao さんは書きました:
On Thu, Oct 31, 2019 at 7:59 PM Ibrar Ahmed <ibrar.ahmad@gmail.com>
wrote:On Thu, Oct 31, 2019 at 12:32 PM Fujii Masao <masao.fujii@gmail.com>
wrote:On Thu, Oct 31, 2019 at 1:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fujii Masao <masao.fujii@gmail.com> writes:
Currently CREATE OR REPLACE VIEW command fails if the column names
are changed.That is, I believe, intentional. It's an effective aid to catching
mistakes in view redefinitions, such as misaligning the new set of
columns relative to the old. That's particularly important given
that we allow you to add columns during CREATE OR REPLACE VIEW.
Consider the oversimplified case where you start withCREATE VIEW v AS SELECT 1 AS x, 2 AS y;
and you want to add a column z, and you get sloppy and write
CREATE OR REPLACE VIEW v AS SELECT 1 AS x, 3 AS z, 2 AS y;
If we did not throw an error on this, references that formerly
pointed to column y would now point to z (as that's still attnum 2),
which is highly unlikely to be what you wanted.This example makes me wonder if the addtion of column by
CREATE OR REPLACE VIEW also has the same (or even worse) issue.
That is, it may increase the oppotunity for users' mistake.
I'm thinking the case where users mistakenly added new column
into the view when replacing the view definition. This mistake can
happen because CREATE OR REPLACE VIEW allows new column to
be added. But what's the worse is that, currently there is no way to
drop the column from the view, except recreation of the view.
Neither CREATE OR REPLACE VIEW nor ALTER TABLE support
the drop of the column from the view. So, to fix the mistake,
users would need to drop the view itself and recreate it. If there
are
some objects depending the view, they also might need to be
recreated.
This looks not good. Since the feature has been supported,
it's too late to say that, though...At least, the support for ALTER VIEW DROP COLUMN might be
necessary to alleviate that situation.- Is this intentional not implemented the "RENAME COLUMN" statement
for
VIEW because it is implemented for Materialized View?Not sure that, but Tom's suggestion to support ALTER VIEW RENAME COLUMN
sounds reasonable whether we support the rename of columns when
replacing
the view definition, or not. Attached is the patch that adds support
for
ALTER VIEW RENAME COLUMN command.I have made just a similar
change to view and it works.Yeah, ISTM that we made the same patch at the same time. You changed
gram.y,
but I added the following changes additionally.- Update the doc
- Add HINT message emit when CRAETE OR REPLACE VIEW fails to rename the
columns
- Update tab-complete.c
I review your patch, and then I found that tab complete of "alter
materialized view" is also not enough.
So, I made a small patch referencing your patch.
Regards,
Attachments:
alter_materialized_view.patchtext/x-diff; name=alter_materialized_view.patchDownload
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 2b1e3cd..c493b3e 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1801,9 +1801,19 @@ psql_completion(const char *text, int start, int end)
"SET SCHEMA");
/* ALTER MATERIALIZED VIEW <name> */
else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny))
- COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME TO",
+ COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
"SET SCHEMA");
-
+ /* ALTER MATERIALIZED VIEW xxx RENAME */
+ else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny, "RENAME"))
+ COMPLETE_WITH_ATTR(prev2_wd, " UNION SELECT 'COLUMN' UNION SELECT 'TO'");
+ else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny, "ALTER|RENAME", "COLUMN"))
+ COMPLETE_WITH_ATTR(prev3_wd, "");
+ /* ALTER MATERIALIZED VIEW xxx RENAME yyy */
+ else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny, "RENAME", MatchAnyExcept("TO")))
+ COMPLETE_WITH("TO");
+ /* ALTER MATERIALIZED VIEW xxx RENAME COLUMN yyy */
+ else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny, "RENAME", "COLUMN", MatchAnyExcept("TO")))
+ COMPLETE_WITH("TO");
/* ALTER POLICY <name> */
else if (Matches("ALTER", "POLICY"))
COMPLETE_WITH_QUERY(Query_for_list_of_policies);
On Thu, Oct 31, 2019 at 9:34 PM Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote:
On Thu, Oct 31, 2019 at 5:28 PM Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote:
On Thu, Oct 31, 2019 at 5:11 PM Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote:
On Thu, Oct 31, 2019 at 5:01 PM Fujii Masao <masao.fujii@gmail.com> wrote:
On Thu, Oct 31, 2019 at 7:59 PM Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote:
On Thu, Oct 31, 2019 at 12:32 PM Fujii Masao <masao.fujii@gmail.com> wrote:
On Thu, Oct 31, 2019 at 1:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fujii Masao <masao.fujii@gmail.com> writes:
Currently CREATE OR REPLACE VIEW command fails if the column names
are changed.That is, I believe, intentional. It's an effective aid to catching
mistakes in view redefinitions, such as misaligning the new set of
columns relative to the old. That's particularly important given
that we allow you to add columns during CREATE OR REPLACE VIEW.
Consider the oversimplified case where you start withCREATE VIEW v AS SELECT 1 AS x, 2 AS y;
and you want to add a column z, and you get sloppy and write
CREATE OR REPLACE VIEW v AS SELECT 1 AS x, 3 AS z, 2 AS y;
If we did not throw an error on this, references that formerly
pointed to column y would now point to z (as that's still attnum 2),
which is highly unlikely to be what you wanted.This example makes me wonder if the addtion of column by
CREATE OR REPLACE VIEW also has the same (or even worse) issue.
That is, it may increase the oppotunity for users' mistake.
I'm thinking the case where users mistakenly added new column
into the view when replacing the view definition. This mistake can
happen because CREATE OR REPLACE VIEW allows new column to
be added. But what's the worse is that, currently there is no way to
drop the column from the view, except recreation of the view.
Neither CREATE OR REPLACE VIEW nor ALTER TABLE support
the drop of the column from the view. So, to fix the mistake,
users would need to drop the view itself and recreate it. If there are
some objects depending the view, they also might need to be recreated.
This looks not good. Since the feature has been supported,
it's too late to say that, though...At least, the support for ALTER VIEW DROP COLUMN might be
necessary to alleviate that situation.- Is this intentional not implemented the "RENAME COLUMN" statement for
VIEW because it is implemented for Materialized View?Not sure that, but Tom's suggestion to support ALTER VIEW RENAME COLUMN
sounds reasonable whether we support the rename of columns when replacing
the view definition, or not. Attached is the patch that adds support for
ALTER VIEW RENAME COLUMN command.I have made just a similar
change to view and it works.Yeah, ISTM that we made the same patch at the same time. You changed gram.y,
but I added the following changes additionally.- Update the doc
- Add HINT message emit when CRAETE OR REPLACE VIEW fails to rename the columns
- Update tab-complete.c
- Add regression testOh, I just sent the patch to ask, good you do that in all the places.
One issue I've not addressed yet is about the command tag of
"ALTER VIEW RENAME COLUMN". Currently "ALTER TABLE" is returned as the tag
like ALTER MATERIALIZED VIEW RENAME COLUMN, but ISTM that "ALTER VIEW"
is better. I started the discussion about the command tag of
"ALTER MATERIALIZED VIEW" at another thread. I will update the patch according
to the result of that discussion.
/messages/by-id/CAHGQGwGUaC03FFdTFoHsCuDrrNvFvNVQ6xyd40==P25WvuBJjg@mail.gmail.comAttached patch contain small change for ALTER MATERIALIZED VIEW.
Hmm, my small change of "ALTER MATERIALIZED VIEW" does not work in some cases need more work on that.
The AlterObjectTypeCommandTag function just take one parameter, but to
show "ALTER MATERIALIZED VIEW" instead of ALTER TABLE we need to
pass "relationType = OBJECT_MATVIEW" along with "renameType = OBJECT_COLUMN"
and handle that in the function. The "AlterObjectTypeCommandTag" function has many
calls. Do you think just for the command tag, we should do all this change?
Thanks for trying to address the issue!
As probably you've already noticed, the commit 979766c0af fixed the issue
thanks to your review. So we can review the patch that I posted.
Regards,
--
Fujii Masao
On Wed, Nov 6, 2019 at 4:14 PM btfujiitkp <btfujiitkp@oss.nttdata.com> wrote:
2019-10-31 21:01 に Fujii Masao さんは書きました:
On Thu, Oct 31, 2019 at 7:59 PM Ibrar Ahmed <ibrar.ahmad@gmail.com>
wrote:On Thu, Oct 31, 2019 at 12:32 PM Fujii Masao <masao.fujii@gmail.com>
wrote:On Thu, Oct 31, 2019 at 1:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fujii Masao <masao.fujii@gmail.com> writes:
Currently CREATE OR REPLACE VIEW command fails if the column names
are changed.That is, I believe, intentional. It's an effective aid to catching
mistakes in view redefinitions, such as misaligning the new set of
columns relative to the old. That's particularly important given
that we allow you to add columns during CREATE OR REPLACE VIEW.
Consider the oversimplified case where you start withCREATE VIEW v AS SELECT 1 AS x, 2 AS y;
and you want to add a column z, and you get sloppy and write
CREATE OR REPLACE VIEW v AS SELECT 1 AS x, 3 AS z, 2 AS y;
If we did not throw an error on this, references that formerly
pointed to column y would now point to z (as that's still attnum 2),
which is highly unlikely to be what you wanted.This example makes me wonder if the addtion of column by
CREATE OR REPLACE VIEW also has the same (or even worse) issue.
That is, it may increase the oppotunity for users' mistake.
I'm thinking the case where users mistakenly added new column
into the view when replacing the view definition. This mistake can
happen because CREATE OR REPLACE VIEW allows new column to
be added. But what's the worse is that, currently there is no way to
drop the column from the view, except recreation of the view.
Neither CREATE OR REPLACE VIEW nor ALTER TABLE support
the drop of the column from the view. So, to fix the mistake,
users would need to drop the view itself and recreate it. If there
are
some objects depending the view, they also might need to be
recreated.
This looks not good. Since the feature has been supported,
it's too late to say that, though...At least, the support for ALTER VIEW DROP COLUMN might be
necessary to alleviate that situation.- Is this intentional not implemented the "RENAME COLUMN" statement
for
VIEW because it is implemented for Materialized View?Not sure that, but Tom's suggestion to support ALTER VIEW RENAME COLUMN
sounds reasonable whether we support the rename of columns when
replacing
the view definition, or not. Attached is the patch that adds support
for
ALTER VIEW RENAME COLUMN command.I have made just a similar
change to view and it works.Yeah, ISTM that we made the same patch at the same time. You changed
gram.y,
but I added the following changes additionally.- Update the doc
- Add HINT message emit when CRAETE OR REPLACE VIEW fails to rename the
columns
- Update tab-complete.cI review your patch, and then I found that tab complete of "alter
materialized view" is also not enough.
So, I made a small patch referencing your patch.
Good catch! The patch basically looks good to me.
But I think that "ALTER MATERIALIZED VIEW xxx <TAB>" should output also
DEPENDS ON EXTENSION, SET TABLESPACE, CLUSTER ON and RESET.
So I added such tab-completes to your patch. Patch attached.
Barring any objection, I'm thinking to commit this patch.
Regards,
--
Fujii Masao
Attachments:
alter_materialized_view_v2.patchapplication/octet-stream; name=alter_materialized_view_v2.patchDownload
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 98c917bf7a..1578568a91 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1801,9 +1801,22 @@ psql_completion(const char *text, int start, int end)
"SET SCHEMA");
/* ALTER MATERIALIZED VIEW <name> */
else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny))
- COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME TO",
- "SET SCHEMA");
-
+ COMPLETE_WITH("ALTER COLUMN", "CLUSTER ON", "DEPENDS ON EXTENSION",
+ "OWNER TO", "RENAME", "RESET (", "SET");
+ /* ALTER MATERIALIZED VIEW xxx RENAME */
+ else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny, "RENAME"))
+ COMPLETE_WITH_ATTR(prev2_wd, " UNION SELECT 'COLUMN' UNION SELECT 'TO'");
+ else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny, "ALTER|RENAME", "COLUMN"))
+ COMPLETE_WITH_ATTR(prev3_wd, "");
+ /* ALTER MATERIALIZED VIEW xxx RENAME yyy */
+ else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny, "RENAME", MatchAnyExcept("TO")))
+ COMPLETE_WITH("TO");
+ /* ALTER MATERIALIZED VIEW xxx RENAME COLUMN yyy */
+ else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny, "RENAME", "COLUMN", MatchAnyExcept("TO")))
+ COMPLETE_WITH("TO");
+ /* ALTER MATERIALIZED VIEW xxx SET */
+ else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny, "SET"))
+ COMPLETE_WITH("(", "SCHEMA", "TABLESPACE", "WITHOUT CLUSTER");
/* ALTER POLICY <name> */
else if (Matches("ALTER", "POLICY"))
COMPLETE_WITH_QUERY(Query_for_list_of_policies);
Barring any objection, I'm thinking to commit this patch.
Regards,
Build and All Test has passed .
Looks good to me .
Regards,
On Wed, Nov 20, 2019 at 1:11 PM btkimurayuzk
<btkimurayuzk@oss.nttdata.com> wrote:
Barring any objection, I'm thinking to commit this patch.
Regards,
Build and All Test has passed .
Looks good to me .
Thanks for reviewing the patch!
I committed the following two patches.
- Allow ALTER VIEW command to rename the column in the view.
- Improve tab-completion for ALTER MATERIALIZED VIEW.
Regards,
--
Fujii Masao