Re: [HACKERS] [PATCHES] GIN improvements
On Tue, 2008-07-08 at 14:51 -0400, Tom Lane wrote: I'd still like to take a look. I was tasked with reviewing this for the current commit fest, although so far I've just been working on grokking the rest of the GIN code. But if you'd like to review it instead, that's fine with me. -Neil -- 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] A GUC variable to replace PGBE_ACTIVITY_SIZE
On Mon, 2008-06-23 at 03:52 +1000, Thomas Lee wrote: * Should it be possible to set this new variable via a command-line option ala shared_buffers? I would say not: most parameters cannot be set by special command-line parameters, and this one is not important enough to warrant special treatment. Instead, --track_activity_query_size=x can be used as-is. * I'm not exactly sure about the best way to add unit/regr tests for this change. AFAICS there isn't an easy way. * I'm also not sure what's required in the way of updates to the documentation. postgresql.conf.sample and doc/src/sgml/config.sgml should be updated at a minimum. -Neil -- 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] small typo in DTrace docs
On Fri, 2008-06-13 at 01:49 -0300, Euler Taveira de Oliveira wrote: Attached is a small patch to fix some typos in probes.d path. Applied, thanks. -Neil -- 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] SQL: table function support
On Tue, 2008-06-10 at 06:42 +0200, Pavel Stehule wrote: internally is table functions implemenation identical with SRF. It's not the internals that I'm concerned about. Semantically is far - user's doesn't specify return type (what is from PostgreSQL), but specifies return table, what is more natural. What more - for users is transparent chaotic joice betwen SETOF RECORD for multicolumns sets and SETOF type. Well, I'd just like to see some thought about how this *entire* feature ought to work, rather than just adding new knobs and syntax variants incrementally and seemingly at random. Just because it happens to be in the standard isn't really a compelling reason to make an overly-complex part of the system even more complicated, IMHO... -Neil -- 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] Minor pedantry for help text
On Mon, 2008-06-09 at 00:25 -0700, Neil Conway wrote: Attached is a patch that makes some minor changes to the text emitted by the new help command. Applied to HEAD. -Neil -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] Minor pedantry for help text
Attached is a patch that makes some minor changes to the text emitted by
the new help command. Previous output:
postgres=# help
You are using psql, the command-line interface to PostgreSQL.
\? for psql help
\h or \help for SQL help
\g or ; to execute a query
\q to quit psql
\copyright to view the copyright
postgres=#
New output:
postgres=# help
You are using psql, the command-line interface to PostgreSQL.
Type: \copyright for distribution terms
\h for help with SQL commands
\? for help with psql commands
\g or terminate with semicolon to execute query
\q to quit
postgres=#
The newlines in the previous text were inconsistent with psql error
message style elsewhere, aside from being distracting. The advice on
commands to enter next was also just emitted, without actually telling
the user that these are possible inputs. Essentially the text was a
regression from the text we've always used in the startup banner, so I
just re-instituted the old text.
-Neil, doing his best to suppress his aesthetic objection to having
help be valid psql input in the first place
*** a/src/bin/psql/mainloop.c
--- b/src/bin/psql/mainloop.c
***
*** 177,188 MainLoop(FILE *source)
(line[4] == '\0' || line[4] == ';' || isspace((unsigned char) line[4])))
{
free(line);
! puts(_(\nYou are using psql, the command-line interface to PostgreSQL.));
! puts(_(\t\\? for psql help));
! puts(_(\t\\h or \\help for SQL help\n));
! puts(_(\t\\g or \;\ to execute a query));
! puts(_(\t\\q to quit psql\n));
! puts(_(\t\\copyright to view the copyright\n));
fflush(stdout);
continue;
--- 177,188
(line[4] == '\0' || line[4] == ';' || isspace((unsigned char) line[4])))
{
free(line);
! puts(_(You are using psql, the command-line interface to PostgreSQL.));
! printf(_(Type: \\copyright for distribution terms\n
! \\h for help with SQL commands\n
! \\? for help with psql commands\n
! \\g or terminate with semicolon to execute query\n
! \\q to quit\n));
fflush(stdout);
continue;
--
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] SQL: table function support
On Tue, 2008-06-03 at 13:03 +0200, Pavel Stehule wrote: this patch add support of table functions syntax like ANSI SQL 2003. I'm not necessarily opposed to this, but I wonder if we really need *more* syntax variants for declaring set-returning functions. The existing patchwork of features is confusing enough as it is... -Neil -- 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] TRUNCATE TABLE with IDENTITY
On Fri, 2008-05-16 at 19:41 -0400, Tom Lane wrote: Applied with corrections. Most notably, since ALTER SEQUENCE RESTART is nontransactional like most other ALTER SEQUENCE operations, I rearranged things to try to ensure that foreseeable failures like deadlock and lack of permissions would be detected before TRUNCATE starts to issue any RESTART commands. Ugh. The fact that the RESTART IDENTITY part of TRUNCATE is non-transactional is a pretty unsightly wort. I would also quarrel with your addition to the docs that suggests this is only an issue in practice if TRUNCATE RESTART IDENTITY is used in a transaction block: unpredictable failures (such as OOM or query cancellation) can certainly occur in practice, and would be very disruptive (e.g. if the sequence values are stored into a column with a UNIQUE constraint, it would break all inserting transactions until the DBA intervenes). I wonder if it would be possible to make the sequence operations performed by TRUNCATE transactional: while the TRUNCATE remains uncommitted, it should be okay to block concurrent access to the sequence. -Neil -- 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] Integer datetime by default
On Tue, 2008-03-25 at 12:54 -0700, Neil Conway wrote: Barring any objections, I'll apply this to HEAD tomorrow. Applied to HEAD. -Neil -- 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] tuplestore_putvalues()
On Thu, 2008-02-28 at 16:37 -0800, Neil Conway wrote: Attached is a patch that allows an array of Datums + nulls to be inserted into a tuplestore without first creating a HeapTuple, per recent suggestion on -hackers. This avoids making an unnecessary copy. Applied to HEAD. -Neil -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] Integer datetime by default
Attached is a refreshed patch that makes integer datetimes the default.
Platforms that don't have a working 64-bit integer type will fail to
configure by default; they can specify --disable-integer-datetimes to
switch back to using floating-point based datetimes.
Barring any objections, I'll apply this to HEAD tomorrow.
-Neil
Index: configure
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/configure,v
retrieving revision 1.587
diff -p -c -r1.587 configure
*** configure 10 Mar 2008 21:50:16 - 1.587
--- configure 25 Mar 2008 19:47:18 -
*** if test -n $ac_init_help; then
*** 1349,1355
Optional Features:
--disable-FEATURE do not include FEATURE (same as --enable-FEATURE=no)
--enable-FEATURE[=ARG] include FEATURE [ARG=yes]
! --enable-integer-datetimes enable 64-bit integer date/time support
--enable-nls[=LANGUAGES] enable Native Language Support
--disable-shareddo not build shared libraries
--disable-rpath do not embed shared library search path in executables
--- 1349,1355
Optional Features:
--disable-FEATURE do not include FEATURE (same as --enable-FEATURE=no)
--enable-FEATURE[=ARG] include FEATURE [ARG=yes]
! --disable-integer-datetimes disable 64-bit integer date/time support
--enable-nls[=LANGUAGES] enable Native Language Support
--disable-shareddo not build shared libraries
--disable-rpath do not embed shared library search path in executables
*** fi
*** 2176,2182
#
! # 64-bit integer date/time storage (--enable-integer-datetimes)
#
{ echo $as_me:$LINENO: checking whether to build with 64-bit integer date/time support 5
echo $ECHO_N checking whether to build with 64-bit integer date/time support... $ECHO_C 6; }
--- 2176,2182
#
! # 64-bit integer date/time storage: enabled by default.
#
{ echo $as_me:$LINENO: checking whether to build with 64-bit integer date/time support 5
echo $ECHO_N checking whether to build with 64-bit integer date/time support... $ECHO_C 6; }
*** echo $as_me: error: no argument expecte
*** 2205,2211
esac
else
! enable_integer_datetimes=no
fi
--- 2205,2215
esac
else
! enable_integer_datetimes=yes
!
! cat confdefs.h \_ACEOF
! #define USE_INTEGER_DATETIMES 1
! _ACEOF
fi
*** fi
*** 23293,23298
--- 23297,23322
+ # If the user did not disable integer datetimes, check that
+ # there is a working 64-bit integral type to use.
+ if test x$USE_INTEGER_DATETIMES = xyes
+test x$HAVE_LONG_INT_64 = xno
+test x$HAVE_LONG_LONG_INT_64 = xno
+test x$HAVE_INT64 = xno ; then
+ { { echo $as_me:$LINENO: error:
+ Integer-based datetime support requires a 64-bit integer type,
+ but no such type could be found. The --disable-integer-datetimes
+ configure option can be used to disable integer-based storage
+ of datetime values. 5
+ echo $as_me: error:
+ Integer-based datetime support requires a 64-bit integer type,
+ but no such type could be found. The --disable-integer-datetimes
+ configure option can be used to disable integer-based storage
+ of datetime values. 2;}
+{ (exit 1); exit 1; }; }
+ fi
+
+
if test $PORTNAME != win32
then
{ echo $as_me:$LINENO: checking for POSIX signal interface 5
Index: configure.in
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/configure.in,v
retrieving revision 1.554
diff -p -c -r1.554 configure.in
*** configure.in 10 Mar 2008 21:50:16 - 1.554
--- configure.in 25 Mar 2008 19:44:55 -
*** PGAC_ARG_REQ(with, libs, [ --with-
*** 128,137
#
! # 64-bit integer date/time storage (--enable-integer-datetimes)
#
AC_MSG_CHECKING([whether to build with 64-bit integer date/time support])
! PGAC_ARG_BOOL(enable, integer-datetimes, no, [ --enable-integer-datetimes enable 64-bit integer date/time support],
[AC_DEFINE([USE_INTEGER_DATETIMES], 1,
[Define to 1 if you want 64-bit integer timestamp and interval support. (--enable-integer-datetimes)])])
AC_MSG_RESULT([$enable_integer_datetimes])
--- 128,137
#
! # 64-bit integer date/time storage: enabled by default.
#
AC_MSG_CHECKING([whether to build with 64-bit integer date/time support])
! PGAC_ARG_BOOL(enable, integer-datetimes, yes, [ --disable-integer-datetimes disable 64-bit integer date/time support],
[AC_DEFINE([USE_INTEGER_DATETIMES], 1,
[Define to 1 if you want 64-bit integer timestamp and interval support. (--enable-integer-datetimes)])])
AC_MSG_RESULT([$enable_integer_datetimes])
*** AC_CHECK_TYPES([int8, uint8, int64, uint
*** 1405,1410
--- 1405,1424
AC_CHECK_TYPES(sig_atomic_t, [], [], [#include signal.h])
+ # If the user did
Re: [PATCHES] tuplestore_putvalues()
On Sat, 2008-03-22 at 21:35 -0400, Tom Lane wrote: After a quick read, looks sane except for one stylistic gripe: in exec_stmt_return_next, you added an initialization of tuple = NULL in order to remove a couple of lines like tuple = NULL; /* keep compiler quiet */ I think this is not good coding style. The point of not having the initialization is so that the compiler will warn us if there are any paths through the function in which we fail to set tuple. You've just disabled that bit of early-warning error detection. It's better if each switch arm has to set tuple for itself. (If only a minority of them needed to do it, I might think differently, but in this case I'd vote for sticking with the way it's being done.) I agree with your general reasoning, but in this case, it seemed cleaner to initialize tuple when it is declared: only 2 of the 6 branches in the function actually initialize tuple to a non-NULL value. I think the warn-about-uninitialized-value argument doesn't hold much water in this particular case, either: each branch of the function either makes use of tuple, or does not. When tuple is utilized, we want to push it into the tuplestore; otherwise, we want to do nothing. Since tuple is only used in two cases, it seems cleaner to me to have the default be do nothing, and only deviate from it where needed. -Neil -- 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] Moving snapshot code around
On Tue, 2008-03-18 at 16:19 -0300, Alvaro Herrera wrote: The other approach, of course, is to just keep all the code in tqual.c and not create a separate module at all. Opinions? I prefer to keep them separate, but I'm not wedded to it if there's any strong reason not to do it. Also, the line is currently blurred because some users of snapshots mess with the internals directly (setting snapshot-curcid), but we could clean that up and make it so that those become external interface users too. Sounds like a good idea to me -- +1 on keeping the code in two separate files, and moving snapshot users toward using the external interface. Given that there's no functional change here, I don't see anything to stop this patch being applied sooner rather than later... -Neil -- 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] Doc patch for DTrace
On Tue, 2008-03-25 at 15:45 -0500, Robert Lor wrote: Attached is the doc patch for the recent DTrace changes Applied, thanks. -Neil -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] tuplestore_putvalues()
Attached is a patch that allows an array of Datums + nulls to be
inserted into a tuplestore without first creating a HeapTuple, per
recent suggestion on -hackers. This avoids making an unnecessary copy.
There isn't a really analogous optimization to be applied to tuplesort:
it takes either a TTS, an IndexTuple or a basic Datum, none of which
involve an extra copy.
BTW, I notice that almost all of the callers of the various
tuplestore_put methods switch into the tuplestore's context first. We
could simplify their lives a bit by having the tuplestore remember the
context in which it is allocated and do the switch itself. Perhaps it's
not worth bothering with at this point, though.
-Neil
Index: src/backend/commands/prepare.c
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/commands/prepare.c,v
retrieving revision 1.80
diff -p -c -r1.80 prepare.c
*** src/backend/commands/prepare.c 1 Jan 2008 19:45:49 - 1.80
--- src/backend/commands/prepare.c 27 Feb 2008 21:55:21 -
*** pg_prepared_statement(PG_FUNCTION_ARGS)
*** 764,770
hash_seq_init(hash_seq, prepared_queries);
while ((prep_stmt = hash_seq_search(hash_seq)) != NULL)
{
- HeapTuple tuple;
Datum values[5];
bool nulls[5];
--- 764,769
*** pg_prepared_statement(PG_FUNCTION_ARGS)
*** 787,797
prep_stmt-plansource-num_params);
values[4] = BoolGetDatum(prep_stmt-from_sql);
- tuple = heap_form_tuple(tupdesc, values, nulls);
-
/* switch to appropriate context while storing the tuple */
MemoryContextSwitchTo(per_query_ctx);
! tuplestore_puttuple(tupstore, tuple);
}
}
--- 786,794
prep_stmt-plansource-num_params);
values[4] = BoolGetDatum(prep_stmt-from_sql);
/* switch to appropriate context while storing the tuple */
MemoryContextSwitchTo(per_query_ctx);
! tuplestore_putvalues(tupstore, tupdesc, values, nulls);
}
}
Index: src/backend/executor/execQual.c
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/executor/execQual.c,v
retrieving revision 1.226
diff -p -c -r1.226 execQual.c
*** src/backend/executor/execQual.c 1 Jan 2008 19:45:49 - 1.226
--- src/backend/executor/execQual.c 27 Feb 2008 22:13:10 -
*** ExecMakeTableFunctionResult(ExprState *f
*** 1547,1553
for (;;)
{
Datum result;
- HeapTuple tuple;
CHECK_FOR_INTERRUPTS();
--- 1547,1552
*** ExecMakeTableFunctionResult(ExprState *f
*** 1649,1663
*/
tmptup.t_len = HeapTupleHeaderGetDatumLength(td);
tmptup.t_data = td;
! tuple = tmptup;
}
else
{
! tuple = heap_form_tuple(tupdesc, result, fcinfo.isnull);
}
-
- oldcontext = MemoryContextSwitchTo(econtext-ecxt_per_query_memory);
- tuplestore_puttuple(tupstore, tuple);
MemoryContextSwitchTo(oldcontext);
/*
--- 1648,1662
*/
tmptup.t_len = HeapTupleHeaderGetDatumLength(td);
tmptup.t_data = td;
!
! oldcontext = MemoryContextSwitchTo(econtext-ecxt_per_query_memory);
! tuplestore_puttuple(tupstore, tmptup);
}
else
{
! oldcontext = MemoryContextSwitchTo(econtext-ecxt_per_query_memory);
! tuplestore_putvalues(tupstore, tupdesc, result, fcinfo.isnull);
}
MemoryContextSwitchTo(oldcontext);
/*
*** no_function_result:
*** 1702,1716
int natts = expectedDesc-natts;
Datum *nulldatums;
bool *nullflags;
- HeapTuple tuple;
MemoryContextSwitchTo(econtext-ecxt_per_tuple_memory);
nulldatums = (Datum *) palloc0(natts * sizeof(Datum));
nullflags = (bool *) palloc(natts * sizeof(bool));
memset(nullflags, true, natts * sizeof(bool));
- tuple = heap_form_tuple(expectedDesc, nulldatums, nullflags);
MemoryContextSwitchTo(econtext-ecxt_per_query_memory);
! tuplestore_puttuple(tupstore, tuple);
}
}
--- 1701,1713
int natts = expectedDesc-natts;
Datum *nulldatums;
bool *nullflags;
MemoryContextSwitchTo(econtext-ecxt_per_tuple_memory);
nulldatums = (Datum *) palloc0(natts * sizeof(Datum));
nullflags = (bool *) palloc(natts * sizeof(bool));
memset(nullflags, true, natts * sizeof(bool));
MemoryContextSwitchTo(econtext-ecxt_per_query_memory);
! tuplestore_putvalues(tupstore, expectedDesc, nulldatums, nullflags);
}
}
Index: src/backend/utils/mmgr/portalmem.c
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/utils/mmgr/portalmem.c,v
retrieving revision 1.106
diff -p -c -r1.106 portalmem.c
*** src/backend/utils/mmgr/portalmem.c 1 Jan 2008 19:45:55 - 1.106
--- src/backend/utils/mmgr/portalmem.c 27 Feb 2008 22:03:33 -
***
Re: [PATCHES] SRF memory leaks
On Tue, 2008-02-26 at 12:09 -0800, Neil Conway wrote: I'd like to apply this change to back branches reasonably soon, so if you have a better way to do the FreeTupleDesc() hack, let me know. Barring any objections, I'll apply this to HEAD and back branches tonight or tomorrow. -Neil ---(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
Re: [PATCHES] SRF memory leaks
On Wed, 2008-02-27 at 15:07 -0500, Tom Lane wrote: Negative refcount does not prove that the SRF itself hasn't still got a pointer to the tupdesc. That sounds quite bizarre. The SRF has already finished execution at this point, so keeping a pointer to the tupledesc around would only make sense if you wanted to use that tupledesc on a *subsequent* invocation of the SRF. The SRF would need to store the pointer in a static variable, too, and it wouldn't have an easy way to distinguish between repeated calls to the SRF within the same query and in different queries (since in the latter case the TupleDesc will be long gone anyway). I can't see why anyone would want to do this. Can't we fix it so that the tupdesc is allocated in the new special context (at least in the primary code paths), and then no explicit free is needed? As I said earlier, the tupdesc is explicitly allocated in the per-query context by the SRFs themselves. If by primary code paths, you mean SRFs in the main source tree, then sure, we can make arbitrary changes to those. That won't help out-of-tree SRFs though. -Neil ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] SRF memory leaks
On Mon, 2008-02-25 at 21:00 -0500, Tom Lane wrote: I find this part of the patch to be a seriously bad idea. nodeFunctionscan has no right to assume that the function has returned an expendable tupdesc; indeed, I would think that the other case is more nearly what's expected by the API for SRFs. AFAICS this is not true of any of the SRFs in the backend, which always return expendable tupdescs. A safer fix would be to try to make the tupdesc be in the new multi_call_ctx when it's being created by the funcapi.c code. funcapi.c doesn't create the tupdesc, though: it is created by user code, and merely assigned to a field of the RSI (note that the TupleDesc in question is the one in the ReturnSetInfo, not in FuncCallContext.) A minor detail is that the lifetime of the tupdesc also needs to exceed the lifetime of the multi_call_ctx, at least as currently implemented. From looking at the existing SRFs in the backend, the TupleDesc is always *explicitly* allocated in the estate's per-query context, so it will leak unless freed. Perhaps it would be sufficient to FreeTupleDesc iff tupdesc-refcount == -1? BTW, I'm thinking of changing the various SRFs that make allocations in the EState's per-query context to instead use the SRF's multi_call_ctx. This means the memory will be reclaimed sooner and avoids possible memory leaks. -Neil ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] SRF memory leaks
On Tue, 2008-02-26 at 03:13 -0500, Tom Lane wrote: It's OK in the built-in SRFs is disastrously different from It's OK. Right, I never said that, I was just commenting on your view that the predominant use-case for SRFs is returning refcounted tupdescs. You didn't comment on my proposed solution (FreeTupleDesc() iff refcount == -1). ISTM that we *need* to free the TupleDesc in at least some cases, in order to defend against the practice of explicitly allocating the TupleDesc in the per-query context. -Neil ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] SRF memory leaks
On Tue, 2008-02-26 at 00:17 -0800, Neil Conway wrote:
You didn't comment on my proposed solution (FreeTupleDesc() iff refcount
== -1).
Attached is a revised version of this patch. It makes the
FreeTupleDesc() change described above, and fixes a bug: in
SRF_RETURN_DONE(), we need to be sure to switch back to the caller's
context before deleting the multi_call_ctx, since some SRFs (e.g.
dblink) call SRF_RETURN_DONE() while still inside the multi_call_ctx.
I'd like to apply this change to back branches reasonably soon, so if
you have a better way to do the FreeTupleDesc() hack, let me know.
-Neil
Index: src/backend/executor/nodeFunctionscan.c
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/executor/nodeFunctionscan.c,v
retrieving revision 1.45
diff -p -c -r1.45 nodeFunctionscan.c
*** src/backend/executor/nodeFunctionscan.c 1 Jan 2008 19:45:49 - 1.45
--- src/backend/executor/nodeFunctionscan.c 26 Feb 2008 19:04:49 -
*** FunctionNext(FunctionScanState *node)
*** 77,83
--- 77,93
* do it always.
*/
if (funcTupdesc)
+ {
tupledesc_match(node-tupdesc, funcTupdesc);
+
+ /*
+ * If it is a dynamically-allocated TupleDesc, free it: it is
+ * typically allocated in the EState's per-query context, so we
+ * must avoid leaking it on rescan.
+ */
+ if (funcTupdesc-tdrefcount == -1)
+ FreeTupleDesc(funcTupdesc);
+ }
}
/*
Index: src/backend/utils/fmgr/funcapi.c
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/utils/fmgr/funcapi.c,v
retrieving revision 1.37
diff -p -c -r1.37 funcapi.c
*** src/backend/utils/fmgr/funcapi.c 1 Jan 2008 19:45:53 - 1.37
--- src/backend/utils/fmgr/funcapi.c 26 Feb 2008 19:32:47 -
***
*** 23,28
--- 23,29
#include utils/array.h
#include utils/builtins.h
#include utils/lsyscache.h
+ #include utils/memutils.h
#include utils/syscache.h
#include utils/typcache.h
*** init_MultiFuncCall(PG_FUNCTION_ARGS)
*** 63,75
/*
* First call
*/
! ReturnSetInfo *rsi = (ReturnSetInfo *) fcinfo-resultinfo;
/*
* Allocate suitably long-lived space and zero it
*/
retval = (FuncCallContext *)
! MemoryContextAllocZero(fcinfo-flinfo-fn_mcxt,
sizeof(FuncCallContext));
/*
--- 64,86
/*
* First call
*/
! ReturnSetInfo *rsi = (ReturnSetInfo *) fcinfo-resultinfo;
! MemoryContext multi_call_ctx;
!
! /*
! * Create a suitably long-lived context to hold cross-call data
! */
! multi_call_ctx = AllocSetContextCreate(fcinfo-flinfo-fn_mcxt,
! SRF multi-call context,
! ALLOCSET_SMALL_MINSIZE,
! ALLOCSET_SMALL_INITSIZE,
! ALLOCSET_SMALL_MAXSIZE);
/*
* Allocate suitably long-lived space and zero it
*/
retval = (FuncCallContext *)
! MemoryContextAllocZero(multi_call_ctx,
sizeof(FuncCallContext));
/*
*** init_MultiFuncCall(PG_FUNCTION_ARGS)
*** 81,87
retval-user_fctx = NULL;
retval-attinmeta = NULL;
retval-tuple_desc = NULL;
! retval-multi_call_memory_ctx = fcinfo-flinfo-fn_mcxt;
/*
* save the pointer for cross-call use
--- 92,98
retval-user_fctx = NULL;
retval-attinmeta = NULL;
retval-tuple_desc = NULL;
! retval-multi_call_memory_ctx = multi_call_ctx;
/*
* save the pointer for cross-call use
*** shutdown_MultiFuncCall(Datum arg)
*** 168,180
flinfo-fn_extra = NULL;
/*
! * Caller is responsible to free up memory for individual struct elements
! * other than att_in_funcinfo and elements.
*/
! if (funcctx-attinmeta != NULL)
! pfree(funcctx-attinmeta);
!
! pfree(funcctx);
}
--- 179,189
flinfo-fn_extra = NULL;
/*
! * Delete context that holds all multi-call data, including the
! * FuncCallContext itself
*/
! MemoryContextSwitchTo(flinfo-fn_mcxt);
! MemoryContextDelete(funcctx-multi_call_memory_ctx);
}
---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
Re: [PATCHES] NUMERIC key word
On Tue, 2008-01-29 at 13:20 -0500, Tom Lane wrote: The reason it was kept was to override the search path --- unqualified NUMERIC will always be taken as pg_catalog.numeric even if you have some other type numeric in front of it. It should be possible to implement this behavior without requiring NUMERIC to be a keyword, though. -Neil ---(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
Re: [PATCHES] [8.4] Updated WITH clause patch (non-recursive)
On Sun, 2008-01-27 at 09:17 +, Gregory Stark wrote: Tom's feeling at the time was that even though it was providing something from the standard, it wasn't actually allowing the user to do anything he couldn't before. I think this feature has value: (1) This is SQL-standard syntax (and not even wacko syntax, at that!), and there is merit in implementing it on those grounds alone. (2) It is supported by DB2, MS SQL and Oracle, so there is a further compatibility argument to be made. (3) It avoids the need to repeat subqueries multiple times in the main query, which can make queries more concise. Defining subqueries outside the main SELECT body can also have readability advantages. If it doesn't provide any additional expressive capabilities then I think he didn't like taking with as a reserved word. Note that we can make WITH a type_func_name_keyword, rather than a full-on reserved_keyword, which reduces the force of this argument slightly. If we're going to implement recursive queries eventually (which we almost surely will, whether in 8.4 or a future release), we'll need to make WITH more reserved at some point anyway, so I don't see much to be gained in the long run by delaying it. -Neil ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [8.4] Updated WITH clause patch (non-recursive)
On Sun, 2008-01-27 at 12:36 -0500, Tom Lane wrote: Both of the above arguments hold water only if we implement compatible *semantics*, not merely syntax, so I find them unconvincing at this stage. How are the semantics of the proposed patch incompatible with the SQL spec or the implementations in other systems? The proposed patch is a *subset* of the functionality in the SQL spec, but it isn't incompatible with it as far as I know (recursive and non-recursive WITH are distinct features). (3) It avoids the need to repeat subqueries multiple times in the main query, which can make queries more concise. Defining subqueries outside the main SELECT body can also have readability advantages. Views fix that too. Sure, if you're willing to resort to DDL, and lose most of the conciseness / readability gain. The point is that when you break people's apps you'll be able to point to some real increment in functionality to justify it. If your application uses an identifier that is a reserved word in SQL-92 and in pretty much all major databases, I don't think you have much cause for grievance when it becomes a reserved word in Postgres -- the writing has been on the wall for a while. Do you have any reason to think that WITH is a particularly common table or column name, by the way? Note also the keywords.c hack in 8.3 for the WITH keyword means that pg_dump already treats WITH as a reserved word, so most dumps should load without changes. With the patch as it stands you'd essentially be saying we're going to cause you pain now for benefit later, which is a hard selling proposition. Again, the readability + compatibility arguments are non-zero benefits, IMHO. But your argument is essentially a public-relations one (it will look bad if...), which I don't find very convincing. If we explain that WITH is a reserved word per SQL spec and is part of the planned support for recursive queries (whether in 8.4 or later), I can't see very many users being annoyed. (Compare that with the irritation we may well see from the removal of implicit casts in 8.3, which will break *far* more applications, for a benefit that many users will no doubt find rather hard to observe.) -Neil ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Proposed patch: synchronized_scanning GUC variable
On Sun, 2008-01-27 at 21:54 +, Gregory Stark wrote: I liked the synchronized_sequential_scans idea myself. I think that's a bit too long. How about synchronized_scans, or synchronized_seqscans? -Neil ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Proposed patch: synchronized_scanning GUC variable
On Mon, 2008-01-28 at 17:27 +1100, Russell Smith wrote: Can somebody explain why it's important to load with synchronized_scanning off? *Loading* with synchronized scanning off is not important (and is not implemented by the patch). *Dumping* with synchronized scanning off is necessary to ensure that the order of the rows in the pg_dump matches the on-disk order of the rows in the table, which is important if you want to preserve the clustering of the table data on restore. See the -hackers thread: http://markmail.org/message/qbytsco6oj2qkxsa -Neil ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
[PATCHES] [8.4] Updated WITH clause patch (non-recursive)
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 haven't looked at the actual functionality of the patch yet (which is quite trivial) -- I just fixed up bitrot and the like. I also removed support for RECURSIVE and the search/cycle clause, along with their associated keywords -- the current patch doesn't approach anything close to adding support for the non-recursive case, so it seems like a net loss to add additional keywords for no gain in functionality. Remaining work is to review the guts of the patch (which shouldn't take long), and write documentation and regression tests. I'm personally hoping to see this get into the tree fairly early in the 8.4 cycle, pending discussion of course. -Neil [1] http://archives.postgresql.org/pgsql-patches/2007-03/msg00139.php http://archives.postgresql.org/pgsql-patches/2007-04/msg00055.php Index: src/backend/nodes/copyfuncs.c === RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/nodes/copyfuncs.c,v retrieving revision 1.387 diff -p -c -r1.387 copyfuncs.c *** src/backend/nodes/copyfuncs.c 1 Jan 2008 19:45:50 - 1.387 --- src/backend/nodes/copyfuncs.c 27 Jan 2008 07:05:29 - *** _copySelectStmt(SelectStmt *from) *** 1930,1935 --- 1930,1936 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: /home/neilc/postgres/cvs_root/pgsql/src/backend/nodes/equalfuncs.c,v retrieving revision 1.317 diff -p -c -r1.317 equalfuncs.c *** src/backend/nodes/equalfuncs.c 1 Jan 2008 19:45:50 - 1.317 --- src/backend/nodes/equalfuncs.c 27 Jan 2008 07:05:29 - *** _equalSelectStmt(SelectStmt *a, SelectSt *** 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: /home/neilc/postgres/cvs_root/pgsql/src/backend/nodes/outfuncs.c,v retrieving revision 1.322 diff -p -c -r1.322 outfuncs.c *** src/backend/nodes/outfuncs.c 9 Jan 2008 08:46:44 - 1.322 --- src/backend/nodes/outfuncs.c 27 Jan 2008 07:05:29 - *** _outSelectStmt(StringInfo str, SelectStm *** 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: /home/neilc/postgres/cvs_root/pgsql/src/backend/parser/analyze.c,v retrieving revision 1.371 diff -p -c -r1.371 analyze.c *** src/backend/parser/analyze.c 1 Jan 2008 19:45:50 - 1.371 --- src/backend/parser/analyze.c 27 Jan 2008 07:23:39 - *** transformSelectStmt(ParseState *pstate, *** 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); Index: src/backend/parser/gram.y === RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/parser/gram.y,v retrieving revision 2.605 diff -p -c -r2.605 gram.y *** src/backend/parser/gram.y 1 Jan 2008 19:45:50 - 2.605 --- src/backend/parser/gram.y 27 Jan 2008 07:21:28 - *** static List *extractArgTypes(List *param *** 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 void insertSelectOptions(SelectStmt *stmt, List *sortClause, List *lockingClause, ! Node *limitOffset, Node *limitCount, ! List *with_cte_list); static Node *makeSetOp(SetOperation op, bool all, Node *larg, Node *rarg); static Node *doNegate(Node *n, int location); static void doNegateFloat(Value *v);
Re: [PATCHES] Revised xml memory allocation patch
On Mon, 2008-01-14 at 20:55 -0500, Tom Lane wrote: Any thoughts whether to apply or not? Seems like a much more sane approach to me -- +1. -Neil ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Fix for _outAgg()
On Tue, 2008-01-08 at 23:08 -0500, Tom Lane wrote: Hmm, I think that must be my fault, but I'm not sure how it got by me ... I'm usually pretty careful about adding outfuncs support when I add a node field. Patch looks good, please apply. Applied to HEAD. -Neil ---(end of broadcast)--- TIP 6: explain analyze is your friend
[PATCHES] Fix for _outAgg()
Attached is a patch which fixes an oversight in _outAgg(): the
grpColIdx and grpOperators fields of the Agg struct were not emitted
by _outAgg(). I don't see any good reason to omit this information.
Note that while the grpOperators field was added during the 8.3 devel
cycle, the grpColIdx field has been around since '02 (without outfuncs
support). So I can backpatch that portion of the patch to back branches
if anyone feels strongly.
-Neil
Index: src/backend/nodes/outfuncs.c
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/nodes/outfuncs.c,v
retrieving revision 1.321
diff -p -c -r1.321 outfuncs.c
*** src/backend/nodes/outfuncs.c 7 Jan 2008 21:33:10 - 1.321
--- src/backend/nodes/outfuncs.c 9 Jan 2008 03:01:33 -
*** _outHashJoin(StringInfo str, HashJoin *n
*** 501,512
--- 501,523
static void
_outAgg(StringInfo str, Agg *node)
{
+ int i;
+
WRITE_NODE_TYPE(AGG);
_outPlanInfo(str, (Plan *) node);
WRITE_ENUM_FIELD(aggstrategy, AggStrategy);
WRITE_INT_FIELD(numCols);
+
+ appendStringInfo(str, :grpColIdx);
+ for (i = 0; i node-numCols; i++)
+ appendStringInfo(str, %d, node-grpColIdx[i]);
+
+ appendStringInfo(str, :grpOperators);
+ for (i = 0; i node-numCols; i++)
+ appendStringInfo(str, %u, node-grpOperators[i]);
+
WRITE_LONG_FIELD(numGroups);
}
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] pgbench - startup delay
On Mon, 2007-12-10 at 19:27 +, Dave Page wrote: Whilst doing some profiling of the server I found it useful to add an option to pgbench to introduce a delay between client connection setup and the start of the benchmark itself to allow me time to attach the profiler to one of the backends. postgres -W n already does this. It is more flexible to put this functionality in the backend that in individual client apps anyway. -Neil ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] pgbench - startup delay
On Mon, 2007-12-10 at 19:12 -0500, Greg Smith wrote: I just poked around the documentation a bit and I didn't find anything that cleared up which options you can pass from a client http://www.postgresql.org/docs/8.2/static/libpq-envars.html Which says only PGOPTIONS sets additional run-time options for the PostgreSQL server. This could probably be elaborated upon -- for the list of options accepted, see PostgresMain() in tcop/postgres.c Perhaps one of the slightly unfortunate consequences of the postmaster = postgres merge is that there is less of a clear distinction between postmaster options and postgres options... -Neil ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] A minor typo fix on pg_standby docs
Applied, thanks. -Neil On Fri, 2007-12-07 at 18:48 +1100, FAST PostgreSQL wrote: Rgds, Arul Shaji ---(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 ---(end of broadcast)--- TIP 6: explain analyze is your friend
[PATCHES] Avoid needless copy in nodeMaterial
Attached is a patch that avoids a needless copy of the result tuple in nodeMaterial, in the case that we don't have a previously-materialized tuple to return. We can just return the TTS produced by executing our child node, rather than returning a copy of it. I didn't bother pulling the MinimalTuple out of outerslot and stuffing it back into the nodeMaterial's result slot, as AFAICS that is not necessary. Although I suppose you could make a cleanliness argument that that would be worth doing instead. (This is 8.4 material...) -Neil Index: source/src/backend/executor/nodeMaterial.c === RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/executor/nodeMaterial.c,v retrieving revision 1.59 diff -p -c -r1.59 nodeMaterial.c *** source/src/backend/executor/nodeMaterial.c 21 May 2007 17:57:33 - 1.59 --- source/src/backend/executor/nodeMaterial.c 16 Oct 2007 04:00:46 - *** ExecMaterial(MaterialState *node) *** 98,103 --- 98,105 eof_tuplestore = true; } + ExecClearTuple(slot); + /* * If necessary, try to fetch another row from the subplan. * *** ExecMaterial(MaterialState *node) *** 124,147 } /* ! * Append returned tuple to tuplestore. NOTE: because the tuplestore ! * is certainly in EOF state, its read position will move forward over ! * the added tuple. This is what we want. */ if (tuplestorestate) tuplestore_puttupleslot(tuplestorestate, outerslot); ! /* ! * And return a copy of the tuple. (XXX couldn't we just return the ! * outerslot?) ! */ ! return ExecCopySlot(slot, outerslot); } ! /* ! * Nothing left ... ! */ ! return ExecClearTuple(slot); } /* --- 126,143 } /* ! * Append a copy of the returned tuple to tuplestore. NOTE: because ! * the tuplestore is certainly in EOF state, its read position will ! * move forward over the added tuple. This is what we want. */ if (tuplestorestate) tuplestore_puttupleslot(tuplestorestate, outerslot); ! return outerslot; } ! /* Nothing left, return empty slot */ ! return slot; } /* ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] Avoid needless copy in nodeMaterial
On Tue, 2007-10-16 at 00:34 -0400, Tom Lane wrote:
Seems like this needs more comments about what's happening, rather
than less ...
Fair point.
Also, it looks to me like the plan node's own resultslot might never be
assigned to at all, when the subplan returns zero rows. Does this
corner case still work correctly?
ISTM the node's own result slot wouldn't be assigned to in this case
regardless: (nodeMaterial.c, circa 116)
outerslot = ExecProcNode(outerNode);
if (TupIsNull(outerslot))
{
node-eof_underlying = true;
return NULL;
}
There's no requirement that we must assign to the result slot, AFAICS.
-Neil
---(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
Re: [PATCHES] tsearch limitations doc correction
On Tue, 2007-10-09 at 19:31 +0100, Heikki Linnakangas wrote: [...] Applied, thanks. -Neil ---(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
Re: [PATCHES] minor compiler warning in libpg/fe-secure.c on win32 (msvc)
On Tue, 2007-10-02 at 23:45 +0200, Hannes Eder wrote: while rebuilding the entire project I ran across following warning: .\src\interfaces\libpq\fe-secure.c(593): warning C4101: 'fp' : unreferenced local variable Applied, thanks. BTW, CC'ing hackers is not necessary for trivial patches like this. -Neil ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] too many variants of relation_open
On Fri, 2007-09-28 at 10:02 +0100, Heikki Linnakangas wrote: Well, yes it could, but why? Keeping them separate looks slightly more readable to me, and the change could break a lot of external modules for no good reason. I agree: it also complicates the common case (calling relation_open() and waiting to acquire for a lock). If anything, you could perhaps refactor relation_open_nowait() to be implemented in terms of relation_open() (first try to get the lock, then do a relation_open(rel, NoLock)), but since you'd only be saving a handful of duplicated lines, it hardly seems worth it. -Neil ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Implementation of aggregate functions
The pgsql-patches mailing list is for submitting modifications to Postgres, not for asking these sorts of questions. On Thu, 2007-20-09 at 06:49 +0100, sayali k wrote: I am keen in implementing certain additional aggregate functions like percentage, standard deviation etc. I would like to know the complexity of this and also the parts of the code which would have to be modified for the same, stddev() already exists. Take a look at the implementation of the existing aggregate functions, which can be found in src/backend/utils/adt (float.c, numeric.c, etc.) There is no need to modify the core Postgres backend code to add new aggregate functions, of course. I suggest doing a modicum of research first, and then asking on pgsql-hackers if you have more specific questions about Postgres internals. -Neil ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Incorrect results from corr()
On Tue, 2007-09-18 at 18:27 -0700, Neil Conway wrote: The builtin corr() aggregate doesn't produce the correct results in some circumstances. Applied to HEAD and REL8_2_STABLE. -Neil ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
[PATCHES] Incorrect results from corr()
The builtin corr() aggregate doesn't produce the correct results in some circumstances. Per the SQL spec, corr(x, y) is defined as equivalent to covar_pop(x, y) / (stddev_pop(x) * stddev_pop(y)). postgres=# create table t1 (x float8, y float8); CREATE TABLE postgres=# copy t1 from stdin with csv; 0.940839937888086,0.539830380585045 0.84795232815668,0.396385048050433 0.601479615084827,0.85123967528 0.785623408854008,0.302559469360858 0.829138438683003,0.0211085784249008 0.926528611686081,0.315794581547379 0.25934984581545,0.609216409735382 0.976522764191031,0.877208305988461 \. postgres=# select corr(x, y) from t1; corr --- 0.214150892978763 (1 row) postgres=# select covar_pop(x, y) / (stddev_pop(x) * stddev_pop(y)) from t1; ?column? -0.214150892978763 (1 row) With the attached patch, we get the expected results: postgres=# select corr(x, y) from t1; corr -0.214150892978763 (1 row) Credit: Jie Zhang at Greenplum and Gavin Sherry for reporting the issue. Barring any objections, I'll apply this to HEAD and 8.2 later tonight or tomorrow. -Neil Index: src/backend/utils/adt/float.c === RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/utils/adt/float.c,v retrieving revision 1.150 diff -p -c -r1.150 float.c *** src/backend/utils/adt/float.c 5 Jun 2007 21:31:06 - 1.150 --- src/backend/utils/adt/float.c 19 Sep 2007 00:51:27 - *** float8_corr(PG_FUNCTION_ARGS) *** 2274,2281 if (numeratorX = 0 || numeratorY = 0) PG_RETURN_NULL(); ! PG_RETURN_FLOAT8(sqrt((numeratorXY * numeratorXY) / ! (numeratorX * numeratorY))); } Datum --- 2274,2280 if (numeratorX = 0 || numeratorY = 0) PG_RETURN_NULL(); ! PG_RETURN_FLOAT8(numeratorXY / sqrt(numeratorX * numeratorY)); } Datum ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] Memory leak in nodeAgg
On Mon, 2007-08-06 at 14:21 -0700, Neil Conway wrote: Attached is a patch that fixes a gradual memory leak in ExecReScanAgg(), when the AGG_HASHED strategy is used Applied to HEAD, and backpatched back to 7.4. -Neil ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Default location for docbook stylesheets in Gentoo
On Thu, 2007-08-09 at 04:08 +1000, Brendan Jurd wrote: This patch adds the default location for the DocBook DSSSL stylesheets in gentoo's package system to the configure script. FYI, patches should typically be submitted as context diffs. For the specific case of configure, you should submit patches against the source file (configure.in), not the generated file (configure). -Neil ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] MemoryContextStats tweak: show tree structure
On Mon, 2007-06-08 at 17:48 -0700, Neil Conway wrote: Obviously this is just for debugging, but I've found it useful while looking at some memory-related issues. Any comments or objections to including this in HEAD? Applied, with an indentation of two spaces per level. -Neil ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
[PATCHES] Memory leak in nodeAgg
Attached is a patch that fixes a gradual memory leak in ExecReScanAgg(),
when the AGG_HASHED strategy is used:
* the aggregation hash table is allocated in a newly-created
sub-context of the agg's aggcontext
* MemoryContextReset() resets the memory allocated in child
contexts, but not the child contexts themselves
* ExecReScanAgg() builds a brand-new hash table, which allocates
a brand-new sub-context, thus leaking the header for the
previous hashtable sub-context
The patch fixes this by using MemoryContextDeleteAndResetChildren(). (I
briefly looked at other call-sites of hash_create() to see if this
problem exists elsewhere, but I didn't see anything obvious.)
We run into the leak quite easily at Truviso; with a sufficiently
long-lived query in vanilla Postgres, you should be able to reproduce
the same problem.
Credit: Sailesh Krishnamurthy at Truviso for diagnosing the cause of the
leak.
-Neil
Index: src/backend/executor/nodeAgg.c
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/executor/nodeAgg.c,v
retrieving revision 1.152
diff -p -c -r1.152 nodeAgg.c
*** src/backend/executor/nodeAgg.c 2 Apr 2007 03:49:38 - 1.152
--- src/backend/executor/nodeAgg.c 6 Aug 2007 19:29:11 -
*** ExecReScanAgg(AggState *node, ExprContex
*** 1646,1653
MemSet(econtext-ecxt_aggvalues, 0, sizeof(Datum) * node-numaggs);
MemSet(econtext-ecxt_aggnulls, 0, sizeof(bool) * node-numaggs);
! /* Release all temp storage */
! MemoryContextReset(node-aggcontext);
if (((Agg *) node-ss.ps.plan)-aggstrategy == AGG_HASHED)
{
--- 1646,1657
MemSet(econtext-ecxt_aggvalues, 0, sizeof(Datum) * node-numaggs);
MemSet(econtext-ecxt_aggnulls, 0, sizeof(bool) * node-numaggs);
! /*
! * Release all temp storage. Note that in the AGG_HASHED case the agg
! * hash table is allocated in a sub-context, so we need to use
! * MemoryContextResetAndDeleteChildren() to reclaim that storage.
! */
! MemoryContextResetAndDeleteChildren(node-aggcontext);
if (((Agg *) node-ss.ps.plan)-aggstrategy == AGG_HASHED)
{
---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at
http://www.postgresql.org/about/donate
Re: [PATCHES] Memory leak in nodeAgg
On Mon, 2007-08-06 at 18:52 -0400, Tom Lane wrote: Hmm. Good catch, but I can't help wondering if this is just the tip of the iceberg. Should *every* MemoryContextReset be MemoryContextResetAndDeleteChildren? Yeah, the same thought occurred to me. Certainly having the current behavior as the default is error-prone: it's quite easy to leak child contexts on Reset. Perhaps we could redefine Reset to mean ResetAndDeleteChildren, and add another name for the current Reset functionality. ResetAndPreserveChildren, maybe? If we redefined MemoryContextReset to be the same as MemoryContextResetAndDeleteChildren, it'd be possible to keep the headers for child contexts in their parent context, thus easing traffic in TopMemoryContext, and perhaps saving a few pfree cycles when resetting the parent The fact that MemoryContextCreate allocates the context header in TopMemoryContext has always made me uneasy, so getting rid of that would be nice. I wonder if there's not at least *one* place that depends on the current Reset behavior, though... Anyone want to investigate what happens if we make MemoryContextReset the same as MemoryContextResetAndDeleteChildren? Sure, I'll take a look, but I'll apply the attached patch in the mean time (above cleanup is probably 8.4 material anyway). -Neil ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
[PATCHES] MemoryContextStats tweak: show tree structure
Previously, MemoryContextStats() simply emitted a line of output for
each MemoryContext. This is fine, but it makes it difficult to see the
shape of the MemoryContext hierarchy. Attached is a trivial patch to
indent each context by 4 * level spaces, where level is the depth of
the node within the subtree printed by MemoryContextStats().
For example, suppose we have three contexts beneath TopMemoryContext:
TopMemoryContext (...)
FooContext (...)
BarContext (...)
BazContext (...)
With the patch, these might be printed as:
TopMemoryContext (...)
FooContext (...)
BarContext (...)
BazContext (...)
Assuming that's the parent/child relationship between them, of course.
Obviously this is just for debugging, but I've found it useful while
looking at some memory-related issues. Any comments or objections to
including this in HEAD?
-Neil
Index: source/src/backend/utils/mmgr/aset.c
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/utils/mmgr/aset.c,v
retrieving revision 1.72
diff -p -c -r1.72 aset.c
*** source/src/backend/utils/mmgr/aset.c 30 Apr 2007 00:12:08 - 1.72
--- source/src/backend/utils/mmgr/aset.c 7 Aug 2007 00:29:41 -
*** static void AllocSetReset(MemoryContext
*** 214,220
static void AllocSetDelete(MemoryContext context);
static Size AllocSetGetChunkSpace(MemoryContext context, void *pointer);
static bool AllocSetIsEmpty(MemoryContext context);
! static void AllocSetStats(MemoryContext context);
#ifdef MEMORY_CONTEXT_CHECKING
static void AllocSetCheck(MemoryContext context);
--- 214,220
static void AllocSetDelete(MemoryContext context);
static Size AllocSetGetChunkSpace(MemoryContext context, void *pointer);
static bool AllocSetIsEmpty(MemoryContext context);
! static void AllocSetStats(MemoryContext context, int level);
#ifdef MEMORY_CONTEXT_CHECKING
static void AllocSetCheck(MemoryContext context);
*** AllocSetIsEmpty(MemoryContext context)
*** 1034,1040
* Displays stats about memory consumption of an allocset.
*/
static void
! AllocSetStats(MemoryContext context)
{
AllocSet set = (AllocSet) context;
long nblocks = 0;
--- 1034,1040
* Displays stats about memory consumption of an allocset.
*/
static void
! AllocSetStats(MemoryContext context, int level)
{
AllocSet set = (AllocSet) context;
long nblocks = 0;
*** AllocSetStats(MemoryContext context)
*** 1044,1049
--- 1044,1050
AllocBlock block;
AllocChunk chunk;
int fidx;
+ int i;
for (block = set-blocks; block != NULL; block = block-next)
{
*** AllocSetStats(MemoryContext context)
*** 1060,1065
--- 1061,1070
freespace += chunk-size + ALLOC_CHUNKHDRSZ;
}
}
+
+ for (i = 0; i level; i++)
+ fprintf(stderr, );
+
fprintf(stderr,
%s: %lu total in %ld blocks; %lu free (%ld chunks); %lu used\n,
set-header.name, totalspace, nblocks, freespace, nchunks,
Index: source/src/backend/utils/mmgr/mcxt.c
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/utils/mmgr/mcxt.c,v
retrieving revision 1.61
diff -p -c -r1.61 mcxt.c
*** source/src/backend/utils/mmgr/mcxt.c 25 Jul 2007 12:22:52 - 1.61
--- source/src/backend/utils/mmgr/mcxt.c 7 Aug 2007 00:30:26 -
*** MemoryContext CurTransactionContext = NU
*** 49,54
--- 49,56
/* This is a transient link to the active portal's memory context: */
MemoryContext PortalContext = NULL;
+ static void MemoryContextStatsInternal(MemoryContext context, int level);
+
/*
* EXPORTED ROUTINES *
*** MemoryContextIsEmpty(MemoryContext conte
*** 321,336
void
MemoryContextStats(MemoryContext context)
{
MemoryContext child;
AssertArg(MemoryContextIsValid(context));
! (*context-methods-stats) (context);
for (child = context-firstchild; child != NULL; child = child-nextchild)
! MemoryContextStats(child);
}
-
/*
* MemoryContextCheck
* Check all chunks in the named context.
--- 323,343
void
MemoryContextStats(MemoryContext context)
{
+ MemoryContextStatsInternal(context, 0);
+ }
+
+ static void
+ MemoryContextStatsInternal(MemoryContext context, int level)
+ {
MemoryContext child;
AssertArg(MemoryContextIsValid(context));
! (*context-methods-stats) (context, level);
for (child = context-firstchild; child != NULL; child = child-nextchild)
! MemoryContextStatsInternal(child, level + 1);
}
/*
* MemoryContextCheck
* Check all chunks in the named context.
Index: source/src/include/nodes/memnodes.h
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/include/nodes/memnodes.h,v
Re: [PATCHES] MemoryContextStats tweak: show tree structure
On Mon, 2007-08-06 at 20:56 -0400, Tom Lane wrote: Seems reasonable except I'd vote for less indentation --- the lines are too long already. How do you feel about 1 or 2 spaces per level? Sure, 2 spaces per level is fine with me (1 makes it a bit hard to see the tree structure, IMHO). -Neil ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Memory leak in tuplestore_end()
On Wed, 2007-08-01 at 16:49 -0700, Neil Conway wrote: Attached is a patch which fixes a memory leak in tuplestore_end(). Applied to HEAD, and to back branches as far back as 7.4. -Neil ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
[PATCHES] Memory leak in tuplestore_end()
Attached is a patch which fixes a memory leak in tuplestore_end(). Barring any objections, I'll apply this to HEAD and back branches tomorrow. -Neil Index: source/src/backend/utils/sort/tuplestore.c === RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/utils/sort/tuplestore.c,v retrieving revision 1.33 diff -p -c -r1.33 tuplestore.c *** source/src/backend/utils/sort/tuplestore.c 7 Jun 2007 19:19:57 - 1.33 --- source/src/backend/utils/sort/tuplestore.c 1 Aug 2007 23:45:24 - *** tuplestore_end(Tuplestorestate *state) *** 322,327 --- 322,328 pfree(state-memtuples[i]); pfree(state-memtuples); } + pfree(state); } /* ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
[PATCHES] RETURN QUERY
Attached is a patch implementing RETURN QUERY, per earlier discussion, and based on a patch from Pavel Stehule. Like RETURN NEXT, RETURN QUERY doesn't immediately return from the function, allowing RETURN NEXT and RETURN QUERY to be intermixed in a single function. Barring any objections, I'll apply this tomorrow. -Neil Index: doc/src/sgml/plpgsql.sgml === RCS file: /home/neilc/postgres/cvs_root/pgsql/doc/src/sgml/plpgsql.sgml,v retrieving revision 1.115 diff -p -c -r1.115 plpgsql.sgml *** doc/src/sgml/plpgsql.sgml 16 Jul 2007 17:01:10 - 1.115 --- doc/src/sgml/plpgsql.sgml 24 Jul 2007 06:40:47 - *** *** 135,141 applicationPL/pgSQL/ functions can also be declared to return a quoteset/, or table, of any data type they can return a single instance of. Such a function generates its output by executing ! literalRETURN NEXT/ for each desired element of the result set. /para para --- 135,143 applicationPL/pgSQL/ functions can also be declared to return a quoteset/, or table, of any data type they can return a single instance of. Such a function generates its output by executing ! commandRETURN NEXT/ for each desired element of the result ! set, or by using commandRETURN QUERY/ to output the result of ! executing a query. /para para *** RETURN replaceableexpression/replacea *** 1349,1400 /sect3 sect3 ! titlecommandRETURN NEXT//title synopsis RETURN NEXT replaceableexpression/replaceable; /synopsis para When a applicationPL/pgSQL/ function is declared to return literalSETOF replaceablesometype//literal, the procedure to follow is slightly different. In that case, the individual ! items to return are specified in commandRETURN NEXT/command ! commands, and then a final commandRETURN/command command ! with no argument is used to indicate that the function has ! finished executing. commandRETURN NEXT/command can be used ! with both scalar and composite data types; with a composite result ! type, an entire quotetable/quote of results will be returned. /para para ! commandRETURN NEXT/command does not actually return from the ! function mdash; it simply saves away the value of the expression. ! Execution then continues with the next statement in ! the applicationPL/pgSQL/ function. As successive ! commandRETURN NEXT/command commands are executed, the result ! set is built up. A final commandRETURN/command, which should ! have no argument, causes control to exit the function (or you can ! just let control reach the end of the function). /para para If you declared the function with output parameters, write just commandRETURN NEXT/command with no expression. On each ! execution, the current values ! of the output parameter variable(s) will be saved for eventual return ! as a row of the result. ! Note that you must declare the function as returning ! literalSETOF record/literal when there are ! multiple output parameters, or ! literalSETOF replaceablesometype//literal when there is ! just one output parameter of type replaceablesometype/, in ! order to create a set-returning function with output parameters. /para para ! Functions that use commandRETURN NEXT/command should be ! called in the following fashion: programlisting SELECT * FROM some_func(); --- 1351,1419 /sect3 sect3 ! titlecommandRETURN NEXT/ and commandRETURN QUERY/command/title ! indexterm ! primaryRETURN NEXT/primary ! secondaryin PL/PgSQL/secondary ! /indexterm ! indexterm ! primaryRETURN QUERY/primary ! secondaryin PL/PgSQL/secondary ! /indexterm synopsis RETURN NEXT replaceableexpression/replaceable; + RETURN QUERY replaceablequery/replaceable; /synopsis para When a applicationPL/pgSQL/ function is declared to return literalSETOF replaceablesometype//literal, the procedure to follow is slightly different. In that case, the individual ! items to return are specified by a sequence of commandRETURN ! NEXT/command or commandRETURN QUERY/command commands, and ! then a final commandRETURN/command command with no argument ! is used to indicate that the function has finished executing. ! commandRETURN NEXT/command can be used with both scalar and ! composite data types; with a composite result type, an entire ! quotetable/quote of results will be returned. ! commandRETURN QUERY/command appends the results of executing ! a query to the function's result set. commandRETURN ! NEXT/command and commandRETURN
Re: [PATCHES] RETURN QUERY
On Mon, 2007-23-07 at 23:57 -0700, Neil Conway wrote: Attached is a patch implementing RETURN QUERY, per earlier discussion Applied to HEAD. -Neil ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] execl() sentinel
On Wed, 2007-07-18 at 17:22 -0400, Alvaro Herrera wrote: I wouldn't know how to look for other variadic functions using NULL sentinels though. You would need something with more knowledge of C than grep has, at any rate. Perhaps you could teach sparse to do this analysis, if it can't do it already... -Neil ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
On Tue, 2007-10-07 at 12:53 +0530, NikhilS wrote:
Yes, in the CREATE..LIKE case, options will be NULL and in the normal
CREATE..INDEX case inhreloptions will be NULL. Note that options is a
List of DefElem entries, whereas inhreloptions is a char pointer.
Hmm, right -- ugly. I'll just stick with your approach.
BTW, I notice a problem with the patch as implemented:
# create table abc (a int, b int);
CREATE TABLE
# create index abc_a_idx on abc using hash (a);
CREATE INDEX
# create index abc_a_idx2 on abc (a);
CREATE INDEX
# create table abc2 (like abc including indexes);
CREATE TABLE
# \d abc2
Table public.abc2
Column | Type | Modifiers
+-+---
a | integer |
b | integer |
Indexes:
abc2_a_key hash (a)
This occurs because transformIndexConstraints() eliminates duplicate
indexes from the index list by looking for two indexes with equal()
column lists. This makes some sense for the normal CREATE TABLE case,
but the above behavior is certainly objectionable -- when the access
method differs it is merely surprising, but when partial indexes are
involved it is surely a bug.
One way to fix this would be to check for duplicates by comparing all
the fields of the two IndexStmts, *except* the index name and is PK?
status. But that's ugly -- it seems much cleaner to keep index
definitions arising from LIKE ... INCLUDING INDEXES in a separate list
from the indexes derived from constraints.
Attached is a revised patch that does that. Barring any objections, I'll
apply this to HEAD tomorrow.
-Neil
Index: doc/src/sgml/ref/create_table.sgml
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/doc/src/sgml/ref/create_table.sgml,v
retrieving revision 1.108
diff -p -c -r1.108 create_table.sgml
*** doc/src/sgml/ref/create_table.sgml 3 Jun 2007 17:06:03 - 1.108
--- doc/src/sgml/ref/create_table.sgml 16 Jul 2007 05:19:43 -
*** PostgreSQL documentation
*** 23,29
CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE replaceable class=PARAMETERtable_name/replaceable ( [
{ replaceable class=PARAMETERcolumn_name/replaceable replaceable class=PARAMETERdata_type/replaceable [ DEFAULT replaceabledefault_expr/ ] [ replaceable class=PARAMETERcolumn_constraint/replaceable [ ... ] ]
| replaceabletable_constraint/replaceable
! | LIKE replaceableparent_table/replaceable [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS } ] ... }
[, ... ]
] )
[ INHERITS ( replaceableparent_table/replaceable [, ... ] ) ]
--- 23,29
CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE replaceable class=PARAMETERtable_name/replaceable ( [
{ replaceable class=PARAMETERcolumn_name/replaceable replaceable class=PARAMETERdata_type/replaceable [ DEFAULT replaceabledefault_expr/ ] [ replaceable class=PARAMETERcolumn_constraint/replaceable [ ... ] ]
| replaceabletable_constraint/replaceable
! | LIKE replaceableparent_table/replaceable [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS | INDEXES } ] ... }
[, ... ]
] )
[ INHERITS ( replaceableparent_table/replaceable [, ... ] ) ]
*** and replaceable class=PARAMETERtable
*** 237,243
/varlistentry
varlistentry
! termliteralLIKE replaceableparent_table/replaceable [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS } ]/literal/term
listitem
para
The literalLIKE/literal clause specifies a table from which
--- 237,243
/varlistentry
varlistentry
! termliteralLIKE replaceableparent_table/replaceable [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS | INDEXES } ]/literal/term
listitem
para
The literalLIKE/literal clause specifies a table from which
*** and replaceable class=PARAMETERtable
*** 266,275
requested, all check constraints are copied.
/para
para
Note also that unlike literalINHERITS/literal, copied columns and
constraints are not merged with similarly named columns and constraints.
If the same name is specified explicitly or in another
! literalLIKE/literal clause an error is signalled.
/para
/listitem
/varlistentry
--- 266,280
requested, all check constraints are copied.
/para
para
+ Any indexes on the original table will not be created on the new
+ table, unless the literalINCLUDING INDEXES/literal clause is
+ specified.
+ /para
+ para
Note also that unlike literalINHERITS/literal, copied columns and
constraints are not merged with similarly named columns and constraints.
If the same name is specified explicitly or in another
! literalLIKE/literal clause, an error is signalled.
/para
/listitem
/varlistentry
Index: src/backend/bootstrap/bootparse.y
===
RCS
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
On Mon, 2007-16-07 at 14:28 +0530, NikhilS wrote: Guess you want to use src_options here to be uniform with usages elsewhere that you have replaced. Thanks, good catch. You suppose src_options is much more readable than inhreloptions or inh_idxoptions? Your call :). Yeah, I'm not necessarily sure that it is. inhreloptions made me think of table inheritance, whereas LIKE in general is more of a source = target copying operation. But I'm not convinced about src_options, either ... suggestions/comments welcome. -Neil ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] bogus unixware compiler warnings
On Mon, 2007-07-16 at 13:11 +0200, Stefan Kaltenbrunner wrote: I'm slowly working on submitting patches to reduce the amount of (bogus) compiler warnings generated by various buildfarm members. Warthog(the unixware box) generates some 1140 lines of: UX:cc: WARNING: debugging and optimization mutually exclusive; -O disabled UX:cc: WARNING: debugging and optimization mutually exclusive; -O disabled Applied, thanks for the patch. -Neil ---(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
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
I've applied the latest version of the patch to HEAD. Thanks for the patches, Nikhil and Trevor -- we can take a look at improving INCLUDING CONSTRAINTS for 8.4. On Mon, 2007-16-07 at 15:50 +0530, NikhilS wrote: I agree, since its a LIKE operation and not inheritance as such, how about src_idxoptions, just to make the reference to the source index clearer? I decided to keep it as src_options, but we can always change it later. -Neil ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
On Tue, 2007-05-06 at 14:50 +0530, NikhilS wrote:
PFA, a patch which provides the CREATE .. INCLUDING INDEXES
functionality. This patch uses the same functionality introduced by
the earlier patches in this thread albeit for the INCLUDING INDEXES
case.
Attached is a revised version of this patch. Sorry for taking so long to
make progress on this: my new job been keeping my busy, and I've
recently been ill.
This version updates the patch to CVS HEAD and has various fixes and
refactoring, including proper docs. I refactored get_opclass_name() into
lsyscache.c, but then realized that this means that lsyscache.c will
depend on commands/indexcmds.c (for GetDefaultOpClass()), which is
arguably improper, so I'm tempted to revert and just duplicate the
syscache lookups in both ruleutils.c and parse_utilcmd.c
Nikhil: why are both options and inhreloptions necessary in
IndexStmt? Won't at least one be NULL?
BTW, I notice that include/defrem.h contains declarations for several
different, marginally-related .c files (indexcmds.c, functioncmds.c,
operatorcmds.c, aggregatecmds.c, opclasscmds.c, define.c). I'm inclined
to separate these declarations into separate header files; any
objections to doing that?
-Neil
Index: doc/src/sgml/ref/create_table.sgml
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/doc/src/sgml/ref/create_table.sgml,v
retrieving revision 1.108
diff -p -c -r1.108 create_table.sgml
*** doc/src/sgml/ref/create_table.sgml 3 Jun 2007 17:06:03 - 1.108
--- doc/src/sgml/ref/create_table.sgml 9 Jul 2007 04:34:40 -
*** PostgreSQL documentation
*** 23,29
CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE replaceable class=PARAMETERtable_name/replaceable ( [
{ replaceable class=PARAMETERcolumn_name/replaceable replaceable class=PARAMETERdata_type/replaceable [ DEFAULT replaceabledefault_expr/ ] [ replaceable class=PARAMETERcolumn_constraint/replaceable [ ... ] ]
| replaceabletable_constraint/replaceable
! | LIKE replaceableparent_table/replaceable [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS } ] ... }
[, ... ]
] )
[ INHERITS ( replaceableparent_table/replaceable [, ... ] ) ]
--- 23,29
CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE replaceable class=PARAMETERtable_name/replaceable ( [
{ replaceable class=PARAMETERcolumn_name/replaceable replaceable class=PARAMETERdata_type/replaceable [ DEFAULT replaceabledefault_expr/ ] [ replaceable class=PARAMETERcolumn_constraint/replaceable [ ... ] ]
| replaceabletable_constraint/replaceable
! | LIKE replaceableparent_table/replaceable [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS | INDEXES } ] ... }
[, ... ]
] )
[ INHERITS ( replaceableparent_table/replaceable [, ... ] ) ]
*** and replaceable class=PARAMETERtable
*** 237,243
/varlistentry
varlistentry
! termliteralLIKE replaceableparent_table/replaceable [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS } ]/literal/term
listitem
para
The literalLIKE/literal clause specifies a table from which
--- 237,243
/varlistentry
varlistentry
! termliteralLIKE replaceableparent_table/replaceable [ { INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS | INDEXES } ]/literal/term
listitem
para
The literalLIKE/literal clause specifies a table from which
*** and replaceable class=PARAMETERtable
*** 266,275
requested, all check constraints are copied.
/para
para
Note also that unlike literalINHERITS/literal, copied columns and
constraints are not merged with similarly named columns and constraints.
If the same name is specified explicitly or in another
! literalLIKE/literal clause an error is signalled.
/para
/listitem
/varlistentry
--- 266,280
requested, all check constraints are copied.
/para
para
+ Any indexes on the original table will not be created on the new
+ table, unless the literalINCLUDING INDEXES/literal clause is
+ specified.
+ /para
+ para
Note also that unlike literalINHERITS/literal, copied columns and
constraints are not merged with similarly named columns and constraints.
If the same name is specified explicitly or in another
! literalLIKE/literal clause, an error is signalled.
/para
/listitem
/varlistentry
Index: src/backend/bootstrap/bootparse.y
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/bootstrap/bootparse.y,v
retrieving revision 1.88
diff -p -c -r1.88 bootparse.y
*** src/backend/bootstrap/bootparse.y 13 Mar 2007 00:33:39 - 1.88
--- src/backend/bootstrap/bootparse.y 8 Jul 2007 00:49:29 -
*** Boot_DeclareIndexStmt:
*** 252,258
Re: [PATCHES] [DOCS] rename of a view
On Tue, 2007-07-03 at 09:56 -0400, Tom Lane wrote: Neil, according to http://developer.postgresql.org/index.php/Todo:PatchStatus you have accepted two patches to review --- what is happening with those? Yeah, sorry, forgot about those -- I'll take a look at them tonight. -Neil ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [DOCS] rename of a view
On Sun, 2007-07-01 at 12:55 -0700, David Fetter wrote: Here's a new patch + file. This one allows ALTER [SEQUENCE | VIEW] to work only on the respective database objects, but permits the old ALTER TABLE syntax. Applied with some fixes. Thanks for the patch. ALTER VIEW ... SET SCHEMA might be another worthwhile thing to add, for consistency. -Neil ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [DOCS] rename of a view
On Mon, 2007-02-07 at 23:13 -0400, Tom Lane wrote: Er, was this on the agenda for 8.3? Well, it seemed fairly harmless to me (no behavioral changes and very little new code, just syntax), so I didn't see a compelling reason to delay applying it for a few months. But I can revert it if you'd prefer. -Neil ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] [DOCS] rename of a view
On Sun, 2007-01-07 at 12:55 -0700, David Fetter wrote: Here's a new patch + file. This one allows ALTER [SEQUENCE | VIEW] to work only on the respective database objects, but permits the old ALTER TABLE syntax. How about taking a look at the more thorough documentation updates Tom suggested? -Neil ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] SPI-header-files safe for C++-compiler
On Thu, 2007-06-28 at 12:08 +0200, Jacob Rief wrote: If there is any chance to get this patch applied, I will of course regenerate it out of the HEAD-revision of the CVS repository. I don't see a reason to reject the patch. All the arguments about why using C++ in the backend is ill-advised are well-taken, but the patch does *not* require making a real commitment to making C++ usable as a backend extension language, it just obviates the need for some people to patch the source. So +1 from me. -Neil ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] SPI-header-files safe for C++-compiler
On Thu, 2007-28-06 at 02:07 -0400, Tom Lane wrote:
It was already pointed out upthread that wrapping the inclusions in
extern C {...} would fix the identifier part of the problem from
the user side
No, my point about extern C was that I don't think we need to add it
to the Postgres headers. As far as I can see, it doesn't help with the
identifier part of the problem:
% cat foo.cpp
extern C {
#include postgres.h
#include commands/trigger.h
}
int
main(void)
{
return 0;
}
% g++ -I$HOME/postgres/head/prefix/include/server foo.cpp
/home/neilc/postgres/head/prefix/include/server/nodes/primnodes.h:1078:
error: expected unqualified-id before 'using'
/home/neilc/postgres/head/prefix/include/server/nodes/primnodes.h:1078:
error: abstract declarator 'List*' used as declaration
/home/neilc/postgres/head/prefix/include/server/nodes/primnodes.h:1078:
error: expected ';' before 'using'
/home/neilc/postgres/head/prefix/include/server/nodes/parsenodes.h:167:
error: expected unqualified-id before 'typeid'
/home/neilc/postgres/head/prefix/include/server/nodes/parsenodes.h:238:
error: expected unqualified-id before 'typename'
[ ... and so on ... ]
-Neil
---(end of broadcast)---
TIP 6: explain analyze is your friend
Re: [PATCHES] psql: add volatility to \df+
On Tue, 2007-26-06 at 15:10 -0700, Neil Conway wrote: Attached is a patch that adds information about function volatility to the output of psql's \df+ slash command. I'll apply this to HEAD tomorrow, barring any objections. Applied. -Neil ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] psql: add volatility to \df+
On Wed, 2007-27-06 at 00:16 -0400, Alvaro Herrera wrote: +1 as well but I'm wondering whether the output is too wide. Well, the output of \df+ is already likely to be wider than 72 characters, so I'm not sure the patch would really make the situation materially worse (\df+ nextval is 118 characters wide without the patch, for example). You could also argue that if the user specifies +, presumably they are less worried about a concise output format. Is it possible to find some shorter string to convey the same meaning? Nothing obvious comes to mind, but I'm not opposed in principle. Any suggestions? -Neil ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] SPI-header-files safe for C++-compiler
On Thu, 2007-06-28 at 00:43 +0200, Jacob Rief wrote:
But the C++-compiler rejects to compile legal C code,
because some of the included Postgres-headers, ie. postgres.h,
executor/spi.h, commands/trigger.h, fmgr.h use a few C++ keywords to
defined a some struct members and function arguments. The incriminating
C++-keywords used in the Postgres-headers are: 'typeid', 'typename' and
'using'.
It would do no harm to the Postgres-sources if these keywords would be
replaced with a similar identifier.
No objection, although it would be nice if we could find something nicer
to rename using to than using_. What about using_clause or
using_list?
You also changed namespace = name_space in builtins.h; is that
necessary?
I wrote a patch which applies cleanly onto version 8.2.4
Patches should be submitted against the CVS HEAD code: we wouldn't want
to apply a patch like this to the REL8_2_STABLE branch, in any case.
BTW, I notice the patch also adds 'extern C { ... }' statements to a
few random header files. Can't client programs do this before including
the headers, if necessary?
-Neil
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [PATCHES] SPI-header-files safe for C++-compiler
On Wed, 2007-06-27 at 20:51 -0400, Andrew Dunstan wrote: He's used the usual #ifdef __cplusplus wrapper - isn't that good enough? Well, there's nothing wrong with it per se, I'm just wondering (1) what the proper set of headers to add it to is -- the patch just does a handful (2) whether we need to bother doing it at all -- can't clients do the extern C trick before including any Postgres headers? -Neil ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] SPI-header-files safe for C++-compiler
On Thu, 2007-28-06 at 01:15 -0400, Tom Lane wrote: The patch as given merely renames some random identifiers that happen to be keywords in some non-C language (thereby breaking not only a lot of core backend code, which we can fix, but who knows what other user-written extensions that we can't fix so easily). The fact is, any user-written extensions that depend on types defined in parsenodes.h and primnodes.h are going to get broken all the time *anyway*, so I don't see this as a major disadvantage. Doing s/typename/type_name/g is likely to be the least of your concerns if your extension integrates with Postgres that closely. It would be one thing if we made a significant effort to preserve internal API stability -- but we plainly do not. (See the varlena API changes made in 8.3 for an example of an API change that will break far more user-written extensions.) The *real* problem here is that to make this useful, we have to buy into the assumption that C++ should work in the backend. I agree that C++ in the backend is always going to be a little hokey, but (a) I don't agree that it is completely impossible (b) if we can make people's lives a bit easier, I don't see a good reason not to. Like it or not, people have called into C++ libraries from C UDFs in the past, and likely will do so in the future. The error handling assumptions are completely incompatible (setjmp and throw do not usually interoperate) AFAIK this is resolvable with some degree of pain: before entering C++ land, wrap the call site in a C++ exception handler, and before calling back into Postgres, use a PG_TRY() block to rethrow elog(ERROR) as a C++ exception (and then rethrow as an elog(ERROR) once you've unwound the C++ portion of the stack) ... hey, I didn't say it was clean ;-) It's also worth noting that some people use C++ as C with classes, and disallow the use of exceptions, RTTI, and that sort of stuff. Calling into such code from the backend is marginally more sane. -Neil ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
[PATCHES] psql: add volatility to \df+
Attached is a patch that adds information about function volatility to the output of psql's \df+ slash command. I'll apply this to HEAD tomorrow, barring any objections. -Neil Index: src/bin/psql/describe.c === RCS file: /home/neilc/postgres/cvs_root/pgsql/src/bin/psql/describe.c,v retrieving revision 1.155 diff -p -c -r1.155 describe.c *** src/bin/psql/describe.c 19 Mar 2007 23:38:31 - 1.155 --- src/bin/psql/describe.c 26 Jun 2007 22:01:55 - *** describeFunctions(const char *pattern, b *** 205,215 if (verbose) appendPQExpBuffer(buf, ,\n r.rolname as \%s\,\n l.lanname as \%s\,\n p.prosrc as \%s\,\n pg_catalog.obj_description(p.oid, 'pg_proc') as \%s\, ! _(Owner), _(Language), _(Source code), _(Description)); if (!verbose) --- 205,220 if (verbose) appendPQExpBuffer(buf, + ,\n CASE\n + WHEN p.provolatile = 'i' THEN 'immutable'\n + WHEN p.provolatile = 's' THEN 'stable'\n + WHEN p.provolatile = 'v' THEN 'volatile'\n + END as \%s\ ,\n r.rolname as \%s\,\n l.lanname as \%s\,\n p.prosrc as \%s\,\n pg_catalog.obj_description(p.oid, 'pg_proc') as \%s\, ! _(Volatility), _(Owner), _(Language), _(Source code), _(Description)); if (!verbose) ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] psql: add volatility to \df+
On Tue, 2007-26-06 at 20:29 -0700, Joshua D. Drake wrote: Tom Lane wrote: +1, but are there not any documentation changes to make? Sure, I'll update the psql ref page. Well here is a question (just because I haven't seen it) is there a list of functions and their volatility in the docs? Not relevant to this thread, but AFAICS there is no explicit list of the volatilities of the builtin functions. -Neil ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] psql: flush output in cursor-fetch mode
On Wed, 2007-06-20 at 15:51 -0700, Neil Conway wrote: Attached is a patch that fixes this, by calling fflush() on the psql output stream after each call to printQuery() in ExecQueryUsingCursor(). Applied to HEAD. -Neil ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] psql: flush output in cursor-fetch mode
On Thu, 2007-21-06 at 22:09 -0400, Tom Lane wrote: Seems reasonable to back-patch into 8.2 too... Okay, done. -Neil ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
[PATCHES] psql: flush output in cursor-fetch mode
psql's FETCH_COUNT feature is useful for incrementally displaying the results of a long-running query. However, psql fails to flush its output stream as new rows from the partial result set are produced, which means that partial query results may not be visible to the client until the stdio buffer is eventually flushed or the query produces its complete result set. Example: $ cat ~/test.sql -- a contrived function to get a query that slowly produces -- more rows create function slow_func() returns boolean as $$ begin perform pg_sleep(2); return true; end; $$ language plpgsql; neilc=# \i ~/test.sql CREATE FUNCTION neilc=# create table t1 (a int, b int); CREATE TABLE neilc=# insert into t1 values (5, 10), (10, 15), (20, 25), (30, 35); INSERT 0 4 neilc=# \set FETCH_COUNT 1 neilc=# select * from t1 where slow_func() is true; With CVS HEAD, no output is visible until the complete result set has been produced, at which point all 4 rows are printed. This is undesirable: since the client has gone to the trouble of FETCH'ing the rows one-at-a-time, it should display the partial result set before issuing another FETCH. Attached is a patch that fixes this, by calling fflush() on the psql output stream after each call to printQuery() in ExecQueryUsingCursor(). -Neil Index: src/bin/psql/common.c === RCS file: /home/neilc/postgres/cvs_root/pgsql/src/bin/psql/common.c,v retrieving revision 1.134 diff -p -c -r1.134 common.c *** src/bin/psql/common.c 16 Apr 2007 20:15:38 - 1.134 --- src/bin/psql/common.c 20 Jun 2007 22:34:04 - *** ExecQueryUsingCursor(const char *query, *** 1076,1081 --- 1076,1087 printQuery(results, my_popt, pset.queryFout, pset.logfile); + /* + * Make sure to flush the output stream, so intermediate + * results are visible to the client immediately. + */ + fflush(pset.queryFout); + /* after the first result set, disallow header decoration */ my_popt.topt.start_table = false; my_popt.topt.prior_records += ntuples; ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
On Mon, 2007-21-05 at 12:23 +0530, NikhilS wrote: I had spent some time on this earlier so decided to complete and send the patch to you for review. This patch supports copying of expressions, predicates, opclass, amorder, reloptions etc. The test case also contains some more additions with this patch. Please let me know if there are any issues. Attached is a revised version of this patch. Note that this pattern is always unsafe: ht_am = SearchSysCache(AMOID, ...); if (!HeapTupleIsValid(ht_am)) elog(ERROR, ...); amrec = (Form_pg_am) GETSTRUCT(ht_am); index-accessMethod = NameStr(amrec-amname); /* ... */ ReleaseSysCache(ht_am); return index; Before calling ReleaseSysCache(), all the data you need from the syscache entry needs to be deep-copied to allow subsequent access, but NameStr() doesn't do a deep-copy. Adding -DFORCE_CATCACHE_RELEASE is a useful way to catch these kinds of problems (I wonder if this is worth adding to the default CFLAGS when assertions are enabled?) I also made a bunch of editorial changes, including moving the varattnos_map_schema() call out of the loop in transformInhRelation(). BTW, comments like This function is based on code from ruleutils.c would be helpful for reviewers (whether in the patch itself or just in the email containing the patch). There's still a few things I need to fix in the patch, but I'll apply a revised version of the attached patch to HEAD tomorrow, barring any objections. -Neil Index: doc/src/sgml/ref/create_table.sgml === RCS file: /home/neilc/postgres/cvs_root/pgsql/doc/src/sgml/ref/create_table.sgml,v retrieving revision 1.107 diff -p -c -r1.107 create_table.sgml *** doc/src/sgml/ref/create_table.sgml 1 Feb 2007 00:28:18 - 1.107 --- doc/src/sgml/ref/create_table.sgml 2 Jun 2007 19:52:22 - *** and replaceable class=PARAMETERtable *** 259,275 /para para Not-null constraints are always copied to the new table. ! literalCHECK/literal constraints will only be copied if ! literalINCLUDING CONSTRAINTS/literal is specified; other types of ! constraints will never be copied. Also, no distinction is made between ! column constraints and table constraints mdash; when constraints are ! requested, all check constraints are copied. /para para Note also that unlike literalINHERITS/literal, copied columns and constraints are not merged with similarly named columns and constraints. If the same name is specified explicitly or in another ! literalLIKE/literal clause an error is signalled. /para /listitem /varlistentry --- 259,276 /para para Not-null constraints are always copied to the new table. ! literalCHECK/literal, literalUNIQUE/literal, and ! literalPRIMARY KEY/literal constraints will only be copied ! if literalINCLUDING CONSTRAINTS/literal is specified. Also, ! no distinction is made between column constraints and table ! constraints mdash; when constraints are requested, all eligible ! constraints are copied. /para para Note also that unlike literalINHERITS/literal, copied columns and constraints are not merged with similarly named columns and constraints. If the same name is specified explicitly or in another ! literalLIKE/literal clause, an error is signalled. /para /listitem /varlistentry Index: src/backend/bootstrap/bootparse.y === RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/bootstrap/bootparse.y,v retrieving revision 1.88 diff -p -c -r1.88 bootparse.y *** src/backend/bootstrap/bootparse.y 13 Mar 2007 00:33:39 - 1.88 --- src/backend/bootstrap/bootparse.y 2 Jun 2007 19:46:07 - *** Boot_DeclareIndexStmt: *** 252,258 LexIDStr($8), NULL, $10, ! NULL, NIL, false, false, false, false, false, true, false, false); do_end(); --- 252,258 LexIDStr($8), NULL, $10, ! NULL, NIL, NULL, false, false, false, false, false, true, false, false); do_end(); *** Boot_DeclareUniqueIndexStmt: *** 270,276 LexIDStr($9), NULL, $11, ! NULL, NIL, true, false, false, false, false, true, false, false); do_end(); --- 270,276 LexIDStr($9), NULL, $11, ! NULL, NIL, NULL, true, false, false, false, false, true, false, false); do_end(); Index: src/backend/commands/indexcmds.c === RCS file:
Re: [PATCHES] boolean = text explicit casts
On Mon, 2007-28-05 at 15:38 -0400, Tom Lane wrote:
More generally, I'm really hoping to get rid of bespoke text-whatever
cast functions in favor of using datatypes' I/O functions. To what
extent can we make the boolean I/O functions serve for this? It seems
relatively painless on the input side --- just allow whitespace --- but
I suppose we can't change boolout's historical result of t/f without
causing problems.
Attached is a revised version of this patch that modifies boolin() to
ignore leading and trailing whitespace. This makes text = boolean
trivial, but boolean = text is still distinct from boolout().
Barring any objections, I'll apply this later today or tomorrow.
-Neil
Index: src/backend/utils/adt/bool.c
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/utils/adt/bool.c,v
retrieving revision 1.38
diff -p -c -r1.38 bool.c
*** src/backend/utils/adt/bool.c 5 Jan 2007 22:19:40 - 1.38
--- src/backend/utils/adt/bool.c 30 May 2007 18:55:26 -
***
*** 15,20
--- 15,22
#include postgres.h
+ #include ctype.h
+
#include libpq/pqformat.h
#include utils/builtins.h
***
*** 33,73
Datum
boolin(PG_FUNCTION_ARGS)
{
! char *b = PG_GETARG_CSTRING(0);
! switch (*b)
{
case 't':
case 'T':
! if (pg_strncasecmp(b, true, strlen(b)) == 0)
PG_RETURN_BOOL(true);
break;
case 'f':
case 'F':
! if (pg_strncasecmp(b, false, strlen(b)) == 0)
PG_RETURN_BOOL(false);
break;
case 'y':
case 'Y':
! if (pg_strncasecmp(b, yes, strlen(b)) == 0)
PG_RETURN_BOOL(true);
break;
case '1':
! if (pg_strncasecmp(b, 1, strlen(b)) == 0)
PG_RETURN_BOOL(true);
break;
case 'n':
case 'N':
! if (pg_strncasecmp(b, no, strlen(b)) == 0)
PG_RETURN_BOOL(false);
break;
case '0':
! if (pg_strncasecmp(b, 0, strlen(b)) == 0)
PG_RETURN_BOOL(false);
break;
--- 35,88
Datum
boolin(PG_FUNCTION_ARGS)
{
! const char *in_str = PG_GETARG_CSTRING(0);
! const char *str;
! size_t len;
!
! /*
! * Skip leading and trailing whitespace
! */
! str = in_str;
! while (isspace((unsigned char) *str))
! str++;
!
! len = strlen(str);
! while (len 0 isspace((unsigned char) str[len - 1]))
! len--;
! switch (*str)
{
case 't':
case 'T':
! if (pg_strncasecmp(str, true, len) == 0)
PG_RETURN_BOOL(true);
break;
case 'f':
case 'F':
! if (pg_strncasecmp(str, false, len) == 0)
PG_RETURN_BOOL(false);
break;
case 'y':
case 'Y':
! if (pg_strncasecmp(str, yes, len) == 0)
PG_RETURN_BOOL(true);
break;
case '1':
! if (pg_strncasecmp(str, 1, len) == 0)
PG_RETURN_BOOL(true);
break;
case 'n':
case 'N':
! if (pg_strncasecmp(str, no, len) == 0)
PG_RETURN_BOOL(false);
break;
case '0':
! if (pg_strncasecmp(str, 0, len) == 0)
PG_RETURN_BOOL(false);
break;
*** boolin(PG_FUNCTION_ARGS)
*** 77,83
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg(invalid input syntax for type boolean: \%s\, b)));
/* not reached */
PG_RETURN_BOOL(false);
--- 92,98
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! errmsg(invalid input syntax for type boolean: \%s\, in_str)));
/* not reached */
PG_RETURN_BOOL(false);
*** boolsend(PG_FUNCTION_ARGS)
*** 127,132
--- 142,178
PG_RETURN_BYTEA_P(pq_endtypsend(buf));
}
+ /*
+ * textbool - cast function for text = bool
+ */
+ Datum
+ textbool(PG_FUNCTION_ARGS)
+ {
+ Datum in_text = PG_GETARG_DATUM(0);
+ char *str;
+
+ str = DatumGetCString(DirectFunctionCall1(textout, in_text));
+
+ PG_RETURN_DATUM(DirectFunctionCall1(boolin, CStringGetDatum(str)));
+ }
+
+ /*
+ * booltext - cast function for bool = text
+ */
+ Datum
+ booltext(PG_FUNCTION_ARGS)
+ {
+ bool arg1 = PG_GETARG_BOOL(0);
+ char *str;
+
+ if (arg1)
+ str = true;
+ else
+ str = false;
+
+ PG_RETURN_DATUM(DirectFunctionCall1(textin, CStringGetDatum(str)));
+ }
+
/*
* PUBLIC ROUTINES *
Index: src/include/catalog/pg_cast.h
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/include/catalog/pg_cast.h,v
retrieving revision 1.32
diff -p -c -r1.32 pg_cast.h
*** src/include/catalog/pg_cast.h 2 Apr 2007 03:49:40 - 1.32
--- src/include/catalog/pg_cast.h 30 May 2007 17:59:20 -
*** DATA(insert ( 1700 25 1688 i ));
*** 302,307
--- 302,309
DATA(insert ( 25 1700 1686 e ));
DATA(insert ( 142 25 2922 e ));
DATA(insert ( 25 142 2896 e ));
+ DATA(insert ( 16 25 2971 e ));
+ DATA(insert (
Re: [PATCHES] boolean = text explicit casts
On Wed, 2007-30-05 at 21:23 +0200, Peter Eisentraut wrote: I'm not sure what your rationale was for creating lower-case words instead of upper case, except for it looks nicer. Is there a technical reason? There's no real technical reason: the standard says upper-case, but PG's general philosophy of case folding would suggest folding to lower-case. If we were compliant with the spec's case folding requirements then emitting uppercase would be the clear choice, but since we aren't, I don't have strong feelings either way. -Neil ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Interval input: usec, msec
On Mon, 2007-28-05 at 10:50 -0400, Tom Lane wrote: I'd argue that it's an oversight. I don't have a problem with adding up the values of units that really translate to the same thing (eg, '1 week 1 day' - '8 days'), but I think '1 second 2 second' should be rejected because it's almost certainly user error. I don't see why 1 week 1 week is any less likely to be user error than 1 second 1 second. Does your patch allow '1 millisec 2 microsec', which should be allowed by this argument? Yes: in order to allow the above input, the straightforward coding also allows 1 second 2 second. I suspect that to make it work unsurprisingly, we'd need to allocate a distinct tmask bit to each logically distinct unit. Okay, attached is a patch that does this. To summarize, the changes are: * add tmask bits for msec, usec (I reordered the #defines to keep them logically contiguous, but AFAICS this isn't necessary) * if the user specifies multiple instances of usec, msec, or sec, reject as invalid input * if the user specifies a fractional second (5.5 seconds), also consider that to be millisecond and microsecond input from the POV of rejecting duplicate units. So 5.5 seconds 1 millisecond will be rejected. Docs and regression tests updated. If people are happy with the above behavior, I'll commit this to HEAD shortly (today/tomorrow), and perhaps backport it: not accepting 1 msec and similar inputs is a clear bug, IMHO. BTW, does anyone know why a few of the regression tests (tinterval, point, geometry, etc.) explicitly disable and then re-enable GEQO? AFAICS the regression tests are just doing fairly routine two-table joins, so GEQO will not be invoked with a sane configuration anyway. -Neil Index: doc/src/sgml/datatype.sgml === RCS file: /home/neilc/postgres/cvs_root/pgsql/doc/src/sgml/datatype.sgml,v retrieving revision 1.201 diff -p -c -r1.201 datatype.sgml *** doc/src/sgml/datatype.sgml 21 May 2007 17:10:28 - 1.201 --- doc/src/sgml/datatype.sgml 28 May 2007 17:11:14 - *** January 8 04:05:06 1999 PST *** 1880,1886 /programlisting Where: replaceablequantity/ is a number (possibly signed); ! replaceableunit/ is literalsecond/literal, literalminute/literal, literalhour/literal, literalday/literal, literalweek/literal, literalmonth/literal, literalyear/literal, literaldecade/literal, literalcentury/literal, literalmillennium/literal, --- 1880,1887 /programlisting Where: replaceablequantity/ is a number (possibly signed); ! replaceableunit/ is literalmicrosecond/literal, ! literalmillisecond/literal, literalsecond/literal, literalminute/literal, literalhour/literal, literalday/literal, literalweek/literal, literalmonth/literal, literalyear/literal, literaldecade/literal, literalcentury/literal, literalmillennium/literal, Index: src/backend/utils/adt/datetime.c === RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/utils/adt/datetime.c,v retrieving revision 1.179 diff -p -c -r1.179 datetime.c *** src/backend/utils/adt/datetime.c 27 May 2007 20:32:16 - 1.179 --- src/backend/utils/adt/datetime.c 28 May 2007 17:25:09 - *** DecodeDateTime(char **field, int *ftype, *** 924,929 --- 924,930 #else *fsec = frac; #endif + tmask = DTK_ALL_SECS_M; } break; *** DecodeTimeOnly(char **field, int *ftype, *** 1699,1704 --- 1700,1706 #else *fsec = frac; #endif + tmask = DTK_ALL_SECS_M; } break; *** DecodeInterval(char **field, int *ftype, *** 2805,2810 --- 2807,2813 #else *fsec += (val + fval) * 1e-6; #endif + tmask = DTK_M(MICROSECOND); break; case DTK_MILLISEC: *** DecodeInterval(char **field, int *ftype, *** 2813,2818 --- 2816,2822 #else *fsec += (val + fval) * 1e-3; #endif + tmask = DTK_M(MILLISECOND); break; case DTK_SECOND: *** DecodeInterval(char **field, int *ftype, *** 2822,2828 #else *fsec += fval; #endif ! tmask = DTK_M(SECOND); break; case DTK_MINUTE: --- 2826,2840 #else *fsec += fval; #endif ! /* ! * If any subseconds were specified, consider ! * this microsecond and millisecond input as ! * well. ! */ ! if (fval == 0) ! tmask = DTK_M(SECOND); ! else ! tmask = DTK_ALL_SECS_M; break; case DTK_MINUTE: Index: src/include/utils/datetime.h === RCS file: /home/neilc/postgres/cvs_root/pgsql/src/include/utils/datetime.h,v retrieving revision 1.65 diff -p -c -r1.65 datetime.h ***
[PATCHES] boolean = text explicit casts
I noticed that SQL:2003 specifies explicit casts between boolean and
the character string types. Attached is a patch that implements them,
and adds some simple regression tests.
A few points worth noting:
(1) The SQL spec requires that text::boolean trim leading and trailing
whitespace from the input
(2) The spec also requires that boolean::varchar(n) should raise an
error if n is not large enough to accomodate the textual
representation of the boolean value. We currently truncate:
= select true::boolean::varchar(3);
varchar
-
TRU
Not sure offhand if there's an easy way to satisfy the spec's
requirement...
(3) The spec suggests that true/false should be upper-cased when
converted to text, so that's what I've implemented, but one could argue
that converting to lower-case would be more consistent with PG's general
approach to case folding.
-Neil
Index: src/backend/utils/adt/bool.c
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/utils/adt/bool.c,v
retrieving revision 1.38
diff -p -c -r1.38 bool.c
*** src/backend/utils/adt/bool.c 5 Jan 2007 22:19:40 - 1.38
--- src/backend/utils/adt/bool.c 28 May 2007 18:47:59 -
*** boolsend(PG_FUNCTION_ARGS)
*** 127,132
--- 127,177
PG_RETURN_BYTEA_P(pq_endtypsend(buf));
}
+ /*
+ * textbool - cast function for text = bool
+ */
+ Datum
+ textbool(PG_FUNCTION_ARGS)
+ {
+ text *arg1 = PG_GETARG_TEXT_P(0);
+ text *trim;
+ char *trim_str;
+
+ trim = DatumGetTextP(DirectFunctionCall1(btrim1,
+ PointerGetDatum(arg1)));
+ trim_str = DatumGetCString(DirectFunctionCall1(textout,
+ PointerGetDatum(trim)));
+
+ if (pg_strcasecmp(trim_str, true) == 0)
+ PG_RETURN_BOOL(true);
+ else if (pg_strcasecmp(trim_str, false) == 0)
+ PG_RETURN_BOOL(false);
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_CHARACTER_VALUE_FOR_CAST),
+ errmsg(invalid input syntax for type boolean: \%s\,
+ trim_str)));
+
+ PG_RETURN_BOOL(false); /* keep the compiler quiet */
+ }
+
+ /*
+ * booltext - cast function for bool = text
+ */
+ Datum
+ booltext(PG_FUNCTION_ARGS)
+ {
+ bool arg1 = PG_GETARG_BOOL(0);
+ char *str;
+
+ if (arg1)
+ str = TRUE;
+ else
+ str = FALSE;
+
+ PG_RETURN_DATUM(DirectFunctionCall1(textin, CStringGetDatum(str)));
+ }
+
/*
* PUBLIC ROUTINES *
Index: src/include/catalog/pg_cast.h
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/include/catalog/pg_cast.h,v
retrieving revision 1.32
diff -p -c -r1.32 pg_cast.h
*** src/include/catalog/pg_cast.h 2 Apr 2007 03:49:40 - 1.32
--- src/include/catalog/pg_cast.h 28 May 2007 18:41:38 -
*** DATA(insert ( 1700 25 1688 i ));
*** 302,307
--- 302,309
DATA(insert ( 25 1700 1686 e ));
DATA(insert ( 142 25 2922 e ));
DATA(insert ( 25 142 2896 e ));
+ DATA(insert ( 16 25 2971 e ));
+ DATA(insert ( 25 16 2970 e ));
/*
* Cross-category casts to and from VARCHAR
*** DATA(insert ( 1700 1043 1688 a ));
*** 342,347
--- 344,351
DATA(insert ( 1043 1700 1686 e ));
DATA(insert ( 142 1043 2922 e ));
DATA(insert ( 1043 142 2896 e ));
+ DATA(insert ( 16 1043 2971 e ));
+ DATA(insert ( 1043 16 2970 e ));
/*
* Cross-category casts to and from BPCHAR
Index: src/include/catalog/pg_proc.h
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/include/catalog/pg_proc.h,v
retrieving revision 1.456
diff -p -c -r1.456 pg_proc.h
*** src/include/catalog/pg_proc.h 21 May 2007 17:10:29 - 1.456
--- src/include/catalog/pg_proc.h 28 May 2007 18:40:53 -
*** DESCR(List all files in a directory);
*** 3221,3226
--- 3221,3230
DATA(insert OID = 2626 ( pg_sleep PGNSP PGUID 12 1 0 f f t f v 1 2278 701 _null_ _null_ _null_ pg_sleep - _null_ ));
DESCR(Sleep for the specified time in seconds);
+ DATA(insert OID = 2970 ( boolean PGNSP PGUID 12 1 0 f f t f i 1 16 25 _null_ _null_ _null_ textbool - _null_ ));
+ DESCR(text to boolean);
+ DATA(insert OID = 2971 ( textPGNSP PGUID 12 1 0 f f t f i 1 25 16 _null_ _null_ _null_ booltext - _null_ ));
+ DESCR(boolean to text);
/* Aggregates (moved here from pg_aggregate for 7.3) */
Index: src/include/utils/builtins.h
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/include/utils/builtins.h,v
retrieving revision 1.293
diff -p -c -r1.293 builtins.h
*** src/include/utils/builtins.h 17 May 2007 23:31:49 - 1.293
--- src/include/utils/builtins.h 28 May 2007 18:13:15 -
*** extern Datum boolin(PG_FUNCTION_ARGS);
*** 70,75
--- 70,77
extern Datum
Re: [PATCHES] Interval input: usec, msec
On Mon, 2007-28-05 at 15:05 -0400, Tom Lane wrote: Right. I guess you misunderstood me: I was arguing for rejecting double occurrences of the same unit name, but not occurrences of different unit names that we happen to map into the same interval field internally. Makes sense to me. I'll send a patch for this shortly (but I'll commit the patch for the usec/msec issue first -- this second change shouldn't be backpatched). I forget --- are the tmask bits used in stored typmod values for intervals? It'd probably be best not to change the meanings of typmod bits, since those are visible to client code if it wants to look. According to datetime.h, only some of the tmask bits are used in stored typmods: YEAR, MONTH, DAY, HOUR, MINUTE, and SECOND, and the patch doesn't modify any of those. So I think we're okay. Hmmm ... if you check the cvs history for those tests you might find some evidence. Modify regression tests to allow GEQ optimizer (order results)., according to the 1997 CVS commit from Thomas Lockhart that added the lines in question. Presumably that is no longer relevant. Nothing unexpected happens if the disabling of GEQO is removed, so I'll apply the attached patch shortly barring any objections. -Neil Index: src/test/regress/expected/geometry.out === RCS file: /home/neilc/postgres/cvs_root/pgsql/src/test/regress/expected/geometry.out,v retrieving revision 1.25 diff -p -c -r1.25 geometry.out *** src/test/regress/expected/geometry.out 1 Feb 2007 19:10:30 - 1.25 --- src/test/regress/expected/geometry.out 29 May 2007 02:16:43 - *** SELECT '' AS twenty, b.f1 / p.f1 AS rota *** 284,290 -- -- Paths -- - SET geqo TO 'off'; SELECT '' AS eight, npoints(f1) AS npoints, f1 AS path FROM PATH_TBL; eight | npoints | path ---+-+--- --- 284,289 *** SELECT '' AS eight, p1.f1 * point '(2,-1 *** 337,343 | ((34,13),(40,15)) (8 rows) - RESET geqo; -- -- Polygons -- --- 336,341 Index: src/test/regress/expected/geometry_1.out === RCS file: /home/neilc/postgres/cvs_root/pgsql/src/test/regress/expected/geometry_1.out,v retrieving revision 1.10 diff -p -c -r1.10 geometry_1.out *** src/test/regress/expected/geometry_1.out 1 Feb 2007 20:11:18 - 1.10 --- src/test/regress/expected/geometry_1.out 29 May 2007 02:16:50 - *** SELECT '' AS twenty, b.f1 / p.f1 AS rota *** 284,290 -- -- Paths -- - SET geqo TO 'off'; SELECT '' AS eight, npoints(f1) AS npoints, f1 AS path FROM PATH_TBL; eight | npoints | path ---+-+--- --- 284,289 *** SELECT '' AS eight, p1.f1 * point '(2,-1 *** 337,343 | ((34,13),(40,15)) (8 rows) - RESET geqo; -- -- Polygons -- --- 336,341 Index: src/test/regress/expected/geometry_2.out === RCS file: /home/neilc/postgres/cvs_root/pgsql/src/test/regress/expected/geometry_2.out,v retrieving revision 1.7 diff -p -c -r1.7 geometry_2.out *** src/test/regress/expected/geometry_2.out 1 Feb 2007 20:11:18 - 1.7 --- src/test/regress/expected/geometry_2.out 29 May 2007 02:16:56 - *** SELECT '' AS twenty, b.f1 / p.f1 AS rota *** 284,290 -- -- Paths -- - SET geqo TO 'off'; SELECT '' AS eight, npoints(f1) AS npoints, f1 AS path FROM PATH_TBL; eight | npoints | path ---+-+--- --- 284,289 *** SELECT '' AS eight, p1.f1 * point '(2,-1 *** 337,343 | ((34,13),(40,15)) (8 rows) - RESET geqo; -- -- Polygons -- --- 336,341 Index: src/test/regress/expected/point.out === RCS file: /home/neilc/postgres/cvs_root/pgsql/src/test/regress/expected/point.out,v retrieving revision 1.16 diff -p -c -r1.16 point.out *** src/test/regress/expected/point.out 9 Jan 2007 02:14:16 - 1.16 --- src/test/regress/expected/point.out 29 May 2007 02:16:00 - *** SELECT '' AS six, p.f1, p.f1 - point ' *** 105,111 | (5.1,34.5) | 34.8749193547455 (6 rows) - SET geqo TO 'off'; SELECT '' AS thirtysix, p1.f1 AS point1, p2.f1 AS point2, p1.f1 - p2.f1 AS dist FROM POINT_TBL p1, POINT_TBL p2 ORDER BY dist, p1.f1[0], p2.f1[0]; --- 105,110 *** SELECT '' AS three, p1.f1 AS point1, p2. *** 222,225 | (5.1,34.5) | (10,10) | 24.9851956166046 (3 rows) - RESET geqo; --- 221,223 Index: src/test/regress/expected/tinterval.out === RCS file: /home/neilc/postgres/cvs_root/pgsql/src/test/regress/expected/tinterval.out,v retrieving revision 1.12 diff
Re: [PATCHES] Interval input: usec, msec
On Mon, 2007-28-05 at 13:54 -0400, Neil Conway wrote: Okay, attached is a patch that does this. To summarize, the changes are: * add tmask bits for msec, usec (I reordered the #defines to keep them logically contiguous, but AFAICS this isn't necessary) * if the user specifies multiple instances of usec, msec, or sec, reject as invalid input * if the user specifies a fractional second (5.5 seconds), also consider that to be millisecond and microsecond input from the POV of rejecting duplicate units. So 5.5 seconds 1 millisecond will be rejected. Applied to HEAD, backported to 8.2 and 8.1 After I applied the patches, it occurred to me that this patch actually slightly changes previous behavior: the previous coding accepted inputs like 1 microsecond 1 microsecond 1 second, whereas the patch rejects this input. I don't think this is a major problem (input of this kind is likely a client bug), but I figured I'd mention it... -Neil ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
[PATCHES] Interval input: usec, msec
neilc=# select '1 day 1 millisecond'::interval; interval 1 day 00:00:00.001 (1 row) neilc=# select '1 millisecond'::interval; ERROR: invalid input syntax for type interval: 1 millisecond neilc=# select '0.001 seconds'::interval; interval -- 00:00:00.001 (1 row) Similarly for microsecond. I think allowing the first input but not the second is pretty inconsistent. Attached is a patch that fixes this for both milliseconds and microseconds. The previous coding allowed x seconds y milliseconds to be specified, so this patch preserves that behavior. It *also* allows x seconds y seconds as valid input, although you could make a consistency argument for that anyway: (with CVS HEAD, unpatched) neilc=# select '5 days 10 days'::interval; interval -- 15 days neilc=# select '1 week 2 week 3 weeks'::interval; interval -- 42 days neilc=# select '1 second 2 second'::interval; ERROR: invalid input syntax for type interval: 1 second 2 second Is there any reason to why DecodeInterval() is willing to accept multiple specifications for some time units but not others? -Neil Index: src/backend/utils/adt/datetime.c === RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/utils/adt/datetime.c,v retrieving revision 1.179 diff -p -c -r1.179 datetime.c *** src/backend/utils/adt/datetime.c 27 May 2007 20:32:16 - 1.179 --- src/backend/utils/adt/datetime.c 28 May 2007 04:37:15 - *** DecodeInterval(char **field, int *ftype, *** 2805,2810 --- 2805,2811 #else *fsec += (val + fval) * 1e-6; #endif + tmask = (fmask DTK_M(SECOND)) ? 0 : DTK_M(SECOND); break; case DTK_MILLISEC: *** DecodeInterval(char **field, int *ftype, *** 2813,2818 --- 2814,2820 #else *fsec += (val + fval) * 1e-3; #endif + tmask = (fmask DTK_M(SECOND)) ? 0 : DTK_M(SECOND); break; case DTK_SECOND: *** DecodeInterval(char **field, int *ftype, *** 2822,2828 #else *fsec += fval; #endif ! tmask = DTK_M(SECOND); break; case DTK_MINUTE: --- 2824,2830 #else *fsec += fval; #endif ! tmask = (fmask DTK_M(SECOND)) ? 0 : DTK_M(SECOND); break; case DTK_MINUTE: ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [DOCS] Autovacuum and XID wraparound
On Tue, 2007-15-05 at 09:07 -0400, Alvaro Herrera wrote: I agree, the note should be added there (but it should be a short one and refer the reader someplace else for more complete details). I've applied the attached patch to HEAD and REL8_2_STABLE. -Neil Index: doc/src/sgml/config.sgml === RCS file: /home/neilc/postgres/cvs_root/pgsql/doc/src/sgml/config.sgml,v retrieving revision 1.122 diff -c -p -r1.122 config.sgml *** doc/src/sgml/config.sgml 20 Apr 2007 02:37:37 - 1.122 --- doc/src/sgml/config.sgml 15 May 2007 15:04:35 - *** SELECT * FROM parent WHERE key = 2400; *** 3172,3177 --- 3172,3183 This parameter can only be set in the filenamepostgresql.conf/ file or on the server command line. /para +para + Note that even when this parameter is disabled, the system + will periodically launch autovacuum processes in order to + prevent transaction ID wraparound. See xref + linkend=vacuum-for-wraparound for more information. +/para /listitem /varlistentry ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [DOCS] Autovacuum and XID wraparound
On Mon, 2007-14-05 at 16:22 -0400, Bruce Momjian wrote: I agree with Tom. I don't think the current behavior is a major issue for users for it to be mentioned more than it already is Are you really suggesting that we shouldn't modify config.sgml to note that autovacuum = off does not actually imply that the autovacuum daemon is disabled? ISTM that plainly violates the principle of least surprise -- it is almost the definition of what an entry in config.sgml *should* include. though if you want to move one of those, we can do that. So the change would be okay if we also removed one of the other mentions in an unrelated section of the manual? I don't see the logic. -Neil ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
On Fri, 2007-27-04 at 18:59 -0400, Neil Conway wrote: This patch needs more work. Has a revised version of this patch been submitted? -Neil ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [DOCS] Autovacuum and XID wraparound
On Sun, 2007-13-05 at 22:06 -0400, Tom Lane wrote: This fact is already documented in at least three places; do we really need two more? I think we need to at least modify the documentation for the autovacuum GUC parameter, which currently states only that it controls whether the server should run the autovacuum launcher daemon -- this is not strictly true, and in any case, it isn't the whole story. The proposed addition to postgresql.conf seems particularly over-the-top I agree that this information doesn't really belong in postgresql.conf. -Neil ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] [DOCS] OS/X startup scripts
On Sun, 2007-13-05 at 18:58 -0700, David Fetter wrote: cvs diff works just great until you want to add or remove a file without write permissions to the CVS repository, i.e. when you've checked out as anonymous. Personally, I usually work against a checkout from a local mirror of the CVS repository (which you can create via cvsup or rsync). With that setup, cvs add and cvs diff -N work fine, since you can arrange for write access to the local mirror. (I'm always surprised to hear that anyone does a non-trivial amount of work on Postgres without setting up a CVS mirror...) -Neil ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Hash function for numeric (WIP)
On Sun, 2007-06-05 at 21:30 -0400, Tom Lane wrote: It'd be a good idea if you repeat the previous number-of-collisions experiment on this code. I repeated the same experiment, and got essentially the same number of collisions (829 collisions on ~2 million randomly generated numerics, with 273 duplicates). Since the modified hash still uses hash_any() and really only differs when there are leading/trailing zeros, this is consistent with what I'd expect. Patch applied to HEAD. -Neil ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Implemented current_query
On Mon, 2007-07-05 at 19:48 +0100, Tomas Doran wrote: As suggested in the TODO list (and as I need the functionality myself), I have implemented the current_query interface to debug_query_string. Comments: * docs need a bit more detail (they should emphasize that it is the current query string submitted by the client, as opposed to the currently executing SPI command or the like). Also, the docs currently claim current_query() returns name. * use textin() to convert C-style strings to text, rather than constructing a text datum by hand * perhaps we can get away with marking current_query() stable? * AFAIK debug_query_string() still does the wrong thing when the user submits multiple queries in a single protocol message (separated by semi-colons). Not sure there's a way to fix that that is both easy and efficient, though... -Neil ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Enable integer datetimes by default
On Sun, 2007-06-05 at 00:20 -0400, Andrew Dunstan wrote: I object to the short notice. I think we need to give people a chance to adjust their configs Sure, I can wait a few days (although if we're going to do this for 8.3, we should do it promptly). On reflection, it might actually be wiser to delay making this change until the beginning of the 8.4 cycle... -Neil ---(end of broadcast)--- TIP 6: explain analyze is your friend
[PATCHES] Enable integer datetimes by default
Attached is a patch that enables integer datetimes by default, per
recent discussion on -hackers. It makes --enable-integer-datetimes the
default, and documents the --disable-integer-datetimes configure
option as a means to get the previous default behavior.
Barring any objections, I'll apply this to HEAD tomorrow.
-Neil
Index: configure
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/configure,v
retrieving revision 1.547
diff -c -p -r1.547 configure
*** configure 4 May 2007 15:20:50 - 1.547
--- configure 6 May 2007 03:10:08 -
*** if test -n $ac_init_help; then
*** 859,865
Optional Features:
--disable-FEATURE do not include FEATURE (same as --enable-FEATURE=no)
--enable-FEATURE[=ARG] include FEATURE [ARG=yes]
! --enable-integer-datetimes enable 64-bit integer date/time support
--enable-nls[=LANGUAGES] enable Native Language Support
--disable-shareddo not build shared libraries
--disable-rpath do not embed shared library search path in executables
--- 859,865
Optional Features:
--disable-FEATURE do not include FEATURE (same as --enable-FEATURE=no)
--enable-FEATURE[=ARG] include FEATURE [ARG=yes]
! --disable-integer-datetimes disable 64-bit integer date/time support
--enable-nls[=LANGUAGES] enable Native Language Support
--disable-shareddo not build shared libraries
--disable-rpath do not embed shared library search path in executables
*** fi;
*** 1699,1705
#
! # 64-bit integer date/time storage (--enable-integer-datetimes)
#
echo $as_me:$LINENO: checking whether to build with 64-bit integer date/time support 5
echo $ECHO_N checking whether to build with 64-bit integer date/time support... $ECHO_C 6
--- 1699,1705
#
! # 64-bit integer date/time storage: enabled by default.
#
echo $as_me:$LINENO: checking whether to build with 64-bit integer date/time support 5
echo $ECHO_N checking whether to build with 64-bit integer date/time support... $ECHO_C 6
*** echo $as_me: error: no argument expecte
*** 1729,1735
esac
else
! enable_integer_datetimes=no
fi;
--- 1729,1739
esac
else
! enable_integer_datetimes=yes
!
! cat confdefs.h \_ACEOF
! #define USE_INTEGER_DATETIMES 1
! _ACEOF
fi;
*** fi
*** 22277,22282
--- 22281,22306
+ # If the user did not disable integer datetimes, check that
+ # there is a working 64-bit integral type to use.
+ if test x$USE_INTEGER_DATETIMES = xyes
+test x$HAVE_LONG_INT_64 = xno
+test x$HAVE_LONG_LONG_INT_64 = xno
+test x$HAVE_INT64 = xno ; then
+ { { echo $as_me:$LINENO: error:
+ Integer-based datetime support requires a 64-bit integer type,
+ but no such type could be found. The --disable-integer-datetimes
+ configure option can be used to disable integer-based storage
+ of datetime values. 5
+ echo $as_me: error:
+ Integer-based datetime support requires a 64-bit integer type,
+ but no such type could be found. The --disable-integer-datetimes
+ configure option can be used to disable integer-based storage
+ of datetime values. 2;}
+{ (exit 1); exit 1; }; }
+ fi
+
+
if test $PORTNAME != win32
then
echo $as_me:$LINENO: checking for POSIX signal interface 5
Index: configure.in
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/configure.in,v
retrieving revision 1.514
diff -c -p -r1.514 configure.in
*** configure.in 4 May 2007 15:20:52 - 1.514
--- configure.in 6 May 2007 03:08:04 -
*** PGAC_ARG_REQ(with, libs, [ --with-
*** 137,146
#
! # 64-bit integer date/time storage (--enable-integer-datetimes)
#
AC_MSG_CHECKING([whether to build with 64-bit integer date/time support])
! PGAC_ARG_BOOL(enable, integer-datetimes, no, [ --enable-integer-datetimes enable 64-bit integer date/time support],
[AC_DEFINE([USE_INTEGER_DATETIMES], 1,
[Define to 1 if you want 64-bit integer timestamp and interval support. (--enable-integer-datetimes)])])
AC_MSG_RESULT([$enable_integer_datetimes])
--- 137,146
#
! # 64-bit integer date/time storage: enabled by default.
#
AC_MSG_CHECKING([whether to build with 64-bit integer date/time support])
! PGAC_ARG_BOOL(enable, integer-datetimes, yes, [ --disable-integer-datetimes disable 64-bit integer date/time support],
[AC_DEFINE([USE_INTEGER_DATETIMES], 1,
[Define to 1 if you want 64-bit integer timestamp and interval support. (--enable-integer-datetimes)])])
AC_MSG_RESULT([$enable_integer_datetimes])
*** AC_CHECK_TYPES([int8, uint8, int64, uint
*** 1362,1367
--- 1362,1381
AC_CHECK_TYPES(sig_atomic_t, [], [], [#include signal.h])
+ # If the user did not disable integer
Re: [PATCHES] Enable integer datetimes by default
On Sat, 2007-05-05 at 22:49 -0500, Michael Glaesemann wrote: Would it make more sense to have phrase it in the positive sense? i.e., --enable-floating-point-datetimes? I guess that's a bit longer, but it says what you're doing, rather than what you're *not* doing. I think the primary reason people will want to use FP-based datetimes is because they can't use integer-based datetimes for compatibility reasons (e.g. no OS support for 64-bit integers, or they need to remain compatible with old applications). The situation is analogous to --without-spinlocks: we could call that --enable-slow-locking or something, but that would sound like we're enabling an additional feature. It would also mean there would be an implicit relationship between --enable-integer-datetimes and --enable-fp-datetimes (at most one can be true). IMHO it would be simpler to just keep a single boolean variable (integer datetimes enabled or not). -Neil ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Hash function for numeric (WIP)
On Mon, 2007-30-04 at 00:04 -0400, Tom Lane wrote:
I'm still not very comfortable with that. You're proposing to add a
pretty obvious failure mechanism --- any numeric-returning function
that failed to normalize its output would now create a subtle,
hard-to-find bug.
What about teaching hash_numeric() to explicitly ignore leading and
trailing zero digits?
Perhaps a suitable test would be to compare the number of
hash collisions in a large set of randomly-chosen-but-distinct
numeric values.
Okay, I did a little testing. I created a table with ~2 million
numerics, generated with equal probability by one of the two
expressions:
random()::numeric * ((random() * 100) ^ 20) * 100
random()::numeric * -100;
There were 251 duplicate inputs that I didn't bother eliminating; of
course, they should have effected both hash functions identically.
Results:
neilc=# create temp table r1 as select hash_numeric(a), count(*) from x
group by hash_numeric(a);
neilc=# create temp table r2 as select old_hash_numeric(a), count(*)
from x group by old_hash_numeric(a);
neilc=# select count(*) from r1 where count 1;
count
123342
(1 row)
neilc=# select count(*) from r2 where count 1;
count
---
756
(1 row)
old_hash_numeric() is the hash_any()-based hash, hash_numeric() is my
coding of your suggested hash function (see attached patch).
So it seems we need a better hash if we're not going to use hash_any().
The analysis required to write a robust hash function from scratch is
precisely why I'd prefer to use hash_any() if possible.
-Neil
Index: src/backend/utils/adt/numeric.c
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/utils/adt/numeric.c,v
retrieving revision 1.101
diff -c -p -r1.101 numeric.c
*** src/backend/utils/adt/numeric.c 27 Feb 2007 23:48:08 - 1.101
--- src/backend/utils/adt/numeric.c 3 May 2007 20:59:55 -
***
*** 26,31
--- 26,32
#include limits.h
#include math.h
+ #include access/hash.h
#include catalog/pg_type.h
#include libpq/pqformat.h
#include utils/array.h
*** cmp_numerics(Numeric num1, Numeric num2)
*** 1149,1154
--- 1150,1213
return result;
}
+ Datum
+ old_hash_numeric(PG_FUNCTION_ARGS)
+ {
+ Numeric key = PG_GETARG_NUMERIC(0);
+ Datum digit_hash;
+ Datum result;
+
+ /* If it's NaN, don't try to hash the rest of the fields */
+ if (NUMERIC_IS_NAN(key))
+ PG_RETURN_UINT32(0);
+
+ /* If n_weight is zero, it is a zero, regardless of any other fields */
+ if (NUMERIC_NDIGITS(key) == 0)
+ PG_RETURN_UINT32(-1);
+
+ /*
+ * XXX: we don't hash on the Numeric's scale, since two numerics
+ * can compare equal but have different scales. We also don't hash
+ * on the sign, although we easily could (since a sign difference
+ * means inequality, it shouldn't affect correctness).
+ */
+ digit_hash = hash_any((unsigned char *) NUMERIC_DIGITS(key),
+ NUMERIC_NDIGITS(key) * sizeof(NumericDigit));
+
+ /* Mix in the weight, via XOR */
+ result = digit_hash ^ key-n_weight;
+ PG_RETURN_DATUM(result);
+ }
+
+ Datum
+ hash_numeric(PG_FUNCTION_ARGS)
+ {
+ Numeric key = PG_GETARG_NUMERIC(0);
+ NumericDigit *digits;
+ uint32 hash;
+ int32 shift;
+ int i;
+
+ /* If it's NaN, don't try to hash the rest of the fields */
+ if (NUMERIC_IS_NAN(key))
+ PG_RETURN_UINT32(0);
+
+ hash = 0;
+ shift = 3 * key-n_weight;
+ digits = NUMERIC_DIGITS(key);
+
+ for (i = 0; i NUMERIC_NDIGITS(key); i++)
+ {
+ int32 this_shift = (shift 31);
+ hash |= ((uint32) digits[i]) this_shift;
+ if (this_shift 0)
+ hash |= ((uint32) digits[i]) (32 - this_shift);
+ shift -= 3;
+ }
+
+ PG_RETURN_UINT32(hash);
+ }
+
/* --
*
Index: src/include/catalog/pg_amop.h
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/include/catalog/pg_amop.h,v
retrieving revision 1.80
diff -c -p -r1.80 pg_amop.h
*** src/include/catalog/pg_amop.h 2 Apr 2007 03:49:40 - 1.80
--- src/include/catalog/pg_amop.h 3 May 2007 19:02:44 -
*** DATA(insert ( 2232 19 19 1 f 2334 405
*** 568,573
--- 568,575
DATA(insert ( 2235 1033 1033 1 f 974 405 ));
/* uuid_ops */
DATA(insert ( 2969 2950 2950 1 f 2972 405 ));
+ /* numeric_ops */
+ DATA(insert ( 1998 1700 1700 1 f 1752 405 ));
/*
Index: src/include/catalog/pg_amproc.h
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/include/catalog/pg_amproc.h,v
retrieving revision 1.64
diff -c -p -r1.64 pg_amproc.h
*** src/include/catalog/pg_amproc.h 2 Apr 2007 03:49:40 - 1.64
--- src/include/catalog/pg_amproc.h 3 May 2007 19:02:44 -
*** DATA(insert ( 1990 26 26 1 453 ));
*** 148,153
--- 148,154
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
On Wed, 2007-02-05 at 17:09 +0530, NikhilS wrote: Since this patch is only supposed to copy unique/primary indexes, I dont think we will ever have predicates associated to such indexes? Nope: neilc=# create table t1 (a int, b int); CREATE TABLE neilc=# create unique index t1_a_idx on t1 ((a + b)) where (a 5); CREATE INDEX -Neil ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] Hash function for numeric (WIP)
On Fri, 2007-04-27 at 04:09 -0400, Tom Lane wrote: I feel uncomfortable about this proposal because it will compute different hashes for values that differ only in having different numbers of trailing zeroes. Now the numeric.c code is supposed to suppress extra trailing zeroes on output, but that's never been a correctness property ... are we willing to make it one? I don't think that is such an onerous requirement: we could easily add code to enforce this invariant (that might even be worth doing regardless, to verify that the comments remain consistent with reality). There are various related cases involving unstripped leading zeroes. numeric.h claims that leading zeros will also be stripped -- is that not correct? Another point is that sign = NUMERIC_NAN makes it a NAN regardless of any other fields; ignoring the sign does not get the right result here. I believe the patch was actually correct for NUMERIC_NAN (it already special-cased it). On Fri, 2007-04-27 at 10:02 -0400, Tom Lane wrote: Something else I just remembered is that ndigits = 0 makes it a zero regardless of the weight. Good point, fixed. Perhaps a sufficiently robust way would be to form the hash as the XOR of each supplied digit, circular-shifted by say 3 times the digit's weight. -Neil ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Hash function for numeric (WIP)
Sorry for fat-fingering the previous reply -- I wanted to add:
On Fri, 2007-04-27 at 10:02 -0400, Tom Lane wrote:
Perhaps a sufficiently robust way would be to form the hash as the
XOR of each supplied digit, circular-shifted by say 3 times the
digit's weight.
The only objection I have to this is that it means we need to have
another hash function in the backend. The Jenkins hash we use in
hash_any() has been studied and we can have at least some confidence in
its collision-resistance, etc.
Anyway, attached is a revised version of the hash_any()-based patch.
-Neil
Index: src/backend/utils/adt/numeric.c
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/utils/adt/numeric.c,v
retrieving revision 1.101
diff -c -p -r1.101 numeric.c
*** src/backend/utils/adt/numeric.c 27 Feb 2007 23:48:08 - 1.101
--- src/backend/utils/adt/numeric.c 28 Apr 2007 22:28:54 -
***
*** 26,31
--- 26,32
#include limits.h
#include math.h
+ #include access/hash.h
#include catalog/pg_type.h
#include libpq/pqformat.h
#include utils/array.h
*** cmp_numerics(Numeric num1, Numeric num2)
*** 1149,1154
--- 1150,1184
return result;
}
+ Datum
+ hash_numeric(PG_FUNCTION_ARGS)
+ {
+ Numeric key = PG_GETARG_NUMERIC(0);
+ Datum digit_hash;
+ Datum result;
+
+ /* If it's NaN, don't try to hash the rest of the fields */
+ if (NUMERIC_IS_NAN(key))
+ PG_RETURN_UINT32(0);
+
+ /* If n_weight is zero, it is a zero, regardless of any other fields */
+ if (NUMERIC_NDIGITS(0) == 0)
+ PG_RETURN_UINT32(-1);
+
+ /*
+ * XXX: we don't hash on the Numeric's scale, since two numerics
+ * can compare equal but have different scales. We also don't hash
+ * on the sign, although we easily could (since a sign difference
+ * means inequality, it shouldn't affect correctness).
+ */
+ digit_hash = hash_any((unsigned char *) NUMERIC_DIGITS(key),
+ NUMERIC_NDIGITS(key) * sizeof(NumericDigit));
+
+ /* Mix in the weight, via XOR */
+ result = digit_hash ^ key-n_weight;
+ PG_RETURN_DATUM(result);
+ }
+
/* --
*
Index: src/include/catalog/pg_amop.h
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/include/catalog/pg_amop.h,v
retrieving revision 1.80
diff -c -p -r1.80 pg_amop.h
*** src/include/catalog/pg_amop.h 2 Apr 2007 03:49:40 - 1.80
--- src/include/catalog/pg_amop.h 28 Apr 2007 22:11:42 -
*** DATA(insert ( 2232 19 19 1 f 2334 405
*** 568,573
--- 568,575
DATA(insert ( 2235 1033 1033 1 f 974 405 ));
/* uuid_ops */
DATA(insert ( 2969 2950 2950 1 f 2972 405 ));
+ /* numeric_ops */
+ DATA(insert ( 1998 1700 1700 1 f 1752 405 ));
/*
Index: src/include/catalog/pg_amproc.h
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/include/catalog/pg_amproc.h,v
retrieving revision 1.64
diff -c -p -r1.64 pg_amproc.h
*** src/include/catalog/pg_amproc.h 2 Apr 2007 03:49:40 - 1.64
--- src/include/catalog/pg_amproc.h 28 Apr 2007 22:11:42 -
*** DATA(insert ( 1990 26 26 1 453 ));
*** 148,153
--- 148,154
DATA(insert ( 1992 30 30 1 457 ));
DATA(insert ( 1995 25 25 1 400 ));
DATA(insert ( 1997 1083 1083 1 452 ));
+ DATA(insert ( 1998 1700 1700 1 432 ));
DATA(insert ( 1999 1184 1184 1 452 ));
DATA(insert ( 2001 1266 1266 1 1696 ));
DATA(insert ( 2040 1114 1114 1 452 ));
Index: src/include/catalog/pg_opclass.h
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/include/catalog/pg_opclass.h,v
retrieving revision 1.75
diff -c -p -r1.75 pg_opclass.h
*** src/include/catalog/pg_opclass.h 2 Apr 2007 03:49:40 - 1.75
--- src/include/catalog/pg_opclass.h 28 Apr 2007 22:11:42 -
*** DATA(insert ( 405 macaddr_ops PGNSP P
*** 129,134
--- 129,135
DATA(insert ( 403 name_ops PGNSP PGUID 1986 19 t 0 ));
DATA(insert ( 405 name_ops PGNSP PGUID 1987 19 t 0 ));
DATA(insert ( 403 numeric_ops PGNSP PGUID 1988 1700 t 0 ));
+ DATA(insert ( 405 numeric_ops PGNSP PGUID 1998 1700 t 0 ));
DATA(insert OID = 1981 ( 403 oid_ops PGNSP PGUID 1989 26 t 0 ));
#define OID_BTREE_OPS_OID 1981
DATA(insert ( 405 oid_opsPGNSP PGUID 1990 26 t 0 ));
Index: src/include/catalog/pg_operator.h
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/include/catalog/pg_operator.h,v
retrieving revision 1.151
diff -c -p -r1.151 pg_operator.h
*** src/include/catalog/pg_operator.h 2 Apr 2007 03:49:40 - 1.151
--- src/include/catalog/pg_operator.h 28 Apr 2007 22:11:42 -
*** DATA(insert OID = 1630 ( !~~* PGNSP
*** 675,681
Re: [PATCHES] actualised forgotten Magnus's patch for plpgsql MOVE statement
On Fri, 2007-04-20 at 09:46 +0200, Pavel Stehule wrote: I refreshed Magnus's patch http://archives.postgresql.org/pgsql-patches/2007-02/msg00275.php from februar. Applied, thanks. BTW, I notice that the documentation for PL/PgSQL's FETCH command states that only the direction variants that fetch a *single* row are allowed. This is not actually the case: FETCH RELATIVE 2 FROM c INTO v results in assigning the first row from c into v, and then discarding the second row. Is this the best behavior? At the least, we should describe it in the documentation. p.s. scrollable cursors in plpgsql need little work still. I forgot for nonstandard (postgresql extension) direction forward all, forward n, backward n. Forward all propably hasn't sense. Yes, these are certainly needed for MOVE, and we may as well allow them for FETCH as well. -Neil ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
[PATCHES] Hash function for numeric (WIP)
There is currently no support for hashing numerics. This prevents
numerics from being hash indexed or used in a hashed aggregation. This
patch makes the necessary changes to the catalogs to enable hashing for
numerics, and implements a first sketch at a hash function for numerics.
For any two numerics that compare equal, we need to compute the same
hash value for both datums, even if their bit patterns differ. This
patch computes the hash from the n_weight and n_data fields of the
numeric -- notably, it doesn't use the numeric's scale field, since
two equal numerics may have different scales. The hash seems to return
the correct results for the simple test cases that I've tried, but I'm
not very familiar with the details of the numeric implementation -- can
anyone comment on whether this hash function is correct?
Thanks to Sailesh Krishnamurthy for reporting this problem.
-Neil
Index: src/backend/utils/adt/numeric.c
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/utils/adt/numeric.c,v
retrieving revision 1.101
diff -c -p -r1.101 numeric.c
*** src/backend/utils/adt/numeric.c 27 Feb 2007 23:48:08 - 1.101
--- src/backend/utils/adt/numeric.c 27 Apr 2007 07:25:38 -
***
*** 26,31
--- 26,32
#include limits.h
#include math.h
+ #include access/hash.h
#include catalog/pg_type.h
#include libpq/pqformat.h
#include utils/array.h
*** cmp_numerics(Numeric num1, Numeric num2)
*** 1149,1154
--- 1150,1184
return result;
}
+ Datum
+ hash_numeric(PG_FUNCTION_ARGS)
+ {
+ Numeric key = PG_GETARG_NUMERIC(0);
+ Datum digit_hash;
+ Datum weight_hash;
+ Datum result;
+
+ if (NUMERIC_IS_NAN(key))
+ PG_RETURN_UINT32(0);
+
+ /*
+ * XXX: we don't hash on the Numeric's scale, since two numerics
+ * can compare equal but have different scales. We also don't hash
+ * on the sign, although we easily could (since a sign difference
+ * means inequality, it shouldn't affect correctness).
+ */
+ digit_hash = hash_any((unsigned char *) NUMERIC_DIGITS(key),
+ NUMERIC_NDIGITS(key) * sizeof(NumericDigit));
+ weight_hash = DirectFunctionCall1(hashint2,
+ Int16GetDatum(key-n_weight));
+
+ /*
+ * XXX: we combine the hashes with XOR. Is this the right choice?
+ */
+ result = digit_hash ^ weight_hash;
+ PG_RETURN_DATUM(result);
+ }
+
/* --
*
Index: src/include/catalog/pg_amop.h
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/include/catalog/pg_amop.h,v
retrieving revision 1.80
diff -c -p -r1.80 pg_amop.h
*** src/include/catalog/pg_amop.h 2 Apr 2007 03:49:40 - 1.80
--- src/include/catalog/pg_amop.h 27 Apr 2007 07:38:32 -
*** DATA(insert ( 2232 19 19 1 f 2334 405
*** 568,573
--- 568,575
DATA(insert ( 2235 1033 1033 1 f 974 405 ));
/* uuid_ops */
DATA(insert ( 2969 2950 2950 1 f 2972 405 ));
+ /* numeric_ops */
+ DATA(insert ( 1998 1700 1700 1 f 1752 405 ));
/*
Index: src/include/catalog/pg_amproc.h
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/include/catalog/pg_amproc.h,v
retrieving revision 1.64
diff -c -p -r1.64 pg_amproc.h
*** src/include/catalog/pg_amproc.h 2 Apr 2007 03:49:40 - 1.64
--- src/include/catalog/pg_amproc.h 27 Apr 2007 07:37:45 -
*** DATA(insert ( 1990 26 26 1 453 ));
*** 148,153
--- 148,154
DATA(insert ( 1992 30 30 1 457 ));
DATA(insert ( 1995 25 25 1 400 ));
DATA(insert ( 1997 1083 1083 1 452 ));
+ DATA(insert ( 1998 1700 1700 1 432 ));
DATA(insert ( 1999 1184 1184 1 452 ));
DATA(insert ( 2001 1266 1266 1 1696 ));
DATA(insert ( 2040 1114 1114 1 452 ));
Index: src/include/catalog/pg_opclass.h
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/include/catalog/pg_opclass.h,v
retrieving revision 1.75
diff -c -p -r1.75 pg_opclass.h
*** src/include/catalog/pg_opclass.h 2 Apr 2007 03:49:40 - 1.75
--- src/include/catalog/pg_opclass.h 27 Apr 2007 07:06:54 -
*** DATA(insert ( 405 macaddr_ops PGNSP P
*** 129,134
--- 129,135
DATA(insert ( 403 name_ops PGNSP PGUID 1986 19 t 0 ));
DATA(insert ( 405 name_ops PGNSP PGUID 1987 19 t 0 ));
DATA(insert ( 403 numeric_ops PGNSP PGUID 1988 1700 t 0 ));
+ DATA(insert ( 405 numeric_ops PGNSP PGUID 1998 1700 t 0 ));
DATA(insert OID = 1981 ( 403 oid_ops PGNSP PGUID 1989 26 t 0 ));
#define OID_BTREE_OPS_OID 1981
DATA(insert ( 405 oid_opsPGNSP PGUID 1990 26 t 0 ));
Index: src/include/catalog/pg_operator.h
===
RCS file:
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
This patch needs more work. How carefully did you test it?
* the patch failed to copy index constraints if there was not also at
least one CHECK constraint defined on the table
* the patch is broken for expressional indexes, and silently omits
copying the predicate that may be associated with an index. It also
doesn't copy the index's amoptions (WITH clause), or the NULLS
FIRST/etc. options that may be associated with any of the index's
columns.
In other words, copying column names does not suffice to duplicate the
index constraint. Perhaps the right fix is to implement support for
INCLUDING INDEXES, and then use that code to copy the definition of any
constraint indexes in INCLUDING INDEXES?
* should we copy invalid indexes? I suppose there's nothing wrong with
copying them...
* we should probably hold the AccessShareLock on copied indexes till end
of xact, per the comments at the end of transformInhRelation()
* index_open() should be matched with index_close(), not
relation_close(). (In the current implementation, index_close() and
relation_close() happen to be identical, so it still worked.)
I've attached a revised version of the patch, but I didn't fix the
second or third bullets in the list.
-Neil
Index: doc/src/sgml/ref/create_table.sgml
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/doc/src/sgml/ref/create_table.sgml,v
retrieving revision 1.107
diff -c -p -r1.107 create_table.sgml
*** doc/src/sgml/ref/create_table.sgml 1 Feb 2007 00:28:18 - 1.107
--- doc/src/sgml/ref/create_table.sgml 27 Apr 2007 21:49:20 -
*** and replaceable class=PARAMETERtable
*** 259,269
/para
para
Not-null constraints are always copied to the new table.
! literalCHECK/literal constraints will only be copied if
! literalINCLUDING CONSTRAINTS/literal is specified; other types of
! constraints will never be copied. Also, no distinction is made between
! column constraints and table constraints mdash; when constraints are
! requested, all check constraints are copied.
/para
para
Note also that unlike literalINHERITS/literal, copied columns and
--- 259,268
/para
para
Not-null constraints are always copied to the new table.
! literalCHECK, UNIQUE, and PRIMARY KEY/literal constraints will only
! be copied if literalINCLUDING CONSTRAINTS/literal is specified. Also,
! no distinction is made between column constraints and table constraints
! mdash; when constraints are requested, all check constraints are copied.
/para
para
Note also that unlike literalINHERITS/literal, copied columns and
Index: src/backend/parser/analyze.c
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/parser/analyze.c,v
retrieving revision 1.362
diff -c -p -r1.362 analyze.c
*** src/backend/parser/analyze.c 13 Mar 2007 00:33:41 - 1.362
--- src/backend/parser/analyze.c 27 Apr 2007 22:50:32 -
***
*** 27,32
--- 27,33
#include postgres.h
+ #include access/genam.h
#include access/heapam.h
#include catalog/heap.h
#include catalog/index.h
***
*** 54,59
--- 55,61
#include utils/acl.h
#include utils/builtins.h
#include utils/lsyscache.h
+ #include utils/relcache.h
#include utils/syscache.h
*** transformInhRelation(ParseState *pstate,
*** 1325,1359
* If default expr could contain any vars, we'd need to fix 'em,
* but it can't; so default is ready to apply to child.
*/
-
def-cooked_default = pstrdup(this_default);
}
}
! /*
! * Copy CHECK constraints if requested, being careful to adjust
! * attribute numbers
! */
! if (including_constraints tupleDesc-constr)
{
! AttrNumber *attmap = varattnos_map_schema(tupleDesc, cxt-columns);
! int ccnum;
! for (ccnum = 0; ccnum tupleDesc-constr-num_check; ccnum++)
{
! char *ccname = tupleDesc-constr-check[ccnum].ccname;
! char *ccbin = tupleDesc-constr-check[ccnum].ccbin;
! Node *ccbin_node = stringToNode(ccbin);
! Constraint *n = makeNode(Constraint);
! change_varattnos_of_a_node(ccbin_node, attmap);
! n-contype = CONSTR_CHECK;
! n-name = pstrdup(ccname);
! n-raw_expr = NULL;
! n-cooked_expr = nodeToString(ccbin_node);
! n-indexspace = NULL;
! cxt-ckconstraints = lappend(cxt-ckconstraints, (Node *) n);
}
}
--- 1327,1424
* If default expr could contain any vars, we'd need to fix 'em,
* but it can't; so default is ready to apply to child.
*/
def-cooked_default = pstrdup(this_default);
}
}
! if (including_constraints)
{
! /*
! * Copy CHECK constraints, being careful to adjust attribute
! * numbers.
! */
! if (tupleDesc-constr)
! {
! AttrNumber
Re: [PATCHES] fix LOCK_DEBUG
On Mon, 2007-04-23 at 14:22 +0100, Heikki Linnakangas wrote: 8.2 branch doesn't compile with LOCK_DEBUG enabled Applied, thanks. -Neil ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] RESET SESSION v3
On Sun, 2007-04-08 at 11:08 +0300, Marko Kreen wrote: I think implicit ABORT would annoy various tools that partially parse user sql and expect to know what transaction state currently is. For them a new tranaction control statement would be nuisance. That's not the only alternative: we could also either disallow all of the ALL variants in a transaction block, or allow RESET SESSION inside a transaction block. I've committed the patch basically as-is: thanks for the patch. I don't feel strongly about the above, but if there's a consensus, we can change the behavior later. -Neil ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
