Bug: COPY IN doesn't test domain constraints

Started by Tom Laneover 23 years ago5 messages
#1Tom Lane
tgl@sss.pgh.pa.us

In CVS tip:

regression=# create domain nnint int not null;
CREATE DOMAIN
regression=# create table foo (f1 nnint);
CREATE TABLE
regression=# insert into foo values(null);
ERROR: Domain nnint does not allow NULL values -- okay
regression=# \copy foo from stdin
123
\N
\.
regression=# select * from foo;
f1
-----
123
-- not okay
(2 rows)

regression=# create domain vc4 varchar(4);
CREATE DOMAIN
regression=# create table foot (f1 vc4);
CREATE TABLE
regression=# \copy foot from stdin
1234567890
\.
regression=# select * from foot;
f1
------------
1234567890 -- not okay
(1 row)

regards, tom lane

#2Rod Taylor
rbt@rbt.ca
In reply to: Tom Lane (#1)
Re: Bug: COPY IN doesn't test domain constraints

On Mon, 2002-09-16 at 17:54, Tom Lane wrote:

In CVS tip:

regression=# create domain nnint int not null;
CREATE DOMAIN

Ok, I'll take a look at this today.

Thanks

--
Rod Taylor

#3Rod Taylor
rbt@rbt.ca
In reply to: Tom Lane (#1)
1 attachment(s)
Re: Bug: COPY IN doesn't test domain constraints

Fixed this problem and added regression tests in domain.sql.

Also:
- Changed header file order (alphabetical)
- Changed to m = attnum - 1 in binary copy code for consistency

On Mon, 2002-09-16 at 17:54, Tom Lane wrote:

In CVS tip:

regression=# create domain nnint int not null;
CREATE DOMAIN
regression=# create table foo (f1 nnint);
CREATE TABLE
regression=# insert into foo values(null);
ERROR: Domain nnint does not allow NULL values -- okay
regression=# \copy foo from stdin
123
\N
\.
regression=# select * from foo;
f1
-----
123
-- not okay
(2 rows)

regression=# create domain vc4 varchar(4);
CREATE DOMAIN
regression=# create table foot (f1 vc4);
CREATE TABLE
regression=# \copy foot from stdin
1234567890
\.
regression=# select * from foot;
f1
------------
1234567890 -- not okay
(1 row)

regards, tom lane

--
Rod Taylor

Attachments:

domain_copy.patchtext/plain; charset=ISO-8859-1; name=domain_copy.patchDownload
? src/test/regress/sql/.domain.sql.swp
Index: src/backend/commands/copy.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/backend/commands/copy.c,v
retrieving revision 1.171
diff -c -r1.171 copy.c
*** src/backend/commands/copy.c	2002/09/04 20:31:14	1.171
--- src/backend/commands/copy.c	2002/09/17 15:17:07
***************
*** 28,37 ****
  #include "commands/copy.h"
  #include "commands/trigger.h"
  #include "executor/executor.h"
- #include "rewrite/rewriteHandler.h"
  #include "libpq/libpq.h"
  #include "miscadmin.h"
  #include "parser/parse_relation.h"
  #include "tcop/pquery.h"
  #include "tcop/tcopprot.h"
  #include "utils/acl.h"
--- 28,40 ----
  #include "commands/copy.h"
  #include "commands/trigger.h"
  #include "executor/executor.h"
  #include "libpq/libpq.h"
+ #include "mb/pg_wchar.h"
  #include "miscadmin.h"
+ #include "nodes/makefuncs.h"
+ #include "parser/parse_coerce.h"
  #include "parser/parse_relation.h"
+ #include "rewrite/rewriteHandler.h"
  #include "tcop/pquery.h"
  #include "tcop/tcopprot.h"
  #include "utils/acl.h"
***************
*** 39,45 ****
  #include "utils/relcache.h"
  #include "utils/lsyscache.h"
  #include "utils/syscache.h"
- #include "mb/pg_wchar.h"
  
  #define ISOCTAL(c) (((c) >= '0') && ((c) <= '7'))
  #define OCTVALUE(c) ((c) - '0')
--- 42,47 ----
***************
*** 744,749 ****
--- 746,753 ----
  				num_defaults;
  	FmgrInfo   *in_functions;
  	Oid		   *elements;
+ 	bool	   *isDomain;
+ 	bool		hasDomain = false;
  	int			i;
  	List	   *cur;
  	Oid			in_func_oid;
***************
*** 796,801 ****
--- 800,806 ----
  	defexprs = (Node **) palloc(sizeof(Node *) * num_phys_attrs);
  	in_functions = (FmgrInfo *) palloc(num_phys_attrs * sizeof(FmgrInfo));
  	elements = (Oid *) palloc(num_phys_attrs * sizeof(Oid));
+ 	isDomain = (bool *) palloc(num_phys_attrs * sizeof(bool));
  
  	for (i = 0; i < num_phys_attrs; i++)
  	{
***************
*** 803,808 ****
--- 808,823 ----
  		if (attr[i]->attisdropped)
  			continue;
  
+ 		/* Test for the base type */
+ 		if (getBaseType(attr[i]->atttypid) != attr[i]->atttypid)
+ 		{
+ 			hasDomain = true;
+ 			isDomain[i] = true;
+ 		}
+ 		else
+ 			isDomain[i] = false;
+ 
+ 		/* Fetch the input function */
  		in_func_oid = (Oid) GetInputFunction(attr[i]->atttypid);
  		fmgr_info(in_func_oid, &in_functions[i]);
  		elements[i] = GetTypeElement(attr[i]->atttypid);
***************
*** 1011,1016 ****
--- 1026,1032 ----
  			foreach(cur, attnumlist)
  			{
  				int			attnum = lfirsti(cur);
+ 				int			m = attnum - 1;
  
  				i++;
  
***************
*** 1019,1027 ****
  					elog(ERROR, "COPY BINARY: unexpected EOF");
  				if (fld_size == 0)
  					continue;	/* it's NULL; nulls[attnum-1] already set */
! 				if (fld_size != attr[attnum - 1]->attlen)
  					elog(ERROR, "COPY BINARY: sizeof(field %d) is %d, expected %d",
! 					  i, (int) fld_size, (int) attr[attnum - 1]->attlen);
  				if (fld_size == -1)
  				{
  					/* varlena field */
--- 1035,1043 ----
  					elog(ERROR, "COPY BINARY: unexpected EOF");
  				if (fld_size == 0)
  					continue;	/* it's NULL; nulls[attnum-1] already set */
! 				if (fld_size != attr[m]->attlen)
  					elog(ERROR, "COPY BINARY: sizeof(field %d) is %d, expected %d",
! 					  i, (int) fld_size, (int) attr[m]->attlen);
  				if (fld_size == -1)
  				{
  					/* varlena field */
***************
*** 1040,1048 ****
  								fp);
  					if (CopyGetEof(fp))
  						elog(ERROR, "COPY BINARY: unexpected EOF");
! 					values[attnum - 1] = PointerGetDatum(varlena_ptr);
  				}
! 				else if (!attr[attnum - 1]->attbyval)
  				{
  					/* fixed-length pass-by-reference */
  					Pointer		refval_ptr;
--- 1056,1064 ----
  								fp);
  					if (CopyGetEof(fp))
  						elog(ERROR, "COPY BINARY: unexpected EOF");
! 					values[m] = PointerGetDatum(varlena_ptr);
  				}
! 				else if (!attr[m]->attbyval)
  				{
  					/* fixed-length pass-by-reference */
  					Pointer		refval_ptr;
***************
*** 1052,1058 ****
  					CopyGetData(refval_ptr, fld_size, fp);
  					if (CopyGetEof(fp))
  						elog(ERROR, "COPY BINARY: unexpected EOF");
! 					values[attnum - 1] = PointerGetDatum(refval_ptr);
  				}
  				else
  				{
--- 1068,1074 ----
  					CopyGetData(refval_ptr, fld_size, fp);
  					if (CopyGetEof(fp))
  						elog(ERROR, "COPY BINARY: unexpected EOF");
! 					values[m] = PointerGetDatum(refval_ptr);
  				}
  				else
  				{
***************
*** 1067,1076 ****
  					CopyGetData(&datumBuf, fld_size, fp);
  					if (CopyGetEof(fp))
  						elog(ERROR, "COPY BINARY: unexpected EOF");
! 					values[attnum - 1] = fetch_att(&datumBuf, true, fld_size);
  				}
  
! 				nulls[attnum - 1] = ' ';
  			}
  		}
  
--- 1083,1138 ----
  					CopyGetData(&datumBuf, fld_size, fp);
  					if (CopyGetEof(fp))
  						elog(ERROR, "COPY BINARY: unexpected EOF");
! 					values[m] = fetch_att(&datumBuf, true, fld_size);
  				}
