Marko et al,<br /> This is my first ever attempt of a patch review just for learning the procedure. I'm not a postgres
developer,so the review is partial and mostly from the usability prospective.<br />The original message id of the patch
is<a href="http://archives.postgresql.org/pgsql-hackers/2010-04/msg01200.php">4BD58DB3.4070605@cs.helsinki.fi</a><br
/><br/><br /> Submssion review<br /> =========================<br /><br /> * Is he patch in context diff format?<br />
Ys<br/><br /> * Dos it apply cleanly to the current CVS HEAD?<br /> Applies cleanly to a source code snapshot<br /><br
/>* Dos it include reasonable tests, necessary doc patches, etc?<br /> I does not require a doc patch. A test is not
included,but it looks pretty trivial:<br /><br /> -- Prpare the test tables<br /><br /> drop table if exists foo;<br />
droptable if exists boo;<br /><br /> crate table foo(<br /> a nt,<br /> b nt,<br /> c nt);<br /><br /> crate table
boo(<br/> a nt,<br /> b nt,<br /> c nt);<br /><br /> insert into boo values (10,20,30);<br /><br /> -- Actual test<br
/><br/> INSERT INTO foo(a,b,c) SELECT (a,b,c) FROM boo;<br /> INSERT INTO foo(a,b,c) VALUES((0,1,2));<br /><br />
UsabilityReview<br /> =========================<br /><br /> The patch provides a HINT for unclear error. This should
clarifyfor a user what exactly is wrong with the sql.<br /> However, the actual HINT text provided with the patch is
notvery clear, too.<br /> The Stephen Frost's suggestion would add clarity:<br /><br /> errhint("insert appears to be a
singlecolumn with a record-type rather than multiple columns of non-composite type."),<br /><br /><br /> Feature
test<br/> =========================<br /><br /> The feature works as advertised for the test provided above.<br /> No
failuresor crashes.<br /> No visible affect on performance
