Re: [HACKERS] Fix PL/Python metadata when there is no result
On fre, 2012-02-10 at 17:44 +0100, Jean-Baptiste Quenot wrote: Dear hackers, Thanks for the work on PLPython result metadata, it is very useful! I just came across a crash when trying to access this metadata on the result of an UPDATE, which obviously cannot return any tuple (unless you specify a RETURNING clause maybe?). Please find attached a patch that solves this issue. Instead of a PG crash, we get the following message: ERROR: plpy.Error: no result fetched After much soul-searched about the API I have now committed your fix that throws the exception. Thanks. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix PL/Python metadata when there is no result
On tor, 2012-04-05 at 19:49 +0200, Jean-Baptiste Quenot wrote: I consider that this is an error to request metadata when the query does not return some. For example: UPDATE mytable SET value = 1 does not return column metadata, so user is not supposed to col coltypes(). That's why I propose to return an error. coltypes() is supposed to return a sequence, not None. Checking for None is a bad coding practise IMO, especially when dealing with lists. What would you suggest instead then? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix PL/Python metadata when there is no result
On lör, 2012-03-24 at 16:24 -0400, Tom Lane wrote: Peter Eisentraut pete...@gmx.net writes: On ons, 2012-03-07 at 17:14 -0500, Tom Lane wrote: I said it was a reasonable alternative, not that it was the only one we should consider. The behavior of .nrows() might be accidental, but perhaps it is a preferable model to adopt. After pondering this for several days now I still think the best approach is to change .nrows() to return None for utility commands and have the other metadata functions throw exceptions. OK, I don't have strong feelings about it. Well, scratch that. This whole area is sloppily documented. resultset.nrows() returns SPI_processed, which is the number of rows, er, processed by the statement, which may or may not be the number of rows returned. So a statement that returns no result set could very well have nrows() 0. The number of rows returned can be obtained by using len(resultset) (not documented, but one could perhaps guess it). But len() cannot return None, so we cannot use that as a marker. The alternatives are now to introduce a new function like has_rows() that returns True iff result rows exist and therefore result metadata can be fetched, or go back to having coltypes() et al. return None when no metadata exists. I'm in favor of the latter, because the former would add somewhat needless complications and doesn't really add any robustness or the like. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix PL/Python metadata when there is no result
2012/4/5 Peter Eisentraut pete...@gmx.net On lör, 2012-03-24 at 16:24 -0400, Tom Lane wrote: Peter Eisentraut pete...@gmx.net writes: On ons, 2012-03-07 at 17:14 -0500, Tom Lane wrote: I said it was a reasonable alternative, not that it was the only one we should consider. The behavior of .nrows() might be accidental, but perhaps it is a preferable model to adopt. After pondering this for several days now I still think the best approach is to change .nrows() to return None for utility commands and have the other metadata functions throw exceptions. OK, I don't have strong feelings about it. Well, scratch that. This whole area is sloppily documented. resultset.nrows() returns SPI_processed, which is the number of rows, er, processed by the statement, which may or may not be the number of rows returned. So a statement that returns no result set could very well have nrows() 0. The number of rows returned can be obtained by using len(resultset) (not documented, but one could perhaps guess it). But len() cannot return None, so we cannot use that as a marker. The alternatives are now to introduce a new function like has_rows() that returns True iff result rows exist and therefore result metadata can be fetched, or go back to having coltypes() et al. return None when no metadata exists. I'm in favor of the latter, because the former would add somewhat needless complications and doesn't really add any robustness or the like. I consider that this is an error to request metadata when the query does not return some. For example: UPDATE mytable SET value = 1 does not return column metadata, so user is not supposed to col coltypes(). That's why I propose to return an error. coltypes() is supposed to return a sequence, not None. Checking for None is a bad coding practise IMO, especially when dealing with lists. But anyway, returning None or raising an error is still much better than crashing :-) Cheers, -- Jean-Baptiste Quenot
Re: [HACKERS] Fix PL/Python metadata when there is no result
Peter Eisentraut pete...@gmx.net writes: The alternatives are now to introduce a new function like has_rows() that returns True iff result rows exist and therefore result metadata can be fetched, or go back to having coltypes() et al. return None when no metadata exists. I'm in favor of the latter, because the former would add somewhat needless complications and doesn't really add any robustness or the like. Seems sensible to me. We had better also document what nrows() really does. Should we also introduce a new function that is number of rows in the resultset, rather than depending on len()? I think it might be useful, or at least consistent, to have a function defined as number of rows, or None if the resultset does not contain rows. In particular, I think it is important to be able to distinguish between a command result (which cannot possibly contain rows) and a query result that happens to contain zero rows. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix PL/Python metadata when there is no result
On ons, 2012-03-07 at 17:14 -0500, Tom Lane wrote: Peter Eisentraut pete...@gmx.net writes: On ons, 2012-03-07 at 16:49 -0500, Tom Lane wrote: Still, it seems rather arbitrary to say that the row count property is the thing to test for that purpose and no other is. Why not return None for any property that's not sensible? Hmm, above you said you were in favor of throwing an error rather than returning None? I said it was a reasonable alternative, not that it was the only one we should consider. The behavior of .nrows() might be accidental, but perhaps it is a preferable model to adopt. It turns out I was mistaken about the .nrows() behavior. It returns 0 even for utility commands, because the value comes straight from SPI_processed. But SPI_processed is a C variable, which can't have a not applicable value, so that doesn't necessarily mean other languages can't handle this differently. After pondering this for several days now I still think the best approach is to change .nrows() to return None for utility commands and have the other metadata functions throw exceptions. Then the programming style would be if there are rows, give me metadata about them. The alternative would be to introduce another function has_rows or something, but then how would that be more intuitive than saying nrows() is not None? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix PL/Python metadata when there is no result
Peter Eisentraut pete...@gmx.net writes: On ons, 2012-03-07 at 17:14 -0500, Tom Lane wrote: I said it was a reasonable alternative, not that it was the only one we should consider. The behavior of .nrows() might be accidental, but perhaps it is a preferable model to adopt. After pondering this for several days now I still think the best approach is to change .nrows() to return None for utility commands and have the other metadata functions throw exceptions. OK, I don't have strong feelings about it. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix PL/Python metadata when there is no result
On lör, 2012-02-25 at 18:03 +0100, Jean-Baptiste Quenot wrote: IMO raising an error is much better because: 1) It is not a valid usecase to retrieve result metadata when no rows are expected to be returned Which led me to think, how are you actually expected to know when no rows are expected to be returned, in PL/Python? You can look at result.status(), which returns a numeric SPI status, but that seems fragile. I notice that result.nrows() returns None when no rows are returned. Is that good enough? In that case, we should document that and then make the new functions throw exceptions like you suggest. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix PL/Python metadata when there is no result
Peter Eisentraut pete...@gmx.net writes: On lör, 2012-02-25 at 18:03 +0100, Jean-Baptiste Quenot wrote: IMO raising an error is much better because: 1) It is not a valid usecase to retrieve result metadata when no rows are expected to be returned Which led me to think, how are you actually expected to know when no rows are expected to be returned, in PL/Python? You can look at result.status(), which returns a numeric SPI status, but that seems fragile. I notice that result.nrows() returns None when no rows are returned. Is that good enough? In that case, we should document that and then make the new functions throw exceptions like you suggest. Wait a minute ... I don't understand why that's not a valid use-case. I have seen more than one piece of code that does a SELECT ... LIMIT 0 or equivalent for the express purpose of finding out the rowtype produced by a particular query. Why would we make it impossible to do that in pl/python? Or are we talking about two different things? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix PL/Python metadata when there is no result
On ons, 2012-03-07 at 15:59 -0500, Tom Lane wrote: Which led me to think, how are you actually expected to know when no rows are expected to be returned, in PL/Python? You can look at result.status(), which returns a numeric SPI status, but that seems fragile. I notice that result.nrows() returns None when no rows are returned. Is that good enough? In that case, we should document that and then make the new functions throw exceptions like you suggest. Wait a minute ... I don't understand why that's not a valid use-case. I have seen more than one piece of code that does a SELECT ... LIMIT 0 or equivalent for the express purpose of finding out the rowtype produced by a particular query. Why would we make it impossible to do that in pl/python? Or are we talking about two different things? I think so. I'm wondering here how to detect whether the execution of a statement has yielded a result set at all. (For example, you ran SELECT or INSERT ... RETURNING, versus CREATE TABLE or VACUUM.) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix PL/Python metadata when there is no result
Peter Eisentraut pete...@gmx.net writes: On ons, 2012-03-07 at 15:59 -0500, Tom Lane wrote: Or are we talking about two different things? I think so. I'm wondering here how to detect whether the execution of a statement has yielded a result set at all. (For example, you ran SELECT or INSERT ... RETURNING, versus CREATE TABLE or VACUUM.) Got it. I agree that throwing an error for resultset property inquiries is reasonable in such cases, as long as there is some non-error-throwing way to test whether a resultset was returned or not. Still, it seems rather arbitrary to say that the row count property is the thing to test for that purpose and no other is. Why not return None for any property that's not sensible? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix PL/Python metadata when there is no result
On ons, 2012-03-07 at 16:49 -0500, Tom Lane wrote: Peter Eisentraut pete...@gmx.net writes: On ons, 2012-03-07 at 15:59 -0500, Tom Lane wrote: Or are we talking about two different things? I think so. I'm wondering here how to detect whether the execution of a statement has yielded a result set at all. (For example, you ran SELECT or INSERT ... RETURNING, versus CREATE TABLE or VACUUM.) Got it. I agree that throwing an error for resultset property inquiries is reasonable in such cases, as long as there is some non-error-throwing way to test whether a resultset was returned or not. Well, that's the question. The only two ways currently to find out whether a result set was returned is by looking at .nrows(), which I think works by accident, or at .status(), which is the SPI status code, and that's quite cumbersome. Still, it seems rather arbitrary to say that the row count property is the thing to test for that purpose and no other is. Why not return None for any property that's not sensible? Hmm, above you said you were in favor of throwing an error rather than returning None? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix PL/Python metadata when there is no result
Peter Eisentraut pete...@gmx.net writes: On ons, 2012-03-07 at 16:49 -0500, Tom Lane wrote: Still, it seems rather arbitrary to say that the row count property is the thing to test for that purpose and no other is. Why not return None for any property that's not sensible? Hmm, above you said you were in favor of throwing an error rather than returning None? I said it was a reasonable alternative, not that it was the only one we should consider. The behavior of .nrows() might be accidental, but perhaps it is a preferable model to adopt. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix PL/Python metadata when there is no result
2012/2/24 Peter Eisentraut pete...@gmx.net: On fre, 2012-02-10 at 17:44 +0100, Jean-Baptiste Quenot wrote: Please find attached a patch that solves this issue. Instead of a PG crash, we get the following message: ERROR: plpy.Error: no result fetched Hmm, should it be an error or just return None? Python DB-API cursor.description returns None if no result set was returned. IMO raising an error is much better because: 1) It is not a valid usecase to retrieve result metadata when no rows are expected to be returned 2) The various metadata methods return a sequence. Checking for null value in this case is not a very good programming style. I expect to find an empty list when no data is available. Cheers, -- Jean-Baptiste Quenot -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix PL/Python metadata when there is no result
On fre, 2012-02-10 at 17:44 +0100, Jean-Baptiste Quenot wrote: Thanks for the work on PLPython result metadata, it is very useful! I just came across a crash when trying to access this metadata on the result of an UPDATE, which obviously cannot return any tuple (unless you specify a RETURNING clause maybe?). Please find attached a patch that solves this issue. Instead of a PG crash, we get the following message: ERROR: plpy.Error: no result fetched Hmm, should it be an error or just return None? Python DB-API cursor.description returns None if no result set was returned. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Fix PL/Python metadata when there is no result
Dear hackers, Thanks for the work on PLPython result metadata, it is very useful! I just came across a crash when trying to access this metadata on the result of an UPDATE, which obviously cannot return any tuple (unless you specify a RETURNING clause maybe?). Please find attached a patch that solves this issue. Instead of a PG crash, we get the following message: ERROR: plpy.Error: no result fetched All the best, -- Jean-Baptiste Quenot 0001-Fix-PLPython-metadata-access-when-there-is-no-result.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