+ 
+ 				nulls[m] = ' ';
+ 			}
+ 		}
+ 
+ 		/* Deal with domains */
+ 		if (hasDomain)
+ 		{
+ 			ParseState *pstate;
+ 			pstate = make_parsestate(NULL);
+ 
+ 			foreach(cur, attnumlist)
+ 			{
+ 				int			attnum = lfirsti(cur);
+ 				int			m = attnum - 1;
+ 
+ 				Const	   *con;
+ 				Node	   *node;
+ 				bool	   isNull = (nulls[m] == 'n');
+ 
+ 				/* This is not a domain, so lets skip it */
+ 				if (!isDomain[m])
+ 					continue;
+ 
+ 				/*
+ 				 * This is a domain.  As such, we must process it's input
+ 				 * function and coerce_type_constraints.  The simplest way
+ 				 * of doing that is to allow coerce_type to accomplish its
+ 				 * job from an unknown constant
+ 				 */
+ 
+ 				/* Create a constant */
+ 				con = makeConst(attr[m]->atttypid,
+ 								attr[m]->attlen,
+ 								values[m],
+ 								isNull,
+ 								attr[m]->attbyval,
+ 								false,		/* not a set */
+ 								false);		/* not coerced */
+ 
+ 				/* Process constraints */
+ 				node = coerce_type_constraints(pstate, (Node *) con,
+ 											   attr[m]->atttypid, true);
+ 
+ 				values[m] = ExecEvalExpr(node, econtext,
+ 										 &isNull, NULL);
  
! 				nulls[m] = isNull ? 'n' : ' ';
  			}
  		}
  
