Re: My first PL/pgSQL function

From: Dane Foster <studdugie(at)gmail(dot)com>
To: Merlin Moncure <mmoncure(at)gmail(dot)com>
Cc: pgsql-general <pgsql-general(at)postgresql(dot)org>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Subject: Re: My first PL/pgSQL function
Date: 2015-10-20 20:22:02
Message-ID: CA+WxinKz87nYa2FjHDU_vzSSosbM6V0UrJUd=MuJr-AvZ8wygg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-general

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 $$
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;

Dane

On Tue, Oct 20, 2015 at 1:45 PM, Dane Foster <studdugie(at)gmail(dot)com> wrote:

> On Tue, Oct 20, 2015 at 12:43 PM, Merlin Moncure <mmoncure(at)gmail(dot)com>
> wrote:
>
>> On Tue, Oct 20, 2015 at 9:45 AM, Dane Foster <studdugie(at)gmail(dot)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
> ​
>
>

In response to

Responses

Browse pgsql-general by date

  From Date Subject
Next Message John R Pierce 2015-10-20 20:35:49 Re: My first PL/pgSQL function
Previous Message Vick Khera 2015-10-20 19:56:14 Re: Returning JSON or JSONB