Re: [PATCHES] [HACKERS] Show INHERIT in \du
On Tue, Mar 25, 2008 at 2:41 AM, Tom Lane [EMAIL PROTECTED] wrote: Brendan Jurd [EMAIL PROTECTED] writes: This makes me wonder whether print.c could offer something a bit more helpful to callers wishing to DIY a table; we could have a table-building struct with methods like addHeader and addCell. What do you think? Overkill, or worthy pursuit? Once you have two occurrences of a pattern, it's reasonable to assume there will be more later. +1 for building a little bit of infrastructure. I've written a patch which implements the same \du behaviour as my previous patch, but using the new printTable API I submitted in [1]. If the printTable API patch is rejected or substantially changed, we will need to revisit this patch. The new patch constructs a table manually, in the same manner as describeOneTableDetails, so that we get the same outputs as the original patch but without any of the localisation issues identified by Tom and Alvaro. I have attached a patch against my printTable code, containing only the changes I made to describeRoles() (du-attributes_1.diff.bz2), and a combined patch against HEAD containing the full printTable API changes as well as the changes to describeRoles() (du-attributes-print-table_1.diff.bz2). No memory problems detected by valgrind, and all regression tests passed on x86_64 gentoo. I've added this item to the May CommitFest wiki page. Cheers, BJ [1[ http://archives.postgresql.org/message-id/[EMAIL PROTECTED] du-attributes_1.diff.bz2 Description: BZip2 compressed data du-attributes-print-table_1.diff.bz2 Description: BZip2 compressed data -- 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] libpq patch for pqtypes hook api and PGresult creation
Kind of a long post, but if you take the time to read it we think it accurately
clarifies how we interrupt the current objections and how we see this working.
NOTE: any references to overhead are in regards to library size, not performance.
would be to insert hooks at library
_init() time, meaning that the mere linking of libpgtypes
Alvaro Herrera wrote:
Maybe there's a way we can have libpqtypes adding calls into some
hypothetical libpq hooks. So libpqtypes registers its hooks in _init()
or some such, and it gets picked up automatically by any app that links
to it.
the hook name concept
Not needed anymore if we do per-conn hooks. I was doing library wide hooks, it
felt natural to allow them to be removed (ability not needed per-conn). You can
only remove hooks if you have a means of referencing what you want to remove.
From that perspective, the names served a purpose - PQremoveObjectHooks(myhook);
you've got it holding a process-wide lock
Okay, easy change to install per-conn. I was trying to avoid having to set these
hooks on every connection.
There are some dirty details in regards to locking. Even if you remove the
locking from libpq hooks, you still incur a lock at every hook point inside
pqtypes. pqtypes has to map a conn and result (we'll call this a pqobj) to
pqtypes typeData. Adding a void* to the hook funcs doesn't help because non-hook
functions like getf, paramcreate, etc. only have a ptr to a pqobj: PQgetf(res,
..), PQparamCreate(conn, ..). Since the private storage of a pqobj is not
directly accessible, you have to either A) map pqobj addresses to typeData in a
pqtypes global array that must be locked or B) add two libpq API calls
PQtypeData(conn), PQresultTypeData(res).
libpgtypes calls PQinitTypes(PGconn *conn)
As this stands, it wouldn't work. You need some hook funcptr arguments. Without
them, there is no way to communicate with pqtypes.
Tom Lane wrote:
hooks that could be used by either libpgtypes or something that would like to
do something roughly similar
I don't think PQinitTypes, private type data per conn/result or the need for
PQtypeData(conn), PQresultTypeData(res) (to avoid locking in pqtypes) keeps
things in line with this requirement (generic hook api). Has this requirement
changed? BTW, Tom was not the only one to suggest generic design. That's why I
came up with object hooks - notifications of libpq object states. Best
name/concept I can come up with. PQinitTypes(conn) is really
PQaddObjectHook(conn, hook) -- addition of the conn argument -- to keep it generic.
In the end, the problem is that the wrong tool hooks is being used for
pqtypes. Hooks normally allow a completely unrelated library to receive events.
I think we are forcing a hook design on to something that is completely
related (libpqtypes is an extension of libpq, getvalue and getf are siblings).
There is no need for hooks. If we wanted to add
PQsetBillingMethod(PQconn*,PQbillingMethod*), then you could make a case for
hooks (obviously the billing api doesn't fit). But that is not the case for
PQgetf, PQputf, PQparamExec, PQparamSend,
The argument against pqtypes being part of libpq was library size overhead.
This was verbalized by many people (issues with redhat packaging were also
brought up). I never heard a complaint about the 10 API calls we wanted to add.
Only that those 10 API calls came at a 50K library bloat cost, and there were
no buyers.
This brings me back to the dlopen idea. If you want to use pqtypes,
PQtypesLoad(). The guts of the library are in libpqtypes.so so the only thing
left in libpq are simple functions like below:
// libpq
PQparamExec(...)
{
if(libpqtypes-paramExec)//typesLoad issued dlsym calls
// we are in libpq, access to private conn storage granted
return libpqtypes-paramExec(conn, conn-typeData, ...);
return library not loaded: call PQtypesLoad;
}
// end user
#include libpqtypes.h // pqtypes typedefs, includes libpq-fe.h
PQtypesLoad(); // call before using libpq
res = PQparamExec(conn, param, .);
The library size issue is resolved. I never heard any complaints about this
approach. Andrew Dunstan said Please make sure that any scheme you have along
these lines will work on Windows DLLs too., which didn't sound like a complaint
to me.
#ifdef WIN32
# define dlopen(libname, flags) LoadLibraryA(libname)
# define dlsym(handle, sym) GetProcAddress(handle, sym)
#endif
Tom also weighed in but he thought I was confused about his hook idea (as the
proposed dlopen is completely different):
Tom Wrote:
This is still 100% backwards. My idea of a libpq hook is something that
could be used by libpgtypes *and other things*. What you are proposing
is something where the entire API of the supposed add-on is hard-wired
into libpq.
He is 100% correct, the dlopen idea is 100% backwards from a hook concept. It
was not an implementation idea for the hooks concept, it was a different
approach
Re: [PATCHES] [HACKERS] Terminating a backend
Bruce Momjian wrote: When we get the termination signal, why can't we just set a global boolean, do a query cancel, and in the setjmp() code block check the global and exit --- at that stage we know we have released all locks and can exit cleanly. I have implemented this idea with the attached patch. One problem I have with my patch is that SIGTERM is currently used by the postmaster to shut down backends. Now because the postmaster knows that all backend are terminating, it can accept a dirtier shutdown than one where we are terminating just one backend and the rest are going to keep running. The new SIGTERM coding is going to exit a backend only in a place where cancel is checked. I have a idea --- to have pg_terminate_backend() set a PGPROC boolean and then send a query cancel signal to the backend --- the backend can then check the boolean and exit if required. I will work on a new version of this patch tomorrow/Monday. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] datum passed to macro which expects a pointer
Hi all,
Attached are more fixes.
Thanks,
Gavin, with Feng Tian
Index: src/backend/access/common/heaptuple.c
===
RCS file: /Users/swm/pgsql-cvs/pgsql/src/backend/access/common/heaptuple.c,v
retrieving revision 1.120
diff -c -p -r1.120 heaptuple.c
*** src/backend/access/common/heaptuple.c 1 Jan 2008 19:45:45 - 1.120
--- src/backend/access/common/heaptuple.c 13 Apr 2008 13:06:53 -
*** heap_form_tuple(TupleDesc tupleDescripto
*** 890,896
else if (att[i]-attlen == -1
att[i]-attalign == 'd'
att[i]-attndims == 0
! !VARATT_IS_EXTENDED(values[i]))
{
values[i] = toast_flatten_tuple_attribute(values[i],
att[i]-atttypid,
--- 890,896
else if (att[i]-attlen == -1
att[i]-attalign == 'd'
att[i]-attndims == 0
! !VARATT_IS_EXTENDED(DatumGetPointer(values[i])))
{
values[i] = toast_flatten_tuple_attribute(values[i],
att[i]-atttypid,
*** heap_formtuple(TupleDesc tupleDescriptor
*** 1001,1007
else if (att[i]-attlen == -1
att[i]-attalign == 'd'
att[i]-attndims == 0
! !VARATT_IS_EXTENDED(values[i]))
{
values[i] = toast_flatten_tuple_attribute(values[i],
att[i]-atttypid,
--- 1001,1007
else if (att[i]-attlen == -1
att[i]-attalign == 'd'
att[i]-attndims == 0
! !VARATT_IS_EXTENDED(DatumGetPointer(values[i])))
{
values[i] = toast_flatten_tuple_attribute(values[i],
att[i]-atttypid,
Index: src/backend/access/common/indextuple.c
===
RCS file: /Users/swm/pgsql-cvs/pgsql/src/backend/access/common/indextuple.c,v
retrieving revision 1.85
diff -c -p -r1.85 indextuple.c
*** src/backend/access/common/indextuple.c 1 Jan 2008 19:45:45 - 1.85
--- src/backend/access/common/indextuple.c 13 Apr 2008 18:16:44 -
*** index_form_tuple(TupleDesc tupleDescript
*** 73,79
* If value is stored EXTERNAL, must fetch it so we are not depending
* on outside storage. This should be improved someday.
*/
! if (VARATT_IS_EXTERNAL(values[i]))
{
untoasted_values[i] =
PointerGetDatum(heap_tuple_fetch_attr((struct varlena *)
--- 73,79
* If value is stored EXTERNAL, must fetch it so we are not depending
* on outside storage. This should be improved someday.
*/
! if (VARATT_IS_EXTERNAL(DatumGetPointer(values[i])))
{
untoasted_values[i] =
PointerGetDatum(heap_tuple_fetch_attr((struct varlena *)
*** index_form_tuple(TupleDesc tupleDescript
*** 85,92
* If value is above size target, and is of a compressible datatype,
* try to compress it in-line.
*/
! if (!VARATT_IS_EXTENDED(untoasted_values[i])
! VARSIZE(untoasted_values[i]) TOAST_INDEX_TARGET
(att-attstorage == 'x' || att-attstorage == 'm'))
{
Datum cvalue = toast_compress_datum(untoasted_values[i]);
--- 85,92
* If value is above size target, and is of a compressible datatype,
* try to compress it in-line.
*/
! if (!VARATT_IS_EXTENDED(DatumGetPointer(untoasted_values[i]))
! VARSIZE(DatumGetPointer(untoasted_values[i])) TOAST_INDEX_TARGET
(att-attstorage == 'x' || att-attstorage == 'm'))
{
Datum cvalue = toast_compress_datum(untoasted_values[i]);
Index: src/backend/access/common/printtup.c
===
RCS file: /Users/swm/pgsql-cvs/pgsql/src/backend/access/common/printtup.c,v
retrieving revision 1.101
diff -c -p -r1.101 printtup.c
*** src/backend/access/common/printtup.c 1 Jan 2008 19:45:45 - 1.101
--- src/backend/access/common/printtup.c 13 Apr 2008 13:14:52 -
*** printtup(TupleTableSlot *slot, DestRecei
*** 340,346
}
/* Clean up detoasted copy, if any */
! if (attr != origattr)
pfree(DatumGetPointer(attr));
}
--- 340,346
}
/* Clean up detoasted copy, if any */
! if (DatumGetPointer(attr) != DatumGetPointer(origattr))
pfree(DatumGetPointer(attr));
}
*** printtup_20(TupleTableSlot *slot, DestRe
*** 423,429
pfree(outputstr);
/* Clean up detoasted copy, if any */
! if (attr != origattr)
pfree(DatumGetPointer(attr));
}
--- 423,429
pfree(outputstr);
/* Clean up detoasted copy, if any */
! if (DatumGetPointer(attr) != DatumGetPointer(origattr))
pfree(DatumGetPointer(attr));
}
*** debugtup(TupleTableSlot *slot, DestRecei
*** 537,543
pfree(value);
/* Clean up detoasted copy, if any */
! if (attr != origattr)
pfree(DatumGetPointer(attr));
}
printf(\t\n);
--- 537,543
pfree(value);
/* Clean up detoasted copy, if any */
! if (DatumGetPointer(attr) != DatumGetPointer(origattr))
Re: [PATCHES] printTable API (was: Show INHERIT in \du)
Brendan Jurd escribió: I'd like to submit my first version of this patch for review. I have introduced a new struct in print.h called printTableContent, which is used to compose the contents of a psql table. The methods exposed for this struct are as follows: Looks cool -- on a first read, I think you should add some more code comments at the top of each function specifying whether the texts need to be translated by the caller or done by the function itself. Also it would be good if it is consistent, too :-) -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Terminating a backend
Bruce Momjian wrote: I have a idea --- to have pg_terminate_backend() set a PGPROC boolean and then send a query cancel signal to the backend --- the backend can then check the boolean and exit if required. I will work on a new version of this patch tomorrow/Monday. That's fine, even if it fails on 0.1% of the cases, but you can't use SIGTERM to implement that because it already has a different meaning. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Remove lossy-operator RECHECK flag?
I wrote: I've committed changes that move the determination of whether recheck is required into the index AMs. Right now, GIST and GIN just always set the recheck flag to TRUE. Obviously that control should be pushed down to the opclass consistent() functions, but I don't know that code well enough to be clear on exactly what should happen. Are you willing to do that part? Actually, I think I figured it out. Please look over the GIST/GIN parts of this patch and see if they're OK with you. This is still WIP because I haven't touched any contrib code, but as far as the main backend goes I think it's ready to apply. regards, tom lane binI2STY1wRy8.bin Description: run-time-recheck-1.patch.gz -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
