pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Anthony Bykov <a.bykov@postgrespro.ru>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Transform for pl/perl
Date: Fri, 1 Dec 2017 15:49:21 -0300
Message-ID: <20171201184921.27he6vbp62yegpvm@alvherre.pgsql> (raw)
In-Reply-To: <20171127132439.23dc26ef@anthony-24-g082ur>

A few very minor comments while quickly paging through:

1. non-ASCII tests are going to cause problems in one platform or
another.  Please don't include that one.

2. error messages
   a) please use ereport() not elog()
   b) conform to style guidelines: errmsg() start with lowercase, others
      are complete phrases (start with uppercase, end with period)
   c) replace
      "The type you was trying to transform can't be represented in JSONB"
      maybe with
      errmsg("could not transform to type \"%s\"", "jsonb"),
      errdetail("The type you are trying to transform can't be represented in JSON")
   d) same errmsg() for the other error; figure out suitable errdetail.

3. whitespace: don't add newlines to while, DirectFunctionCallN, pnstrdup.

4. the "relocatability" test seems pointless to me.

5. "#undef _" looks bogus.  Don't do it.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services




view thread (62+ messages)  latest in thread

Message-ID: <20171201184921.27he6vbp62yegpvm@alvherre.pgsql>
Permalink:  ../20171201184921.27he6vbp62yegpvm@alvherre.pgsql/
Also on:    postgresql.org/message-id/20171201184921.27he6vbp62yegpvm@alvherre.pgsql

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: alvherre@alvh.no-ip.org, a.bykov@postgrespro.ru, pgsql-hackers@lists.postgresql.org
  Subject: Re: Transform for pl/perl
  In-Reply-To: <20171201184921.27he6vbp62yegpvm@alvherre.pgsql>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox