* Stephen Frost (sfr...@snowman.net) wrote: > > Looks like there is an issue here with CHECK constraints and NOT NULL > > constraints, yes. The uniqueness check complains about the key already > > existing and returns the key, but I don't think that's actually a > > problem- to get that to happen you have to specify the new key and > > that's what is returned. > > Yeah, I take that back. If there is a composite key involved then you > can run into the same issue- you update one of the columns to a > conflicting value and get back the entire key, including columns you > shouldn't be allowed to see.
Alright, attached is a patch which addresses this by checking if the
user has SELECT rights on the relation first and, if so, the existing
error message is returned with the full row as usual, but if not, then
the row data is omitted.
Also attached is a patch for 9.4 which does the same, but also removes
the row reporting (completely) from the WITH CHECK case. It could be
argued that it would be helpful to have it there also, perhaps, but I'm
not convinced at this point that it's really valuable- and we'd have to
think about how this would work with views (permission on the view? or
on the table underneath? what if there is more than one?, etc).
Thanks,
Stephen
diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c
new file mode 100644
index 17d9765..f9a8bff
*** a/src/backend/access/nbtree/nbtinsert.c
--- b/src/backend/access/nbtree/nbtinsert.c
***************
*** 21,26 ****
--- 21,27 ----
#include "miscadmin.h"
#include "storage/lmgr.h"
#include "storage/predicate.h"
+ #include "utils/acl.h"
#include "utils/inval.h"
#include "utils/tqual.h"
*************** _bt_check_unique(Relation rel, IndexTupl
*** 387,400 ****
index_deform_tuple(itup, RelationGetDescr(rel),
values, isnull);
! ereport(ERROR,
! (errcode(ERRCODE_UNIQUE_VIOLATION),
! errmsg("duplicate key value violates unique constraint \"%s\"",
! RelationGetRelationName(rel)),
! errdetail("Key %s already exists.",
! BuildIndexValueDescription(rel,
! values, isnull)),
! errtableconstraint(heapRel,
RelationGetRelationName(rel))));
}
}
--- 388,420 ----
index_deform_tuple(itup, RelationGetDescr(rel),
values, isnull);
! /*
! * When reporting a failure, it can be handy to have
! * the key which failed reported. Unfortunately, when
! * using column-level privileges, this could expose
! * a column the user does not have rights for. Instead,
! * only report the key if the user has full table-level
! * SELECT rights on the relation.
! */
! if (pg_class_aclcheck(RelationGetRelid(rel),
! GetUserId(), ACL_SELECT) ==
! ACLCHECK_OK)
! ereport(ERROR,
! (errcode(ERRCODE_UNIQUE_VIOLATION),
! errmsg("duplicate key value violates unique constraint \"%s\"",
! RelationGetRelationName(rel)),
! errdetail("Key %s already exists.",
! BuildIndexValueDescription(rel,
! values,
! isnull)),
! errtableconstraint(heapRel,
! RelationGetRelationName(rel))));
! else
! ereport(ERROR,
! (errcode(ERRCODE_UNIQUE_VIOLATION),
! errmsg("duplicate key value violates unique constraint \"%s\"",
! RelationGetRelationName(rel)),
! errtableconstraint(heapRel,
RelationGetRelationName(rel))));
}
}
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
new file mode 100644
index 85d1c63..fe98599
*** a/src/backend/executor/execMain.c
--- b/src/backend/executor/execMain.c
*************** ExecConstraints(ResultRelInfo *resultRel
*** 1600,1614 ****
{
if (tupdesc->attrs[attrChk - 1]->attnotnull &&
slot_attisnull(slot, attrChk))
! ereport(ERROR,
! (errcode(ERRCODE_NOT_NULL_VIOLATION),
! errmsg("null value in column \"%s\" violates not-null constraint",
! NameStr(tupdesc->attrs[attrChk - 1]->attname)),
! errdetail("Failing row contains %s.",
! ExecBuildSlotValueDescription(slot,
! tupdesc,
! 64)),
! errtablecol(rel, attrChk)));
}
}
--- 1600,1631 ----
{
if (tupdesc->attrs[attrChk - 1]->attnotnull &&
slot_attisnull(slot, attrChk))
! {
! /*
! * When reporting failure, it's handy to have the whole row
! * which failed returned, but we can only show it if the user
! * has full SELECT rights on the relation, otherwise we might
! * end up showing fields the user does not have access to, due
! * to column-level privileges.
! */
! if (pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
! ACL_SELECT) == ACLCHECK_OK)
! ereport(ERROR,
! (errcode(ERRCODE_NOT_NULL_VIOLATION),
! errmsg("null value in column \"%s\" violates not-null constraint",
! NameStr(tupdesc->attrs[attrChk - 1]->attname)),
! errdetail("Failing row contains %s.",
! ExecBuildSlotValueDescription(slot,
! tupdesc,
! 64)),
! errtablecol(rel, attrChk)));
! else
! ereport(ERROR,
! (errcode(ERRCODE_NOT_NULL_VIOLATION),
! errmsg("null value in column \"%s\" violates not-null constraint",
! NameStr(tupdesc->attrs[attrChk - 1]->attname)),
! errtablecol(rel, attrChk)));
! }
}
}
*************** ExecConstraints(ResultRelInfo *resultRel
*** 1617,1631 ****
const char *failed;
if ((failed = ExecRelCheck(resultRelInfo, slot, estate)) != NULL)
! ereport(ERROR,
! (errcode(ERRCODE_CHECK_VIOLATION),
! errmsg("new row for relation \"%s\" violates check constraint \"%s\"",
! RelationGetRelationName(rel), failed),
! errdetail("Failing row contains %s.",
! ExecBuildSlotValueDescription(slot,
! tupdesc,
! 64)),
! errtableconstraint(rel, failed)));
}
}
--- 1634,1659 ----
const char *failed;
if ((failed = ExecRelCheck(resultRelInfo, slot, estate)) != NULL)
! {
! /* Check table-level SELECT rights on the relation, see above */
! if (pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
! ACL_SELECT) == ACLCHECK_OK)
! ereport(ERROR,
! (errcode(ERRCODE_CHECK_VIOLATION),
! errmsg("new row for relation \"%s\" violates check constraint \"%s\"",
! RelationGetRelationName(rel), failed),
! errdetail("Failing row contains %s.",
! ExecBuildSlotValueDescription(slot,
! tupdesc,
! 64)),
! errtableconstraint(rel, failed)));
! else
! ereport(ERROR,
! (errcode(ERRCODE_CHECK_VIOLATION),
! errmsg("new row for relation \"%s\" violates check constraint \"%s\"",
! RelationGetRelationName(rel), failed),
! errtableconstraint(rel, failed)));
! }
}
}
diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c
new file mode 100644
index 59d7006..0af3a4f
*** a/src/backend/access/nbtree/nbtinsert.c
--- b/src/backend/access/nbtree/nbtinsert.c
***************
*** 21,26 ****
--- 21,27 ----
#include "miscadmin.h"
#include "storage/lmgr.h"
#include "storage/predicate.h"
+ #include "utils/acl.h"
#include "utils/tqual.h"
*************** _bt_check_unique(Relation rel, IndexTupl
*** 391,404 ****
index_deform_tuple(itup, RelationGetDescr(rel),
values, isnull);
! ereport(ERROR,
! (errcode(ERRCODE_UNIQUE_VIOLATION),
! errmsg("duplicate key value violates unique constraint \"%s\"",
! RelationGetRelationName(rel)),
! errdetail("Key %s already exists.",
! BuildIndexValueDescription(rel,
! values, isnull)),
! errtableconstraint(heapRel,
RelationGetRelationName(rel))));
}
}
--- 392,424 ----
index_deform_tuple(itup, RelationGetDescr(rel),
values, isnull);
! /*
! * When reporting a failure, it can be handy to have
! * the key which failed reported. Unfortunately, when
! * using column-level privileges, this could expose
! * a column the user does not have rights for. Instead,
! * only report the key if the user has full table-level
! * SELECT rights on the relation.
! */
! if (pg_class_aclcheck(RelationGetRelid(rel),
! GetUserId(), ACL_SELECT) ==
! ACLCHECK_OK)
! ereport(ERROR,
! (errcode(ERRCODE_UNIQUE_VIOLATION),
! errmsg("duplicate key value violates unique constraint \"%s\"",
! RelationGetRelationName(rel)),
! errdetail("Key %s already exists.",
! BuildIndexValueDescription(rel,
! values,
! isnull)),
! errtableconstraint(heapRel,
! RelationGetRelationName(rel))));
! else
! ereport(ERROR,
! (errcode(ERRCODE_UNIQUE_VIOLATION),
! errmsg("duplicate key value violates unique constraint \"%s\"",
! RelationGetRelationName(rel)),
! errtableconstraint(heapRel,
RelationGetRelationName(rel))));
}
}
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
new file mode 100644
index 01eda70..b1f27a2
*** a/src/backend/executor/execMain.c
--- b/src/backend/executor/execMain.c
*************** ExecConstraints(ResultRelInfo *resultRel
*** 1602,1616 ****
{
if (tupdesc->attrs[attrChk - 1]->attnotnull &&
slot_attisnull(slot, attrChk))
! ereport(ERROR,
! (errcode(ERRCODE_NOT_NULL_VIOLATION),
! errmsg("null value in column \"%s\" violates not-null constraint",
! NameStr(tupdesc->attrs[attrChk - 1]->attname)),
! errdetail("Failing row contains %s.",
! ExecBuildSlotValueDescription(slot,
! tupdesc,
! 64)),
! errtablecol(rel, attrChk)));
}
}
--- 1602,1633 ----
{
if (tupdesc->attrs[attrChk - 1]->attnotnull &&
slot_attisnull(slot, attrChk))
! {
! /*
! * When reporting failure, it's handy to have the whole row
! * which failed returned, but we can only show it if the user
! * has full SELECT rights on the relation, otherwise we might
! * end up showing fields the user does not have access to, due
! * to column-level privileges.
! */
! if (pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
! ACL_SELECT) == ACLCHECK_OK)
! ereport(ERROR,
! (errcode(ERRCODE_NOT_NULL_VIOLATION),
! errmsg("null value in column \"%s\" violates not-null constraint",
! NameStr(tupdesc->attrs[attrChk - 1]->attname)),
! errdetail("Failing row contains %s.",
! ExecBuildSlotValueDescription(slot,
! tupdesc,
! 64)),
! errtablecol(rel, attrChk)));
! else
! ereport(ERROR,
! (errcode(ERRCODE_NOT_NULL_VIOLATION),
! errmsg("null value in column \"%s\" violates not-null constraint",
! NameStr(tupdesc->attrs[attrChk - 1]->attname)),
! errtablecol(rel, attrChk)));
! }
}
}
*************** ExecConstraints(ResultRelInfo *resultRel
*** 1619,1633 ****
const char *failed;
if ((failed = ExecRelCheck(resultRelInfo, slot, estate)) != NULL)
! ereport(ERROR,
! (errcode(ERRCODE_CHECK_VIOLATION),
! errmsg("new row for relation \"%s\" violates check constraint \"%s\"",
! RelationGetRelationName(rel), failed),
! errdetail("Failing row contains %s.",
! ExecBuildSlotValueDescription(slot,
! tupdesc,
! 64)),
! errtableconstraint(rel, failed)));
}
}
--- 1636,1661 ----
const char *failed;
if ((failed = ExecRelCheck(resultRelInfo, slot, estate)) != NULL)
! {
! /* Check table-level SELECT rights on the relation, see above */
! if (pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
! ACL_SELECT) == ACLCHECK_OK)
! ereport(ERROR,
! (errcode(ERRCODE_CHECK_VIOLATION),
! errmsg("new row for relation \"%s\" violates check constraint \"%s\"",
! RelationGetRelationName(rel), failed),
! errdetail("Failing row contains %s.",
! ExecBuildSlotValueDescription(slot,
! tupdesc,
! 64)),
! errtableconstraint(rel, failed)));
! else
! ereport(ERROR,
! (errcode(ERRCODE_CHECK_VIOLATION),
! errmsg("new row for relation \"%s\" violates check constraint \"%s\"",
! RelationGetRelationName(rel), failed),
! errtableconstraint(rel, failed)));
! }
}
}
*************** ExecWithCheckOptions(ResultRelInfo *resu
*** 1669,1679 ****
ereport(ERROR,
(errcode(ERRCODE_WITH_CHECK_OPTION_VIOLATION),
errmsg("new row violates WITH CHECK OPTION for view \"%s\"",
! wco->viewname),
! errdetail("Failing row contains %s.",
! ExecBuildSlotValueDescription(slot,
! RelationGetDescr(resultRelInfo->ri_RelationDesc),
! 64))));
}
}
--- 1697,1703 ----
ereport(ERROR,
(errcode(ERRCODE_WITH_CHECK_OPTION_VIOLATION),
errmsg("new row violates WITH CHECK OPTION for view \"%s\"",
! wco->viewname)));
}
}
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
new file mode 100644
index 507b6a2..b835335
*** a/src/test/regress/expected/updatable_views.out
--- b/src/test/regress/expected/updatable_views.out
*************** SELECT * FROM information_schema.views W
*** 1396,1413 ****
INSERT INTO rw_view1 VALUES(3,4); -- ok
INSERT INTO rw_view1 VALUES(4,3); -- should fail
ERROR: new row violates WITH CHECK OPTION for view "rw_view1"
- DETAIL: Failing row contains (4, 3).
INSERT INTO rw_view1 VALUES(5,null); -- should fail
ERROR: new row violates WITH CHECK OPTION for view "rw_view1"
- DETAIL: Failing row contains (5, null).
UPDATE rw_view1 SET b = 5 WHERE a = 3; -- ok
UPDATE rw_view1 SET b = -5 WHERE a = 3; -- should fail
ERROR: new row violates WITH CHECK OPTION for view "rw_view1"
- DETAIL: Failing row contains (3, -5).
INSERT INTO rw_view1(a) VALUES (9); -- ok
INSERT INTO rw_view1(a) VALUES (10); -- should fail
ERROR: new row violates WITH CHECK OPTION for view "rw_view1"
- DETAIL: Failing row contains (10, 10).
SELECT * FROM base_tbl;
a | b
---+----
--- 1396,1409 ----
*************** SELECT * FROM information_schema.views W
*** 1446,1456 ****
INSERT INTO rw_view2 VALUES (-5); -- should fail
ERROR: new row violates WITH CHECK OPTION for view "rw_view1"
- DETAIL: Failing row contains (-5).
INSERT INTO rw_view2 VALUES (5); -- ok
INSERT INTO rw_view2 VALUES (15); -- should fail
ERROR: new row violates WITH CHECK OPTION for view "rw_view2"
- DETAIL: Failing row contains (15).
SELECT * FROM base_tbl;
a
---
--- 1442,1450 ----
*************** SELECT * FROM base_tbl;
*** 1459,1468 ****
UPDATE rw_view2 SET a = a - 10; -- should fail
ERROR: new row violates WITH CHECK OPTION for view "rw_view1"
- DETAIL: Failing row contains (-5).
UPDATE rw_view2 SET a = a + 10; -- should fail
ERROR: new row violates WITH CHECK OPTION for view "rw_view2"
- DETAIL: Failing row contains (15).
CREATE OR REPLACE VIEW rw_view2 AS SELECT * FROM rw_view1 WHERE a < 10
WITH LOCAL CHECK OPTION;
\d+ rw_view2
--- 1453,1460 ----
*************** SELECT * FROM information_schema.views W
*** 1487,1493 ****
INSERT INTO rw_view2 VALUES (-10); -- ok, but not in view
INSERT INTO rw_view2 VALUES (20); -- should fail
ERROR: new row violates WITH CHECK OPTION for view "rw_view2"
- DETAIL: Failing row contains (20).
SELECT * FROM base_tbl;
a
-----
--- 1479,1484 ----
*************** DETAIL: Valid values are "local" and "c
*** 1501,1510 ****
ALTER VIEW rw_view1 SET (check_option=local);
INSERT INTO rw_view2 VALUES (-20); -- should fail
ERROR: new row violates WITH CHECK OPTION for view "rw_view1"
- DETAIL: Failing row contains (-20).
INSERT INTO rw_view2 VALUES (30); -- should fail
ERROR: new row violates WITH CHECK OPTION for view "rw_view2"
- DETAIL: Failing row contains (30).
ALTER VIEW rw_view2 RESET (check_option);
\d+ rw_view2
View "public.rw_view2"
--- 1492,1499 ----
*************** INSERT INTO rw_view2 VALUES (-2); -- ok,
*** 1560,1566 ****
INSERT INTO rw_view2 VALUES (2); -- ok
INSERT INTO rw_view3 VALUES (-3); -- should fail
ERROR: new row violates WITH CHECK OPTION for view "rw_view2"
- DETAIL: Failing row contains (-3).
INSERT INTO rw_view3 VALUES (3); -- ok
DROP TABLE base_tbl CASCADE;
NOTICE: drop cascades to 3 other objects
--- 1549,1554 ----
*************** CREATE VIEW rw_view1 AS SELECT * FROM ba
*** 1574,1589 ****
INSERT INTO rw_view1 VALUES (1, ARRAY[1,2,3]); -- ok
INSERT INTO rw_view1 VALUES (10, ARRAY[4,5]); -- should fail
ERROR: new row violates WITH CHECK OPTION for view "rw_view1"
- DETAIL: Failing row contains (10, {4,5}).
UPDATE rw_view1 SET b[2] = -b[2] WHERE a = 1; -- ok
UPDATE rw_view1 SET b[1] = -b[1] WHERE a = 1; -- should fail
ERROR: new row violates WITH CHECK OPTION for view "rw_view1"
- DETAIL: Failing row contains (1, {-1,-2,3}).
PREPARE ins(int, int[]) AS INSERT INTO rw_view1 VALUES($1, $2);
EXECUTE ins(2, ARRAY[1,2,3]); -- ok
EXECUTE ins(10, ARRAY[4,5]); -- should fail
ERROR: new row violates WITH CHECK OPTION for view "rw_view1"
- DETAIL: Failing row contains (10, {4,5}).
DEALLOCATE PREPARE ins;
DROP TABLE base_tbl CASCADE;
NOTICE: drop cascades to view rw_view1
--- 1562,1574 ----
*************** CREATE VIEW rw_view1 AS
*** 1598,1608 ****
INSERT INTO rw_view1 VALUES (5); -- ok
INSERT INTO rw_view1 VALUES (15); -- should fail
ERROR: new row violates WITH CHECK OPTION for view "rw_view1"
- DETAIL: Failing row contains (15).
UPDATE rw_view1 SET a = a + 5; -- ok
UPDATE rw_view1 SET a = a + 5; -- should fail
ERROR: new row violates WITH CHECK OPTION for view "rw_view1"
- DETAIL: Failing row contains (15).
EXPLAIN (costs off) INSERT INTO rw_view1 VALUES (5);
QUERY PLAN
---------------------------------------------------------------
--- 1583,1591 ----
*************** CREATE VIEW rw_view1 AS SELECT * FROM ba
*** 1650,1659 ****
INSERT INTO rw_view1 VALUES (5,0); -- ok
INSERT INTO rw_view1 VALUES (15, 20); -- should fail
ERROR: new row violates WITH CHECK OPTION for view "rw_view1"
- DETAIL: Failing row contains (15, 10).
UPDATE rw_view1 SET a = 20, b = 30; -- should fail
ERROR: new row violates WITH CHECK OPTION for view "rw_view1"
- DETAIL: Failing row contains (20, 10).
DROP TABLE base_tbl CASCADE;
NOTICE: drop cascades to view rw_view1
DROP FUNCTION base_tbl_trig_fn();
--- 1633,1640 ----
*************** CREATE VIEW rw_view2 AS
*** 1684,1695 ****
SELECT * FROM rw_view1 WHERE a > 0 WITH LOCAL CHECK OPTION;
INSERT INTO rw_view2 VALUES (-5); -- should fail
ERROR: new row violates WITH CHECK OPTION for view "rw_view2"
- DETAIL: Failing row contains (-5).
INSERT INTO rw_view2 VALUES (5); -- ok
INSERT INTO rw_view2 VALUES (50); -- ok, but not in view
UPDATE rw_view2 SET a = a - 10; -- should fail
ERROR: new row violates WITH CHECK OPTION for view "rw_view2"
- DETAIL: Failing row contains (-5).
SELECT * FROM base_tbl;
a | b
----+----
--- 1665,1674 ----
signature.asc
Description: Digital signature
