Re: [PATCHES] stored procedure stats in collector
On Mar 23, 2008, at 9:25 PM, Volkan YAZICI wrote: Hi, On Sun, 23 Mar 2008, Martin Pihlak [EMAIL PROTECTED] writes: Attached is a patch that enables tracking function calls through the stats subsystem. Original discussion: http://archives.postgresql.org/pgsql-hackers/2007-07/msg00377.php Introduces new guc variable - track_functions. Possible values are: none - no collection, default pl - tracks procedural language functions all - tracks procedural, SQL and C (not internal) functions I might have missed the discussion, but I'd love to see a more flexible interface for configuration parameters. For instance, it'd be great if we can specify which procedural languages to track in the `pl' GUC. Moreover, if it'd be possible to specify which specific functions we want to try, then that would be awesome as well. For instance, possible configuration combinations for track_functions can be: `pl:*'- Tracks procedural, SQL and C (not internal) functions in the `public' schema. `pl:pgsql'- Tracks only PL/pgSQL functions. `pl:scheme' - Tracks only PL/scheme functions. `foo(int, int)' - Tracks related `foo' function in the public schema. `stock.foo(int, int)' - Tracks related `foo' function in the `stock' schema. `pl:stock.*' - Tracks procedural, SQL and C (not internal) functions in the `stock' schema. Syntax can obviously be much more consistent. (These are just what I come up with for the very moment.) And I'm aware of the overhead and complexity(?) this sort of scheme will bring, but I really want to use such a useful feature with mentioned flexibilities. this patch is quite cool already. it would be even cooler if we could define on a per-function basis. eg. CREATE FUNCTION ... TRACK | NOTRACK in addition to that we could define a GUC defining whether TRACK or NOTRACK is used as default. in many cases you are only interested in a special set of functions anyway. as every operator is basically a procedure in postgres, i am not quite happy about the per-language approach. best regards, hans -- Cybertec Schönig Schönig GmbH PostgreSQL Solutions and Support Gröhrmühlgasse 26, 2700 Wiener Neustadt Tel: +43/1/205 10 35 / 340 www.postgresql-support.de, www.postgresql-support.com
Re: [PATCHES] [HACKERS] quote_literal with NULL
On 23/03/2008, Tom Lane [EMAIL PROTECTED] wrote: Applied with some revisions to sync it with CVS HEAD --- primarily, since we now have a quote_literal(anyelement) function, it seemed important to add a quote_nullable(anyelement) variant. I also editorialized on the documentation example a bit. Thanks Tom, good call on the (anyelement) version. I like the changes to the documentation too. I didn't know that the DISTINCT FROM operator was relatively slow. Regards, BJ - 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] [HACKERS] Function structure in formatting.c
On 23/03/2008, Tom Lane [EMAIL PROTECTED] wrote:
Working through this patch now. I found one thing that seems to be
a mistake (probably an overenthusiastic searchreplace): the patch
changes
- {iy, 2, dch_date, DCH_IY, TRUE},
to
+ {iyear, 2, DCH_IY, TRUE},
The removal of dch_date is intended, but surely the keyword should
still be iy. I'm proceeding on that assumption, but if this change
was actually intended, please explain.
Nice catch. Not sure how that got in there, but your theory about a
search replace gone awry seems the most likely.
Now that the functions have been refactored, I'm looking forward to
getting back into improving the sanity checking in to_date.
Cheers,
BJ
-
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] [HACKERS] Show INHERIT in \du
On 21/03/2008, Tom Lane [EMAIL PROTECTED] wrote: Brendan Jurd [EMAIL PROTECTED] writes: We can't just build the output table by hand like describeOneTableDetails does? Admittedly it's kludgy, but it's not an unprecedented kludge. Oh, I had forgotten the existence of that entry point in print.c. Yeah, it might be workable --- want to have a go at it? I've had a chance to look at this now, and although it certainly does seem workable, there's a lot of duplication of code that I feel uneasy about. describeOneTableDetails essentially already duplicates the table buildling code in printQuery, so I would be creating a third copy of the same logic. This makes me wonder whether print.c could offer something a bit more helpful to callers wishing to DIY a table; we could have a table-building struct with methods like addHeader and addCell. What do you think? Overkill, or worthy pursuit? - 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] [HACKERS] Show INHERIT in \du
Brendan Jurd [EMAIL PROTECTED] writes: I've had a chance to look at this now, and although it certainly does seem workable, there's a lot of duplication of code that I feel uneasy about. describeOneTableDetails essentially already duplicates the table buildling code in printQuery, so I would be creating a third copy of the same logic. This makes me wonder whether print.c could offer something a bit more helpful to callers wishing to DIY a table; we could have a table-building struct with methods like addHeader and addCell. What do you think? Overkill, or worthy pursuit? Once you have two occurrences of a pattern, it's reasonable to assume there will be more later. +1 for building a little bit of infrastructure. regards, tom lane - 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] script binaries renaming
Where are we on this? Tom thinks we don't want this. TODO has: * Prefix command-line utilities like createuser with 'pg_' http://archives.postgresql.org/pgsql-hackers/2007-06/msg00025.php I think we need to make a decision. --- Zdenek Kotala wrote: I attach complete patch which renames following binaries createdb createlang createuser dropdb droplang dropuser clusterdb vacuumdb reindexdb to pg_createdb pg_createlang pg_createuser pg_dropdb pg_droplang pg_dropuser pg_clusterdb pg_vacuumdb pg_reindexdb Symlinks (or copy on win32) are created for backward compatibility. This renaming was discussed there: http://archives.postgresql.org/pgsql-hackers/2007-06/msg00145.php I create also separate unified patch for documentation. Zdenek [ application/x-gzip is not supported, skipping... ] ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://postgres.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] generate_subscripts
Hello
This patch contains generate_subscripts functions, that generate
series of array's subscripts of some dimension:
postgres=#
create or replace function unnest2(anyarray)
returns setof anyelement as $$
select $1[i][j]
from generate_subscripts($1,1) g1(i),
generate_subscripts($1,2) g2(j);
$$ language sql immutable;
postgres=# select * from unnest2(array[[1,2],[3,4]]);
unnest2
-
1
2
3
4
(4 rows)
Proposal: http://archives.postgresql.org/pgsql-hackers/2007-10/msg00874.php
Regards
Pavel Stehule
*** ./doc/src/sgml/func.sgml.orig 2008-03-24 18:01:54.0 +0100
--- ./doc/src/sgml/func.sgml 2008-03-24 19:03:43.0 +0100
***
*** 10562,10569
para
This section describes functions that possibly return more than one row.
-Currently the only functions in this class are series generating functions,
-as detailed in xref linkend=functions-srf-series.
/para
table id=functions-srf-series
--- 10562,10567
***
*** 10641,10646
--- 10639,10697
(3 rows)
/programlisting
/para
+
+ table id=functions-srf-subscripts
+titleSubscripts Generating Functions/title
+tgroup cols=3
+ thead
+ row
+ entryFunction/entry
+ entryReturn Type/entry
+ entryDescription/entry
+ /row
+ /thead
+
+ tbody
+ row
+ entryliteralfunctiongenerate_subscripts/function(parameterarray annyarray/parameter, parameterdim int/parameter)/literal/entry
+ entrytypesetof int/type/entry
+ entry
+Generate a series of array's subscripts.
+ /entry
+ /row
+
+ row
+ entryliteralfunctiongenerate_subscripts/function(parameterarray annyarray/parameter, parameterdim int/parameter, parameterreverse boolean/parameter)/literal/entry
+ entrytypesetof int/type/entry
+ entry
+Generate a series of array's subscripts. When parameterreverse/parameter is true, then series is reverse order.
+ /entry
+ /row
+
+ /tbody
+/tgroup
+ /table
+
+ programlisting
+ -- unnest 2D array
+ create or replace function unnest2(anyarray)
+ returns setof anyelement as $$
+ select $1[i][j]
+from generate_subscripts($1,1) g1(i),
+ generate_subscripts($1,2) g2(j);
+ $$ language sql immutable;
+ CREATE FUNCTION
+ postgres=# select * from unnest2(array[[1,2],[3,4]]);
+ unnest2
+ -
+1
+2
+3
+4
+ (4 rows)
+ /programlisting
+
+
/sect1
sect1 id=functions-info
*** ./src/backend/utils/adt/arrayfuncs.c.orig 2008-03-24 17:38:45.0 +0100
--- ./src/backend/utils/adt/arrayfuncs.c 2008-03-24 18:12:35.0 +0100
***
*** 16,21
--- 16,22
#include ctype.h
+ #include funcapi.h
#include access/tupmacs.h
#include libpq/pqformat.h
#include parser/parse_coerce.h
***
*** 96,101
--- 97,108
int typlen, bool typbyval, char typalign);
static int array_cmp(FunctionCallInfo fcinfo);
+ typedef struct generate_subscripts_fctx
+ {
+ int4lower;
+ int4upper;
+ boolreverse;
+ } generate_subscripts_fctx;
/*
* array_in :
***
*** 4237,4239
--- 4244,4315
PG_RETURN_ARRAYTYPE_P(result);
}
+
+ /*
+ * array_subscripts(array anyarray, dim int, reverse bool)
+ * returns all subscripts of array for any dimension
+ */
+ Datum
+ array_subscripts_direction(PG_FUNCTION_ARGS)
+ {
+ return array_subscripts(fcinfo);
+ }
+
+ Datum
+ array_subscripts(PG_FUNCTION_ARGS)
+ {
+ FuncCallContext *funcctx;
+ MemoryContext oldcontext;
+ generate_subscripts_fctx *fctx;
+
+ /* stuff done only on the first call of the function */
+ if (SRF_IS_FIRSTCALL())
+ {
+ ArrayType *v = PG_GETARG_ARRAYTYPE_P(0);
+ int reqdim = PG_GETARG_INT32(1);
+ int *lb, *dimv;
+
+ /* create a function context for cross-call persistence */
+ funcctx = SRF_FIRSTCALL_INIT();
+
+ /* Sanity check: does it look like an array at all? */
+ if (ARR_NDIM(v) = 0 || ARR_NDIM(v) MAXDIM)
+ SRF_RETURN_DONE(funcctx);
+
+ /* Sanity check: was the requested dim valid */
+ if (reqdim = 0 || reqdim ARR_NDIM(v))
+ SRF_RETURN_DONE(funcctx);
+
+ /*
+ * switch to memory context appropriate for multiple function calls
+ */
+ oldcontext = MemoryContextSwitchTo(funcctx-multi_call_memory_ctx);
+ fctx = (generate_subscripts_fctx *) palloc(sizeof(generate_subscripts_fctx));
+
+ lb = ARR_LBOUND(v);
+ dimv = ARR_DIMS(v);
+
+ fctx-lower = lb[reqdim - 1];
+ fctx-upper = dimv[reqdim - 1] + lb[reqdim - 1] - 1;
+
Re: [PATCHES] script binaries renaming
Bruce Momjian [EMAIL PROTECTED] writes: Where are we on this? Tom thinks we don't want this. TODO has: * Prefix command-line utilities like createuser with 'pg_' http://archives.postgresql.org/pgsql-hackers/2007-06/msg00025.php I think we need to make a decision. Well, I don't have any particular objection to adding pg_ prefixes as alternate names for the existing scripts. However, it's not clear what is the point unless we have the intention to remove the old names at some time in the foreseeable future. And the consensus of the previous thread on -patches seemed to be that nobody except Zdenek was very eager to do that. In any case, there is no value in discussing this further on -patches since the readers of this list already weighed in. If you want to make a decision then it needs to be made on -hackers or -general. regards, tom lane - Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] actualised execute using patch
Hello
http://archives.postgresql.org/pgsql-patches/2007-10/msg00161.php
I actualized this patch for current CVS
Regards
Pavel Stehule
*** ./doc/src/sgml/plpgsql.sgml.orig 2008-03-23 01:24:19.0 +0100
--- ./doc/src/sgml/plpgsql.sgml 2008-03-24 20:41:27.0 +0100
***
*** 1005,1011
commandEXECUTE/command statement is provided:
synopsis
! EXECUTE replaceable class=commandcommand-string/replaceable optional INTO optionalSTRICT/optional replaceabletarget/replaceable /optional;
/synopsis
where replaceablecommand-string/replaceable is an expression
--- 1005,1011
commandEXECUTE/command statement is provided:
synopsis
! EXECUTE replaceable class=commandcommand-string/replaceable optional INTO optionalSTRICT/optional replaceabletarget/replaceable /optional optional USING replaceable class=parameterexpression/replaceable optional, .../optional /optional;
/synopsis
where replaceablecommand-string/replaceable is an expression
***
*** 1046,1051
--- 1046,1066
If the literalSTRICT/ option is given, an error is reported
unless the query produces exactly one row.
/para
+
+ para
+ The commandEXECUTE/command statement can take parameters. To refer
+ to the parameters use $1, $2, $3, etc. Any parameter have to be bind to
+ any variable or any expression with USING clause. You cannot use bind
+ arguments to pass the names of schema objects to a dynamic SQL statement.
+ The use of arguments is perfect protection from SQL injection.
+ programlisting
+ EXECUTE 'SELECT count(*) FROM '
+ || tabname::regclass
+ || ' WHERE inserted_by = $1 AND inserted = $2'
+INTO c
+USING checked_user, checked_date;
+ /programlisting
+ /para
para
commandSELECT INTO/command is not currently supported within
***
*** 1997,2003
rows:
synopsis
optional lt;lt;replaceablelabel/replaceablegt;gt; /optional
! FOR replaceabletarget/replaceable IN EXECUTE replaceabletext_expression/replaceable LOOP
replaceablestatements/replaceable
END LOOP optional replaceablelabel/replaceable /optional;
/synopsis
--- 2012,2018
rows:
synopsis
optional lt;lt;replaceablelabel/replaceablegt;gt; /optional
! FOR replaceabletarget/replaceable IN EXECUTE replaceabletext_expression/replaceable optional USING replaceable class=parameterexpression/replaceable optional, .../optional /optional LOOP
replaceablestatements/replaceable
END LOOP optional replaceablelabel/replaceable /optional;
/synopsis
*** ./src/pl/plpgsql/src/gram.y.orig 2008-01-01 20:46:00.0 +0100
--- ./src/pl/plpgsql/src/gram.y 2008-03-24 20:41:27.0 +0100
***
*** 21,26
--- 21,27
static PLpgSQL_expr *read_sql_construct(int until,
int until2,
+ int until3,
const char *expected,
const char *sqlstart,
bool isexpression,
***
*** 200,205
--- 201,207
%token K_THEN
%token K_TO
%token K_TYPE
+ %token K_USING
%token K_WARNING
%token K_WHEN
%token K_WHILE
***
*** 892,899
{
PLpgSQL_stmt_dynfors *new;
PLpgSQL_expr *expr;
! expr = plpgsql_read_expression(K_LOOP, LOOP);
new = palloc0(sizeof(PLpgSQL_stmt_dynfors));
new-cmd_type = PLPGSQL_STMT_DYNFORS;
--- 894,907
{
PLpgSQL_stmt_dynfors *new;
PLpgSQL_expr *expr;
+ int term;
! expr = read_sql_construct(K_LOOP,
! K_USING,
! 0,
! LOOP|USING,
! SELECT ,
! true, true, term);
new = palloc0(sizeof(PLpgSQL_stmt_dynfors));
new-cmd_type = PLPGSQL_STMT_DYNFORS;
***
*** 920,925
--- 928,948
yyerror(loop variable of loop over rows must be a record or row variable or list of scalar variables);
}
new-query = expr;
+
+ if (term == K_USING)
+ {
+ for(;;)
+ {
+ int term;
+
+ expr = read_sql_construct(',', K_LOOP, 0, , or LOOP,
+ SELECT ,
+ true, true, term);
+ new-params = lappend(new-params, expr);
+ if (term == K_LOOP)
+ break;
+ }
+ }
$$ = (PLpgSQL_stmt *) new;
}
***
*** 954,959
--- 977,983
*/
expr1 = read_sql_construct(K_DOTDOT,
K_LOOP,
+ 0,
LOOP,
SELECT ,
true,
***
*** 975,980
--- 999,1005
/* Read and check the second one */
expr2 = read_sql_construct(K_LOOP,
K_BY,
+ 0,
LOOP,
SELECT ,
true,
***
*** 1222,1228
Re: [PATCHES] script binaries renaming
Tom Lane napsal(a): Bruce Momjian [EMAIL PROTECTED] writes: Where are we on this? Tom thinks we don't want this. TODO has: * Prefix command-line utilities like createuser with 'pg_' http://archives.postgresql.org/pgsql-hackers/2007-06/msg00025.php I think we need to make a decision. Well, I don't have any particular objection to adding pg_ prefixes as alternate names for the existing scripts. However, it's not clear what is the point unless we have the intention to remove the old names at some time in the foreseeable future. And the consensus of the previous thread on -patches seemed to be that nobody except Zdenek was very eager to do that. Yeah, I have to had two reason for this patch. First is my personal, because I don't like these names since 1999. And second is that Solaris architects do not like these names. Especially createdb and createuser. It could clash with some system utility. In any case, there is no value in discussing this further on -patches since the readers of this list already weighed in. If you want to make a decision then it needs to be made on -hackers or -general. I think both are important (maybe general is more important). Maybe we can put also a survey on webpage. On other side. The question is also if we really still need these utilities? If you look on them there are missing features. E.g vacuumdb does not allow make VACUUM FREEZ or set some modern version of vacuum parameters. There is not createtablespace command and so on... Zdenek - 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] script binaries renaming
On Mon, Mar 24, 2008 at 09:19:42PM +0100, Zdenek Kotala wrote: Tom Lane napsal(a): Bruce Momjian [EMAIL PROTECTED] writes: Where are we on this? Tom thinks we don't want this. TODO has: * Prefix command-line utilities like createuser with 'pg_' http://archives.postgresql.org/pgsql-hackers/2007-06/msg00025.php I think we need to make a decision. Well, I don't have any particular objection to adding pg_ prefixes as alternate names for the existing scripts. However, it's not clear what is the point unless we have the intention to remove the old names at some time in the foreseeable future. And the consensus of the previous thread on -patches seemed to be that nobody except Zdenek was very eager to do that. Yeah, I have to had two reason for this patch. First is my personal, because I don't like these names since 1999. And second is that Solaris architects do not like these names. Especially createdb and createuser. It could clash with some system utility. +1 for renaming the utilities. Not stomping on the global namespace is one place where MySQL is really out ahead of us. Cheers, David. -- David Fetter [EMAIL PROTECTED] http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: [EMAIL PROTECTED] Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate - 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] script binaries renaming
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Mon, 24 Mar 2008 14:12:19 -0700 David Fetter [EMAIL PROTECTED] wrote: Yeah, I have to had two reason for this patch. First is my personal, because I don't like these names since 1999. And second is that Solaris architects do not like these names. Especially createdb and createuser. It could clash with some system utility. +1 for renaming the utilities. Not stomping on the global namespace is one place where MySQL is really out ahead of us. - -1 I have not yet seen an argument that has compelled me to actually want to have us enter the Gnome world of binary naming. However, if we *must* go down this route let us please use pgcreatedb *not* pg_createdb. Sincerely, Joshua D. Drake - -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFH6BohATb/zqfZUUQRAjm5AJ0QFb1C5/BaAIMjnu/OdqTsCO/1EACfX3XL PNC+b1WIXd1fgJz23e9Gles= =UkoA -END PGP SIGNATURE- - Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
int8/float8/time/timestamp[tz]/float4 passed by value, was Re: [PATCHES] Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
Hi, I got around to (almost) finalize the patch. What it does: - fixes the configure define described in my previous mail - when the value of HAVE_LONG_INT_64 == 1, the following types are passed by value: int8, float8, time, timestamp, timestamptz The time[stamp[tz]] types caused segfaults in the regression test if their attbyval setting was different from int8/float8, so it's really needed. I am not sure there's another type that needs a similat switch, the type regression tests are running fine. - In the HAVE_LONG_INT_64 == 1 case, Int64GetDatum() becomes a casting macro instead of a function, some others become functions. The implementation of DatumGetFloat4()/Float4GetDatum()/etc functions may change into an inline or change internals. - float4 is now unconditionally passed by value - the int8inc(), int2_sum() and int4_sum() used pointers directly from the Datums for performance, that code path is now commented out, the other code path is correct for the AggState and !AggState runs and correct every time and now because of the passbyval nature of int8, the !AggState version is not slower than using the pointer directly. - removed deprecated DatumGetFloat32/Float32GetDatum/etc macros, they aren't used anywhere in the core and contrib source. There is only one regression, in the tsearch tests. Some selects in tsearch now return no records but they don't segfault. I couldn't hunt the bug down yet, not sure I will have time in the near future for that. Comments welcome. Best regards, Zoltán Böszörményi Zoltan Boszormenyi írta: Hi, I am working on this TODO item: * Consider allowing 64-bit integers and floats to be passed by value on 64-bit platforms Also change 32-bit floats (float4) to be passed by value at the same time. For genbki.sh, to correctly determine whether HAVE_LONG_INT_64 is defined, the attached bugfix is needed in the configure.in machinery. This way the pg_config.h actually conforms to the comments for HAVE_LONG_INT_64 and HAVE_LONG_LONG_INT_64. Best regards, Zoltán Böszörményi -- -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH http://www.postgresql.at/ pg84-passedbyval.patch.gz Description: Unix tar archive - 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] script binaries renaming
Which part of this is the wrong list wasn't clear to you guys? regards, tom lane - 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] script binaries renaming
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Mon, 24 Mar 2008 18:30:46 -0400 Tom Lane [EMAIL PROTECTED] wrote: Which part of this is the wrong list wasn't clear to you guys? I actually didn't even notice. Sorry Tom. Joshua D. Drake - -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFH6C1YATb/zqfZUUQRAlenAJwIBnHS0rWIyx2gE/lbeHWEmeGVgACbB/1/ HmcDiHcVbe5zhJcXW8oir1g= =KS1F -END PGP SIGNATURE- - 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] int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
Zoltan Boszormenyi [EMAIL PROTECTED] writes: - the int8inc(), int2_sum() and int4_sum() used pointers directly from the Datums for performance, that code path is now commented out, the other code path is correct for the AggState and !AggState runs and correct every time and now because of the passbyval nature of int8, the !AggState version is not slower than using the pointer directly. Does this mean count() and sum() are slower on a 32-bit machine? -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training! - 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] int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
Hi, Gregory Stark írta: Zoltan Boszormenyi [EMAIL PROTECTED] writes: - the int8inc(), int2_sum() and int4_sum() used pointers directly from the Datums for performance, that code path is now commented out, the other code path is correct for the AggState and !AggState runs and correct every time and now because of the passbyval nature of int8, the !AggState version is not slower than using the pointer directly. Does this mean count() and sum() are slower on a 32-bit machine? If you mean slower than on a 64-bit machine then yes. If you mean slower than before, then no. I didn't express myself correctly. The original code path is not commented out, it is just conditionally compiled. BTW I found the tsearch bug, it was a missing conversion of float4 in gistproc.c, it was an unfortunate detail that this didn't cause a segfault, it woul have been easier to find. Now there are no failing regression tests. -- -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH http://www.postgresql.at/ pg84-passedbyval-v2.patch.gz Description: Unix tar archive -- 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] int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
Ok, ignore my previous message. I've read the patch now and that's not an issue. The old code path is not commented out, it's #ifdef'd conditionally on HAVE_LONG_INT_64 is right (well it seems right, it's a bit hard to tell in patch form). A few comments: 1) Please don't include configure in your patch. I don't know why it's checked into CVS but it is so that means manually removing it from any patch. It's usually a huge portion of the diff so it's worth removing. 2) The genbki.sh change could be a bit tricky for multi-platform builds (ie OSX). I don't really see an alternative so it's just something to note for the folks setting that up (Hi Dave). Actually there is an alternative but I prefer the approach you've taken. The alternative would be to have a special value in the catalogs for 8-bit maybe-pass-by-value data types and handle the check at run-time. Another alternative would be to have initdb fix up these values in C code instead of fixing them directly in the bki scripts. That seems like more hassle than it's worth though and a bigger break with the rest. 3) You could get rid of a bunch of #ifndef HAVE_LONG_INT_64 snippets by having a #define like INT64PASSBYVALUE which is defined to be either true or false. It might start getting confusing having three different defines for the same thing though. But personally I hate having more #ifdefs than necessary, it makes it hard to read the code. 4) Your problems with tsearch and timestamp etc raise an interesting problem. We don't need to mark this in pg_control because it's a purely a run-time issue and doesn't affect on-disk storage. However it does affect ABI compatibility with modules. Perhaps it should be added to PG_MODULE_MAGIC_DATA. Actually, why isn't sizeof(Datum) in there already? Do we have any protection against loading 64-bit compiled modules in a 32-bit server or vice versa? But generally this is something I've been wanting to do for a while and basically the same approach I would have taken. It seems sound to me. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
Gregory Stark írta: Ok, ignore my previous message. I've read the patch now and that's not an issue. The old code path is not commented out, it's #ifdef'd conditionally on HAVE_LONG_INT_64 is right (well it seems right, it's a bit hard to tell in patch form). A few comments: 1) Please don't include configure in your patch. I don't know why it's checked into CVS but it is so that means manually removing it from any patch. It's usually a huge portion of the diff so it's worth removing. Noted. 2) The genbki.sh change could be a bit tricky for multi-platform builds (ie OSX). I don't really see an alternative so it's just something to note for the folks setting that up (Hi Dave). Actually there is an alternative but I prefer the approach you've taken. The alternative would be to have a special value in the catalogs for 8-bit maybe-pass-by-value data types and handle the check at run-time. Another alternative would be to have initdb fix up these values in C code instead of fixing them directly in the bki scripts. That seems like more hassle than it's worth though and a bigger break with the rest. 3) You could get rid of a bunch of #ifndef HAVE_LONG_INT_64 snippets by having a #define like INT64PASSBYVALUE which is defined to be either true or false. It might start getting confusing having three different defines for the same thing though. But personally I hate having more #ifdefs than necessary, it makes it hard to read the code. OK, this would also make the patch smaller. Is pg_config_manual.h good for this setting? Or which header would you suggest? 4) Your problems with tsearch and timestamp etc raise an interesting problem. We don't need to mark this in pg_control because it's a purely a run-time issue and doesn't affect on-disk storage. However it does affect ABI compatibility with modules. Perhaps it should be added to PG_MODULE_MAGIC_DATA. I am looking into it. Actually, why isn't sizeof(Datum) in there already? Do we have any protection against loading 64-bit compiled modules in a 32-bit server or vice versa? You can't mix 32-bit executables with 64-bit shared libraries, well, without tricks. I don't see any problem here. But generally this is something I've been wanting to do for a while and basically the same approach I would have taken. It seems sound to me. Thanks for commenting and encouragement. -- -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH http://www.postgresql.at/ -- 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] Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
Zoltan Boszormenyi írta: Gregory Stark írta: 4) Your problems with tsearch and timestamp etc raise an interesting problem. We don't need to mark this in pg_control because it's a purely a run-time issue and doesn't affect on-disk storage. However it does affect ABI compatibility with modules. Perhaps it should be added to PG_MODULE_MAGIC_DATA. I am looking into it. Do you think it's actually needed? PG_MODULE_MAGIC_DATA contains the server version the external module was compiled for. This patch won't go to older versions, so it's already protected from the unconditional float4 change. And because you can't mix server and libraries with different bitsize, it's protected from the conditional int64, float8, etc. changes. Best regards, Zoltán Böszörményi -- -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH http://www.postgresql.at/ -- 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] int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
Zoltan Boszormenyi írta: BTW I found the tsearch bug, it was a missing conversion of float4 in gistproc.c, it was an unfortunate detail that this didn't cause a segfault, it woul have been easier to find. Now there are no failing regression tests. Disregards this patch, the bugfix for tsearch is not real. -- -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH http://www.postgresql.at/ -- 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] Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
Zoltan Boszormenyi [EMAIL PROTECTED] writes: Zoltan Boszormenyi írta: Gregory Stark írta: 4) Your problems with tsearch and timestamp etc raise an interesting problem. We don't need to mark this in pg_control because it's a purely a run-time issue and doesn't affect on-disk storage. However it does affect ABI compatibility with modules. Perhaps it should be added to PG_MODULE_MAGIC_DATA. I am looking into it. Do you think it's actually needed? PG_MODULE_MAGIC_DATA contains the server version the external module was compiled for. This patch won't go to older versions, so it's already protected from the unconditional float4 change. And because you can't mix server and libraries with different bitsize, it's protected from the conditional int64, float8, etc. changes. Right, it does seem like we're conservative about adding things to PG_MODULE_MAGIC. As long as this is hard coded based on HAVE_LONG_INT_64 then we don't strictly need it. And I don't see much reason to make this something the user can override. If there are modules which use the wrong macros and assume certain other data types are pass-by-reference when they're not then they're going to fail regardless of what version of postgres they're compiled against anyways. So I would say in response to your other query to _not_ use pg_config_manual.h which is intended for variables which the user can override. If you put anything there then we would have to worry about incompatibilities. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's Slony Replication 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 HAVE_LONG[_LONG]_INT_64 to really define to 1
Zoltan Boszormenyi wrote: For genbki.sh, to correctly determine whether HAVE_LONG_INT_64 is defined, the attached bugfix is needed in the configure.in machinery. This way the pg_config.h actually conforms to the comments for HAVE_LONG_INT_64 and HAVE_LONG_LONG_INT_64. I think this part of the patch can be committed right away. -- 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] Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
Gregory Stark írta: Zoltan Boszormenyi [EMAIL PROTECTED] writes: Zoltan Boszormenyi írta: Gregory Stark írta: 4) Your problems with tsearch and timestamp etc raise an interesting problem. We don't need to mark this in pg_control because it's a purely a run-time issue and doesn't affect on-disk storage. However it does affect ABI compatibility with modules. Perhaps it should be added to PG_MODULE_MAGIC_DATA. I am looking into it. Do you think it's actually needed? PG_MODULE_MAGIC_DATA contains the server version the external module was compiled for. This patch won't go to older versions, so it's already protected from the unconditional float4 change. And because you can't mix server and libraries with different bitsize, it's protected from the conditional int64, float8, etc. changes. Right, it does seem like we're conservative about adding things to PG_MODULE_MAGIC. As long as this is hard coded based on HAVE_LONG_INT_64 then we don't strictly need it. And I don't see much reason to make this something the user can override. If there are modules which use the wrong macros and assume certain other data types are pass-by-reference when they're not then they're going to fail regardless of what version of postgres they're compiled against anyways. So I would say in response to your other query to _not_ use pg_config_manual.h which is intended for variables which the user can override. If you put anything there then we would have to worry about incompatibilities OK, third version, the #define INT64PASSBYVAL is used now for less #ifdef'd code, I used postgres.h for the defines for now. As for the tsearch problem, it seems that bth tsearch and gist in general uses the internal type for passing pointers to different datatypes around for modifying them in-place, float4 among them. So, somewhere tsearch or gist uses hardcoded passed-by-ref where it really a float4, not internal. Someone with more knowledge about tsearch could look into this... -- -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH http://www.postgresql.at/ pg84-passedbyval-v3.patch.gz Description: Unix tar archive -- 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] 2WRS [WIP]
We need more testing to show this is a good idea. --- Manolo _ wrote: HI. I send you the diff of my code against the current CVS TIP. Please tell me if it's what you were asking for. Thanks. Regards, Manolo di Domenico Date: Wed, 6 Feb 2008 17:03:16 -0800 From: [EMAIL PROTECTED] To: [EMAIL PROTECTED] Subject: Re: [PATCHES] 2WRS [WIP] Go here and snoop around a bit. http://neilconway.org/talks/hacking _ Express yourself instantly with MSN Messenger! Download today it's FREE! http://messenger.msn.click-url.com/go/onm00200471ave/direct/01/ [ Attachment, skipping... ] ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://postgres.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] [8.4] Updated WITH clause patch (non-recursive)
Hi, From: Neil Conway [EMAIL PROTECTED] Subject: [PATCHES] [8.4] Updated WITH clause patch (non-recursive) Date: Sat, 26 Jan 2008 23:58:40 -0800 Attached is an updated version of Greg Stark's patch to add support for the non-recursive variant of the SQL99 WITH clause[1]. I found a bug with the following SQL. postgres=# WITH x AS (SELECT 1), y AS (SELECT 2) SELECT * FROM x UNION ALL SELECT * FROM y; ERROR: relation x does not exist Attached patch transforms WITH clause in transformSetOperationStmt(). It works correctly with the attached patch. postgres=# WITH x AS (SELECT 1), y AS (SELECT 2) SELECT * FROM x UNION ALL SELECT * FROM y; ?column? -- 1 2 (2 rows) Regards, -- Yoshiyuki Asaba [EMAIL PROTECTED] Index: src/backend/nodes/copyfuncs.c === RCS file: /projects/cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v retrieving revision 1.390 diff -c -r1.390 copyfuncs.c *** src/backend/nodes/copyfuncs.c 21 Mar 2008 22:41:48 - 1.390 --- src/backend/nodes/copyfuncs.c 25 Mar 2008 04:18:06 - *** *** 1939,1944 --- 1939,1945 COPY_NODE_FIELD(limitOffset); COPY_NODE_FIELD(limitCount); COPY_NODE_FIELD(lockingClause); + COPY_NODE_FIELD(with_cte_list); COPY_SCALAR_FIELD(op); COPY_SCALAR_FIELD(all); COPY_NODE_FIELD(larg); Index: src/backend/nodes/equalfuncs.c === RCS file: /projects/cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v retrieving revision 1.320 diff -c -r1.320 equalfuncs.c *** src/backend/nodes/equalfuncs.c 21 Mar 2008 22:41:48 - 1.320 --- src/backend/nodes/equalfuncs.c 25 Mar 2008 04:18:07 - *** *** 821,826 --- 821,827 COMPARE_NODE_FIELD(limitOffset); COMPARE_NODE_FIELD(limitCount); COMPARE_NODE_FIELD(lockingClause); + COMPARE_NODE_FIELD(with_cte_list); COMPARE_SCALAR_FIELD(op); COMPARE_SCALAR_FIELD(all); COMPARE_NODE_FIELD(larg); Index: src/backend/nodes/outfuncs.c === RCS file: /projects/cvsroot/pgsql/src/backend/nodes/outfuncs.c,v retrieving revision 1.324 diff -c -r1.324 outfuncs.c *** src/backend/nodes/outfuncs.c21 Mar 2008 22:41:48 - 1.324 --- src/backend/nodes/outfuncs.c25 Mar 2008 04:18:08 - *** *** 1599,1604 --- 1599,1605 WRITE_NODE_FIELD(limitOffset); WRITE_NODE_FIELD(limitCount); WRITE_NODE_FIELD(lockingClause); + WRITE_NODE_FIELD(with_cte_list); WRITE_ENUM_FIELD(op, SetOperation); WRITE_BOOL_FIELD(all); WRITE_NODE_FIELD(larg); Index: src/backend/parser/analyze.c === RCS file: /projects/cvsroot/pgsql/src/backend/parser/analyze.c,v retrieving revision 1.371 diff -c -r1.371 analyze.c *** src/backend/parser/analyze.c1 Jan 2008 19:45:50 - 1.371 --- src/backend/parser/analyze.c25 Mar 2008 04:18:09 - *** *** 688,693 --- 688,696 /* make FOR UPDATE/FOR SHARE info available to addRangeTableEntry */ pstate-p_locking_clause = stmt-lockingClause; + /* process the WITH clause (pull CTEs into the pstate's ctenamespace) */ + transformWithClause(pstate, stmt-with_cte_list); + /* process the FROM clause */ transformFromClause(pstate, stmt-fromClause); *** *** 1021,1026 --- 1024,1032 (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg(SELECT FOR UPDATE/SHARE is not allowed with UNION/INTERSECT/EXCEPT))); + /* process the WITH clause (pull CTEs into the pstate's ctenamespace) */ + transformWithClause(pstate, stmt-with_cte_list); + /* * Recursively transform the components of the tree. */ Index: src/backend/parser/gram.y === RCS file: /projects/cvsroot/pgsql/src/backend/parser/gram.y,v retrieving revision 2.610 diff -c -r2.610 gram.y *** src/backend/parser/gram.y 21 Mar 2008 22:41:48 - 2.610 --- src/backend/parser/gram.y 25 Mar 2008 04:18:16 - *** *** 103,109 static SelectStmt *findLeftmostSelect(SelectStmt *node); static void insertSelectOptions(SelectStmt *stmt, List *sortClause, List *lockingClause, ! Node *limitOffset, Node *limitCount); static Node *makeSetOp(SetOperation op, bool all, Node *larg, Node *rarg); static Node *doNegate(Node *n, int location); static void doNegateFloat(Value *v); --- 103,110 static SelectStmt *findLeftmostSelect(SelectStmt *node); static
