Re: [PATCHES] variadic function support
2008/6/25 Tom Lane [EMAIL PROTECTED]:
Pavel Stehule [EMAIL PROTECTED] writes:
Tom Lane wrote:
Your point about the syntax is good though. It would be better if
the syntax were like
create function foo (a text, variadic b int[])
or maybe even better
create function foo (a text, variadic b int)
I don't see problem with your syntax. It well block combination OUT
and VARIADIC parameter - my one request, variadic parameter have to be
array.
Well, we should certainly store the parameter type as an array in
proargtypes, because that makes this feature transparent to all the
PLs. However, it doesn't follow that the CREATE FUNCTION syntax
has to specify the array type rather than the element type. I think
the Java precedent might be good reason to go with using the element
type in the function declaration.
regards, tom lane
Hello
this is third variant with variadic argumen as scalar. But I still
strongly prefer second variant with conformance declared variadic
array with used array variable.
Regards
Pavel Stehule
*** ./doc/src/sgml/ref/create_function.sgml.orig 2008-06-24 16:46:47.0 +0200
--- ./doc/src/sgml/ref/create_function.sgml 2008-06-24 16:47:46.0 +0200
***
*** 102,108
listitem
para
The mode of an argument: either literalIN/, literalOUT/,
!or literalINOUT/. If omitted, the default is literalIN/.
/para
/listitem
/varlistentry
--- 102,109
listitem
para
The mode of an argument: either literalIN/, literalOUT/,
!literalINOUT/ or literalVARIADIC/literal. If omitted,
!the default is literalIN/.
/para
/listitem
/varlistentry
*** ./doc/src/sgml/xfunc.sgml.orig 2008-06-24 16:53:58.0 +0200
--- ./doc/src/sgml/xfunc.sgml 2008-06-26 13:34:20.0 +0200
***
*** 578,584
para
Parameters can be marked as literalIN/ (the default),
! literalOUT/, or literalINOUT/. An literalINOUT/
parameter serves as both an input parameter (part of the calling
argument list) and an output parameter (part of the result record type).
/para
--- 578,585
para
Parameters can be marked as literalIN/ (the default),
! literalOUT/, literalINOUT/, or literalVARIADIC/literal.
! An literalINOUT/
parameter serves as both an input parameter (part of the calling
argument list) and an output parameter (part of the result record type).
/para
***
*** 805,810
--- 806,833
/screen
/para
/sect2
+
+sect2
+ titleVariadic acronymSQL/acronym Functions/title
+
+ para
+ acronymSQL/acronym functions can be declared to accept
+ variable number of arguments.
+ screen
+ CREATE FUNCTION mleast(variadic args numeric) RETURNS numeric AS $$
+ SELECT min($1[i])
+FROM generate_subscripts($1,1) g(i);
+ $$ LANGUAGE SQL;
+
+ SELECT mleast(10, -1, 5, 4);
+ mleast
+
+ -1
+ (1 row)
+ /screen
+ /para
+/sect2
+
/sect1
sect1 id=xfunc-overload
*** ./src/backend/catalog/namespace.c.orig 2008-06-24 11:24:34.0 +0200
--- ./src/backend/catalog/namespace.c 2008-06-26 16:41:06.0 +0200
***
*** 606,614
int pronargs = procform-pronargs;
int pathpos = 0;
FuncCandidateList newResult;
/* Ignore if it doesn't match requested argument count */
! if (nargs = 0 pronargs != nargs)
continue;
if (OidIsValid(namespaceId))
--- 606,645
int pronargs = procform-pronargs;
int pathpos = 0;
FuncCandidateList newResult;
+ Oid va_oid = InvalidOid;
+ bool variadic = false;
+ bool isnull;
+ Datum proargmodes;
+
+ /*
+ * Search type of variadic argument,
+ */
+ proargmodes = SysCacheGetAttr(PROCOID, proctup,
+ Anum_pg_proc_proargmodes, isnull);
+ if (!isnull)
+ {
+ ArrayType *ar = DatumGetArrayTypeP(proargmodes);
+ char *argmodes;
+ int j;
+
+ argmodes = ARR_DATA_PTR(ar);
+ for (j = 0; j ARR_DIMS(ar)[0]; j++)
+ if (argmodes[j] == PROARGMODE_VARIADIC)
+ {
+ variadic = true;
+ va_oid = get_variadic_element_type(
+ procform-proargtypes.values[j]);
+ Assert(OidIsValid(va_oid));
+ break;
+ }
+ }
/* Ignore if it doesn't match requested argument count */
! if (nargs = 0 pronargs != nargs !variadic)
! continue;
!
! /* Ignore variadic function with less arguments */
! if (nargs = 0 pronargs nargs variadic)
continue;
if (OidIsValid(namespaceId))
***
*** 691,706
/*
* Okay to add it to result list
*/
! newResult = (FuncCandidateList)
! palloc(sizeof(struct _FuncCandidateList) - sizeof(Oid)
! + pronargs * sizeof(Oid));
newResult-pathpos = pathpos;
newResult-oid = HeapTupleGetOid(proctup);
- newResult-nargs = pronargs;
-
Re: [PATCHES] Fix pg_ctl restart bug
Bruce Momjian wrote: Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: However, as of 2004-10-15, this has not worked. :-( The attached patch is the one that caused the bug --- on non-Unix systems, SYSTEMQUOTE is , meaning zero-length string. I should have seen the bug when I applied the contributed patch in 2004. So, shouldn't this fix be back-patched? Well, no one has actually complained about the breakage, and it has been a few years. Also there is always the risk of a new bug being introduced, so I am unsure. Why do we need someone to complain? We know the bug is there. Has the code changed a lot in that area? -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Fix pg_ctl restart bug
Alvaro Herrera wrote: Bruce Momjian wrote: Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: However, as of 2004-10-15, this has not worked. :-( The attached patch is the one that caused the bug --- on non-Unix systems, SYSTEMQUOTE is , meaning zero-length string. I should have seen the bug when I applied the contributed patch in 2004. So, shouldn't this fix be back-patched? Well, no one has actually complained about the breakage, and it has been a few years. Also there is always the risk of a new bug being introduced, so I am unsure. Why do we need someone to complain? We know the bug is there. Has the code changed a lot in that area? Do we have the policy of backpatching every fix? I thought it was only the major bugs we fixed in back branches. If someone wants to backpatch it, feel free to do so. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Fix pg_ctl restart bug
Bruce Momjian wrote:
, meaning zero-length string. I should have seen the bug when I
applied the contributed patch in 2004.
So, shouldn't this fix be back-patched?
Well, no one has actually complained about the breakage, and it has been
a few years. Also there is always the risk of a new bug being
introduced, so I am unsure.
Why do we need someone to complain? We know the bug is there. Has the
code changed a lot in that area?
Do we have the policy of backpatching every fix? I thought it was only
the major bugs we fixed in back branches. If someone wants to backpatch
it, feel free to do so.
OK, I started looking at what it would take to backpatch this and found
another bug I have fixed in CVS HEAD. What back branchs (8.0-8.3.X) are
doing is pretty odd. On non-Win32 systems, it is looking for the null
byte, then putting a null byte before it, and passing a NULL back as the
options and binary location. The test:
if (postgres_path != NULL)
postgres_path = optline;
is backwards, which means that if in 8.3.X you start the server with any
arguments, like:
/usr/var/local/postgres/bin/postgres -i -o -d5
and you use pg_ctl to specify the binary location:
pg_ctl -p /u/pg/bin/postmaster restart
the server actually fails to restart because it chops off the last byte
(a bug) and the test above is wrong (another bug), and it thinks the
binary name is the full string, in quotes:
/usr/var/local/postgres/bin/postgres -i -o -d
and you get this error from pg_ctl:
sh: /usr/var/local/postgres/bin/postgres -i -o -d: not found
This is more than just ignoring the documentation, it is a failure.
I am attaching a minimal patch that will fix the bug in back branches.
Keep in mind that a patched pg_ctl will not be able to restart a backend
that was not patched.
--
Bruce Momjian [EMAIL PROTECTED]http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/backend/postmaster/postmaster.c
===
RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.551
diff -c -c -r1.551 postmaster.c
*** src/backend/postmaster/postmaster.c 11 Jan 2008 00:54:09 - 1.551
--- src/backend/postmaster/postmaster.c 26 Jun 2008 18:53:42 -
***
*** 4163,4169
fprintf(fp, %s, fullprogname);
for (i = 1; i argc; i++)
! fprintf(fp, %s%s%s, SYSTEMQUOTE, argv[i], SYSTEMQUOTE);
fputs(\n, fp);
if (fclose(fp))
--- 4163,4169
fprintf(fp, %s, fullprogname);
for (i = 1; i argc; i++)
! fprintf(fp, \%s\, argv[i]);
fputs(\n, fp);
if (fclose(fp))
Index: src/bin/pg_ctl/pg_ctl.c
===
RCS file: /cvsroot/pgsql/src/bin/pg_ctl/pg_ctl.c,v
retrieving revision 1.92.2.3
diff -c -c -r1.92.2.3 pg_ctl.c
*** src/bin/pg_ctl/pg_ctl.c 29 Feb 2008 23:31:42 - 1.92.2.3
--- src/bin/pg_ctl/pg_ctl.c 26 Jun 2008 18:53:43 -
***
*** 613,627
{
char *arg1;
! arg1 = strchr(optline, *SYSTEMQUOTE);
! if (arg1 == NULL || arg1 == optline)
! post_opts = ;
! else
{
! *(arg1 - 1) = '\0'; /* this should be a
space */
! post_opts = arg1;
}
! if (postgres_path != NULL)
postgres_path = optline;
}
else
--- 613,628
{
char *arg1;
! /*
!* Are we at the first option, as defined by
space and
!* double-quote?
!*/
! if ((arg1 = strstr(optline, \)) != NULL)
{
! *arg1 = '\0'; /* terminate so we get
only program name */
! post_opts = arg1 + 1; /* point past
whitespace */
}
! if (postgres_path == NULL)
postgres_path = optline;
}
else
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Fix pg_ctl restart bug
Bruce Momjian wrote:
I am attaching a minimal patch that will fix the bug in back branches.
Keep in mind that a patched pg_ctl will not be able to restart a backend
that was not patched.
I think this patch will work for unpatched backends as well. I am still
uncertain if it should be backpatched.
--
Bruce Momjian [EMAIL PROTECTED]http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/backend/postmaster/postmaster.c
===
RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.551
diff -c -c -r1.551 postmaster.c
*** src/backend/postmaster/postmaster.c 11 Jan 2008 00:54:09 - 1.551
--- src/backend/postmaster/postmaster.c 26 Jun 2008 19:11:37 -
***
*** 4163,4169
fprintf(fp, %s, fullprogname);
for (i = 1; i argc; i++)
! fprintf(fp, %s%s%s, SYSTEMQUOTE, argv[i], SYSTEMQUOTE);
fputs(\n, fp);
if (fclose(fp))
--- 4163,4169
fprintf(fp, %s, fullprogname);
for (i = 1; i argc; i++)
! fprintf(fp, \%s\, argv[i]);
fputs(\n, fp);
if (fclose(fp))
Index: src/bin/pg_ctl/pg_ctl.c
===
RCS file: /cvsroot/pgsql/src/bin/pg_ctl/pg_ctl.c,v
retrieving revision 1.92.2.3
diff -c -c -r1.92.2.3 pg_ctl.c
*** src/bin/pg_ctl/pg_ctl.c 29 Feb 2008 23:31:42 - 1.92.2.3
--- src/bin/pg_ctl/pg_ctl.c 26 Jun 2008 19:11:37 -
***
*** 613,627
{
char *arg1;
! arg1 = strchr(optline, *SYSTEMQUOTE);
! if (arg1 == NULL || arg1 == optline)
! post_opts = ;
! else
{
! *(arg1 - 1) = '\0'; /* this should be a space */
! post_opts = arg1;
}
! if (postgres_path != NULL)
postgres_path = optline;
}
else
--- 613,629
{
char *arg1;
! /*
! * Are we at the first option, as defined by space and
! * double-quote?
! */
! if ((arg1 = strstr(optline, \)) != NULL ||
! (arg1 = strstr(optline, -)) != NULL)
{
! *arg1 = '\0'; /* terminate so we get only program name */
! post_opts = arg1 + 1; /* point past whitespace */
}
! if (postgres_path == NULL)
postgres_path = optline;
}
else
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Fix pg_ctl restart bug
Bruce Momjian wrote: Alvaro Herrera wrote: Why do we need someone to complain? We know the bug is there. Has the code changed a lot in that area? Do we have the policy of backpatching every fix? I thought it was only the major bugs we fixed in back branches. If someone wants to backpatch it, feel free to do so. I think the policy is we fix the bugs in supported releases. If you start making exceptions, it becomes needlessly complex. I've always assumed that I'm supposed to backpatch the bugs I fix in HEAD, however far is reasonable. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Fix pg_ctl restart bug
Alvaro Herrera wrote: Bruce Momjian wrote: Alvaro Herrera wrote: Why do we need someone to complain? We know the bug is there. Has the code changed a lot in that area? Do we have the policy of backpatching every fix? I thought it was only the major bugs we fixed in back branches. If someone wants to backpatch it, feel free to do so. I think the policy is we fix the bugs in supported releases. If you start making exceptions, it becomes needlessly complex. I've always assumed that I'm supposed to backpatch the bugs I fix in HEAD, however far is reasonable. I thought we only backatched major bugs to prevent possible instability when fixing minor bugs. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [Fwd: Re: [HACKERS] pg_dump additional options for performance]
Added to July patch queue. Thanks. --- Simon Riggs wrote: Re-sending post as discussed with Bruce... On Sun, 2008-03-23 at 12:45 -0300, Alvaro Herrera wrote: Bruce Momjian wrote: Added to TODO: o Allow pre/data/post files when dumping a single object, for performance reasons http://archives.postgresql.org/pgsql-hackers/2008-02/msg00205.php When dumping a single object?? Do you mean database? It would be for whatever set of objects are specified through the use of databases, table include/exclude switches. I've written a patch that implements these new switches on the commands as shown pg_dump --schema-pre-load pg_dump --schema-post-load pg_restore --schema-pre-load pg_restore --schema-post-load I have not implemented --schema-pre-file=xxx style because they don't make any sense when using pg_restore in direct database connection mode. On reflection I don't see any particular need to produce multiple files as output, which just complicates an already horrendous user interface. This is a minimal set of changes and includes nothing at all about directories, parallelisation in the code etc.. This has the following use cases amongst others... * dump everything to a file, then use pg_restore first --schema-pre-load and then --data-only directly into the database, then pg_restore --schema-post-load to a file so we can edit that file into multiple pieces to allow index creation in parallel * dump of database into multiple files by manually specifying which tables go where, then reload in parallel using multiple psql sessions The patch tests OK after some testing, though without a test suite that probably isn't more than a few percentage points of all the possible code paths. There are no docs for it, as yet. --- Further thinking on this Some further refinement might replace --data-only and --schema-only with --want-schema-pre --want-data --want-schema-post --want-schema (same as --want-schema-pre --want-schema-post) These could be used together e.g. --want-schema-pre --want-data whereas the existing --data-only type switches cannot. Which would be a straightforward and useful change to the enclosed patch. That way of doing things is hierarchically extensible to include further subdivisions of the set of SQL commands produced, e.g. divide --want-post-schema into objects required to support various inter-table dependencies and those that don't such as additional indexes. I don't personally think we need that though. Comments? -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com [ Attachment, skipping... ] -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] Removal of the patches email list
We have come to agreement that there is no longer a need for a separate 'patches' email list --- the size of patches isn't a significant issue anymore, and tracking threads between the patches and hackers lists is confusing. I propose we close the patches list and tell everyone to start using only the hackers list. This will require email server changes and web site updates, and some people who are only subscribed to patches have to figure out if they want to subscribe to hackers. I have CC'ed hackers, patches, and www because this does affect all those lists. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Fix pg_ctl restart bug
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Alvaro Herrera wrote: I've always assumed that I'm supposed to backpatch the bugs I fix in HEAD, however far is reasonable. I thought we only backatched major bugs to prevent possible instability when fixing minor bugs. Actually, Bruce, this *is* a minor bug; if it were major we'd have heard about it from the field. My take on it is that pg_ctl restart must be practically unused. Given that we now know it's completely broken, the only way that patching it could make the situation worse would be if the patch affected some other code path that people actually do use. As long as you're sure it doesn't do that, I see no downside to an attempted fix, even if it fails. OK, done; backpatched from 8.0.X to 8.3.X. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
