Hi Nikhil,
Nikhil Sontakke <[email protected]> wrote:
> The attached patch calls the existing function from the drop path, after
> the column is checked for droppability and before recursion to
> inheritance children, so both calls and every level are covered.
I reviewed v2 on master at 9e17d25e79d. It applies cleanly, builds
without warnings, and make check and contrib/test_decoding pass. The
new tests in alter_table fail on master without the tablecmds.c change,
so they do exercise the fix. Both of your cases, ALTER TYPE ... DROP
ATTRIBUTE with a column of the type and ALTER TABLE ... DROP COLUMN with
a column of the table's row type, are refused, and the stored values
stay distinct.
One gap remains: find_composite_type_dependencies() only looks for
columns of the row type, so the drop still goes through when a value of
the type is stored as a constant inside an expression, with the same
effect you describe for columns. On v2:
create type s1 as (a int, b int);
create view vs1 as select '(1,2)'::s1 = '(1,3)'::s1 as eq; -- f
alter type s1 drop attribute b; -- accepted
select eq from vs1; -- t
A CHECK constraint is worse, because the table stops accepting rows:
create type s3 as (a int, b int);
create table ts3 (i int check ('(1,2)'::s3 <> '(1,3)'::s3));
alter type s3 drop attribute b; -- accepted
insert into ts3 values (1);
ERROR: new row for relation "ts3" violates check constraint "ts3_check"
The same happens with a column DEFAULT, a function parameter default,
and with ALTER TABLE ... DROP COLUMN when the table's row type is used
the same way.
On pgsql-bugs, in a thread about a related problem [1], I posted a
patch that makes find_composite_type_dependencies() look inside stored
expressions for constants of the row type (pg_attrdef, pg_constraint,
pg_policy, pg_proc, pg_rewrite, pg_statistic_ext, pg_trigger, and index
and partition key expressions). It was written for ALTER COLUMN TYPE,
but because your patch calls the same function from the drop path, the
two compose: with both applied, all six stored-expression cases are
refused and the stored values keep their meaning, while a function that
only takes the type as a parameter, or a view that selects NULL::s1,
is still allowed. The two patches apply cleanly together, and make
check and test_decoding pass with both. Neither covers all of this on
its own.
Testing them together also showed a problem in mine rather than in
yours: for a stand-alone composite type my new messages said
cannot alter table "s1" because rule _RETURN on view vs1 stores a
constant of its row type
where the existing ones, which your patch reaches, correctly say
"cannot alter type". That was already wrong in my v1 on its own (ALTER
TYPE ... ALTER ATTRIBUTE ... TYPE shows it too), and v2, posted on that
thread, fixes it. With your v2 and mine, the drops above report
"cannot alter type".
Heikki, I've added you because d78040a469b touched this call site "to
make backpatching future patches a little easier", in case this
overlaps with what you have in mind.
The SQL for all the cases above, with controls, is attached.
[1]
https://postgr.es/m/CA+bCEdDvsVApd+AADX=5uyj6q5c3r5aruwnzofayqafqlwe...@mail.gmail.com
Regards,
Manu
-- #7169 matriz: DROP ATTRIBUTE / DROP COLUMN sobre un tipo compuesto cuyo
-- valor esta GUARDADO en algun lado. Por escenario: el DROP (OK o ERROR con
-- SQLSTATE) y que devuelve DESPUES lo que estaba guardado.
-- Cada escenario usa su propio tipo para no contaminar al siguiente.
\set ON_ERROR_STOP off
\set SHOW_CONTEXT never
\pset footer off
\pset tuples_only on
\echo '--- N1 (Nikhil) columna de tabla + unique; ALTER TYPE DROP ATTRIBUTE'
create type n1 as (a int, b int);
create table tn1 (v n1); create unique index on tn1 (v);
insert into tn1 values (row(1,2)::n1), (row(1,3)::n1);
alter type n1 drop attribute b;
\if :ERROR \echo ' => DROP ERROR' :LAST_ERROR_SQLSTATE \else \echo ' =>
DROP OK' \endif
\echo ' => POST count(distinct v) (era 2):'
select count(distinct v) from tn1;
\echo '--- N2 rowtype de tabla guardado en otra tabla; ALTER TABLE DROP COLUMN'
create table rn2 (a int, b int);
create table tn2 (v rn2); insert into tn2 values (row(1,2)::rn2),
(row(1,3)::rn2);
alter table rn2 drop column b;
\if :ERROR \echo ' => DROP ERROR' :LAST_ERROR_SQLSTATE \else \echo ' =>
DROP OK' \endif
\echo ' => POST count(distinct v) (era 2):'
select count(distinct v) from tn2;
\echo '--- S1 vista con constante del tipo: (1,2) = (1,3)'
create type s1 as (a int, b int);
create view vs1 as select '(1,2)'::s1 = '(1,3)'::s1 as eq;
alter type s1 drop attribute b;
\if :ERROR \echo ' => DROP ERROR' :LAST_ERROR_SQLSTATE \else \echo ' =>
DROP OK' \endif
\echo ' => POST eq (era f):'
select eq from vs1;
\echo '--- S2 vista que muestra la constante como texto'
create type s2 as (a int, b int);
create view vs2 as select '(1,2)'::s2::text as t;
alter type s2 drop attribute b;
\if :ERROR \echo ' => DROP ERROR' :LAST_ERROR_SQLSTATE \else \echo ' =>
DROP OK' \endif
\echo ' => POST t (era (1,2)):'
select t from vs2;
\echo '--- S3 CHECK de una tabla int que compara constantes del tipo'
create type s3 as (a int, b int);
create table ts3 (i int check ('(1,2)'::s3 <> '(1,3)'::s3));
alter type s3 drop attribute b;
\if :ERROR \echo ' => DROP ERROR' :LAST_ERROR_SQLSTATE \else \echo ' =>
DROP OK' \endif
\echo ' => POST insert into ts3 (antes se podia):'
insert into ts3 values (1);
\if :ERROR \echo ' insert ERROR' :LAST_ERROR_SQLSTATE \else \echo '
insert OK' \endif
\echo '--- S4 DEFAULT con la constante como texto'
create type s4 as (a int, b int);
create table ts4 (t text default '(1,2)'::s4::text);
alter type s4 drop attribute b;
\if :ERROR \echo ' => DROP ERROR' :LAST_ERROR_SQLSTATE \else \echo ' =>
DROP OK' \endif
insert into ts4 default values;
\echo ' => POST default insertado (era (1,2)):'
select t from ts4;
\echo '--- S5 default de parametro de funcion'
create type s5 as (a int, b int);
create function fs5(x s5 default '(1,2)'::s5) returns text language sql as
'select x::text';
alter type s5 drop attribute b;
\if :ERROR \echo ' => DROP ERROR' :LAST_ERROR_SQLSTATE \else \echo ' =>
DROP OK' \endif
\echo ' => POST fs5() (era (1,2)):'
select fs5();
\echo '--- S6 rowtype de TABLA en una vista; ALTER TABLE DROP COLUMN'
create table rs6 (a int, b int);
create view vs6 as select '(1,2)'::rs6 = '(1,3)'::rs6 as eq;
alter table rs6 drop column b;
\if :ERROR \echo ' => DROP ERROR' :LAST_ERROR_SQLSTATE \else \echo ' =>
DROP OK' \endif
\echo ' => POST eq (era f):'
select eq from vs6;
\echo '--- C1 control: funcion que solo NOMBRA el tipo (sin constante)'
create type c1 as (a int, b int);
create function fc1(x c1) returns int language sql as 'select 1';
alter type c1 drop attribute b;
\if :ERROR \echo ' => DROP ERROR' :LAST_ERROR_SQLSTATE \else \echo ' =>
DROP OK' \endif
\echo '--- C2 control: vista con NULL del tipo (no hay imagen guardada)'
create type c2 as (a int, b int);
create view vc2 as select null::c2 as x;
alter type c2 drop attribute b;
\if :ERROR \echo ' => DROP ERROR' :LAST_ERROR_SQLSTATE \else \echo ' =>
DROP OK' \endif
