Обсуждение: My first PL/pgSQL function
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,
Dane
/**
* Returns the status of a coupon or voucher.
* @param _code The discount code.
* @return NULL if the discount does not exist otherwise a JSON object.
*
* 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 cart.
* @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 JSON AS $$
DECLARE
discount RECORD;
BEGIN
SELECT
ok,
v.value,
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,
-- 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,
-- The coupon's discount value as a dollar amount or percent.
COALESCE(
discount_rate || '%',
'$' || to_char(discount_amount, '999999999999999D99')
) AS discount,
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
IF discount.danglingcoupon
THEN
-- This should NEVER happen!
DELETE FROM discount_codes WHERE code = _code;
RAISE WARNING 'Removed dangling coupon code: %', _code;
ELSE
IF discount.maxuse OR NOT discount.ok
THEN
RETURN json_build_object('status', 'void', 'type', 'coupon');
END IF;
IF discount.expired
THEN
RETURN json_build_object(
'type', 'coupon',
'status', 'expired',
'date', discount.expd,
'datetime', discount.expires
);
END IF;
IF discount.notyet THEN
RETURN json_build_object(
'type', 'coupon',
'date', discount.edate,
'status', 'inactive',
'datetime', discount.effective_date
);
END IF;
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 json_build_object(
'status', 'min',
'type', 'coupon',
'min', discount.min
);
END IF;
ELSIF discount.min::INT > cpnxtra[0]::INT
THEN
RETURN json_build_object(
'status', 'min',
'type', 'coupon',
'min', discount.min
);
END IF;
RETURN json_build_object(
'status', 'ok',
'type', 'coupon',
'min', discount.min,
'value', discount.discount
);
END IF;
END IF;
RETURN json_build_object(
'status', 'ok',
'type', 'coupon',
'value', discount.discount
);
END IF;
ELSE
IF discount.danglingvoucher
THEN
-- This should NEVER happen!
DELETE FROM discount_codes WHERE code = _code;
RAISE WARNING 'Removed dangling voucher: %', _code;
ELSE
IF NOT discount.ok
THEN
RETURN json_build_object(
'status', 'void',
'type', discount.type,
'date', discount.mdate,
'datetime', discount.modified
);
END IF;
IF discount.expired
THEN
RETURN json_build_object(
'status', 'expired',
'type', discount.type,
'date', discount.expd,
'datetime', discount.expires
);
END IF;
IF discount.notyet
THEN
RETURN json_build_object(
'type', discount.type,
'date', discount.edate,
'status', 'inactive',
'datetime', discount.effective_date,
'value', to_char(discount.value, '999999999999999D99')
);
END IF;
IF discount.value > 0
THEN
RETURN json_build_object(
'status', 'ok',
'type', discount.type,
'date', discount.expd,
'datetime', discount.expires,
'value', to_char(discount.value, '999999999999999D99')
);
END IF;
RETURN json_build_object('status', 'depleted', 'type', discount.type);
END IF;
END CASE;
END IF;
RETURN NULL;
END;
$$ LANGUAGE plpgsql STRICT;
* Returns the status of a coupon or voucher.
* @param _code The discount code.
* @return NULL if the discount does not exist otherwise a JSON object.
*
* 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 cart.
* @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 JSON AS $$
DECLARE
discount RECORD;
BEGIN
SELECT
ok,
v.value,
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,
-- 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,
-- The coupon's discount value as a dollar amount or percent.
COALESCE(
discount_rate || '%',
'$' || to_char(discount_amount, '999999999999999D99')
) AS discount,
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
IF discount.danglingcoupon
THEN
-- This should NEVER happen!
DELETE FROM discount_codes WHERE code = _code;
RAISE WARNING 'Removed dangling coupon code: %', _code;
ELSE
IF discount.maxuse OR NOT discount.ok
THEN
RETURN json_build_object('status', 'void', 'type', 'coupon');
END IF;
IF discount.expired
THEN
RETURN json_build_object(
'type', 'coupon',
'status', 'expired',
'date', discount.expd,
'datetime', discount.expires
);
END IF;
IF discount.notyet THEN
RETURN json_build_object(
'type', 'coupon',
'date', discount.edate,
'status', 'inactive',
'datetime', discount.effective_date
);
END IF;
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 json_build_object(
'status', 'min',
'type', 'coupon',
'min', discount.min
);
END IF;
ELSIF discount.min::INT > cpnxtra[0]::INT
THEN
RETURN json_build_object(
'status', 'min',
'type', 'coupon',
'min', discount.min
);
END IF;
RETURN json_build_object(
'status', 'ok',
'type', 'coupon',
'min', discount.min,
'value', discount.discount
);
END IF;
END IF;
RETURN json_build_object(
'status', 'ok',
'type', 'coupon',
'value', discount.discount
);
END IF;
ELSE
IF discount.danglingvoucher
THEN
-- This should NEVER happen!
DELETE FROM discount_codes WHERE code = _code;
RAISE WARNING 'Removed dangling voucher: %', _code;
ELSE
IF NOT discount.ok
THEN
RETURN json_build_object(
'status', 'void',
'type', discount.type,
'date', discount.mdate,
'datetime', discount.modified
);
END IF;
IF discount.expired
THEN
RETURN json_build_object(
'status', 'expired',
'type', discount.type,
'date', discount.expd,
'datetime', discount.expires
);
END IF;
IF discount.notyet
THEN
RETURN json_build_object(
'type', discount.type,
'date', discount.edate,
'status', 'inactive',
'datetime', discount.effective_date,
'value', to_char(discount.value, '999999999999999D99')
);
END IF;
IF discount.value > 0
THEN
RETURN json_build_object(
'status', 'ok',
'type', discount.type,
'date', discount.expd,
'datetime', discount.expires,
'value', to_char(discount.value, '999999999999999D99')
);
END IF;
RETURN json_build_object('status', 'depleted', 'type', discount.type);
END IF;
END CASE;
END IF;
RETURN NULL;
END;
$$ LANGUAGE plpgsql STRICT;
2015-10-20 16:45 GMT+02:00 Dane Foster <studdugie@gmail.com>:
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,
Hi
I have only one objection - the returning result in JSON. I prefer two functions - one: check_discount and second: format_to_JSON(result_of_check_discount). JSON is good format for usage outer of plpgsql - so formatting to JSON should be last operation. Processing tuple - internal PostgreSQL format for composites is significantly faster than JSON.
Regards
PavelOn 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
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
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@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.
merlinThank 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
On 10/20/2015 1:22 PM, Dane Foster wrote: > 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. I would stick with developing/debugging on the current release version, 9.4. 9.6 isn't even in beta yet. -- john r pierce, recycling bits in santa cruz
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).* @param int seats The number of seats in the user's order.
*
* 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.RETURNS TABLE (
* @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[]
)
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,
DaneOn 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.
merlinThank 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
On Tue, Oct 20, 2015 at 4:35 PM, John R Pierce <pierce@hogranch.com> wrote:
On 10/20/2015 1:22 PM, Dane Foster wrote: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.
I would stick with developing/debugging on the current release version, 9.4. 9.6 isn't even in beta yet.
--
john r pierce, recycling bits in santa cruz
--
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general
We are in the "let's think about how we wanna approach this" phase of migration. So all we are doing now is researching and testing. In essence we are playing. So by the time we get serious enough to reach beta internally I suspect 9.6 should be in beta too. And if we happen to be ready for production before 9.6 is ready there is always 9.5 which will definitely be production ready before we are.
My larger point is this. I've been lurking at the edge of the PostgreSQL community for a couple years now and I finally have a project that has PostgreSQL written all over it. But instead of just taking the "free software" and running w/ it I'd like to participate and one of the things that PostgreSQL always needs is people willing to use/test the bleeding edge of the code w/ "real" workloads. I hope to be able to contribute in that area.
Regards,Dane
On 10/20/15 11:43 AM, Merlin Moncure wrote: > *) 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. Related to that (I think), you might want to move the documentation out of the comment block and into a COMMENT ON FUNCTION ... IS; statement. That way it's available through tools like pgAdmin and psql. -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com
On Tue, Oct 20, 2015 at 5:53 PM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:
On 10/20/15 11:43 AM, Merlin Moncure wrote:*) 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.
Related to that (I think), you might want to move the documentation out of the comment block and into a COMMENT ON FUNCTION ... IS; statement. That way it's available through tools like pgAdmin and psql.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
Done! Thanks for the heads up that COMMENT ON FUNCTION ... exists.
Dane
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).* @param int seats The number of seats in the user's order.
*
* 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.RETURNS TABLE (
* @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[]
)
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 typeCREATE 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,DaneOn 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.
merlinThank 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
2015-10-21 4:08 GMT+02:00 Dane Foster <studdugie@gmail.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,DaneOn 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).* @param int seats The number of seats in the user's order.
*
* 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.RETURNS TABLE (
* @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[]
)
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 typeCREATE 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,DaneOn 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.
merlinThank 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
On Wed, Oct 21, 2015 at 3:20 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
2015-10-21 4:08 GMT+02:00 Dane Foster <studdugie@gmail.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.RegardsPavel
Thanks,DaneOn 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).* @param int seats The number of seats in the user's order.
*
* 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.RETURNS TABLE (
* @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[]
)
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 typeCREATE 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,DaneOn 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.
merlinThank 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.
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;
_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;
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.
yes - the variadic *functions* can have parameters with default values or *variadic parameters* or mix. It is similar to Visual Basic for example. The specific PostgreSQL feature is support function overloading and variadic functions together. When you call any function, then PostgreSQL have to choose function with the closest param types - and usually Postgres try to cast parameters to different type less times than other databases.
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?
JSON in Postgres is string. If you need a access to any field, then this string have to be parsed and the data must be deserialized. The same behave has XML format. If I don't plan to process result of function on server side, then returning XML or JSON is ok. But If I'll process result in other server side functions, then native tuples are better (if nested structures are not needed.)
Regards
Pavel
Dane
On Wed, Oct 21, 2015 at 10:24 AM, Dane Foster <studdugie@gmail.com> wrote: > 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. *) I really dislike your formatting relative to what it was. In fact, it's a mess. You went to four spaces vs two, which causes a lot of issues IMO. SQL relative to other languages requires a lot of nested indentation and this makes things challenging to format well. This is opinion I guess but I think it's a regression. aside: do not use tabs in sql, ever -- I dislike tabs generally but in postgres .sql it breaks pasting to psql. *) your indent spacing is not regular (meaning, not always exactly 4). For example, END CASE; END IF; Irregular indentation leads directly to: *) lots of extra work and irrelevant-to-the-actual-change whitespace adjustments *) coding difficulty locating the end of the block in the face of indentation changes. At the bottom portion of your function, I'm not easily finding the matching END IF to the: ELSE -- This should NEVER happen! IF danglingvoucher merlin