Review: information schema parameter_default implementation (v2) This is a review of the patch submitted in http://archives.postgresql.org/message-id/1384483678.5008.1.ca...@vanquo.pezone.net (information schema parameter_default implementation).
Previous review from Amit Khandekar covers technical aspects: http://www.postgresql.org/message-id/CACoZds0eB3-yZ+pVLp9Sf5Xs_9xsDZ=jdp1d83ra-hjvjjo...@mail.gmail.com Submission review ================= * Is the patch in a patch format which has context? (eg: context diff format) Yes * Does it apply cleanly to the current git master? I had to apply "fromdos" to remove trailing whitespace. After that, the patch applies cleanly to HEAD. Make builds without warnings, except for: In file included from gram.y:13675:0: scan.c: In function ‘yy_try_NUL_trans’: scan.c:10185:23: warning: unused variable ‘yyg’ [-Wunused-variable] but from previous messages in this mailing list I think that's unrelated to this patch and normal. The regression tests all pass successfully against the new patch. * Does it include reasonable tests, necessary doc patches, etc? Yes Usability review ================ * Does the patch actually implement that? The patch implements the column "parameter_default" of information schema view "parameters", defined in the SQL:2011 standard. I could not get a hand to the spec, but I found a document where it is mentioned: http://msdn.microsoft.com/en-us/library/jj191733(v=sql.105).aspx * Do we want that? I think we do, as it is defined in the spec. * Do we already have it? No * Does it follow SQL spec, or the community-agreed behavior? SQL:2011. * Does it include pg_dump support (if applicable)? N/A * Are there dangers? None AFAICS. * Have all the bases been covered? Yes. Feature test ============ * Does the feature work as advertised? Yes * Are there corner cases the author has failed to consider? None that I can see. * Are there any assertion failures or crashes? No Performance review ================== N/A Coding review ============= I'm not skilled enough to do a code review; see previous review from Amit: http://www.postgresql.org/message-id/CACoZds0eB3-yZ+pVLp9Sf5Xs_9xsDZ=jdp1d83ra-hjvjjo...@mail.gmail.com 2013/11/21 Peter Eisentraut <pete...@gmx.net> > On 11/20/13, 8:39 PM, Rodolfo Campero wrote: > > 2013/11/20 Peter Eisentraut <pete...@gmx.net <mailto:pete...@gmx.net>> > > > > Updated patch > > > > > > I can't apply the patch; maybe I'm doing something wrong? > > It looks like you are not in the right directory. > > -- Rodolfo Campero Anachronics S.R.L. Tel: (54 11) 4899 2088 rodolfo.camp...@anachronics.com http://www.anachronics.com