Index: src/test/regress/expected/domain.out
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/test/regress/expected/domain.out,v
retrieving revision 1.9
diff -c -r1.9 domain.out
*** src/test/regress/expected/domain.out	2002/08/31 22:10:48	1.9
--- src/test/regress/expected/domain.out	2002/09/17 15:17:08
***************
*** 33,44 ****
  INSERT INTO basictest values ('88', 'haha', 'short text', '123.12'); -- Bad varchar
  ERROR:  value too long for type character varying(5)
  INSERT INTO basictest values ('88', 'haha', 'short', '123.1212');    -- Truncate numeric
  select * from basictest;
   testint4 | testtext | testvarchar | testnumeric 
  ----------+----------+-------------+-------------
   88       | haha     | short       | 123.12
   88       | haha     | short       | 123.12
! (2 rows)
  
  -- check that domains inherit operations from base types
  select testtext || testvarchar as concat, testnumeric + 42 as sum
--- 33,50 ----
  INSERT INTO basictest values ('88', 'haha', 'short text', '123.12'); -- Bad varchar
  ERROR:  value too long for type character varying(5)
  INSERT INTO basictest values ('88', 'haha', 'short', '123.1212');    -- Truncate numeric
+ -- Test copy
+ COPY basictest (testvarchar) FROM stdin; -- fail
+ ERROR:  copy: line 1, value too long for type character varying(5)
+ lost synchronization with server, resetting connection
+ COPY basictest (testvarchar) FROM stdin;
  select * from basictest;
   testint4 | testtext | testvarchar | testnumeric 
  ----------+----------+-------------+-------------
   88       | haha     | short       | 123.12
   88       | haha     | short       | 123.12
