Re: My first PL/pgSQL function

On Wed, Oct 21, 2015 at 3:20 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
wrote:

>
>
> 2015-10-21 4:08 GMT+02:00 Dane Foster <studdugie(at)gmail(dot)com>:
>
>> 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?
>>
>
> It strongly depends on what do you do. I artificial benchmarks you can
> find tens percent difference (based on massive cycles), but in life there
> will be zero difference probably. The bottleneck in PLpgSQL functions are
> SQL statements usually, and the overhead of "glue" is pretty less. Mainly
> if you has not any loop there.
>
> Regards
>
> Pavel
>
>
>
>>
>> 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
>>>>> ​
>>>>>
>>>>>
>>>>
>>>
>>
> ​For posterity here is the final version. I ran it through PostgreSQL
9.5beta1 this morning so it's at least syntactically valid. Additionally I
went w/ a list of INTO targets instead of a RECORD because it's a more
elegant solution in that it made the code a little less verbose and a
little less repetitive. The fact that in some cases it's faster is a
serendipitous bonus.

Though the conversation around this function has improved my understanding
of PL/pgSQL immensely there are a couple things that happened that I don't
fully understand:

1. I've changed the function's argument list from: (text, variadic
numeric[]) to: (text, int default, numeric default) because I couldn't get
the variadic version to work when only one argument was passed to the
function. For example:
SELECT * FROM check_discount_code('blah')
caused PostreSQL to complained that "no function w/ that signature exists
you may need to cast" (I'm paraphrasing). In order to get it to work I had
to provide at least two arguments.

2. I was under the impression that the runtime environment of PL/pgSQL is
the same environment PostgreSQL uses to execute all SQL commands and
functions. So if that's true why is returning JSON from inside a PL/pgSQL
function so much more expensive than doing it outside?

Dane
​
​CREATE OR REPLACE FUNCTION public.check_discount_code(
_code TEXT,
seats INT DEFAULT -1,
subtotal NUMERIC DEFAULT -1,
OUT type TEXT,
OUT status TEXT,
OUT date TEXT,
OUT datetime TIMESTAMPTZ,
OUT value TEXT,
OUT min TEXT
) AS $$
DECLARE
-- The (formatted) expiration date of the discount.
expd TEXT;
-- The (formatted) last modification date of the discount.
mdate TEXT;
-- The (formatted) effective date of the discount.
edate TEXT;
-- TRUE means the discount is valid (i.e., not void).
ok BOOLEAN;
-- The effective date of the discount is in the future.
notyet BOOLEAN;
-- The coupon has been used up. This is necessary because some coupons
can be
-- used a limited number of times.
maxuse BOOLEAN;
-- The discount has expired.
expired BOOLEAN;
-- There exists a coupon in discount_codes that does not exist in
coupons. This
-- should NEVER happen! But there is no harm in checking.
danglingcoupon BOOLEAN;
-- There exists a voucher in discount_codes that does not exist in
vouchers. This
-- should NEVER happen! But there is no harm in checking.
danglingvoucher BOOLEAN;
-- The expiration date of the discount.
expires TIMESTAMPTZ;
-- The last modification date/time of the discount's status. The
primary purpose
-- of this column is so that we can tell users when something was
voided.
modified TIMESTAMPTZ;
-- The date/time discount became effective (think start date).
effectivedate TIMESTAMPTZ;
BEGIN

SELECT
dc.ok,
dc.expires,
dc.modified,
effective_date,
effective_date > CURRENT_DATE,
dc.expires < CURRENT_DATE,
cpn.code IS NULL,
v.code IS NULL,
-- Determines the type of discount (coupon, voucher, or giftcert).
CASE dc.type::TEXT
WHEN 'voucher'
THEN
CASE WHEN gd.code IS NOT NULL THEN 'giftcert' END
ELSE
dc.type::TEXT
END,
-- The minimum quantity or dollar amount required to use the coupon.
COALESCE(
lower(qty_range)::TEXT,
'$' || to_char(lower(amount_range), '999999999999999D99')
),
to_char(dc.expires, 'Dy, MM Mon. YYYY'),
to_char(dc.modified, 'Dy, MM Mon. YYYY'),
to_char(effective_date, 'Dy, MM Mon. YYYY'),
-- The gift certificate's remaining value or the coupon's discount
value as a
-- dollar amount or percent.
COALESCE(
v.value::TEXT,
discount_rate || '%',
'$' || to_char(discount_amount, '999999999999999D99')
),
-- Determines if the coupon has been used up.
CASE WHEN cpn.maxuse > 0 THEN cpn.maxuse - used <= 0 ELSE FALSE END
INTO ok,
expires,
modified,
effectivedate,
notyet,
expired,
danglingcoupon,
danglingvoucher,
type,
min,
expd,
mdate,
edate,
value,
maxuse
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 type
WHEN 'coupon'
THEN
-- This should NEVER happen!
IF danglingcoupon
THEN
DELETE FROM discount_codes WHERE code = _code;
RAISE WARNING 'Removed dangling coupon code: %', _code;
END IF;

IF maxuse OR NOT ok THEN status := 'void'; RETURN; END IF;

IF expired
THEN
date := expd;
status := 'expired';
datetime := expires;
RETURN;
END IF;

IF notyet
THEN
date := edate;
status := 'inactive';
datetime := effectivedate;
RETURN;
END IF;

IF min IS NOT NULL
THEN
IF min ~ '^\$'
THEN
IF right(min, -1)::NUMERIC > subtotal
THEN
date := edate;
status := 'min';
datetime := effectivedate;
RETURN;
END IF;
ELSIF min::INT > seats
THEN
date := edate;
status := 'min';
datetime := effectivedate;
RETURN;
END IF;
END IF;

status := 'ok';
date := edate;
datetime := effectivedate;
RETURN;
ELSE
-- This should NEVER happen!
IF danglingvoucher
THEN
DELETE FROM discount_codes WHERE code = _code;
RAISE WARNING 'Removed dangling voucher: %', _code;
END IF;

IF NOT ok
THEN
date := mdate;
status := 'void';
datetime := modified;
RETURN;
END IF;

IF expired
THEN
date := expd;
status := 'expired';
datetime := expires;
RETURN;
END IF;

IF notyet
THEN
date := edate;
status := 'inactive';
datetime := effectivedate;
value := to_char(value, '999999999999999D99');
RETURN;
END IF;

-- Please note that even though the voucher is valid we
return the
-- expiration date information because the data is shown to
the user
-- to inform them of when their gift certificate expires.
IF value > 0
THEN
date := expd;
status := 'ok';
datetime := expires;
value := to_char(value, '999999999999999D99');
RETURN;
END IF;

status := 'depleted';
END CASE;
END IF;

END;
$$ LANGUAGE plpgsql STRICT;​