Re: My first PL/pgSQL function

2015-10-20 22:22 GMT+02:00 Dane Foster <studdugie(at)gmail(dot)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(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
>> ​
>>
>>
>