!           |          | short       | 
! (3 rows)
  
  -- check that domains inherit operations from base types
  select testtext || testvarchar as concat, testnumeric + 42 as sum
***************
*** 47,53 ****
  -----------+--------
   hahashort | 165.12
   hahashort | 165.12
! (2 rows)
  
  drop table basictest;
  drop domain domainvarchar restrict;
--- 53,60 ----
  -----------+--------
   hahashort | 165.12
   hahashort | 165.12
!            |       
! (3 rows)
  
  drop table basictest;
  drop domain domainvarchar restrict;
***************
*** 107,118 ****
  INSERT INTO nulltest values ('a', 'b', NULL, 'd');
  ERROR:  ExecInsert: Fail to add null value in not null attribute col3
  INSERT INTO nulltest values ('a', 'b', 'c', NULL); -- Good
  select * from nulltest;
   col1 | col2 | col3 | col4 
  ------+------+------+------
   a    | b    | c    | d
   a    | b    | c    | 
! (2 rows)
  
  -- Test out coerced (casted) constraints
  SELECT cast('1' as dnotnull);
--- 114,131 ----
  INSERT INTO nulltest values ('a', 'b', NULL, 'd');
  ERROR:  ExecInsert: Fail to add null value in not null attribute col3
  INSERT INTO nulltest values ('a', 'b', 'c', NULL); -- Good
+ -- Test copy
+ COPY nulltest FROM stdin; --fail
+ ERROR:  copy: line 1, CopyFrom: Fail to add null value in not null attribute col3
+ lost synchronization with server, resetting connection
+ COPY nulltest FROM stdin;
  select * from nulltest;
   col1 | col2 | col3 | col4 
  ------+------+------+------
   a    | b    | c    | d
+  a    | b    | c    | 
   a    | b    | c    | 
! (3 rows)
  
  -- Test out coerced (casted) constraints
  SELECT cast('1' as dnotnull);
***************
*** 152,164 ****
  insert into defaulttest default values;
  insert into defaulttest default values;
  insert into defaulttest default values;
  select * from defaulttest;
   col1 | col2 | col3 | col4 | col5 | col6 | col7 | col8  
  ------+------+------+------+------+------+------+-------
   3    | 12   | 5    | 1    | 3    | 88   | 8000 | 12.12
   3    | 12   | 5    | 2    | 3    | 88   | 8000 | 12.12
   3    | 12   | 5    | 3    | 3    | 88   | 8000 | 12.12
! (3 rows)
  
  drop sequence ddef4_seq;
  drop table defaulttest;
--- 165,180 ----
  insert into defaulttest default values;
  insert into defaulttest default values;
  insert into defaulttest default values;
+ -- Test defaults with copy
+ COPY defaulttest(col5) FROM stdin;
  select * from defaulttest;
   col1 | col2 | col3 | col4 | col5 | col6 | col7 | col8  
  ------+------+------+------+------+------+------+-------
   3    | 12   | 5    | 1    | 3    | 88   | 8000 | 12.12
   3    | 12   | 5    | 2    | 3    | 88   | 8000 | 12.12
   3    | 12   | 5    | 3    | 3    | 88   | 8000 | 12.12
!  3    | 12   | 5    | 4    | 42   | 88   | 8000 | 12.12
! (4 rows)
  
  drop sequence ddef4_seq;
  drop table defaulttest;
