Thread: dunction issue
Hi,
i have a problem solving my function trouble.
this function should return an email address stored in a table (tmp_newsletterreg) based on a number (sessions ID).
if the session id is not find it should return a string corresponding to and error.
if the email in found but already exists into another table (users), it should also return a string value relative the this error.
here is my stored procedure.
if the session id is wrong, it works correctly.
however if the session id is correct it does not return me the email address (even if it really exist into table tmp_newsletterreg / but not in table users.)
so i think my eyes are tired, because i do not see an error...
thanks.
--
Alain
------------------------------------
Windows XP SP2
PostgreSQL 8.2.4 / MS SQL server 2005
Apache 2.2.4
PHP 5.2.4
C# 2005-2008
i have a problem solving my function trouble.
this function should return an email address stored in a table (tmp_newsletterreg) based on a number (sessions ID).
if the session id is not find it should return a string corresponding to and error.
if the email in found but already exists into another table (users), it should also return a string value relative the this error.
here is my stored procedure.
CREATE OR REPLACE FUNCTION cust_portal.sp_u_002(id_session character varying)
RETURNS character varying AS
$BODY$
DECLARE
ret_email CHARACTER VARYING(512) :='';
usr_exists INTEGER := 0;
usr_exists_2 INTEGER := 0;
BEGIN
set search_path = cust_portal;
SELECT count(*) INTO usr_exists FROM tmp_newsletterreg WHERE tmp_usr_id = id_session;
IF (usr_exists = 1) THEN
SELECT email INTO ret_email FROM tmp_newsletterreg WHERE tmp_usr_id = id_session;
IF (ret_email IS NULL || ret_email='') THEN
RETURN ('-3');
ELSE
SELECT count(*) INTO usr_exists_2 FROM users WHERE users.email = ret_email;
IF (usr_exists_2 = 0) THEN -- first try of user to get registered
RETURN (ret_email);
ELSE -- user already exists into users tables (several tries to register)
RETURN ('-2');
END IF;
END IF;
ELSE
RETURN('-1');
END IF;
END;
if the session id is wrong, it works correctly.
however if the session id is correct it does not return me the email address (even if it really exist into table tmp_newsletterreg / but not in table users.)
so i think my eyes are tired, because i do not see an error...
thanks.
--
Alain
------------------------------------
Windows XP SP2
PostgreSQL 8.2.4 / MS SQL server 2005
Apache 2.2.4
PHP 5.2.4
C# 2005-2008
-------------- Original message ---------------------- From: "Alain Roger" <raf.news@gmail.com> > Hi, > > i have a problem solving my function trouble. > this function should return an email address stored in a table > (tmp_newsletterreg) based on a number (sessions ID). > if the session id is not find it should return a string corresponding to and > error. > if the email in found but already exists into another table (users), it > should also return a string value relative the this error. > > here is my stored procedure. > > > CREATE OR REPLACE FUNCTION cust_portal.sp_u_002(id_session character > > varying) > > RETURNS character varying AS > > $BODY$ > > > > DECLARE > > > > ret_email CHARACTER VARYING(512) :=''; > > usr_exists INTEGER := 0; > > usr_exists_2 INTEGER := 0; > > > > BEGIN > > set search_path = cust_portal; > > > > SELECT count(*) INTO usr_exists FROM tmp_newsletterreg WHERE > > tmp_usr_id = id_session; > > IF (usr_exists = 1) THEN > > SELECT email INTO ret_email FROM tmp_newsletterreg WHERE > > tmp_usr_id = id_session; > > IF (ret_email IS NULL || ret_email='') THEN See if I can do better this time. I believe your problem is here: IF (ret_email IS NULL || ret_email='') THEN || is the string concatenation operator. If you are trying to test both cases then you need to do something along lines of IF (ret_email IS NULL ) THEN RETURN ('-3') ELSIF (ret_email='') RETURN ('-3') > > RETURN ('-3'); > > ELSE > > SELECT count(*) INTO usr_exists_2 FROM users WHERE users.email= > ret_email; > > IF (usr_exists_2 = 0) THEN -- first try of user to get > > registered > > RETURN (ret_email); > > ELSE -- user already exists into users tables (several > > tries to register) > > RETURN ('-2'); > > END IF; > > END IF; > > ELSE > > RETURN('-1'); > > END IF; > > END; > > > > if the session id is wrong, it works correctly. > however if the session id is correct it does not return me the email address > (even if it really exist into table tmp_newsletterreg / but not in table > users.) > so i think my eyes are tired, because i do not see an error... > > thanks. > -- > Alain > ------------------------------------ > Windows XP SP2 > PostgreSQL 8.2.4 / MS SQL server 2005 > Apache 2.2.4 > PHP 5.2.4 > C# 2005-2008 -- Adrian Klaver aklaver@comcast.net
On Thu, Mar 27, 2008 at 08:43:46PM +0100, Alain Roger wrote: > i have a problem solving my function trouble. > this function should return an email address stored in a table > (tmp_newsletterreg) based on a number (sessions ID). > if the session id is not find it should return a string corresponding to and > error. > if the email in found but already exists into another table (users), it > should also return a string value relative the this error. This is fun isn't it! > here is my stored procedure. And here it is in a single, unreadable, SQL statement: SELECT CASE WHEN s.email = u.email THEN 'email already exists' ELSE COALESCE(s.email, 'no such session') END AS msg FROM (VALUES (1)) x(one) LEFT JOIN ( SELECT email FROM tmp_newsletterreg WHERE sessionid = $1) s ON TRUE LEFT JOIN (SELECT email FROM users) u ON s.email = u.email; Why not put a foreign key on the "email" column to the users table---one less error to handle that way? Sam
On Thursday 27 March 2008 3:17 pm, Sam Mason wrote: > On Thu, Mar 27, 2008 at 08:43:46PM +0100, Alain Roger wrote: > > i have a problem solving my function trouble. > > this function should return an email address stored in a table > > (tmp_newsletterreg) based on a number (sessions ID). > > if the session id is not find it should return a string corresponding to > > and error. > > if the email in found but already exists into another table (users), it > > should also return a string value relative the this error. > > This is fun isn't it! > > > here is my stored procedure. > > And here it is in a single, unreadable, SQL statement: > > SELECT CASE WHEN s.email = u.email THEN 'email already exists' > ELSE COALESCE(s.email, 'no such session') END AS msg > FROM (VALUES (1)) x(one) > LEFT JOIN ( > SELECT email FROM tmp_newsletterreg > WHERE sessionid = $1) s ON TRUE > LEFT JOIN (SELECT email FROM users) u ON s.email = u.email; > > Why not put a foreign key on the "email" column to the users table---one > less error to handle that way? > > > Sam Or a simpler way to do handle my previous suggestion: IF (ret_email IS NULL ) OR (ret_email='') THEN RETURN ('-3') -- Adrian Klaver aklaver@comcast.net
On Thu, Mar 27, 2008 at 03:34:49PM -0700, Adrian Klaver wrote: > Or a simpler way to do handle my previous suggestion: > > IF (ret_email IS NULL ) OR (ret_email='') THEN > RETURN ('-3') That would be the sane fix, yes. Based on the previous emails from the OP, he seems to be missing a lot of the tools that databases' give you. Transactions and unique constraints being two significant ones. Writing stored procedures to do their work is just going to introduce unnecessary bugs and complication. Sam
I do not agree with you Sam.
Stored procedure are safe from hacking (from external access).
From my point of view transitions should be used only as internal purpose or via intrAnet and not thru intErnet.
at list this is how under MS SQL they use to teach.
regarding unique constraint, i already setup it. :-)
unique violation will not help me in this case, or only to know if the email is stored several time.... which i do not test...directly :-)
Al.
--
Alain
------------------------------------
Windows XP SP2
PostgreSQL 8.2.4 / MS SQL server 2005
Apache 2.2.4
PHP 5.2.4
C# 2005-2008
Stored procedure are safe from hacking (from external access).
From my point of view transitions should be used only as internal purpose or via intrAnet and not thru intErnet.
at list this is how under MS SQL they use to teach.
regarding unique constraint, i already setup it. :-)
unique violation will not help me in this case, or only to know if the email is stored several time.... which i do not test...directly :-)
Al.
On Fri, Mar 28, 2008 at 1:19 AM, Sam Mason <sam@samason.me.uk> wrote:
On Thu, Mar 27, 2008 at 03:34:49PM -0700, Adrian Klaver wrote:That would be the sane fix, yes.
> Or a simpler way to do handle my previous suggestion:
>
> IF (ret_email IS NULL ) OR (ret_email='') THEN
> RETURN ('-3')
Based on the previous emails from the OP, he seems to be missing a
lot of the tools that databases' give you. Transactions and unique
constraints being two significant ones. Writing stored procedures to do
their work is just going to introduce unnecessary bugs and complication.
Sam
--
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general
--
Alain
------------------------------------
Windows XP SP2
PostgreSQL 8.2.4 / MS SQL server 2005
Apache 2.2.4
PHP 5.2.4
C# 2005-2008
Alain Roger wrote: > I do not agree with you Sam. > > Stored procedure are safe from hacking (from external access). In that a stored procedure encapsulates a series of data operations, meaning that the client doesn't have to know the details or even have privileges to run the individual operations ? Yes, that can be really useful, but it's hardly the full story. Proper use of things like foreign keys, unique constraints, CHECK constraints, etc adds another level of protection. I'd use those tools before I restored to using a stored procedure. Like stored procedures, users with appropriately limited priveleges are unable to bypass, drop, or modify constraints. That's true in any database with any sort of user privileges model. For example, if you want to enforce the rule that a certain field must have unique values in a table, do you think it's better to do it with a stored procedure, or by adding a UNIQUE constraint to the field? I'd say the UNIQUE constraint is better in every way. It's faster. It's simple and unlike the stored procedure isn't at risk of being bypassed by coding errors in the stored procedure. It's also self documenting, in that the schema clearly shows the unque constraint rather than hiding it in code. It's if anything more secure than a stored procedure because it's simpler and can be easily protected against user modification. The same goes for NOT NULL, CHECK constraints, foreign keys, etc. You're also doing a lot of things in your stored procedure code the long way. For example, instead of selecting a count() aggregate into a variable and testing it, why not just use `EXISTS', or select the information you really wanted and then use `IF FOUND' ? For example: -- It seems like not having an email address on record is an error. -- Ensure that the problem is detected at INSERT/UPDATE time. ALTER TABLE tmp_newsletterreg ALTER COLUMN email SET NOT NULL; -- Really basic valiation of email addresses. It's not worth doing much -- more than this sort of thing IMO because of performance issues and -- transcient errors (MX lookup fail etc) when doing proper email -- validation. At least now you don't have to revalidate in every -- procedure. ALTER TABLE tmp_newsletterreg ADD CONSTRAINT simplistic_email_check CHECK lower(trim(both ' ' from email)) LIKE '%_@_%'; -- Add the same check on the user table. I imagine a NOT NULL -- constraint there would also make sense. ALTER TABLE users ADD CONSTRAINT simplistic_email_check CHECK lower(trim(both ' ' from email)) LIKE '%_@_%'; -- Now, using those rules, redefine the stored procedure CREATE OR REPLACE FUNCTION cust_portal.sp_u_002 (id_session character varying) RETURNS character varying AS $BODY$ DECLARE ret_email CHARACTER VARYING(512) :=''; BEGIN set search_path = cust_portal; -- Find the customer's email address, or NULL (and set NOT FOUND) -- if no such customer exists. SELECT email INTO ret_email FROM tmp_newsletterreg WHERE tmp_usr_id = id_session; IF FOUND THEN IF NOT EXISTS (SELECT 1 FROM users WHERE users.email= ret_email) THEN RETURN (ret_email); ELSE RETURN ('-2'); END IF; ELSE RETURN('-1'); END IF; END; $BODY$ LANGUAGE 'plpgsql'; Personally I think the use of text string error codes gets ugly fast. I'd either rewrite the function to at least return an integer error code as an OUT parameter: CREATE OR REPLACE FUNCTION cust_portal.sp_u_002 (IN id_session character varying, OUT ret_email character varying, OUT err_code integer) RETURNS record AS $BODY$ DECLARE BEGIN set search_path = cust_portal; -- If we don't find the session, return -1 . ret_email := NULL; err_code := -1; -- Find the customer's email address, or NULL (and set NOT FOUND) -- if no such customer exists. SELECT email INTO ret_email FROM tmp_newsletterreg WHERE tmp_usr_id = id_session; IF FOUND THEN IF EXISTS (SELECT 1 FROM users WHERE users.email= ret_email) THEN -- User already registered ret_email := NULL; err_code := '-2'; END IF; END IF; RETURN; END; $BODY$ LANGUAGE 'plpgsql'; [note: the above code hasn't actually been tested] ... or preferably throw informative exceptions. However, I do find it frustrating that I can't attach a value or list of values to a PostgreSQL exception in a way that is easy for the client app to extract - I have to resort to text parsing (mega-ugly and unsafe) if I need to do it. Especially in an internationalised environment that's not nice. Being able to obtain the exact exception name (as opposed to the full error message), the full error string from an exception (without its context) and also obtain the individual parameters substituted into an exception string would be AMAZINGLY handy for use with JDBC etc. > From my point of view transitions should be used only as internal purpose or > via intrAnet and not thru intErnet. > > at list this is how under MS SQL they use to teach. Do you mean transactions? Transactions really aren't a security feature/issue. They're a tool useful for preserving data integrity and consistency by bundling up related operations into a single change that the database guarantees will all be applied at once, or not applied at all. They're critically important in non-trivial database use. Because you're wrapping most of your operations in stored procedures they're happening transactionally anyway, but it's still an important thing to think about. -- Craig Ringer
On Fri, Mar 28, 2008 at 06:43:00PM +0900, Craig Ringer wrote: > Alain Roger wrote: > > I do not agree with you Sam. > > > > Stored procedure are safe from hacking (from external access). > > In that a stored procedure encapsulates a series of data operations, > meaning that the client doesn't have to know the details or even have > privileges to run the individual operations ? Yes, that can be really > useful, but it's hardly the full story. Indeed. And in my experience, it's the program's own developers you've got to be most cautious about. "Hackers" would have very little trouble breaking most software these days---almost everything is far too big and complicated, ignoring rules like keeping it simple, respecting the principle of least authority and other time tested rules. Attackers also tend to go around the barriers you put in their way, not through them, the most general attack would be the physical one, i.e. paying a cleaner to remove something important. Another way of looking at it is to witness the types of bugs being fixed in software, almost all of them have no security implications and are straight human fallibility. > Proper use of things like foreign keys, unique constraints, CHECK > constraints, etc adds another level of protection. I'd use those tools > before I restored to using a stored procedure. Like stored procedures, > users with appropriately limited priveleges are unable to bypass, drop, > or modify constraints. Indeed, use the simplest possible tool to get the job done. And if possible reuse an existing one (i.e. all the work that has gone into getting the constraint handling working correctly in all the known cases). > -- Really basic valiation of email addresses. It's not worth doing much > -- more than this sort of thing IMO because of performance issues and > -- transcient errors (MX lookup fail etc) when doing proper email > -- validation. At least now you don't have to revalidate in every > -- procedure. > ALTER TABLE tmp_newsletterreg ADD CONSTRAINT simplistic_email_check > CHECK lower(trim(both ' ' from email)) LIKE '%_@_%'; Just out of interest, what's the lower() function call doing? I'd almost be tempted to do something like: CREATE DOMAIN emailaddr AS text CHECK (VALUE ~ '^[^ ]+@[^ ]+$'); and then use this instead of text/varchar types. > ... or preferably throw informative exceptions. This would be my preference. It'll probably do the "right" thing if the code is called from other stored procedures then. > However, I do find it > frustrating that I can't attach a value or list of values to a > PostgreSQL exception in a way that is easy for the client app to extract > - I have to resort to text parsing (mega-ugly and unsafe) if I need to > do it. Yup, why is this so often ignored when people write database drivers. I used the "pyPgSQL" python module (I think) for a bit, before realising that it even went as far as to "helpfully" automatically start a new transaction when the last one aborted. The resulting semantics meant my code did the most horrible things. And I'd agree with the remainder of your comments! Sam
Sam Mason wrote: >> ALTER TABLE tmp_newsletterreg ADD CONSTRAINT simplistic_email_check >> CHECK lower(trim(both ' ' from email)) LIKE '%_@_%'; >> > > Just out of interest, what's the lower() function call doing? > > Absolutely nothing. That's what I get for reading my mail at stupid-o-clock in the morning (Australia) instead of something sensible like sleeping. It's there because I was thinking about case insensitive domain comparison, but I couldn't begin to guess how it made its way into the constraint expression. > I'd almost be tempted to do something like: > > CREATE DOMAIN emailaddr AS text > CHECK (VALUE ~ '^[^ ]+@[^ ]+$'); > > and then use this instead of text/varchar types. > I was thinking about something like that, but my own storage of email addresses actually splits them into user part and domain part (so it can handle the case sensitivity differently - user parts are may be case sensitive depending on the mail system so you can't assume they're the same if they only differ in case; domain parts are never case sensitive) and that would've unnecessarily complicated the example. I didn't think to go for the half way point. >> ... or preferably throw informative exceptions. >> > > This would be my preference. It'll probably do the "right" thing if the > code is called from other stored procedures then. > Yep, it's what I'll do in almost all cases. I often land up writing client / UI data validation code to perform the same checks and catch the issue before submitting anything to the DB, but I don't consider this unreasonable. The DB's checks are protecting data integrity and consistency; the UI's checks are improving the user/app interaction by providing early (and usually more friendly) notification of data issues. They're really quite different jobs. Occasionally, though, I do have something where the DB-using app must just submit a request to the DB and see if it works. Either the UI doesn't have the privileges to run the same checks its self, or they're just too expensive to do from the client (or to do twice). In those cases I start to find Pg's error reporting frustrating, and I either resort to a "return value" sort of approach or embed a unique error code and some parseable values in the exception string. Eg: Some kind of human-readable error description goes here [ERR114:ID=12200;CONFLICTING-ID=1111] It's not pretty, but it works. > Yup, why is this so often ignored when people write database drivers. I > used the "pyPgSQL" python module (I think) for a bit, before realising > that it even went as far as to "helpfully" automatically start a new > transaction when the last one aborted. The resulting semantics meant my > code did the most horrible things. > That is indeed horrible, and I'd be running from a DB interface like that as fast as I could. Much of what I've done with PostgreSQL has been with Python (I do a lot of C++ too, but not with databases) and I've thankfully not run into anything like that. psycopg (the alternative PostgreSQL interface for Python) handles exceptions about as well as is possible with PostgreSQL's purely text based exception reporting, and I've found it very useful. I understand that it's also a LOT faster than PyPgSQL, though I don't have any direct experience there as I've never used PyPgSQL. It sounds like I unwittingly dodged a bullet there. As far as I'm concerned any DB interface that's ignoring errors behind your back needs to die. Especially in an exception-capable language like Python, where throwing and letting the upper layers handle it is the obviously sane thing to do. -- Craig Ringer
On Sat, Mar 29, 2008 at 04:05:15AM +0900, Craig Ringer wrote: > Sam Mason wrote: > >>ALTER TABLE tmp_newsletterreg ADD CONSTRAINT simplistic_email_check > >>CHECK lower(trim(both ' ' from email)) LIKE '%_@_%'; > > > >Just out of interest, what's the lower() function call doing? > > Absolutely nothing. That's what I get for reading my mail at > stupid-o-clock in the morning (Australia) instead of something sensible > like sleeping. OK, good to know I wasn't missing something :) > >I'd almost be tempted to do something like: > > > > CREATE DOMAIN emailaddr AS text > > CHECK (VALUE ~ '^[^ ]+@[^ ]+$'); > > > >and then use this instead of text/varchar types. > > > I was thinking about something like that, but my own storage of email > addresses actually splits them into user part and domain part (so it can > handle the case sensitivity differently - user parts are may be case > sensitive depending on the mail system so you can't assume they're the > same if they only differ in case; domain parts are never case sensitive) > and that would've unnecessarily complicated the example. I didn't think > to go for the half way point. I'd never bothered to go that way before. My reasoning being that emails get forwarded, aliases exist and other such fun and games. I only want the unique constraint there to keep my code working. > The DB's checks are protecting data integrity and > consistency; the UI's checks are improving the user/app interaction by > providing early (and usually more friendly) notification of data issues. > They're really quite different jobs. Humm, I'm getting the feeling we both learned programming at the same school! > Occasionally, though, I do have something where the DB-using app must > just submit a request to the DB and see if it works. Either the UI > doesn't have the privileges to run the same checks its self, or they're > just too expensive to do from the client (or to do twice). In those > cases I start to find Pg's error reporting frustrating, and I either > resort to a "return value" sort of approach or embed a unique error code > and some parseable values in the exception string. Eg: > > Some kind of human-readable error description goes here > [ERR114:ID=12200;CONFLICTING-ID=1111] > > It's not pretty, but it works. sounds sensible. Do any other databaes/other tools work better that you know of? I keep looking for projects, but this could end up touching quite a lot of code. I'm lucky in that I've got a small userbase and they seem to be OK (after a few initial frights) with the raw error messages from the database. > >Yup, why is this so often ignored when people write database drivers. I > >used the "pyPgSQL" python module (I think) for a bit, before realising > >that it even went as far as to "helpfully" automatically start a new > >transaction when the last one aborted. The resulting semantics meant my > >code did the most horrible things. > > That is indeed horrible, and I'd be running from a DB interface like > that as fast as I could. Yes, luckily I found out reasonably early. I don't do much with python and wanted to see how well it worked. It was a bit of an off putting experience. > Much of what I've done with PostgreSQL has been with Python (I do a lot > of C++ too, but not with databases) and I've thankfully not run into > anything like that. Most of the stuff I do with PG at work is through VB. At least I've managed get away from access at the back end! My little hobby programming things tend to be in much more formally specified things like Haskell, or lower level in C. > psycopg (the alternative PostgreSQL interface for > Python) handles exceptions about as well as is possible with > PostgreSQL's purely text based exception reporting, and I've found it > very useful. I understand that it's also a LOT faster than PyPgSQL, > though I don't have any direct experience there as I've never used > PyPgSQL. It sounds like I unwittingly dodged a bullet there. If I have reason to go back to Python I'll try and remember, thanks! > As far as I'm concerned any DB interface that's ignoring errors behind > your back needs to die. Especially in an exception-capable language like > Python, where throwing and letting the upper layers handle it is the > obviously sane thing to do. I think the author was honestly trying to he helpful. It's just that (s)he hadn't quite realised the consequences of this automatic transaction handling. Looking in its readme it's got comments about "To achieve the DB-API 2.0 mandated behaviour"..."a new transaction is created on the next call to execute()". I think maybe something got changed so that it also did an automatic rollback on an exception (I'm pretty sure I'd turned off autocommit pretty early, but it was a while ago). Sam
Sam Mason wrote: >> Occasionally, though, I do have something where the DB-using app must >> just submit a request to the DB and see if it works. Either the UI >> doesn't have the privileges to run the same checks its self, or they're >> just too expensive to do from the client (or to do twice). In those >> cases I start to find Pg's error reporting frustrating, and I either >> resort to a "return value" sort of approach or embed a unique error code >> and some parseable values in the exception string. Eg: >> >> Some kind of human-readable error description goes here >> [ERR114:ID=12200;CONFLICTING-ID=1111] >> >> It's not pretty, but it works. >> > > sounds sensible. Do any other databaes/other tools work better that you > know of? I keep looking for projects, but this could end up touching > quite a lot of code. > Unfortunately I don't know of anything that already does this sort of more detailed exception reporting. Being able to access: - Individual exception substitution values with their original types or as text; - The original exception text with substituted-in values but no context/location information; and - A user-specified exception "code" or subtype for PL/PgSQL-thrown exceptions would be *so* nice. Even just the last one would make a big difference I think. Anybody have any idea how tricky that might be to implement while maintaining 3.0 protocol compatibility for existing DB interfaces? It might be a pretty cool GSoC project, though I suspect it might involve a bit too much of Pg's internals. Am I the only one who'd want something like that, or does it sound like something that might be a worthwhile wishlist/distant-todo item? -- Craig Ringer