Re: My first PL/pgSQL function

From: Dane Foster <studdugie(at)gmail(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: pgsql-general <pgsql-general(at)postgresql(dot)org>
Subject: Re: My first PL/pgSQL function
Date: 2015-10-21 02:08:10
Message-ID: CA+WxinL=BneG3VE5uDr3A6xwqb5U3tidvqBJxTi7+KbqGXPCOA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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(dot)stehule(at)gmail(dot)com>
wrote:

>
>
> 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
>>> ​
>>>
>>>
>>
>

In response to

Responses

Browse pgsql-general by date

  From Date Subject
Next Message John R Pierce 2015-10-21 05:45:47 Re: trouble downloading postgres 9.4 for RHEL 6.x
Previous Message Dane Foster 2015-10-21 00:56:34 Re: My first PL/pgSQL function