Re: [HACKERS] New functions in sslinfo module
On Mon, Apr 21, 2014 at 2:51 PM, Воронин Дмитрий wrote: > I put patch generated on git diffs to this letter. I make an a thread in > postgresql commit fest: > https://commitfest.postgresql.org/action/patch_view?id=1438 > Thanks! -- Michael
Re: [HACKERS] New functions in sslinfo module
I put patch generated on git diffs to this letter. I make an a thread in postgresql commit fest: https://commitfest.postgresql.org/action/patch_view?id=1438 21.04.2014, 09:12, "Michael Paquier" :On Mon, Apr 21, 2014 at 1:48 PM, Воронин Дмитрийwrote:> Hello,> > I make an a patch, which adds 4 functions to sslinfo extension module: > 1) ssl_get_count_of_extensions() --- get count of X509v3 extensions from> client certificate;> 2) ssl_get_extension_names() --- get short names of X509v3 extensions from> client certificate;> 3) ssl_get_extension_value(text) --- get value of extension from certificate > (argument --- short name of extension);> 4) ssl_is_critical_extension(text) --- returns true, if extension is> critical and false, if is not (argument --- short name of extension).> > You can view some information of certificate's extensions via those > functions.> I want, that my functions will be included in PostgreSQL release.> > What do you think about it?Please avoid creating a new thread each time you send a new version of the same patch. Previous message was here: http://www.postgresql.org/message-id/1135491397673...@web9m.yandex.ruWith my previous answer here:http://www.postgresql.org/message-id/CAB7nPqRVFhnPnQL9ND+K=WA-YF_N1fAirx=s6fawk9f6anl...@mail.gmail.com As I already mentioned last time, please register this patch to the upcoming commit fest beginning in June:https://commitfest.postgresql.org/action/commitfest_view?id=22This way, you will be sure that your patch will get at least one fair review and that progress will be made on the feature you are proposing.The development cycle of 9.4 is over, but your patch could get into 9.5. You seem as well to have developed this patch using a tarball of 9.3.4 code by generating diffs from it, you will need a development environment with git. Here are some guidelines you can refer to (those are the same URLs as in my previous email btw...): https://wiki.postgresql.org/wiki/Submitting_a_Patch https://wiki.postgresql.org/wiki/Working_with_Git https://wiki.postgresql.org/wiki/Creating_Clean_PatchesRegards,-- Michael Best regrads, Dmitry Voronin *** a/contrib/sslinfo/sslinfo--1.0.sql --- b/contrib/sslinfo/sslinfo--1.0.sql *** *** 38,40 LANGUAGE C STRICT; --- 38,56 CREATE FUNCTION ssl_issuer_dn() RETURNS text AS 'MODULE_PATHNAME', 'ssl_issuer_dn' LANGUAGE C STRICT; + + CREATE OR REPLACE FUNCTION ssl_get_extension_value(text) RETURNS text + AS 'MODULE_PATHNAME', 'ssl_get_extension_value' + LANGUAGE C STRICT; + + CREATE OR REPLACE FUNCTION ssl_is_critical_extension(text) RETURNS boolean + AS 'MODULE_PATHNAME', 'ssl_is_critical_extension' + LANGUAGE C STRICT; + + CREATE OR REPLACE FUNCTION ssl_get_count_of_extensions() RETURNS integer + AS 'MODULE_PATHNAME', 'ssl_get_count_of_extensions' + LANGUAGE C STRICT; + + CREATE OR REPLACE FUNCTION ssl_get_extension_names() RETURNS SETOF text + AS 'MODULE_PATHNAME', 'ssl_get_extension_names' + LANGUAGE C STRICT; \ No newline at end of file \ No newline at end of file *** a/contrib/sslinfo/sslinfo--unpackaged--1.0.sql --- b/contrib/sslinfo/sslinfo--unpackaged--1.0.sql *** *** 11,16 ALTER EXTENSION sslinfo ADD function ssl_issuer_field(text); --- 11,21 ALTER EXTENSION sslinfo ADD function ssl_client_dn(); ALTER EXTENSION sslinfo ADD function ssl_issuer_dn(); + ALTER EXTENSION sslinfo ADD function ssl_get_extension_value(); + ALTER EXTENSION sslinfo ADD function ssl_is_critical_extension(); + ALTER EXTENSION sslinfo ADD function ssl_count_of_extensions(); + ALTER EXTENSION sslinfo ADD function ssl_get_extension_names(); + -- These functions were not in 9.0: CREATE FUNCTION ssl_version() RETURNS text *** a/contrib/sslinfo/sslinfo.c --- b/contrib/sslinfo/sslinfo.c *** *** 5,10 --- 5,12 * This file is distributed under BSD-style license. * * contrib/sslinfo/sslinfo.c + * + * Extension functions written by Dmitry Voronin carriingfat...@yandex.ru, CNIIEISU. */ #include "postgres.h" *** *** 14,31 #include "miscadmin.h" #include "utils/builtins.h" #include "mb/pg_wchar.h" #include #include PG_MODULE_MAGIC; ! static Datum X509_NAME_field_to_text(X509_NAME *name, text *fieldName); ! static Datum X509_NAME_to_text(X509_NAME *name); ! static Datum ASN1_STRING_to_text(ASN1_STRING *str); /* * Indicates whether current session uses SSL --- 16,49 #include "miscadmin.h" #include "utils/builtins.h" #include "mb/pg_wchar.h" + #include "funcapi.h" #include #include + #include PG_MODULE_MAGIC; ! Datum ssl_is_used(PG_FUNCTION_ARGS); ! Datum ssl_version(PG_FUNCTION_ARGS); ! Datum ssl_cipher(PG_FUNCTION_ARGS); ! Datum ssl_client_cert_present(PG_FUNCTION_ARGS); ! Datum ssl_client_serial(PG_FUNCTION_ARGS); ! Datum ssl_client_dn_field(PG_FUNCTION_ARGS); ! Datum ssl_issuer_field(PG_FUNCTION_ARGS); ! Datum ssl_client_
Re: [HACKERS] New functions in sslinfo module
On Mon, Apr 21, 2014 at 1:48 PM, Воронин Дмитрий wrote: > Hello, > > I make an a patch, which adds 4 functions to sslinfo extension module: > 1) ssl_get_count_of_extensions() --- get count of X509v3 extensions from > client certificate; > 2) ssl_get_extension_names() --- get short names of X509v3 extensions from > client certificate; > 3) ssl_get_extension_value(text) --- get value of extension from certificate > (argument --- short name of extension); > 4) ssl_is_critical_extension(text) --- returns true, if extension is > critical and false, if is not (argument --- short name of extension). > > You can view some information of certificate's extensions via those > functions. > I want, that my functions will be included in PostgreSQL release. > > What do you think about it? Please avoid creating a new thread each time you send a new version of the same patch. Previous message was here: http://www.postgresql.org/message-id/1135491397673...@web9m.yandex.ru With my previous answer here: http://www.postgresql.org/message-id/CAB7nPqRVFhnPnQL9ND+K=WA-YF_N1fAirx=s6fawk9f6anl...@mail.gmail.com As I already mentioned last time, please register this patch to the upcoming commit fest beginning in June: https://commitfest.postgresql.org/action/commitfest_view?id=22 This way, you will be sure that your patch will get at least one fair review and that progress will be made on the feature you are proposing. The development cycle of 9.4 is over, but your patch could get into 9.5. You seem as well to have developed this patch using a tarball of 9.3.4 code by generating diffs from it, you will need a development environment with git. Here are some guidelines you can refer to (those are the same URLs as in my previous email btw...): https://wiki.postgresql.org/wiki/Submitting_a_Patch https://wiki.postgresql.org/wiki/Working_with_Git https://wiki.postgresql.org/wiki/Creating_Clean_Patches Regards, -- Michael
Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)
I lack time to give this a solid review, but here's a preliminary reaction: On Sun, Apr 20, 2014 at 03:38:23PM -0400, Tom Lane wrote: > On HEAD, this takes about 295-300 msec on my machine in a non-cassert > build. With the patch I sent previously, the time goes to 495-500 msec. > This goes from circa 1250 ms in HEAD to around 1680 ms. > > Some poking around with oprofile says that much of the added time is > coming from added syscache and typcache lookups, as I suspected. > > I did some further work on the patch to make it possible to cache > the element tuple descriptor across calls for these two functions. > With the attached patch, the first test case comes down to about 335 ms > and the second to about 1475 ms (plpgsql is still leaving some extra > cache lookups on the table). More could be done with some further API > changes, but I think this is about as much as is safe to back-patch. That particular performance drop seems acceptable. As I hinted upthread, the large performance risk arises for test cases that do not visit a toast table today but will do so after the change. > At least in the accumArrayResult test case, it would be possible to > bring the runtime back to very close to HEAD if we were willing to > abandon the principle that compressed fields within a tuple should > always get decompressed when the tuple goes into a larger structure. > That would allow flatten_composite_element to quit early if the > tuple doesn't have the HEAP_HASEXTERNAL infomask bit set. I'm not > sure that this would be a good tradeoff however: if we fail to remove nested > compression, we could end up paying for that with double compression. > On the other hand, it's unclear that that case would come up often, > while the overhead of disassembling the tuple is definitely going to > happen every time we assign to an array-of-composites element. (Also, > there is no question here of regression relative to previous releases, > since the existing code fails to prevent nested compression.) I wonder how it would work out to instead delay this new detoast effort until toast_insert_or_update(). That timing has the major advantage of not adding any toast table visits beyond those actually needed to avoid improperly writing an embedded toast pointer to disk. It would give us the flexibility to detoast array elements more lazily than we do today, though I don't propose any immediate change there. To reduce syscache traffic, make the TupleDesc record whether any column requires recursive detoast work. Perhaps also use the TupleDesc as a place to cache the recursive-detoast syscache lookups for tables that do need them. Thoughts? -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] New functions in sslinfo module
Hello, I make an a patch, which adds 4 functions to sslinfo extension module:1) ssl_get_count_of_extensions() --- get count of X509v3 extensions from client certificate;2) ssl_get_extension_names() --- get short names of X509v3 extensions from client certificate;3) ssl_get_extension_value(text) --- get value of extension from certificate (argument --- short name of extension);4) ssl_is_critical_extension(text) --- returns true, if extension is critical and false, if is not (argument --- short name of extension). You can view some information of certificate's extensions via those functions.I want, that my functions will be included in PostgreSQL release. What do you think about it? -- Best regards, Dmitry Voronin --- contrib/sslinfo/sslinfo.c 2014-03-17 23:35:47.0 +0400 +++ contrib/sslinfo/sslinfo.c 2014-04-18 11:09:49.567775647 +0400 @@ -5,6 +5,8 @@ * This file is distributed under BSD-style license. * * contrib/sslinfo/sslinfo.c + * + * Extension functions written by Dmitry Voronin carriingfat...@yandex.ru, CNIIEISU. */ #include "postgres.h" @@ -14,9 +16,11 @@ #include "miscadmin.h" #include "utils/builtins.h" #include "mb/pg_wchar.h" +#include "funcapi.h" #include #include +#include PG_MODULE_MAGIC; @@ -35,6 +39,11 @@ Datum X509_NAME_to_text(X509_NAME *name); Datum ASN1_STRING_to_text(ASN1_STRING *str); +X509_EXTENSION *get_extension(X509* certificate, char *name); +Datum ssl_get_extension_value(PG_FUNCTION_ARGS); +Datum ssl_is_critical_extension(PG_FUNCTION_ARGS); +Datum ssl_get_count_of_extensions(PG_FUNCTION_ARGS); +Datum ssl_get_extension_names(PG_FUNCTION_ARGS); /* * Indicates whether current session uses SSL @@ -371,3 +380,146 @@ PG_RETURN_NULL(); return X509_NAME_to_text(X509_get_issuer_name(MyProcPort->peer)); } + + +X509_EXTENSION *get_extension(X509* certificate, char *name) { + int extension_nid = 0; + int locate = 0; + + extension_nid = OBJ_sn2nid(name); + if (extension_nid == NID_undef) { + extension_nid = OBJ_ln2nid(name); + if (extension_nid == NID_undef) + return NULL; + } + locate = X509_get_ext_by_NID(certificate, extension_nid, -1); + return X509_get_ext(certificate, locate); +} + +/* Returns value of extension. + * + * This function returns value of extension by short name in client certificate. + * + * Returns text datum. + */ + +PG_FUNCTION_INFO_V1(ssl_get_extension_value); +Datum +ssl_get_extension_value(PG_FUNCTION_ARGS) { + X509 *certificate = MyProcPort -> peer; + X509_EXTENSION *extension = NULL; + char *extension_name = text_to_cstring(PG_GETARG_TEXT_P(0)); + BIO *bio = NULL; + char *value = NULL; + char nullterm = '\0'; + text *result = NULL; + + if (certificate == NULL) + PG_RETURN_NULL(); + + extension = get_extension(certificate, extension_name); + if (extension == NULL) + elog(ERROR, "Extension by name \"%s\" is not found in certificate", extension_name); + + bio = BIO_new(BIO_s_mem()); + X509V3_EXT_print(bio, extension, -1, -1); + BIO_write(bio, &nullterm, 1); + BIO_get_mem_data(bio, &value); + + result = cstring_to_text(value); + BIO_free(bio); + + PG_RETURN_TEXT_P(result); +} + +/* Returns status of extension + * + * Returns true, if extension is critical and false, if it is not. + * + * Returns bool datum + */ +PG_FUNCTION_INFO_V1(ssl_is_critical_extension); +Datum +ssl_is_critical_extension(PG_FUNCTION_ARGS) { + X509 *certificate = MyProcPort -> peer; + X509_EXTENSION *extension = NULL; + char *extension_name = text_to_cstring(PG_GETARG_TEXT_P(0)); + int critical = 0; + + if (certificate == NULL) + PG_RETURN_NULL(); + + extension = get_extension(certificate, extension_name); + if (extension == NULL) + elog(ERROR, "Extension name \"%s\" is not found in certificate", extension_name); + + critical = X509_EXTENSION_get_critical(extension); + PG_RETURN_BOOL(critical); +} + +/* Returns count of extensions in client certificate + * + * Returns int datum + */ +PG_FUNCTION_INFO_V1(ssl_get_count_of_extensions); +Datum +ssl_get_count_of_extensions(PG_FUNCTION_ARGS) { + X509 *certificate = MyProcPort -> peer; + + if (certificate == NULL) + PG_RETURN_NULL(); + + PG_RETURN_INT32(X509_get_ext_count(certificate)); +} + +/* Returns short names of extensions in client certificate + * + * Returns setof text datum + */ +PG_FUNCTION_INFO_V1(ssl_get_extension_names); +Datum +ssl_get_extension_names(PG_FUNCTION_ARGS) { + X509*certificate = MyProcPort -> peer; + FuncCallContext *funcctx; + STACK_OF(X509_EXTENSION) *extension_stack = NULL; + MemoryContext oldcontext; + int call = 0; + int max_calls = 0; + X509_EXTENSION *extension = NULL; + ASN1_OBJECT *object = NULL; + int extension_nid = 0; + text*result = NULL; + + if (certificate == NULL) + PG_RETURN_NULL(); + + extension_stack = certificate -> cert_info -> extensions; + if (extension_stack == NULL) + PG_RETURN_NULL(); + + if (SRF_IS_FIRSTCALL()) { + funcctx = SRF_FIRSTCALL_I
Re: [HACKERS] PATCH: Allow empty targets in unaccent dictionary
Please add this to the next commitfest. https://commitfest.postgresql.org/action/commitfest_view?id=22 Cheers, David. On Sun, Apr 20, 2014 at 01:06:43AM +0200, Mohammad Alhashash wrote: > Hi, > > Currently, unaccent extension only allows replacing one source > character with one or more target characters. In Arabic, Hebrew and > possibly other languages, diacritics are standalone characters that > are being added to normal letters. To use unaccent dictionary for > these languages, we need to allow empty targets to remove diacritics > instead of replacing them. > > The attached patch modfies unaacent.c so that dictionary parser uses > zero-length target when the line has no target. > > Best Regards, > > Mohammad Alhashash > > diff --git a/contrib/unaccent/unaccent.c b/contrib/unaccent/unaccent.c > old mode 100644 > new mode 100755 > index a337df6..4e72829 > --- a/contrib/unaccent/unaccent.c > +++ b/contrib/unaccent/unaccent.c > @@ -58,7 +58,9 @@ placeChar(TrieChar *node, unsigned char *str, int lenstr, > char *replaceTo, int r > { > curnode->replacelen = replacelen; > curnode->replaceTo = palloc(replacelen); > - memcpy(curnode->replaceTo, replaceTo, replacelen); > + /* palloc(0) returns a valid address, not NULL */ > + if (replaceTo) /* memcpy() is undefined for NULL > pointers*/ > + memcpy(curnode->replaceTo, replaceTo, > replacelen); > } > } > else > @@ -105,10 +107,10 @@ initTrie(char *filename) > while ((line = tsearch_readline(&trst)) != NULL) > { > /* > - * The format of each line must be "src trg" > where src and trg > + * The format of each line must be "src [trg]" > where src and trg >* are sequences of one or more non-whitespace > characters, >* separated by whitespace. Whitespace at > start or end of > - * line is ignored. > + * line is ignored. If no trg added, a > zero-length string is used. >*/ > int state; > char *ptr; > @@ -160,6 +162,13 @@ initTrie(char *filename) > } > } > > + /* if no trg (loop stops at state 1 or 2), use > zero-length target */ > + if (state == 1 || state == 2) > + { > + trglen = 0; > + state = 5; > + } > + > if (state >= 3) > rootTrie = placeChar(rootTrie, > > (unsigned char *) src, srclen, > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Removing dependency to wsock32.lib when compiling code on WIndows
Hi all, When doing some work on Windows, I noticed that the mkvc specs in src/tools/msvc use wsock32.lib, which is as far as I understand an old, obsolete version of the Windows socket library. Wouldn't it make sense to update the specs to build only with ws2_32.lib like in the patch attached? Regards, -- Michael commit 805e63f0c06c55c7133e9b42a2b57597d1f45494 Author: Michael Paquier Date: Mon Apr 21 12:48:30 2014 +0900 Replace wsock32.lib by ws2_32.lib in mkvc spec for Windows build wsock32.lib is the Windows socket library, an older and obsolete version of ws2_32.lib. diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm index 308a4b4..63d457e 100644 --- a/src/tools/msvc/Mkvcbuild.pm +++ b/src/tools/msvc/Mkvcbuild.pm @@ -44,7 +44,7 @@ my @contrib_uselibpgcommon = ( 'pg_test_fsync', 'pg_test_timing', 'pg_upgrade','pg_xlogdump', 'vacuumlo'); -my $contrib_extralibs = { 'pgbench' => ['wsock32.lib'] }; +my $contrib_extralibs = { 'pgbench' => ['ws2_32.lib'] }; my $contrib_extraincludes = { 'tsearch2' => ['contrib/tsearch2'], 'dblink' => ['src/backend'] }; my $contrib_extrasource = { @@ -113,9 +113,8 @@ sub mkvcbuild $postgres->AddFiles('src\backend\replication', 'repl_scanner.l', 'repl_gram.y'); $postgres->AddDefine('BUILDING_DLL'); - $postgres->AddLibrary('wsock32.lib'); - $postgres->AddLibrary('ws2_32.lib'); $postgres->AddLibrary('secur32.lib'); + $postgres->AddLibrary('ws2_32.lib'); $postgres->AddLibrary('wldap32.lib') if ($solution->{options}->{ldap}); $postgres->FullExportDLL('postgres.lib'); @@ -270,7 +269,6 @@ sub mkvcbuild $libpq->AddDefine('FRONTEND'); $libpq->AddDefine('UNSAFE_STAT_OK'); $libpq->AddIncludeDir('src\port'); - $libpq->AddLibrary('wsock32.lib'); $libpq->AddLibrary('secur32.lib'); $libpq->AddLibrary('ws2_32.lib'); $libpq->AddLibrary('wldap32.lib') if ($solution->{options}->{ldap}); @@ -300,7 +298,7 @@ sub mkvcbuild $libecpg->AddIncludeDir('src\interfaces\libpq'); $libecpg->AddIncludeDir('src\port'); $libecpg->UseDef('src\interfaces\ecpg\ecpglib\ecpglib.def'); - $libecpg->AddLibrary('wsock32.lib'); + $libecpg->AddLibrary('ws2_32.lib'); $libecpg->AddReference($libpq, $pgtypes, $libpgport); my $libecpgcompat = $solution->AddProject( @@ -345,7 +343,7 @@ sub mkvcbuild $isolation_tester->AddIncludeDir('src\interfaces\libpq'); $isolation_tester->AddDefine('HOST_TUPLE="i686-pc-win32vc"'); $isolation_tester->AddDefine('FRONTEND'); - $isolation_tester->AddLibrary('wsock32.lib'); + $isolation_tester->AddLibrary('ws2_32.lib'); $isolation_tester->AddReference($libpq, $libpgcommon, $libpgport); my $pgregress_isolation = @@ -363,7 +361,6 @@ sub mkvcbuild $initdb->AddIncludeDir('src\interfaces\libpq'); $initdb->AddIncludeDir('src\timezone'); $initdb->AddDefine('FRONTEND'); - $initdb->AddLibrary('wsock32.lib'); $initdb->AddLibrary('ws2_32.lib'); my $pgbasebackup = AddSimpleFrontend('pg_basebackup', 1); @@ -500,7 +497,7 @@ sub mkvcbuild 'pgp-mpi-internal.c', 'imath.c'); } $pgcrypto->AddReference($postgres); - $pgcrypto->AddLibrary('wsock32.lib'); + $pgcrypto->AddLibrary('ws2_32.lib'); my $mf = Project::read_file('contrib/pgcrypto/Makefile'); GenerateContribSqlFiles('pgcrypto', $mf); -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] DISCARD ALL (Again)
* Robert Haas (robertmh...@gmail.com) wrote: > On Fri, Apr 18, 2014 at 2:24 AM, Tom Lane wrote: > > 2. While I'm no Python expert, I believe GD is just a specific instance > > of a general capability for global state in Python. Are we going to > > promise that any and all user-created data inside Python goes away? > > What about other PLs? Will users thank us if this suddenly starts > > happening? > > This is not the first time that somebody's asked for a way to throw > away global interpreter state, and I really think we ought to oblige. > In a connection-pooling environment, you really need a way to get the > connection back to its original state rather than some not-so-near > facsimile thereof. Maybe it'll end up as an optional behavior, and > which kind of reset to use will become part of the pooler > configuration, but that doesn't bother me as much as not having it for > those that want it. Drop the connection and reconnect would be the answer to that. For as much as we may hope and wish for a connection to go back to 'the way it was upon first connection', throwing away the interpretor *might* (and I wouldn't be comfortable claiming it absolutely..) get you there when you've only called functions which use interpretors, but people write code in C too and we've seen complaints of memory leaks, etc, from C libraries and C extensions- and there's nothing we're going to be able to do to address that, so this mythical 'DISCARD EVERYTHING' is a pipe dream. (Were we to actually re-exec ourselves into a new process, as if we went through a disconnect/reconnect, I'd be more inclined to support this capability, but I'm not sure what such would really buy us...) > What's a bit odd about this request is that it asks for the ability to > throw away only part of the state. ISTM that if somebody wants to add > that kind of capability, they ought to just package a function which > does precisely that with the plpython extension, or create a Python > function that zaps that particular variable if that's possible. I > think it's clearly useful to have DISCARD ALL be a request to discard > *everything* in one shot, but it's going to be a stretch to come up > with DISCARD variants for every kind of partial state removal somebody > wants to do. Agreed. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Composite Datums containing toasted fields are a bad idea(?)
I wrote: > The main problem with this patch, as I see it, is that it'll introduce > extra syscache lookup overhead even when there are no toasted fields > anywhere. I've not really tried to quantify how much, since that would > require first agreeing on a benchmark case --- anybody have a thought > about what that ought to be? But in at least some of these functions, > we'll be paying a cache lookup or two per array element, which doesn't > sound promising. I created a couple of test cases that I think are close to worst-case for accumArrayResult() and array_set(), which are the two functions that seem most worth worrying about. The accumArrayResult test case is create table manycomplex as select row(random(),random())::complex as c from generate_series(1,100); vacuum analyze manycomplex; then time: select array_dims(array_agg(c)) from manycomplex; On HEAD, this takes about 295-300 msec on my machine in a non-cassert build. With the patch I sent previously, the time goes to 495-500 msec. Ouch. The other test case is create or replace function timearrayset(n int) returns void language plpgsql as $$ declare a complex[]; begin a := array[row(1,2), row(3,4), row(5,6)]; for i in 1..$1 loop a[2] := row(5,6)::complex; end loop; end; $$; select timearrayset(100); This goes from circa 1250 ms in HEAD to around 1680 ms. Some poking around with oprofile says that much of the added time is coming from added syscache and typcache lookups, as I suspected. I did some further work on the patch to make it possible to cache the element tuple descriptor across calls for these two functions. With the attached patch, the first test case comes down to about 335 ms and the second to about 1475 ms (plpgsql is still leaving some extra cache lookups on the table). More could be done with some further API changes, but I think this is about as much as is safe to back-patch. At least in the accumArrayResult test case, it would be possible to bring the runtime back to very close to HEAD if we were willing to abandon the principle that compressed fields within a tuple should always get decompressed when the tuple goes into a larger structure. That would allow flatten_composite_element to quit early if the tuple doesn't have the HEAP_HASEXTERNAL infomask bit set. I'm not sure that this would be a good tradeoff however: if we fail to remove nested compression, we could end up paying for that with double compression. On the other hand, it's unclear that that case would come up often, while the overhead of disassembling the tuple is definitely going to happen every time we assign to an array-of-composites element. (Also, there is no question here of regression relative to previous releases, since the existing code fails to prevent nested compression.) Comments? regards, tom lane diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c index aea9d40..48b09b8 100644 *** a/src/backend/access/common/heaptuple.c --- b/src/backend/access/common/heaptuple.c *** heap_form_tuple(TupleDesc tupleDescripto *** 649,674 * Check for nulls and embedded tuples; expand any toasted attributes in * embedded tuples. This preserves the invariant that toasting can only * go one level deep. - * - * We can skip calling toast_flatten_tuple_attribute() if the attribute - * couldn't possibly be of composite type. All composite datums are - * varlena and have alignment 'd'; furthermore they aren't arrays. Also, - * if an attribute is already toasted, it must have been sent to disk - * already and so cannot contain toasted attributes. */ for (i = 0; i < numberOfAttributes; i++) { if (isnull[i]) hasnull = true; ! 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, ! att[i]->atttypmod); ! } } /* --- 649,661 * Check for nulls and embedded tuples; expand any toasted attributes in * embedded tuples. This preserves the invariant that toasting can only * go one level deep. */ for (i = 0; i < numberOfAttributes; i++) { if (isnull[i]) hasnull = true; ! else ! values[i] = toast_flatten_tuple_attribute(values[i], att[i]); } /* *** heap_form_minimal_tuple(TupleDesc tupleD *** 1403,1428 * Check for nulls and embedded tuples; expand any toasted attributes in * embedded tuples. This preserves the invariant that toasting can only * go one level deep. - * - * We can skip calling toast_flatten_tuple_attribute() if the attribute - * couldn't possibly be of composite type. All composite datums are - * varlena and have alignment 'd'; furthermore they aren't arrays. Also, - * if an attribute is already toasted, it must have b
Re: [HACKERS] PostgreSQL in Windows console and Ctrl-C
OK, here is the first draft against current master. It builds on Windows with VS 2012 and on FreeBSD 10 with clang 3.3. I ran the regression tests on Windows, they all pass. The changed behavior is limited to Windows, where it now silently ignores Ctrl-C and Ctrl-Break when started via pg_ctl start. I don't think there is currently any support for switch-type long options, so rather than invent my own, I squeezed the two lines I added into postmaster.c where they fit best; unfortunately, the result is quite ugly. I'll be happy to refine that if someone can give me a hint on how to do it. Patch attached, will add to CF soon. -- Christian diff --git a/doc/src/sgml/ref/postgres-ref.sgml b/doc/src/sgml/ref/postgres-ref.sgml new file mode 100644 index 8e225e4..731682c *** a/doc/src/sgml/ref/postgres-ref.sgml --- b/doc/src/sgml/ref/postgres-ref.sgml *** PostgreSQL documentation *** 590,595 --- 590,620 + + + Platform-specific Options + + +--background + + background execution + Windows + + + + This option is effective on Windows only and ignored on all other platforms. + + + It instructs the server to ignore the signals caused by pressing + CtrlC or + CtrlBreak + in a console window. It is used automatically by + pg_ctl when called with the + start subcommand. + + + + diff --git a/src/backend/port/win32/signal.c b/src/backend/port/win32/signal.c new file mode 100644 index 322b857..91f8243 *** a/src/backend/port/win32/signal.c --- b/src/backend/port/win32/signal.c *** static pqsigfunc pg_signal_defaults[PG_S *** 41,46 --- 41,48 static DWORD WINAPI pg_signal_thread(LPVOID param); static BOOL WINAPI pg_console_handler(DWORD dwCtrlType); + extern bool IsBackgroundPostmaster; + extern bool IsUnderPostmaster; /* * pg_usleep --- delay the specified number of microseconds, but *** pg_signal_thread(LPVOID param) *** 346,358 static BOOL WINAPI pg_console_handler(DWORD dwCtrlType) { ! if (dwCtrlType == CTRL_C_EVENT || ! dwCtrlType == CTRL_BREAK_EVENT || ! dwCtrlType == CTRL_CLOSE_EVENT || ! dwCtrlType == CTRL_SHUTDOWN_EVENT) { pg_queue_signal(SIGINT); ! return TRUE; } ! return FALSE; } --- 348,369 static BOOL WINAPI pg_console_handler(DWORD dwCtrlType) { ! switch (dwCtrlType) { + case CTRL_C_EVENT: + case CTRL_BREAK_EVENT: + /* Ignore if started with "pg_ctl start". */ + if (IsUnderPostmaster || IsBackgroundPostmaster) + break; + /* fall through */ + case CTRL_CLOSE_EVENT: + case CTRL_SHUTDOWN_EVENT: pg_queue_signal(SIGINT); ! break; ! default: ! return FALSE; } ! ! return TRUE; } + diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c new file mode 100644 index b573fd8..d1ce978 *** a/src/backend/postmaster/postmaster.c --- b/src/backend/postmaster/postmaster.c *** PostmasterMain(int argc, char *argv[]) *** 751,771 *value; ParseLongOption(optarg, &name, &value); ! if (!value) { ! if (opt == '-') ! ereport(ERROR, ! (errcode(ERRCODE_SYNTAX_ERROR), ! errmsg("--%s requires a value", ! optarg))); ! else ! ereport(ERROR, ! (errcode(ERRCODE_SYNTAX_ERROR), ! errmsg("-c %s requires a value", ! optarg))); } - SetConfigOption(name, value, PGC_POSTMASTER, PGC_S_ARGV); free(name); if (value) free(value); --- 751,780 *value; ParseLongOption(optarg, &name, &value); !
Re: [HACKERS] DISCARD ALL (Again)
On Fri, Apr 18, 2014 at 2:24 AM, Tom Lane wrote: > 2. While I'm no Python expert, I believe GD is just a specific instance > of a general capability for global state in Python. Are we going to > promise that any and all user-created data inside Python goes away? > What about other PLs? Will users thank us if this suddenly starts > happening? This is not the first time that somebody's asked for a way to throw away global interpreter state, and I really think we ought to oblige. In a connection-pooling environment, you really need a way to get the connection back to its original state rather than some not-so-near facsimile thereof. Maybe it'll end up as an optional behavior, and which kind of reset to use will become part of the pooler configuration, but that doesn't bother me as much as not having it for those that want it. What's a bit odd about this request is that it asks for the ability to throw away only part of the state. ISTM that if somebody wants to add that kind of capability, they ought to just package a function which does precisely that with the plpython extension, or create a Python function that zaps that particular variable if that's possible. I think it's clearly useful to have DISCARD ALL be a request to discard *everything* in one shot, but it's going to be a stretch to come up with DISCARD variants for every kind of partial state removal somebody wants to do. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Minor improvement in gin_private.h
On Fri, Apr 18, 2014 at 10:43 AM, Etsuro Fujita wrote: > The attached improves a comment in gin_private.h a little bit. Committed also. That comment should probably be rephrased more extensively, something like "We use our own versions of ItemPointerGetBlockNumber and ItemPointerGetOffsetNumber ...". -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Minor improvement in src/backend/access/gin/README
On Fri, Apr 18, 2014 at 10:28 AM, Etsuro Fujita wrote: > The attached improves a document in src/backend/access/gin/README. Committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Perfomance degradation 9.3 (vs 9.2) for FreeBSD
> 20 apr 2014 kl. 12:19 skrev Francois Tigeot : > > Hi, > >> On Sun, Apr 20, 2014 at 11:24:38AM +0200, Palle Girgensohn wrote: >> >> I see performance degradation with PostgreSQL 9.3 vs 9.2 on FreeBSD, and I'm >> wondering who to poke to mitigate the problem. In reference to this thread >> [1], who where the FreeBSD people that Francois mentioned? > > At least one FreeBSD hacker came to discuss it on the #dragonflybsd irc > channel and tried to run the benchmark on a 80-core machine. > > I didn't keep logs and don't remember his/their name(s) but there was > definitely some FreeBSD effort at the time to investigate and fix things. > >> If mmap needs to perform well in the kernel, I'd like to know of someone >> with FreeBSD kernel knowledge who is interested in working with mmap >> perfocmance. If mmap is indeed the cuplrit, I've just tested 9.2.8 vs 9.3.4, >> I nevere isolated the mmap patch, although I believe Francois did just that >> with similar results. > > I did test the 9.3 -devel branch before and after the SysV shared memory => > mmap commit. The performance degradation was visible. > > I recently ran a few benchmarks of PostgreSQL 9.3 with different operating > systems > including DragonFly 3.6 and FreeBSD 10. You may be interested in the results: > > http://lists.dragonflybsd.org/pipermail/users/2014-March/128216.html > Interesting, indeed. The fixes to dragonfly where made quite recently, in 3.2, right? Was it an isolated patch that could perhaps be used as inspiration for a similar fix on freebsd, or is it the major rewrite of the scheduler mentioned in [http://m.slashdot.org/story/177299]? Palle > -- > Francois Tigeot -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Perfomance degradation 9.3 (vs 9.2) for FreeBSD
Hi, On Sun, Apr 20, 2014 at 11:24:38AM +0200, Palle Girgensohn wrote: > > I see performance degradation with PostgreSQL 9.3 vs 9.2 on FreeBSD, and I'm > wondering who to poke to mitigate the problem. In reference to this thread > [1], who where the FreeBSD people that Francois mentioned? At least one FreeBSD hacker came to discuss it on the #dragonflybsd irc channel and tried to run the benchmark on a 80-core machine. I didn't keep logs and don't remember his/their name(s) but there was definitely some FreeBSD effort at the time to investigate and fix things. > If mmap needs to perform well in the kernel, I'd like to know of someone with > FreeBSD kernel knowledge who is interested in working with mmap perfocmance. > If mmap is indeed the cuplrit, I've just tested 9.2.8 vs 9.3.4, I nevere > isolated the mmap patch, although I believe Francois did just that with > similar results. I did test the 9.3 -devel branch before and after the SysV shared memory => mmap commit. The performance degradation was visible. I recently ran a few benchmarks of PostgreSQL 9.3 with different operating systems including DragonFly 3.6 and FreeBSD 10. You may be interested in the results: http://lists.dragonflybsd.org/pipermail/users/2014-March/128216.html -- Francois Tigeot -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers