Re: My first PL/pgSQL function - Mailing list pgsql-general

Since I'm switching to OUT parameters is there any difference (performance/efficiency wise) between using an INTO STRICT RECORD_TYPE_VARIABLE statement which forces me to copy/assign the property values from the RECORD to the OUT parameter variables and simply listing the OUT parameters, i.e., INTO STRICT outparam1, outparam2, ..., outparamN?

Thanks,

Dane

On Tue, Oct 20, 2015 at 4:37 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:

2015-10-20 22:22 GMT+02:00 Dane Foster <studdugie@gmail.com>:

Here is the updated version w/ the feedback incorporated. I'm going to install PostgreSQL 9.6 from source this weekend so I can start testing/debugging. Does anyone here have any experience using the pgAdmin debugger recently? I ask because it seems a little dated (September 26, 2008).

Thanks,

Dane

/**
 * Returns the status of a coupon or voucher.
 * @param _code The discount code.
 * @return NULL if the discount does not exist otherwise a composite type (see return
 * type declaration below).

 *
 * Voucher codes have the following properties:
 * type     - The type of discount (voucher, giftcert).
 *
 * status   - The status of the voucher. The valid values are:
 *            void     - The voucher has been voided.
 *
 *            expired  - The voucher has expired.
 *
 *            inactive - The gift certificate has not been sent yet.
 *
 *            ok       - The voucher has been activated, has not expired, and has a
 *                       current value greater than zero.
 *
 * date     - The expiration or activation or void date of the voucher in a reader
 *            friendly format.
 *
 * datetime - The expiration or activation or void date of the gift certificate in
 *            YYYY-MM-DD HH:MM:SS format.
 *
 * value    - The current value of the voucher.
 *
 * The mandatory properties are type and status. The presence of the other properties
 * are dependent on the value of status.
 ************************************************************************************
 * Coupon codes can provide the following additional parameters that are used to
 * determine if an order meets a coupon's minimum requirements.

 * @param int seats The number of seats in the user's order.

 * @param numeric subtotal The order's subtotal.
 *
 * Coupon codes have the following properties:
 * type     - The type of discount (coupon).
 *
 * status   - The status of the coupon code. The valid values are:
 *            void     - The coupon has been voided.
 *
 *            expired  - The coupon has expired.
 *
 *            inactive - The coupon has not been activated yet.
 *
 *            min      - The minimum seats or dollar amount requirement has not been
 *                       met.
 *
 *            ok       - The coupon can be used.
 *
 * min      - The minimum seats or dollar amount requirement. The value of this
 *            property is either an unsigned integer or dollar amount string w/ the
 *            dollar sign.
 *
 * date     - The expiration or activation or void date of the coupon in a reader
 *            friendly format.
 *
 * datetime - The expiration or activation or void date of the coupon in YYYY-MM-DD
 *             HH:MM:SS format.
 *
 * value    - The current value of the coupon as a string. The value of this property
 *            is either an unsigned integer w/ a percent symbol or dollar amount
 *            string w/ the dollar sign.
 */
CREATE OR REPLACE FUNCTION check_discount_code(
  _code public.CITXT70,
  VARIADIC cpnxtra NUMERIC[]
)

RETURNS TABLE (
  type     TEXT,
  status   TEXT,
  date     TEXT,
  datetime TIMESTAMPTZ,
  value    TEXT,
  min      TEXT
) AS $$

it is wrong, you are return composite, not SETOF composites (table).

Use OUT parameters instead or declared custom type

CREATE TYPE foo_result_type AS (a int, b int, c int);

CREATE OR REPLACE FUNCTION foo(..) RETURNS foo_result_type AS $$ $$

 

DECLARE
  discount RECORD;
BEGIN

  SELECT
    ok,
    created,
    expires,
    modified,
    effective_date,
    -- The minimum quantity or dollar amount required to use the coupon.
    COALESCE(
      lower(qty_range),
      '$' || to_char(lower(amount_range), '999999999999999D99')
    )                                                           AS min,
    CASE type::TEXT
      WHEN 'voucher'
      THEN
        CASE WHEN gd.code IS NOT NULL THEN 'giftcert' END
      ELSE
        type::TEXT
    END                                                         AS type,
    to_char(expires, 'Dy, MM Mon. YYYY')                        AS expd,
    to_char(modified, 'Dy, MM Mon. YYYY')                       AS mdate,
    to_char(effective_date, 'Dy, MM Mon. YYYY')                 AS edate,
    -- The gift certificates remaining value or the coupon's discount value as a
    -- dollar amount or percent.
    COALESCE(
      value,
      discount_rate || '%',
      '$' || to_char(discount_amount, '999999999999999D99')
    )                                                           AS value,
    -- Determines if the coupon has been used up.
    CASE WHEN maxuse > 0 THEN maxuse - used <= 0 ELSE FALSE END AS maxuse,
    effective_date > CURRENT_DATE                               AS notyet,
    expires < CURRENT_DATE                                      AS expired,
    cpn.code IS NULL                                            AS danglingcoupon,
    v.code IS NULL                                              AS danglingvoucher
  INTO STRICT discount
  FROM
    discount_codes        AS dc
    LEFT JOIN coupons     AS cpn USING (code)
    LEFT JOIN vouchers    AS v   USING (code)
    LEFT JOIN giftcerts_d AS gd  USING (code)
  WHERE
    dc.code = _code;

  IF FOUND THEN
    CASE discount.type
      WHEN 'coupon'
      THEN
        -- This should NEVER happen!
        IF discount.danglingcoupon
        THEN
          DELETE FROM discount_codes WHERE code = _code;
          RAISE WARNING 'Removed dangling coupon code: %', _code;
        ELSE
          IF discount.maxuse OR NOT discount.ok
          THEN
              RETURN (discount.type, 'void');
          END IF;

          IF discount.expired
          THEN
            RETURN (discount.type, 'expired', discount.expd, discount.expires);
          END IF;

          IF discount.notyet
          THEN
            RETURN (
              discount.type,
              'inactive',
              discount.edate,
              discount.effective_date
            );
          END IF;
          /**
           * Coupon codes can provide up to two additional parameters that are used
           * to determine if an order meets a coupon's minimum requirements.
           *
           * int seats (i.e., cpnxtra[0]) The number of seats in the user's order.
           * numeric subtotal (i.e., cpnxtra[1]) The order's subtotal.
           */
          IF 2 = array_length(cpnxtra, 1)
          THEN
            IF discount.min IS NOT NULL
            THEN
              -- @TODO - Test the regex to ensure it is escaped properly.
              IF discount.min ~ '^\$'
              THEN
                IF right(discount.min, -1)::NUMERIC > cpnxtra[1]::NUMERIC
                THEN
                  RETURN (
                    discount.type,
                    'min',
                    discount.edate,
                    discount.effective_date,
                    discount.value,
                    discount.min
                  );
                END IF;
              ELSIF discount.min::INT > cpnxtra[0]::INT
              THEN
                RETURN (
                  discount.type,
                  'min',
                  discount.edate,
                  discount.effective_date,
                  discount.value,
                  discount.min
                );
              END IF;

              RETURN (
                'coupon',
                'ok',
                discount.edate,
                discount.effective_date,
                discount.value,
                discount.min
              );
            END IF;
          END IF;

          RETURN (
            'coupon',
            'ok',
            discount.edate,
            discount.effective_date,
            discount.value
          );
        END IF;
      ELSE
        -- This should NEVER happen!
        IF discount.danglingvoucher
        THEN
          DELETE FROM discount_codes WHERE code = _code;
          RAISE WARNING 'Removed dangling voucher: %', _code;
        ELSE
          IF NOT discount.ok
          THEN
            RETURN (discount.type, 'void', discount.mdate, discount.modified);
          END IF;

          IF discount.expired
          THEN
            RETURN (discount.type, 'expired', discount.expd, discount.expires);
          END IF;

          IF discount.notyet
          THEN
            RETURN (
              discount.type,
              'inactive',
              discount.edate,
              discount.effective_date,
              to_char(discount.value, '999999999999999D99')
            );
          END IF;
          -- Please note that even though the gift certificate is valid we return
          -- the expiration date information. This is because the data is shown to
          -- the user to inform them of when their gift certificate expires.
          IF discount.value > 0
          THEN
            RETURN (
              discount.type,
              'ok',
              discount.expd,
              discount.expires,
              to_char(discount.value, '999999999999999D99')
            );
          END IF;

          RETURN (discount.type, 'depleted');
        END IF;
    END CASE;
  END IF;

  RETURN NULL;

END;
$$ LANGUAGE plpgsql STRICT;

this function is pretty long, you can divide it - to two maybe three parts - first - taking data, second - checking,

 

Dane

On Tue, Oct 20, 2015 at 1:45 PM, Dane Foster <studdugie@gmail.com> wrote:

On Tue, Oct 20, 2015 at 12:43 PM, Merlin Moncure <mmoncure@gmail.com> wrote:

On Tue, Oct 20, 2015 at 9:45 AM, Dane Foster <studdugie@gmail.com> wrote:
> Hello,
>
> I'm in the very very very very early stages of migrating a MySQL/PHP app to
> PostgreSQL/PHP/Lua. Because we are moving to PostgreSQL one of the [many]
> things I intend to change is to move ALL the SQL code/logic out of the
> application layer and into the database where it belongs. So after months of
> reading the [fine] PostgreSQL manual my first experiment is to port some
> PHP/SQL code to a PostgreSQL function.
>
> At this stage the function is a purely academic exercise because like I said
> before it's early days so no data has been migrated yet so I don't have data
> to test it against. My reason for sharing at such an early stage is because
> all I've done so far is read the [fine] manual and I'd like to know if I've
> groked at least some of the material.
>
> I would appreciate any feedback you can provide. I am particularly
> interested in learning about the most efficient way to do things in PL/pgSQL
> because I would hate for the first iteration of the new version of the app
> to be slower than the old version.
>
> Thank you for your consideration,

This is beautiful code. It in fact is an for all intents and purposes
an exact replica of my personal style.

Some notes:
*) I agree with Pavel; better to return specific columns if the result
is well defined (mark them in the argument list with OUT and I tend to
not prefix underscore them in that case).  The caller can always do a
json production if necessary, or you can wrap the function.

Some other minor suggestions:
*) I tend to prefer format() to || concatenation in ALL usage these
days.  It's more readable and tends to give better handling of NULL
strings by default.

*) this login should really be documented in line
          IF 2 = array_length(cpnxtra, 1)
          THEN

*) I avoid all right justified code (spaced out AS x, AS y, etc).  I
understand the perceived readability improvements but none of them are
worth the cascading edits when variables get longer.

*) let's compare notes on your doxygen style code markup. I've been
trouble finding a good robust tool that does exactly what I want,
curious if you did better.

*) FYI, since you're obviously not using pgadmin, I use 'Sublime Text
3' for my code editor.  I've significantly enhanced it to support
various postgresqlisms, so if you're maintaining code in a codebase,
you have reasonable support for 'jump to definition' and things like
that.

merlin

​

Thank you Pavel and Merlin for the feedback. I'm delighted that my first PL/pgSQL function wasn't rubbish. I think the credit goes to the authors of the [fine] PostgreSQL manual.

Pavel, I've taken your recommendation to heart but I'll need to do some more reading on composite types because I didn't realize they were on option in this context (i.e., the fields/columns aren't fixed).

Merlin:

I went w/ || on purpose because I want/need its NULL behavior. The relationship between the columns with which || is used is a binary (mutually exclusive) relationship. So they both can't be NULL nor NOT NULL.

I understand that right justification is an issue of personal taste. For me SQL is such a verbose and dense language that I use the justification to help break it up into visually manageable chunks. In traditional programming languages we have curly braces and/or indentation to help us visually organize and parse the code. I try to use justification to the same effect. And since most code is read more frequently than it's written I think a little realigning is a small price to pay.

I haven't investigated or encountered any doxygen processing tools. As a matter of fact I wasn't even aware that the commenting style that I used was called doxygen! Until recently I used to program in Java regularly (since the Java 1.1 days) so I have a tendency to bring that style of commenting w/ me to other languages. The version on display is a PHP'ified variation of JavaDoc which thanks to you I just learned is called doxygen.

Like I said I'm an old Java hack and used to use IntelliJ/IDEA to sling Java. But even though I rarely code in Java anymore I continue to use IDEA for coding everything, except shell scripts. IDEA has support for "jump to definition" and (more importantly) renames across files (i.e., refactoring).

Thanks again for the feedback it is truly appreciated.

Regards,

Dane

​