Index: src/test/regress/sql/domain.sql
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/test/regress/sql/domain.sql,v
retrieving revision 1.6
diff -c -r1.6 domain.sql
*** src/test/regress/sql/domain.sql	2002/08/31 22:10:48	1.6
--- src/test/regress/sql/domain.sql	2002/09/17 15:17:08
***************
*** 35,40 ****
--- 35,50 ----
  INSERT INTO basictest values ('88', 'haha', 'short', '123.12');      -- Good
  INSERT INTO basictest values ('88', 'haha', 'short text', '123.12'); -- Bad varchar
  INSERT INTO basictest values ('88', 'haha', 'short', '123.1212');    -- Truncate numeric
+ 
+ -- Test copy
+ COPY basictest (testvarchar) FROM stdin; -- fail
+ notsoshorttext
+ \.
+ 
+ COPY basictest (testvarchar) FROM stdin;
+ short
+ \.
+ 
  select * from basictest;
  
  -- check that domains inherit operations from base types
***************
*** 84,89 ****
--- 94,109 ----
  INSERT INTO nulltest values ('a', NULL, 'c', 'd');
  INSERT INTO nulltest values ('a', 'b', NULL, 'd');
  INSERT INTO nulltest values ('a', 'b', 'c', NULL); -- Good
+ 
+ -- Test copy
+ COPY nulltest FROM stdin; --fail
+ a	b	\N	d
+ \.
+ 
+ COPY nulltest FROM stdin;
+ a	b	c	\N
+ \.
+ 
  select * from nulltest;
  
  -- Test out coerced (casted) constraints
***************
*** 119,124 ****
--- 139,150 ----
  insert into defaulttest default values;
  insert into defaulttest default values;
  insert into defaulttest default values;
+ 
+ -- Test defaults with copy
+ COPY defaulttest(col5) FROM stdin;
+ 42
+ \.
+ 
  select * from defaulttest;
  
  drop sequence ddef4_seq;
#4Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Rod Taylor (#3)
Re: Bug: COPY IN doesn't test domain constraints

Your patch has been added to the PostgreSQL unapplied patches list at:

http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

---------------------------------------------------------------------------

Rod Taylor wrote:

Fixed this problem and added regression tests in domain.sql.

Also:
- Changed header file order (alphabetical)
- Changed to m = attnum - 1 in binary copy code for consistency

On Mon, 2002-09-16 at 17:54, Tom Lane wrote:

In CVS tip:

regression=# create domain nnint int not null;
CREATE DOMAIN
regression=# create table foo (f1 nnint);
CREATE TABLE
regression=# insert into foo values(null);
ERROR: Domain nnint does not allow NULL values -- okay
regression=# \copy foo from stdin
123
\N
\.
regression=# select * from foo;
f1
-----
123
-- not okay
(2 rows)

regression=# create domain vc4 varchar(4);
CREATE DOMAIN
regression=# create table foot (f1 vc4);
CREATE TABLE
regression=# \copy foot from stdin
1234567890
\.
regression=# select * from foot;
f1
------------
1234567890 -- not okay
(1 row)

regards, tom lane

--
Rod Taylor

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#5Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Rod Taylor (#3)
Re: Bug: COPY IN doesn't test domain constraints

Patch applied. Thanks.

---------------------------------------------------------------------------

Rod Taylor wrote:

Fixed this problem and added regression tests in domain.sql.

Also:
- Changed header file order (alphabetical)
- Changed to m = attnum - 1 in binary copy code for consistency

On Mon, 2002-09-16 at 17:54, Tom Lane wrote:

In CVS tip:

regression=# create domain nnint int not null;
CREATE DOMAIN
regression=# create table foo (f1 nnint);
CREATE TABLE
regression=# insert into foo values(null);
ERROR: Domain nnint does not allow NULL values -- okay
regression=# \copy foo from stdin
123
\N
\.
regression=# select * from foo;
f1
-----
123
-- not okay
(2 rows)

regression=# create domain vc4 varchar(4);
CREATE DOMAIN
regression=# create table foot (f1 vc4);
CREATE TABLE
regression=# \copy foot from stdin
1234567890
\.
regression=# select * from foot;
f1
------------
1234567890 -- not okay
(1 row)

regards, tom lane

--
Rod Taylor

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073