Re: [PATCHES] pgstat: delayed write of stats file
Magnus Hagander [EMAIL PROTECTED] writes: This was not ready to be applied, was it? I don't recall any specific objections that weren't answered. How about the fact that it's already caused one buildfarm failure? http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=waspdt=2006-04-30%2003:05:01 regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] pgstat: delayed write of stats file
This was not ready to be applied, was it? I don't recall any specific objections that weren't answered. How about the fact that it's already caused one buildfarm failure? http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=waspdt=2006 -04-30%2003:05:01 Well, that objection certainly wasn't raised before since it wasn't on buildfarm, and it passed the regression tests many times on my systems. Oh, and apparantly it caused the same issue on firefly. I don't really see the connection between these two platforms and why it should happen just there, though. Any ideas on that part? //Magnus ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] plpython improvements
Hi,
Thu, 27 Apr 2006 17:17:36 +0300, Bruce Momjian pgman@candle.pha.pa.us:
Sorry, I have to revert this patch because it is causing crashes in the
plpython regression tests. Would you please run those tests, fix the
bug, and resubmit. Thanks.
Found and fixed two problems:
1) named parameters handling if there were no parameters at all
2) didn't increment counter for borrowed reference fetched from list
Also integrated tests to plpython test suite and updated existing tests to
use named parameters.
Unfortunately, there is still one problem when using unpatched python,
caused by too aggressive assert.
See
http://mail.python.org/pipermail/python-checkins/2005-August/046571.html.
I guess there should be warning somewhere as Hannu said but didn't know
where to put it.
--
Sven SuursohoIndex: plpython.c
===
RCS file: /projects/cvsroot/pgsql/src/pl/plpython/plpython.c,v
retrieving revision 1.79
diff -u -r1.79 plpython.c
--- plpython.c 27 Apr 2006 14:18:07 - 1.79
+++ plpython.c 29 Apr 2006 10:52:22 -
@@ -19,6 +19,7 @@
#include catalog/pg_type.h
#include commands/trigger.h
#include executor/spi.h
+#include funcapi.h
#include fmgr.h
#include nodes/makefuncs.h
#include parser/parse_type.h
@@ -108,6 +109,11 @@
bool fn_readonly;
PLyTypeInfo result; /* also used to store info for trigger tuple
* type */
+ bool is_setof; /* true, if procedure returns result set */
+ PyObject*setof; /* contents of result set. */
+ int setof_count; /* numbef of items to return in result set */
+ int setof_current; /* current item in result set */
+ char **argnames; /* Argument names */
PLyTypeInfo args[FUNC_MAX_ARGS];
int nargs;
PyObject *code; /* compiled procedure code */
@@ -184,6 +190,7 @@
static HeapTuple PLy_trigger_handler(FunctionCallInfo fcinfo, PLyProcedure *);
static PyObject *PLy_function_build_args(FunctionCallInfo fcinfo, PLyProcedure *);
+static void PLy_function_delete_args(PLyProcedure *);
static PyObject *PLy_trigger_build_args(FunctionCallInfo fcinfo, PLyProcedure *,
HeapTuple *);
static HeapTuple PLy_modify_tuple(PLyProcedure *, PyObject *,
@@ -218,6 +225,7 @@
static PyObject *PLyInt_FromString(const char *);
static PyObject *PLyLong_FromString(const char *);
static PyObject *PLyString_FromString(const char *);
+static HeapTuple PLyDict_ToTuple(PLyTypeInfo *, PyObject *);
/* global data */
@@ -726,11 +734,17 @@
PG_TRY();
{
- plargs = PLy_function_build_args(fcinfo, proc);
- plrv = PLy_procedure_call(proc, args, plargs);
-
- Assert(plrv != NULL);
- Assert(!PLy_error_in_progress);
+ if (!proc-is_setof || proc-setof_count == -1)
+ {
+ /* python function not called yet, do it */
+ plargs = PLy_function_build_args(fcinfo, proc);
+ plrv = PLy_procedure_call(proc, args, plargs);
+ if (!proc-is_setof)
+/* SETOF function parameters are deleted when called last row is returned */
+PLy_function_delete_args(proc);
+ Assert(plrv != NULL);
+ Assert(!PLy_error_in_progress);
+ }
/*
* Disconnect from SPI manager and then create the return values datum
@@ -741,6 +755,82 @@
if (SPI_finish() != SPI_OK_FINISH)
elog(ERROR, SPI_finish failed);
+ if (proc-is_setof)
+ {
+ bool is_done = false;
+ ReturnSetInfo *rsi = (ReturnSetInfo *)fcinfo-resultinfo;
+
+ if (proc-setof_current == -1)
+ {
+/* first time -- do checks and setup */
+if (!rsi || !IsA(rsi, ReturnSetInfo) ||
+ (rsi-allowedModes SFRM_ValuePerCall) == 0)
+{
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg(only value per call is allowed)));
+}
+rsi-returnMode = SFRM_ValuePerCall;
+
+/* fetch information about returned object */
+proc-setof = plrv;
+plrv = NULL;
+if (PyList_Check(proc-setof))
+ /* SETOF as list */
+ proc-setof_count = PyList_GET_SIZE(proc-setof);
+else if (PyIter_Check(proc-setof))
+ /* SETOF as iterator, unknown number of items */
+ proc-setof_current = proc-setof_count = 0;
+else
+{
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg(SETOF must be returned as list, iterator or generator)));
+}
+ }
+
+ Assert(proc-setof != NULL);
+
+ /* Fetch next of SETOF */
+ if (PyList_Check(proc-setof))
+ {
+is_done = ++proc-setof_current == proc-setof_count;
+if (!is_done)
+{
+ plrv = PyList_GET_ITEM(proc-setof, proc-setof_current);
+
+ /* Got 'borrowed reference', must increment reference count
+ * to balance later decrement */
+ Py_INCREF(plrv);
+}
+ }
+ else if (PyIter_Check(proc-setof))
+ {
+plrv = PyIter_Next(proc-setof);
+is_done = plrv == NULL;
+ }
+
+ if (!is_done)
+ {
+rsi-isDone = ExprMultipleResult;
+ }
+ else
+ {
+rsi-isDone = ExprEndResult;
+proc-setof_count = proc-setof_current = -1;
+Py_DECREF(proc-setof);
Re: [PATCHES] plpython improvements
Sven Suursoho [EMAIL PROTECTED] writes: Unfortunately, there is still one problem when using unpatched python, caused by too aggressive assert. See http://mail.python.org/pipermail/python-checkins/2005-August/046571.html. I guess there should be warning somewhere as Hannu said but didn't know where to put it. I don't think we are going to be able to accept a patch that causes the server to crash when using any but a bleeding-edge copy of Python. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] plpython improvements
Sun, 30 Apr 2006 19:14:28 +0300, Tom Lane [EMAIL PROTECTED]: Sven Suursoho [EMAIL PROTECTED] writes: Unfortunately, there is still one problem when using unpatched python, caused by too aggressive assert. See http://mail.python.org/pipermail/python-checkins/2005-August/046571.html. I guess there should be warning somewhere as Hannu said but didn't know where to put it. I don't think we are going to be able to accept a patch that causes the server to crash when using any but a bleeding-edge copy of Python. Actually normal python installations do not cause problem, only debugging versions do. Anyway, if you think that this doesn't count as an argument, there is nothing that we can do from PG-side except drop returning SETOF as iterator/generator and only allow return SETOF as list. -- Sven Suursoho ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] plpython improvements
Sven Suursoho wrote: Sun, 30 Apr 2006 19:14:28 +0300, Tom Lane [EMAIL PROTECTED]: Sven Suursoho [EMAIL PROTECTED] writes: Unfortunately, there is still one problem when using unpatched python, caused by too aggressive assert. See http://mail.python.org/pipermail/python-checkins/2005-August/046571.html. I guess there should be warning somewhere as Hannu said but didn't know where to put it. I don't think we are going to be able to accept a patch that causes the server to crash when using any but a bleeding-edge copy of Python. Actually normal python installations do not cause problem, only debugging versions do. Anyway, if you think that this doesn't count as an argument, there is nothing that we can do from PG-side except drop returning SETOF as iterator/generator and only allow return SETOF as list. Can't we detect a debug build and disable the feature? -- Bruce Momjian http://candle.pha.pa.us EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] plpython improvements
Sun, 30 Apr 2006 20:48:48 +0300, Bruce Momjian pgman@candle.pha.pa.us: Sun, 30 Apr 2006 19:14:28 +0300, Tom Lane [EMAIL PROTECTED]: Sven Suursoho [EMAIL PROTECTED] writes: Unfortunately, there is still one problem when using unpatched python, caused by too aggressive assert. http://mail.python.org/pipermail/python-checkins/2005-August/046571.html. I guess there should be warning somewhere as Hannu said but didn't know where to put it. I don't think we are going to be able to accept a patch that causes the server to crash when using any but a bleeding-edge copy of Python. Actually normal python installations do not cause problem, only debugging versions do. Anyway, if you think that this doesn't count as an argument, there is nothing that we can do from PG-side except drop returning SETOF as iterator/generator and only allow return SETOF as list. Can't we detect a debug build and disable the feature? Yes, we can, but newer python versions are already fixed. So, what about this in configure: if --with-python test_iterator_app_crashes # errcode(FEATURE_NOT_SUPPORTED), errmsg(patch your python) disable_iterator_feature fi In this way we disable feature only if it is absolutely neccessary and will give developer enough information how to fix it. -- Sven Suursoho ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] plpython improvements
Sven Suursoho [EMAIL PROTECTED] writes: So, what about this in configure: if --with-python test_iterator_app_crashes # errcode(FEATURE_NOT_SUPPORTED), errmsg(patch your python) disable_iterator_feature fi Testing it in configure is wrong, because there's no guarantee the same python library will be used at runtime. regards, tom lane ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
[PATCHES] fori stmt with by keyword was:(Re: [HACKERS] for statement, adding a STEP clause?)
On 4/29/06, Andrew Dunstan [EMAIL PROTECTED] wrote:
Tom Lane wrote:
Jaime Casanova [EMAIL PROTECTED] writes:
there is a chance to add a STEP clause to the FOR statement in plpgsql?
This is not free: it'd require making STEP a reserved word (at least
within plpgsql) which is contrary to spec. I think you need to make
a pretty good case why the value of the feature outweighs breaking
applications that have perfectly-legally used step as an identifier.
This isn't available in PL/SQL, is it? That doesn't mean we shouldn't do it, of
course, but it might lessen any perceived imperative.
Maybe using BY instad of STEP as the keyword would make it easier, since its
occurrence in SQL makes it less likely to be used as a variable.
cheers
andrew
Hi,
i make a little patch using BY instead of STEP per Tom's complaint and
Andrew's suggestion.
the patch is not ready yet because i can't figure out how to make the
BY optional and that is mandatory because backward compatibility...
the problem is how to manage it in gram.y
perhaps someone with more expertise in gram.y can make suggestions?
also, you can review the patch and say if it will be added if i can
solve the optional BY problem... then i can start working in fixing
the docs
--
regards,
Jaime Casanova
What they (MySQL) lose in usability, they gain back in benchmarks, and that's
all that matters: getting the wrong answer really fast.
Randal L. Schwartz
diff -rciEb pgsql-8.2dev/src/pl/plpgsql/src/gram.y pgsql-8.2fori/src/pl/plpgsql/src/gram.y
*** pgsql-8.2dev/src/pl/plpgsql/src/gram.y 2006-04-30 09:45:12.0 -0500
--- pgsql-8.2fori/src/pl/plpgsql/src/gram.y 2006-04-30 09:49:05.0 -0500
***
*** 143,148
--- 143,149
%token K_ALIAS
%token K_ASSIGN
%token K_BEGIN
+ %token K_BY
%token K_CLOSE
%token K_CONSTANT
%token K_CONTINUE
***
*** 930,935
--- 931,937
{
/* Saw .., so it must be an integer loop */
PLpgSQL_expr *expr2;
+ PLpgSQL_expr *expr_by;
PLpgSQL_var *fvar;
PLpgSQL_stmt_fori *new;
char*varname;
***
*** 937,943
/* First expression is well-formed */
check_sql_expr(expr1-query);
! expr2 = plpgsql_read_expression(K_LOOP, LOOP);
/* should have had a single variable name */
plpgsql_error_lineno = $2.lineno;
--- 939,951
/* First expression is well-formed */
check_sql_expr(expr1-query);
! expr2 = read_sql_construct(K_BY, K_LOOP,
! BY|LOOP,
! SELECT , true,
! false, tok);
!
! if (tok = K_BY)
! expr_by = plpgsql_read_expression(K_LOOP, LOOP);
/* should have had a single variable name */
plpgsql_error_lineno = $2.lineno;
***
*** 965,970
--- 973,979
new-reverse = reverse;
new-lower = expr1;
new-upper = expr2;
+ new-by = expr_by;
$$ = (PLpgSQL_stmt *) new;
}
diff -rciEb pgsql-8.2dev/src/pl/plpgsql/src/pl_exec.c pgsql-8.2fori/src/pl/plpgsql/src/pl_exec.c
*** pgsql-8.2dev/src/pl/plpgsql/src/pl_exec.c 2006-04-30 09:45:12.0 -0500
--- pgsql-8.2fori/src/pl/plpgsql/src/pl_exec.c 2006-04-30 09:49:54.0 -0500
***
*** 1346,1352
/* --
* exec_stmt_fori Iterate an integer variable
! * from a lower to an upper value.
* Loop can be left with exit.
* --
*/
--- 1346,1353
/* --
* exec_stmt_fori Iterate an integer variable
! * from a lower to an upper value
! * incrementing or decrementing in BY value
* Loop can be left with exit.
* --
*/
***
*** 1355,1360
--- 1356,1362
{
PLpgSQL_var *var;
Datum value;
+ Datum by_value;
Oid valtype;
bool isnull;
bool found = false;
***
*** 1393,1398
--- 1395,1414
exec_eval_cleanup(estate);
/*
+ * Get the by value
+ */
+ by_value = exec_eval_expr(estate, stmt-by, isnull, valtype);
+ by_value = exec_cast_value(by_value, valtype, var-datatype-typoid,
+ (var-datatype-typinput),
+ var-datatype-typioparam,
+ var-datatype-atttypmod, isnull);
+
+ /* If there is no BY, then we assume 1 */
+ if (isnull)
+ by_value = (Datum) 1;
+ exec_eval_cleanup(estate);
+
+ /*
* Now do the loop
*/
for (;;)
***
*** 1468,1476
* Increase/decrease loop var
*/
if (stmt-reverse)
! var-value--;
else
! var-value++;
}
/*
--- 1484,1492
* Increase/decrease loop var
*/
if (stmt-reverse)
! var-value -= by_value;
else
! var-value += by_value;
}
/*
diff -rciEb pgsql-8.2dev/src/pl/plpgsql/src/plpgsql.h pgsql-8.2fori/src/pl/plpgsql/src/plpgsql.h
***
Re: [PATCHES] plpython improvements
Sun, 30 Apr 2006 21:43:03 +0300, Tom Lane [EMAIL PROTECTED]: Sven Suursoho [EMAIL PROTECTED] writes: So, what about this in configure: if --with-python test_iterator_app_crashes # errcode(FEATURE_NOT_SUPPORTED), errmsg(patch your python) disable_iterator_feature fi Testing it in configure is wrong, because there's no guarantee the same python library will be used at runtime. Yes, that's right. Ok, will do Py_DEBUG Py_GetVersion() check at runtime. -- Sven Suursoho ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Patch for %Allow per-database permissions to be set via
Bruce Momjian pgman@candle.pha.pa.us writes: Documentation added, patch attached and applied. Thanks. I just got around to reading this patch. Why is the syntax GRANT CONNECTION and not GRANT CONNECT? Privilege names are generally verbs not nouns. Unless someone can point to a good reason for CONNECTION, I'm going to change it. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Patch for %Allow per-database permissions to be set
On Sun, 2006-04-30 at 15:29 -0400, Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us writes: Documentation added, patch attached and applied. Thanks. I just got around to reading this patch. Why is the syntax GRANT CONNECTION and not GRANT CONNECT? Privilege names are generally verbs not nouns. Unless someone can point to a good reason for CONNECTION, I'm going to change it. The main reason for this was because, in the beginning when I was gathering information for developing this patch, I read something about not introducing a new reserved word. So I used CONNECTION as the first relevant word I could find in the token list from gram.y. Later on we did not discussed anything about the *CONNECT* or *CONNECTION Regards, Gevik. ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Patch for %Allow per-database permissions to be set via
Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us writes: Documentation added, patch attached and applied. Thanks. I just got around to reading this patch. Why is the syntax GRANT CONNECTION and not GRANT CONNECT? Privilege names are generally verbs not nouns. Unless someone can point to a good reason for CONNECTION, I'm going to change it. Sounds good, hit the docs too, thanks. -- Bruce Momjian http://candle.pha.pa.us EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
