Re: [HACKERS] plpgsql: numeric assignment to an integer variable errors out
Hi, Now it is true that a lot of the uses for that were subsumed when we added coerce-via-IO to the native cast capabilities; but I'm still quite scared of what this would break, and I don't see any field demand for a change. Well, we have had one of our EDB connectors facing issues because of this existing conversion mechanism. I think this is a small patch which tries to do the right thing, no? Regards, Nikhils -- http://www.enterprisedb.com -- 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] plpgsql: numeric assignment to an integer variable errors out
Whatever happened to this patch? --- Nikhil Sontakke wrote: Hi, nikhil.sonta...@enterprisedb.com wrote: The following plpgsql function errors out with cvs head: CREATE function test_assign() returns void AS $$ declare x int; BEGIN x := 9E3/2; END $$ LANGUAGE 'plpgsql'; postgres=# select test_assign(); ERROR: invalid input syntax for integer: 4500. CONTEXT: PL/pgSQL function test_assign line 3 at assignment We do have an existing cast from numeric to type integer. But here basically we convert the value to string in exec_cast_value before calling int4in. And then use of strtol in pg_atoi leads to this complaint. Guess converting the value to string is not always a good strategy. PFA, patch which uses find_coercion_pathway to find a direct COERCION_PATH_FUNC function and uses that if it is available. Or is there a better approach? Seems to handle the above issue with this patch. Regards, Nikhils -- http://www.enterprisedb.com [ Attachment, skipping... ] -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do + If your life is a hard drive, Christ can be your backup. + -- 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] plpgsql: numeric assignment to an integer variable errors out
Bruce Momjian br...@momjian.us writes: Whatever happened to this patch? I think we bounced it on the grounds that it would represent a fundamental change in plpgsql behavior and break a whole lot of applications. People have been relying on plpgsql's coerce-via-IO assignment behavior for ten years. If you prefer coerce via cast conversion, you can get that by writing an explicit cast. Now it is true that a lot of the uses for that were subsumed when we added coerce-via-IO to the native cast capabilities; but I'm still quite scared of what this would break, and I don't see any field demand for a change. 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] plpgsql: numeric assignment to an integer variable errors out
Tom Lane wrote: Bruce Momjian br...@momjian.us writes: Whatever happened to this patch? I think we bounced it on the grounds that it would represent a fundamental change in plpgsql behavior and break a whole lot of applications. People have been relying on plpgsql's coerce-via-IO assignment behavior for ten years. If you prefer coerce via cast conversion, you can get that by writing an explicit cast. Now it is true that a lot of the uses for that were subsumed when we added coerce-via-IO to the native cast capabilities; but I'm still quite scared of what this would break, and I don't see any field demand for a change. Thanks. Sorry to be asking so many questions but it is the only way I can be sure we have covered everything. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do + If your life is a hard drive, Christ can be your backup. + -- 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] plpgsql: numeric assignment to an integer variable errors out
Hello I tested patch v2.0, and I thing, so this patch should be used as bug fix. It has same or little bit better speed than current and solve some problems with numeric's implicit casting in plpgsql. But this is only an half solution. The core of problem is in lazy casting of plpgsql. We need to modify execution plan for pl expression when result is different than destination type. For almost expression we know destination type. there is some sample of effectiveness: postgres=# create or replace function test1() returns int as $$ declare s int := 0; begin for i in 1..10 loop s := 4e3; -- numeric end loop; return s; end; $$ language plpgsql immutable; CREATE FUNCTION Time: 5,140 ms postgres=# create or replace function test2() returns int as $$ declare s int := 0; begin for i in 1..10 loop s := 4e3::int; end loop; return s; end; $$ language plpgsql immutable; CREATE FUNCTION Time: 416,240 ms postgres=# select test1(); test1 --- 4000 (1 row) Time: 161,048 ms postgres=# select test2(); test2 --- 4000 (1 row) Time: 68,110 ms postgres=# select test1(); test1 --- 4000 (1 row) Time: 171,020 ms postgres=# select test2(); test2 --- 4000 (1 row) Time: 61,771 ms postgres=# Regards Pavel Stehule 2009/1/22 Bruce Momjian br...@momjian.us: Nikhil Sontakke wrote: PFA, patch which uses find_coercion_pathway to find a direct COERCION_PATH_FUNC function and uses that if it is available. Or is there a better approach? Seems to handle the above issue with this patch. +1 I thing, so some values should by cached, current patch could by slow. Agreed, it can slow things down a bit especially since we are only interested in the COERCION_PATH_FUNC case. What we need is a much simpler pathway function which searches in the SysCache and returns back with the valid/invalid castfunc immediately. PFA, version 2.0 of this patch with these changes in place. I could have added a generic function in parse_coerce.c, but thought the use case was restricted to plpgsql and hence I have kept it within pl_exec.c for now. Where are we on this? 8.5? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] plpgsql: numeric assignment to an integer variable errors out
Nikhil Sontakke wrote: PFA, patch which uses find_coercion_pathway to find a direct COERCION_PATH_FUNC function and uses that if it is available. Or is there a better approach? Seems to handle the above issue with this patch. +1 I thing, so some values should by cached, current patch could by slow. Agreed, it can slow things down a bit especially since we are only interested in the COERCION_PATH_FUNC case. What we need is a much simpler pathway function which searches in the SysCache and returns back with the valid/invalid castfunc immediately. PFA, version 2.0 of this patch with these changes in place. I could have added a generic function in parse_coerce.c, but thought the use case was restricted to plpgsql and hence I have kept it within pl_exec.c for now. Where are we on this? 8.5? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] plpgsql: numeric assignment to an integer variable errors out
2009/1/22 Bruce Momjian br...@momjian.us: Nikhil Sontakke wrote: PFA, patch which uses find_coercion_pathway to find a direct COERCION_PATH_FUNC function and uses that if it is available. Or is there a better approach? Seems to handle the above issue with this patch. +1 I thing, so some values should by cached, current patch could by slow. Agreed, it can slow things down a bit especially since we are only interested in the COERCION_PATH_FUNC case. What we need is a much simpler pathway function which searches in the SysCache and returns back with the valid/invalid castfunc immediately. PFA, version 2.0 of this patch with these changes in place. I could have added a generic function in parse_coerce.c, but thought the use case was restricted to plpgsql and hence I have kept it within pl_exec.c for now. Where are we on this? 8.5? I hope, so this patch will be in next commitfest Pavel -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] plpgsql: numeric assignment to an integer variable errors out
Hi,
nikhil.sonta...@enterprisedb.com wrote:
The following plpgsql function errors out with cvs head:
CREATE function test_assign() returns void
AS
$$ declare x int;
BEGIN
x := 9E3/2;
END
$$ LANGUAGE 'plpgsql';
postgres=# select test_assign();
ERROR: invalid input syntax for integer: 4500.
CONTEXT: PL/pgSQL function test_assign line 3 at assignment
We do have an existing cast from numeric to type integer. But here
basically we convert the value to string in exec_cast_value before calling
int4in. And then use of strtol in pg_atoi leads to this complaint. Guess
converting the value to string is not always a good strategy.
PFA, patch which uses find_coercion_pathway to find a direct
COERCION_PATH_FUNC function and uses that if it is available. Or is there a
better approach? Seems to handle the above issue with this patch.
Regards,
Nikhils
--
http://www.enterprisedb.com
Index: src/pl/plpgsql/src/pl_exec.c
===
RCS file: /repositories/postgreshome/cvs/pgsql/src/pl/plpgsql/src/pl_exec.c,v
retrieving revision 1.225
diff -c -r1.225 pl_exec.c
*** src/pl/plpgsql/src/pl_exec.c 20 Nov 2008 15:36:22 - 1.225
--- src/pl/plpgsql/src/pl_exec.c 29 Dec 2008 15:23:49 -
***
*** 24,29
--- 24,30
#include funcapi.h
#include nodes/nodeFuncs.h
#include parser/scansup.h
+ #include parser/parse_coerce.h
#include tcop/tcopprot.h
#include utils/array.h
#include utils/builtins.h
***
*** 4732,4750
{
if (!isnull)
{
! char *extval;
! extval = convert_value_to_string(value, valtype);
! /* Allow input function to use SPI ... see notes above */
! SPI_push();
! value = InputFunctionCall(reqinput, extval,
! reqtypioparam, reqtypmod);
! SPI_pop();
! pfree(extval);
}
else
{
--- 4733,4777
{
if (!isnull)
{
! /*
! * find out if there is a coercion pathway via a function
! */
! CoercionPathType pathtype;
! Oid castfunc;
! pathtype = find_coercion_pathway(reqtype, valtype,
! COERCION_ASSIGNMENT,
! castfunc);
!
! if (pathtype == COERCION_PATH_FUNC)
! {
! FmgrInfo cast_func_finfo;
!
! fmgr_info_cxt(castfunc, cast_func_finfo,
! CurrentMemoryContext);
! cast_func_finfo.fn_oid = InvalidOid;
!
! /* do we need reqtypmod for cast functions? */
! value = FunctionCall2(cast_func_finfo,
! value,
! reqtypmod);
! }
! else
! {
! char *extval;
! extval = convert_value_to_string(value, valtype);
! /* Allow input function to use SPI ... see notes above */
! SPI_push();
! value = InputFunctionCall(reqinput, extval,
! reqtypioparam, reqtypmod);
! SPI_pop();
!
! pfree(extval);
! }
}
else
{
--
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] plpgsql: numeric assignment to an integer variable errors out
2008/12/29 Nikhil Sontakke nikhil.sonta...@enterprisedb.com: Hi, nikhil.sonta...@enterprisedb.com wrote: The following plpgsql function errors out with cvs head: CREATE function test_assign() returns void AS $$ declare x int; BEGIN x := 9E3/2; END $$ LANGUAGE 'plpgsql'; postgres=# select test_assign(); ERROR: invalid input syntax for integer: 4500. CONTEXT: PL/pgSQL function test_assign line 3 at assignment We do have an existing cast from numeric to type integer. But here basically we convert the value to string in exec_cast_value before calling int4in. And then use of strtol in pg_atoi leads to this complaint. Guess converting the value to string is not always a good strategy. PFA, patch which uses find_coercion_pathway to find a direct COERCION_PATH_FUNC function and uses that if it is available. Or is there a better approach? Seems to handle the above issue with this patch. +1 I thing, so some values should by cached, current patch could by slow. Regards Pavel Stehule Regards, Nikhils -- http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- 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] plpgsql: numeric assignment to an integer variable errors out
Hi,
The following plpgsql function errors out with cvs head:
CREATE function test_assign() returns void
AS
$$ declare x int;
BEGIN
x := 9E3/2;
END
$$ LANGUAGE 'plpgsql';
postgres=# select test_assign();
ERROR: invalid input syntax for integer: 4500.
CONTEXT: PL/pgSQL function test_assign line 3 at assignment
We do have an existing cast from numeric to type integer. But here
basically we convert the value to string in exec_cast_value before
calling
int4in. And then use of strtol in pg_atoi leads to this complaint. Guess
converting the value to string is not always a good strategy.
PFA, patch which uses find_coercion_pathway to find a direct
COERCION_PATH_FUNC function and uses that if it is available. Or is there
a
better approach? Seems to handle the above issue with this patch.
+1
I thing, so some values should by cached, current patch could by slow.
Agreed, it can slow things down a bit especially since we are only
interested in the COERCION_PATH_FUNC case. What we need is a much simpler
pathway function which searches in the SysCache and returns back with the
valid/invalid castfunc immediately.
PFA, version 2.0 of this patch with these changes in place. I could have
added a generic function in parse_coerce.c, but thought the use case was
restricted to plpgsql and hence I have kept it within pl_exec.c for now.
Regards,
Nikhils
--
http://www.enterprisedb.com
Index: src/pl/plpgsql/src/pl_exec.c
===
RCS file: /repositories/postgreshome/cvs/pgsql/src/pl/plpgsql/src/pl_exec.c,v
retrieving revision 1.225
diff -c -r1.225 pl_exec.c
*** src/pl/plpgsql/src/pl_exec.c 20 Nov 2008 15:36:22 - 1.225
--- src/pl/plpgsql/src/pl_exec.c 30 Dec 2008 07:31:36 -
***
*** 20,29
--- 20,31
#include access/transam.h
#include catalog/pg_proc.h
#include catalog/pg_type.h
+ #include catalog/pg_cast.h
#include executor/spi_priv.h
#include funcapi.h
#include nodes/nodeFuncs.h
#include parser/scansup.h
+ #include parser/parse_coerce.h
#include tcop/tcopprot.h
#include utils/array.h
#include utils/builtins.h
***
*** 199,204
--- 201,208
static void free_params_data(PreparedParamsData *ppd);
static Portal exec_dynquery_with_params(PLpgSQL_execstate *estate,
PLpgSQL_expr *query, List *params);
+ static CoercionPathType find_assignment_coercion_pathway(Oid targetTypeId,
+ Oid sourceTypeId, Oid *funcid);
/* --
***
*** 4732,4750
{
if (!isnull)
{
! char *extval;
! extval = convert_value_to_string(value, valtype);
! /* Allow input function to use SPI ... see notes above */
! SPI_push();
! value = InputFunctionCall(reqinput, extval,
! reqtypioparam, reqtypmod);
! SPI_pop();
! pfree(extval);
}
else
{
--- 4736,4779
{
if (!isnull)
{
! /*
! * find out if there is a coercion pathway via a function
! */
! CoercionPathType pathtype;
! Oid castfunc;
! pathtype = find_assignment_coercion_pathway(reqtype, valtype,
! castfunc);
! if (pathtype == COERCION_PATH_FUNC)
! {
! FmgrInfo cast_func_finfo;
!
! fmgr_info_cxt(castfunc, cast_func_finfo,
! CurrentMemoryContext);
! cast_func_finfo.fn_oid = InvalidOid;
!
! /* do we need reqtypmod for cast functions? */
! value = FunctionCall2(cast_func_finfo,
! value,
! reqtypmod);
! }
! else
! {
! char *extval;
! extval = convert_value_to_string(value, valtype);
! /* Allow input function to use SPI ... see notes above */
! SPI_push();
! value = InputFunctionCall(reqinput, extval,
! reqtypioparam, reqtypmod);
!
! SPI_pop();
!
! pfree(extval);
! }
}
else
{
***
*** 5456,5458
--- 5485,5513
return portal;
}
+
+ static CoercionPathType
+ find_assignment_coercion_pathway(Oid targetTypeId, Oid sourceTypeId, Oid *funcid)
+ {
+ CoercionPathType result = COERCION_PATH_NONE;
+ HeapTuple tuple;
+
+ /* Look in pg_cast */
+ tuple = SearchSysCache(CASTSOURCETARGET,
+ ObjectIdGetDatum(sourceTypeId),
+ ObjectIdGetDatum(targetTypeId),
+ 0, 0);
+
+ if (HeapTupleIsValid(tuple))
+ {
+ Form_pg_cast castForm = (Form_pg_cast) GETSTRUCT(tuple);
+
+ *funcid = castForm-castfunc;
+ ReleaseSysCache(tuple);
+ }
+
+ if (OidIsValid(*funcid))
+ result = COERCION_PATH_FUNC;
+
+ return result;
+ }
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
