Re: [HACKERS] Column Redaction
On 10 October 2014 16:45, Rod Taylor rod.tay...@gmail.com wrote: On my laptop I can pull all 10,000 card numbers in less than 1 second. Right. Like I said: covert channels exist. Great example of how to exploit them, thanks. Cool SQL. What could be the use of a security feature that does not prevent security? As soon as you issue the above query, you have clearly indicated your intention to steal. Receiving information is no longer accidental, it is an explicit act that is logged in the auditing system against your name. This is sufficient to bury you in court and it is now a real deterrent. Redaction has worked. Redaction is similar to a 3m high razor wire fence. The fence reminds you of what is correct and dissuades you from going further. The fence does not prevent access by a determined and skillful agent (Rod), but the CCTV cameras that are set out will record the action. It will be almost impossible to claim you were just walking your dog, and the wire cutters were a gift for your brother in law. Redaction prevents accidental information loss only, forcing any loss that occurs to be explicit. It ensures that loss of information can be tied clearly back to an individual, like an ink packet that stains the fingers of a thief. I don't have a word or pithy phrase for this concept. Maybe something related to forcing their hand, flushing game into the open, or simply preventing tipping your hand and inadvertently allowing data loss. Redaction clearly relies completely on auditing before it can have any additional effect. And the effectiveness of redaction needs to be understood next to Rod's example. Since it relies on auditing, we need to do that first. -- 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] jsonb contains behaviour weirdness
This was never committed... On Sat, Sep 13, 2014 at 4:31 PM, Peter Geoghegan p...@heroku.com wrote: On Fri, Sep 12, 2014 at 6:40 AM, Alexander Korotkov aekorot...@gmail.com wrote: It's likely that JB_ROOT_COUNT(val) JB_ROOT_COUNT(tmpl) should be checked only for objects, not arrays. Also, should JsonbDeepContains does same fast check when it deals with nested objects? Attached patch implements something similar to what you describe here, fixing your example. I haven't added the optimization to JsonbDeepContains(). I think that if anything, we should remove the optimization entirely, which is what I've done -- an rhs is it contained within? value is hardly ever going to be an object that has more pairs than the object we're checking it is contained within. It's almost certainly going to have far fewer pairs. Apart from only applying to objects, that optimization just isn't an effective way of eliminating jsonb values from consideration quickly. I'd rather not bother at all, rather than having a complicated comment about why the optimization applies to objects and not arrays. -- Peter Geoghegan -- 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] Column Redaction
On 10 October 2014 11:27, Heikki Linnakangas hlinnakan...@vmware.com wrote: I googled for Oracle Data redaction, and found General Usage guidelines: General Usage Guidelines * Oracle Data Redaction is not intended to protect against attacks by privileged database users who run ad hoc queries directly against the database. * Oracle Data Redaction is not intended to protect against users who run exhaustive SQL queries that attempt to determine the actual values by inference. So it's not actually suitable for the example you gave. I don't think we want this feature... The full quote I read is the following... Even though Oracle Data Redaction is not intended to protect against attacks by database users who run ad hoc queries directly against the database, it can hide sensitive data for these ad hoc query scenarios when you couple it with other preventive and detective controls. That full context would have been useful. -- 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] orangutan seizes up during isolation-check
On 10/11/14 1:41 AM, Noah Misch wrote: Good question. It would be nice to make the change there, for the benefit of other consumers. The patch's setlocale_native_forked() assumes it never runs in a multithreaded process, but libintl_setlocale() must not assume that. I see a few ways libintl/gnulib might proceed: Yeah, it's difficult to see how they might proceed if they keep calling into Core Foundation, which might do anything, now or in the future. I went ahead and submitted a bug report to gettext: https://savannah.gnu.org/bugs/index.php?43404 (They way I understand it is that the files concerned originate in gettext and are copied to gnulib.) Let's see what they say. I like the idea of calling pthread_is_threaded_np() as a verification. This appears to be a OS X-specific function at the moment. If other platforms start adding it, then we'll run into the usual problems of how to link binaries that use pthread functions. Maybe that's not a realistic concern. -- 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] Wait free LW_SHARED acquisition - v0.9
Hi, On 2014-10-11 07:26:57 +0530, Amit Kapila wrote: On Sat, Oct 11, 2014 at 7:00 AM, Andres Freund and...@2ndquadrant.com And since your general performance numbers are a fair bit lower than what I see with, hopefully, the same code on the same machine... You have reported numbers at 1000 scale factor and mine were at 3000 scale factor, so I think the difference is expected. The numbers for 3000 show pretty much the same: SCALE 128 160 175 HEAD352113 339005 336491 LW_SHARED 365874 347931 342528 Hm. I wonder if you're using pgbench without -M prepared? That'd about explain the difference. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] Add CREATE support to event triggers
On Thu, Oct 9, 2014 at 8:29 AM, Michael Paquier michael.paqu...@gmail.com wrote: On Thu, Oct 9, 2014 at 6:26 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: However, if I were to do it, I would instead create a quote.h file and would also add the quote_literal_cstr() prototype to it, perhaps even move the implementations from their current ruleutils.c location to quote.c. (Why is half the stuff in ruleutils.c rather than quote.c is beyond me.) Yes, an extra header file would be cleaner. Now if we are worried about creating much incompatibilities with existing extension (IMO that's not something core should worry much about), then it is better not to do it. Now, if I can vote on it, I think it is worth doing in order to have builtins.h only contain only Datum foo(PG_FUNCTION_ARGS), and move all the quote-related things in quote.c. The attached patch implements this idea, creating a new header quote.h in include/utils that groups all the quote-related functions. This makes the code more organized. Regards, -- Michael From b14000662a52c3f5b40cf43a1f6699e0d024d1fd Mon Sep 17 00:00:00 2001 From: Michael Paquier mich...@otacoo.com Date: Sat, 11 Oct 2014 22:28:05 +0900 Subject: [PATCH] Move all quote-related functions into a single header quote.h Note that this impacts as well contrib modules, and mainly external modules particularly for quote_identifier. --- contrib/dblink/dblink.c | 1 + contrib/postgres_fdw/deparse.c| 1 + contrib/postgres_fdw/postgres_fdw.c | 1 + contrib/test_decoding/test_decoding.c | 1 + contrib/worker_spi/worker_spi.c | 1 + src/backend/catalog/namespace.c | 1 + src/backend/catalog/objectaddress.c | 1 + src/backend/commands/explain.c| 1 + src/backend/commands/extension.c | 1 + src/backend/commands/matview.c| 1 + src/backend/commands/trigger.c| 1 + src/backend/commands/tsearchcmds.c| 1 + src/backend/parser/parse_utilcmd.c| 1 + src/backend/utils/adt/format_type.c | 1 + src/backend/utils/adt/quote.c | 110 ++ src/backend/utils/adt/regproc.c | 1 + src/backend/utils/adt/ri_triggers.c | 1 + src/backend/utils/adt/ruleutils.c | 109 + src/backend/utils/adt/varlena.c | 1 + src/backend/utils/cache/ts_cache.c| 1 + src/backend/utils/misc/guc.c | 1 + src/include/utils/builtins.h | 6 -- src/include/utils/quote.h | 28 + src/pl/plpgsql/src/pl_gram.y | 1 + src/pl/plpython/plpy_plpymodule.c | 1 + 25 files changed, 160 insertions(+), 114 deletions(-) create mode 100644 src/include/utils/quote.h diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c index d77b3ee..cbbc513 100644 --- a/contrib/dblink/dblink.c +++ b/contrib/dblink/dblink.c @@ -56,6 +56,7 @@ #include utils/guc.h #include utils/lsyscache.h #include utils/memutils.h +#include utils/quote.h #include utils/rel.h #include utils/tqual.h diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c index 2045774..0009cd3 100644 --- a/contrib/postgres_fdw/deparse.c +++ b/contrib/postgres_fdw/deparse.c @@ -50,6 +50,7 @@ #include parser/parsetree.h #include utils/builtins.h #include utils/lsyscache.h +#include utils/quote.h #include utils/syscache.h diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 4c49776..feb41f5 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -36,6 +36,7 @@ #include utils/guc.h #include utils/lsyscache.h #include utils/memutils.h +#include utils/quote.h PG_MODULE_MAGIC; diff --git a/contrib/test_decoding/test_decoding.c b/contrib/test_decoding/test_decoding.c index fdbd313..b168fc3 100644 --- a/contrib/test_decoding/test_decoding.c +++ b/contrib/test_decoding/test_decoding.c @@ -25,6 +25,7 @@ #include utils/builtins.h #include utils/lsyscache.h #include utils/memutils.h +#include utils/quote.h #include utils/rel.h #include utils/relcache.h #include utils/syscache.h diff --git a/contrib/worker_spi/worker_spi.c b/contrib/worker_spi/worker_spi.c index 328c722..7ada98d 100644 --- a/contrib/worker_spi/worker_spi.c +++ b/contrib/worker_spi/worker_spi.c @@ -38,6 +38,7 @@ #include lib/stringinfo.h #include pgstat.h #include utils/builtins.h +#include utils/quote.h #include utils/snapmgr.h #include tcop/utility.h diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c index 5eb8fd4..7f8f54b 100644 --- a/src/backend/catalog/namespace.c +++ b/src/backend/catalog/namespace.c @@ -53,6 +53,7 @@ #include utils/inval.h #include utils/lsyscache.h #include utils/memutils.h +#include utils/quote.h #include utils/syscache.h diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index b69b75b..d7a1ec7 100644 ---
Re: [HACKERS] Wait free LW_SHARED acquisition - v0.9
On Sat, Oct 11, 2014 at 6:40 PM, Andres Freund and...@2ndquadrant.com wrote: On 2014-10-11 07:26:57 +0530, Amit Kapila wrote: On Sat, Oct 11, 2014 at 7:00 AM, Andres Freund and...@2ndquadrant.com And since your general performance numbers are a fair bit lower than what I see with, hopefully, the same code on the same machine... You have reported numbers at 1000 scale factor and mine were at 3000 scale factor, so I think the difference is expected. The numbers for 3000 show pretty much the same: SCALE 128 160 175 HEAD352113 339005 336491 LW_SHARED 365874 347931 342528 Hm. I wonder if you're using pgbench without -M prepared? No, I use below statement: ./pgbench -c 128 -j 128 -T 300 -S -M prepared postgres That'd about explain the difference. Here I think first thing to clarify is why the numbers on HEAD are different? Another thing is that I generally see difference in numbers at 1000 and 3000 scale factor (although I have not run lately), but in your case the numbers are almost same. I will try once more by cleaning every thing(installation, data_dir, etc..) but not today... With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: [HACKERS] Column Redaction
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 10/11/2014 02:40 AM, Simon Riggs wrote: As soon as you issue the above query, you have clearly indicated your intention to steal. Receiving information is no longer accidental, it is an explicit act that is logged in the auditing system against your name. This is sufficient to bury you in court and it is now a real deterrent. Redaction has worked. Redaction is similar to a 3m high razor wire fence. The fence reminds you of what is correct and dissuades you from going further. The fence does not prevent access by a determined and skillful agent (Rod), but the CCTV cameras that are set out will record the action. It will be almost impossible to claim you were just walking your dog, and the wire cutters were a gift for your brother in law. Redaction prevents accidental information loss only, forcing any loss that occurs to be explicit. It ensures that loss of information can be tied clearly back to an individual, like an ink packet that stains the fingers of a thief. I don't have a word or pithy phrase for this concept. Maybe something related to forcing their hand, flushing game into the open, or simply preventing tipping your hand and inadvertently allowing data loss. Redaction clearly relies completely on auditing before it can have any additional effect. And the effectiveness of redaction needs to be understood next to Rod's example. Since it relies on auditing, we need to do that first. This is a really good summary. I definitely know of folks who would be interested in this feature, but I also agree, as you have said, it relies on a good audit trail. Joe - -- Joe Conway credativ LLC: http://www.credativ.us Linux, PostgreSQL, and general Open Source Training, Service, Consulting, 24x7 Support -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQIcBAEBAgAGBQJUOTOGAAoJEDfy90M199hlswcP/1qUtwvsb+a4hKqL3FsIIkmK +2f5x+TRm1C5B04QhVa4A7iOr+lfzcoGChV2x2EwCqKJWNzwcpZfB/vBNv593KU4 /WZ+r0o0Hih69dE8gAS602xkrw8x3iAqcTzfyrfiE2O9yhYjoCmqqPls6PtgACc7 JI9pNiPRO+Sd2B308FaD70KkbnGDjMeFPgrxU7NRZwf0NG/bkDq28vSJl5QLg6DO lFEtB1mMVWWmlnfTgw+zTXamxPJZTLK2Z38OBX3mjjD+64kEMjI5YQ39X8T9Ndfu 0dCA6KCqfCiy/ANETv0ScdoO/uiEQ6VfkbXy1lHK9sWDgu7HOwTPo4c0ft4tILDK NIXvCYAFK0aPzuEVLFfwf6wm6BP7kuJ+42fY+VwMwCkt4DoQpLRJChIQzJ9ilmK2 suMSmC/sxHeRkLwRAo4uHyAzLZbectq3VC6Zdjlx35jdWG7We1katBoIU8MOC0sc YFcUJRQk+PTxjp1fOPS7szDZulCMMXP4s0v07hiW5z6EaY82I9mJk6dnuk8eha16 3h4zBgbkM9hZhKLlbwLFSUKZrQdUklRJDXQhUuUqSIOQAU02zEKs2Pl0w1l+h5CY cb0xPfvkIVPgrDMRfEhdbr+rh2jcEE4gQeuWNe0cexuyZiKI+Xc2MLscaeqIeBNJ bEur+OvRj+wlnrYPGA80 =gTcG -END PGP SIGNATURE- -- 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] Column Redaction
On Sat, Oct 11, 2014 at 09:51:28AM +0100, Simon Riggs wrote: So it's not actually suitable for the example you gave. I don't think we want this feature... The full quote I read is the following... Even though Oracle Data Redaction is not intended to protect against attacks by database users who run ad hoc queries directly against the database, it can hide sensitive data for these ad hoc query scenarios when you couple it with other preventive and detective controls. That full context would have been useful. OK, that certainly helps. I think the interesting question, though, is whether we can create a data type that doesn't have any casting or comparison functions, and has limited or no output function, and is useful. Are there are cases where you would want to store data in a database that could not be fully viewed but still would be useful to be stored. For example, for a credit card type, you would output the last four digits, but is there any value to storing the non-visible digits? You can check the checksum of the digits, but that can be done on input and doesn't require the storage of the digits. Is there some function we could provide that would make that data type useful? Could we provide comparison functions with delays or increasing delays? I can think of a useful fully-redacted data type example, and that would be the credit card expire date. You could store that in a field that has no output or comparison functions, but you could provide a useful function that would tell whether the expire date had passed based on the system date. It would be useful to store such a date, and a user could know the data value only after it had expired. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] split builtins.h to quote.h
Started a new thread to raise awareness. Michael Paquier wrote: On Thu, Oct 9, 2014 at 8:29 AM, Michael Paquier michael.paqu...@gmail.com wrote: On Thu, Oct 9, 2014 at 6:26 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: However, if I were to do it, I would instead create a quote.h file and would also add the quote_literal_cstr() prototype to it, perhaps even move the implementations from their current ruleutils.c location to quote.c. (Why is half the stuff in ruleutils.c rather than quote.c is beyond me.) Yes, an extra header file would be cleaner. Now if we are worried about creating much incompatibilities with existing extension (IMO that's not something core should worry much about), then it is better not to do it. Now, if I can vote on it, I think it is worth doing in order to have builtins.h only contain only Datum foo(PG_FUNCTION_ARGS), and move all the quote-related things in quote.c. The attached patch implements this idea, creating a new header quote.h in include/utils that groups all the quote-related functions. This makes the code more organized. If someone wants to object to this, please speak quickly. Ref: this comes from http://www.postgresql.org/message-id/cab7npqr1ivd5r_qn_ngmkbolqmagbosj4wnpo8eybnn6we_...@mail.gmail.com From b14000662a52c3f5b40cf43a1f6699e0d024d1fd Mon Sep 17 00:00:00 2001 From: Michael Paquier mich...@otacoo.com Date: Sat, 11 Oct 2014 22:28:05 +0900 Subject: [PATCH] Move all quote-related functions into a single header quote.h Note that this impacts as well contrib modules, and mainly external modules particularly for quote_identifier. --- contrib/dblink/dblink.c | 1 + contrib/postgres_fdw/deparse.c| 1 + contrib/postgres_fdw/postgres_fdw.c | 1 + contrib/test_decoding/test_decoding.c | 1 + contrib/worker_spi/worker_spi.c | 1 + src/backend/catalog/namespace.c | 1 + src/backend/catalog/objectaddress.c | 1 + src/backend/commands/explain.c| 1 + src/backend/commands/extension.c | 1 + src/backend/commands/matview.c| 1 + src/backend/commands/trigger.c| 1 + src/backend/commands/tsearchcmds.c| 1 + src/backend/parser/parse_utilcmd.c| 1 + src/backend/utils/adt/format_type.c | 1 + src/backend/utils/adt/quote.c | 110 ++ src/backend/utils/adt/regproc.c | 1 + src/backend/utils/adt/ri_triggers.c | 1 + src/backend/utils/adt/ruleutils.c | 109 + src/backend/utils/adt/varlena.c | 1 + src/backend/utils/cache/ts_cache.c| 1 + src/backend/utils/misc/guc.c | 1 + src/include/utils/builtins.h | 6 -- src/include/utils/quote.h | 28 + src/pl/plpgsql/src/pl_gram.y | 1 + src/pl/plpython/plpy_plpymodule.c | 1 + 25 files changed, 160 insertions(+), 114 deletions(-) create mode 100644 src/include/utils/quote.h diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c index d77b3ee..cbbc513 100644 --- a/contrib/dblink/dblink.c +++ b/contrib/dblink/dblink.c @@ -56,6 +56,7 @@ #include utils/guc.h #include utils/lsyscache.h #include utils/memutils.h +#include utils/quote.h #include utils/rel.h #include utils/tqual.h diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c index 2045774..0009cd3 100644 --- a/contrib/postgres_fdw/deparse.c +++ b/contrib/postgres_fdw/deparse.c @@ -50,6 +50,7 @@ #include parser/parsetree.h #include utils/builtins.h #include utils/lsyscache.h +#include utils/quote.h #include utils/syscache.h diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 4c49776..feb41f5 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -36,6 +36,7 @@ #include utils/guc.h #include utils/lsyscache.h #include utils/memutils.h +#include utils/quote.h PG_MODULE_MAGIC; diff --git a/contrib/test_decoding/test_decoding.c b/contrib/test_decoding/test_decoding.c index fdbd313..b168fc3 100644 --- a/contrib/test_decoding/test_decoding.c +++ b/contrib/test_decoding/test_decoding.c @@ -25,6 +25,7 @@ #include utils/builtins.h #include utils/lsyscache.h #include utils/memutils.h +#include utils/quote.h #include utils/rel.h #include utils/relcache.h #include utils/syscache.h diff --git a/contrib/worker_spi/worker_spi.c b/contrib/worker_spi/worker_spi.c index 328c722..7ada98d 100644 --- a/contrib/worker_spi/worker_spi.c +++ b/contrib/worker_spi/worker_spi.c @@ -38,6 +38,7 @@ #include lib/stringinfo.h #include pgstat.h #include utils/builtins.h +#include utils/quote.h #include utils/snapmgr.h #include tcop/utility.h diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c index 5eb8fd4..7f8f54b 100644 ---
Re: [HACKERS] Wait free LW_SHARED acquisition - v0.9
On 2014-10-11 15:10:45 +0200, Andres Freund wrote: Hi, On 2014-10-11 07:26:57 +0530, Amit Kapila wrote: On Sat, Oct 11, 2014 at 7:00 AM, Andres Freund and...@2ndquadrant.com And since your general performance numbers are a fair bit lower than what I see with, hopefully, the same code on the same machine... You have reported numbers at 1000 scale factor and mine were at 3000 scale factor, so I think the difference is expected. The numbers for 3000 show pretty much the same: SCALE 128 160 175 HEAD 352113 339005 336491 LW_SHARED 365874 347931 342528 Hm. I wonder if you're using pgbench without -M prepared? That'd about explain the difference. Started a test for a run without -M prepared (i.e. -M simple): SCALE 1 2 4 8 16 32 64 96 128 160 196 HEAD796815132 31147 63395 123551 180436 242098 263625 249652 244240 232679 LW_SHARED 826815032 31267 63911 118943 180447 247067 269262 259165 247166 231292 This really doesn't look exciting to me. scale 200, -M prepared: SCALE 1 2 4 8 16 32 64 96 128 160 196 HEAD13032 24712 50967 103801 201745 279226 380844 392623 379953 357206 361072 LW_SHARED 12997 25134 51119 102920 199597 282511 422424 460222 447662 436959 418519 My guess is that the primary benefit on systems with few sockets, like this, is that with the new code there's far fewer problems with processes sleeping (being scheduled out) while holding a spinlock. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Function array_agg(array)
Greetings,
While looking for easier items in PostgreSQL Wiki's Todo List (this will be
my 3rd patch), i found this TODO:
Add a built-in array_agg(anyarray) or similar, that can aggregate
1-dimensional arrays into a 2-dimensional array.
I've stumbled by this lack of array_agg(anyarray) sometimes ago in my work,
so i decided to explore this.
Currently, if we try array_agg(anyarray), PostgreSQL behaves like this:
# select array_agg('{1,2}'::int[]);
ERROR: could not find array type for data type integer[]
Reading implementation of array_agg, it looks like the array_agg function
is generic, and can process any input. The error comes from PostgreSQL not
finding array type for int[] (_int4 in pg_proc).
In PostgreSQL, any array is multidimensional, array type for any array is
the same:
- the type of {1,2} is int[]
- {{1,2}, {3,4}} is int[]
- {{{1},{2}, {3} ,{4}}} is still int[]
So, can't we just set the typarray of array types to its self oid? (patch
attached). So far:
- the array_agg is working and returning correct types:
backend select array_agg('{1,2}'::int[]);
1: array_agg(typeid = 1007, len = -1, typmod = -1, byval = f)
1: array_agg = {{1,2}}(typeid = 1007, len = -1, typmod = -1,
byval = f)
select array_agg('{''a'',''b''}'::varchar[]);
1: array_agg(typeid = 1015, len = -1, typmod = -1, byval = f)
1: array_agg = {{'a','b'}}(typeid = 1015, len = -1, typmod =
-1, byval = f)
- Regression tests passed except for the pg_type sanity check while
checking typelem relation with typarray:
SELECT p1.oid, p1.typname as basetype, p2.typname as arraytype,
p2.typelem, p2.typlen
FROM pg_type p1 LEFT JOIN pg_type p2 ON (p1.typarray = p2.oid)
WHERE p1.typarray 0 AND
(p2.oid IS NULL OR p2.typelem p1.oid OR p2.typlen -1);
! oid |basetype| arraytype| typelem | typlen
! --+++-+
! 143 | _xml | _xml | 142 | -1
! 199 | _json | _json | 114 | -1
! 629 | _line | _line | 628 | -1
! 719 | _circle| _circle| 718 | -1
... (cut)
Aside from the sanity check complaints, I don't see any problems in the
resulting array operations.
So, back to the question: Can't we just set the typarray of array types to
its self oid?
Regards,
--
Ali Akbar
diff --git a/src/include/catalog/pg_type.h b/src/include/catalog/pg_type.h
index b7d9256..7934874 100644
--- a/src/include/catalog/pg_type.h
+++ b/src/include/catalog/pg_type.h
@@ -357,8 +357,8 @@ DATA(insert OID = 114 ( json PGNSP PGUID -1 f b U f t \054 0 0 199 json_in j
DATA(insert OID = 142 ( xml PGNSP PGUID -1 f b U f t \054 0 0 143 xml_in xml_out xml_recv xml_send - - - i x f 0 -1 0 0 _null_ _null_ _null_ ));
DESCR(XML content);
#define XMLOID 142
-DATA(insert OID = 143 ( _xml PGNSP PGUID -1 f b A f t \054 0 142 0 array_in array_out array_recv array_send - - array_typanalyze i x f 0 -1 0 0 _null_ _null_ _null_ ));
-DATA(insert OID = 199 ( _json PGNSP PGUID -1 f b A f t \054 0 114 0 array_in array_out array_recv array_send - - array_typanalyze i x f 0 -1 0 0 _null_ _null_ _null_ ));
+DATA(insert OID = 143 ( _xml PGNSP PGUID -1 f b A f t \054 0 142 143 array_in array_out array_recv array_send - - array_typanalyze i x f 0 -1 0 0 _null_ _null_ _null_ ));
+DATA(insert OID = 199 ( _json PGNSP PGUID -1 f b A f t \054 0 114 199 array_in array_out array_recv array_send - - array_typanalyze i x f 0 -1 0 0 _null_ _null_ _null_ ));
DATA(insert OID = 194 ( pg_node_tree PGNSP PGUID -1 f b S f t \054 0 0 0 pg_node_tree_in pg_node_tree_out pg_node_tree_recv pg_node_tree_send - - - i x f 0 -1 0 100 _null_ _null_ _null_ ));
DESCR(string representing an internal node tree);
@@ -395,7 +395,7 @@ DESCR(geometric polygon '(pt1,...)');
DATA(insert OID = 628 ( line PGNSP PGUID 24 f b G f t \054 0 701 629 line_in line_out line_recv line_send - - - d p f 0 -1 0 0 _null_ _null_ _null_ ));
DESCR(geometric line);
#define LINEOID 628
-DATA(insert OID = 629 ( _line PGNSP PGUID -1 f b A f t \054 0 628 0 array_in array_out array_recv array_send - - array_typanalyze d x f 0 -1 0 0 _null_ _null_ _null_ ));
+DATA(insert OID = 629 ( _line PGNSP PGUID -1 f b A f t \054 0 628 629 array_in array_out array_recv array_send - - array_typanalyze d x f 0 -1 0 0 _null_ _null_ _null_ ));
/* OIDS 700 - 799 */
@@ -421,11 +421,11 @@ DESCR();
DATA(insert OID = 718 ( circlePGNSP PGUID 24 f b G f t \054 0 0 719 circle_in circle_out circle_recv circle_send - - - d p f 0 -1 0 0 _null_ _null_ _null_ ));
DESCR(geometric circle '(center,radius)');
#define CIRCLEOID 718
-DATA(insert OID = 719 ( _circle PGNSP PGUID -1 f b A f t \054 0 718 0 array_in array_out array_recv array_send - - array_typanalyze d x f 0 -1 0 0 _null_ _null_ _null_ ));
+DATA(insert OID = 719 ( _circle PGNSP PGUID -1 f b A f t \054 0 718 719
Re: [HACKERS] Function array_agg(array)
Ali Akbar the.ap...@gmail.com writes: So, can't we just set the typarray of array types to its self oid? Seems dangerous as heck; certainly it would have side-effects far more wide-ranging than just making this particular function work. A safer answer is to split array_agg into two functions, array_agg(anynonarray) - anyarray array_agg(anyarray) - anyarray I rather imagine you should do that anyway, because I really doubt that this hack is operating quite as intended. I suspect you are producing arrays containing arrays as elements, not true 2-D arrays. That's not a direction we want to go in I think; certainly there are no other operations that produce such things. regards, tom lane -- 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] jsonb contains behaviour weirdness
Peter Geoghegan p...@heroku.com writes: This was never committed... Yeah ... the discussion trailed off and I think we forgot to actually commit a fix. Will go do so. regards, tom lane -- 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] pg_dump refactor patch to remove global variables
Here's the complete patch in case anyone is wondering.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
index 2f855cf..17e9574 100644
--- a/src/bin/pg_dump/common.c
+++ b/src/bin/pg_dump/common.c
@@ -64,7 +64,7 @@ static DumpableObject **nspinfoindex;
static void flagInhTables(TableInfo *tbinfo, int numTables,
InhInfo *inhinfo, int numInherits);
-static void flagInhAttrs(TableInfo *tblinfo, int numTables);
+static void flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables);
static DumpableObject **buildIndexArray(void *objArray, int numObjs,
Size objSize);
static int DOCatalogIdCompare(const void *p1, const void *p2);
@@ -78,7 +78,7 @@ static int strInArray(const char *pattern, char **arr, int arr_size);
* Collect information about all potentially dumpable objects
*/
TableInfo *
-getSchemaData(Archive *fout, int *numTablesPtr)
+getSchemaData(Archive *fout, DumpOptions *dopt, int *numTablesPtr)
{
ExtensionInfo *extinfo;
InhInfo*inhinfo;
@@ -114,7 +114,7 @@ getSchemaData(Archive *fout, int *numTablesPtr)
*/
if (g_verbose)
write_msg(NULL, reading user-defined tables\n);
- tblinfo = getTables(fout, numTables);
+ tblinfo = getTables(fout, dopt, numTables);
tblinfoindex = buildIndexArray(tblinfo, numTables, sizeof(TableInfo));
/* Do this after we've built tblinfoindex */
@@ -122,11 +122,11 @@ getSchemaData(Archive *fout, int *numTablesPtr)
if (g_verbose)
write_msg(NULL, reading extensions\n);
- extinfo = getExtensions(fout, numExtensions);
+ extinfo = getExtensions(fout, dopt, numExtensions);
if (g_verbose)
write_msg(NULL, reading user-defined functions\n);
- funinfo = getFuncs(fout, numFuncs);
+ funinfo = getFuncs(fout, dopt, numFuncs);
funinfoindex = buildIndexArray(funinfo, numFuncs, sizeof(FuncInfo));
/* this must be after getTables and getFuncs */
@@ -142,7 +142,7 @@ getSchemaData(Archive *fout, int *numTablesPtr)
if (g_verbose)
write_msg(NULL, reading user-defined aggregate functions\n);
- getAggregates(fout, numAggregates);
+ getAggregates(fout, dopt, numAggregates);
if (g_verbose)
write_msg(NULL, reading user-defined operators\n);
@@ -183,7 +183,7 @@ getSchemaData(Archive *fout, int *numTablesPtr)
if (g_verbose)
write_msg(NULL, reading default privileges\n);
- getDefaultACLs(fout, numDefaultACLs);
+ getDefaultACLs(fout, dopt, numDefaultACLs);
if (g_verbose)
write_msg(NULL, reading user-defined collations\n);
@@ -213,7 +213,7 @@ getSchemaData(Archive *fout, int *numTablesPtr)
*/
if (g_verbose)
write_msg(NULL, finding extension members\n);
- getExtensionMembership(fout, extinfo, numExtensions);
+ getExtensionMembership(fout, dopt, extinfo, numExtensions);
/* Link tables to parents, mark parents of target tables interesting */
if (g_verbose)
@@ -222,11 +222,11 @@ getSchemaData(Archive *fout, int *numTablesPtr)
if (g_verbose)
write_msg(NULL, reading column info for interesting tables\n);
- getTableAttrs(fout, tblinfo, numTables);
+ getTableAttrs(fout, dopt, tblinfo, numTables);
if (g_verbose)
write_msg(NULL, flagging inherited columns in subtables\n);
- flagInhAttrs(tblinfo, numTables);
+ flagInhAttrs(dopt, tblinfo, numTables);
if (g_verbose)
write_msg(NULL, reading indexes\n);
@@ -307,7 +307,7 @@ flagInhTables(TableInfo *tblinfo, int numTables,
* modifies tblinfo
*/
static void
-flagInhAttrs(TableInfo *tblinfo, int numTables)
+flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables)
{
int i,
j,
@@ -384,7 +384,7 @@ flagInhAttrs(TableInfo *tblinfo, int numTables)
attrDef-adef_expr = pg_strdup(NULL);
/* Will column be dumped explicitly? */
-if (shouldPrintColumn(tbinfo, j))
+if (shouldPrintColumn(dopt, tbinfo, j))
{
attrDef-separate = false;
/* No dependency needed: NULL cannot have dependencies */
diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h
index b387aa1..06763ed 100644
--- a/src/bin/pg_dump/dumputils.h
+++ b/src/bin/pg_dump/dumputils.h
@@ -19,6 +19,23 @@
#include libpq-fe.h
#include pqexpbuffer.h
+/*
+ * Data structures for simple lists of OIDs and strings. The support for
+ * these is very primitive compared to the backend's List facilities, but
+ * it's all we need in pg_dump.
+ */
+typedef struct SimpleOidListCell
+{
+ struct SimpleOidListCell *next;
+ Oid val;
+} SimpleOidListCell;
+
+typedef struct SimpleOidList
+{
+ SimpleOidListCell *head;
+ SimpleOidListCell *tail;
+} SimpleOidList;
+
typedef struct SimpleStringListCell
{
struct SimpleStringListCell *next;
@@ -31,6 +48,7 @@ typedef struct SimpleStringList
SimpleStringListCell *tail;
} SimpleStringList;
+#define atooid(x) ((Oid) strtoul((x), NULL, 10))
extern int quote_all_identifiers;
extern PQExpBuffer (*getLocalPQExpBuffer) (void);
diff --git
Re: [HACKERS] Shared memory changes in 9.4?
Uh, this patch was never applied. Do we want it? --- On Thu, Jun 12, 2014 at 03:39:14PM +0200, Christoph Berg wrote: Re: Andres Freund 2014-06-12 20140612094112.gz8...@alap3.anarazel.de * Make initdb determine the best shm type for this platform and write it into postgresql.conf as it does now. * If no dynamic_shared_memory_type is found in the config, default to none. * Modify the three identical error messages concerned about shm segments to include the shm type instead of always just saying FATAL: could not open shared memory segment * Add a HINT to the POSIX error message: HINT: This might indicate that /dev/shm is not mounted, or its permissions do not allow the database user to create files there Sounds like a sane plan to me. Here are two patches, one that implements the annotated error messages, and one that selects none as default. It might also make sense to add a Note that POSIX depends on /dev/shm, and also a Note that dynamic_shared_memory_type is not related to the shared_buffers shm segments, which I didn't include here. Christoph -- c...@df7cb.de | http://www.df7cb.de/ diff --git a/src/backend/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c new file mode 100644 index 0819641..780e3a5 *** a/src/backend/storage/ipc/dsm_impl.c --- b/src/backend/storage/ipc/dsm_impl.c *** dsm_impl_posix(dsm_op op, dsm_handle han *** 289,296 if (errno != EEXIST) ereport(elevel, (errcode_for_dynamic_shared_memory(), ! errmsg(could not open shared memory segment \%s\: %m, ! name))); return false; } --- 289,299 if (errno != EEXIST) ereport(elevel, (errcode_for_dynamic_shared_memory(), ! errmsg(could not open POSIX shared memory segment \%s\: %m, ! name), ! errhint(This error usually means that /dev/shm is not mounted, or its ! permissions do not allow the database user to create files ! there.))); return false; } *** dsm_impl_posix(dsm_op op, dsm_handle han *** 313,319 ereport(elevel, (errcode_for_dynamic_shared_memory(), ! errmsg(could not stat shared memory segment \%s\: %m, name))); return false; } --- 316,322 ereport(elevel, (errcode_for_dynamic_shared_memory(), ! errmsg(could not stat POSIX shared memory segment \%s\: %m, name))); return false; } *** dsm_impl_posix(dsm_op op, dsm_handle han *** 332,338 ereport(elevel, (errcode_for_dynamic_shared_memory(), ! errmsg(could not resize shared memory segment %s to %zu bytes: %m, name, request_size))); return false; } --- 335,341 ereport(elevel, (errcode_for_dynamic_shared_memory(), ! errmsg(could not resize POSIX shared memory segment %s to %zu bytes: %m, name, request_size))); return false; } *** dsm_impl_posix(dsm_op op, dsm_handle han *** 358,364 ereport(elevel, (errcode_for_dynamic_shared_memory(), !errmsg(could not unmap shared memory segment \%s\: %m, name))); return false; } --- 361,367 ereport(elevel, (errcode_for_dynamic_shared_memory(), !errmsg(could not unmap POSIX shared memory segment \%s\: %m, name))); return false; } *** dsm_impl_posix(dsm_op op, dsm_handle han *** 382,388 ereport(elevel, (errcode_for_dynamic_shared_memory(), ! errmsg(could not map shared memory segment
Re: [HACKERS] uninitialized values in revised prepared xact code
Uh, was this fixed. I see a cleanup commit for this C file, but this report is from June: commit 07a4a93a0e35a778c77ffbbbc18de29e859e18f0 Author: Heikki Linnakangas heikki.linnakan...@iki.fi Date: Fri May 16 09:47:50 2014 +0300 Initialize tsId and dbId fields in WAL record of COMMIT PREPARED. Commit dd428c79 added dbId and tsId to the xl_xact_commit struct but missed that prepared transaction commits reuse that struct. Fix that. Because those fields were left unitialized, replaying a commit prepared WAL record in a hot standby node would fail to remove the relcache init file. That can lead to could not open file errors on the standby. Relcache init file only needs to be removed when a system table/index is rewritten in the transaction using two phase commit, so that should be rare in practice. In HEAD, the incorrect dbId/tsId values are also used for filtering in logical replication code, causing the transaction to always be filtered out. Analysis and fix by Andres Freund. Backpatch to 9.0 where hot standby was introduced. --- On Mon, Jun 30, 2014 at 11:58:59AM +0200, Andres Freund wrote: Hi, I've just rerun valgrind for the first time in a while and saw the following splat. My guess is it exists since bb38fb0d43c, but that's blindly guessing: ==2049== Use of uninitialised value of size 8 ==2049==at 0x4FE66D: EndPrepare (twophase.c:1063) ==2049==by 0x4F231B: PrepareTransaction (xact.c:2217) ==2049==by 0x4F2A38: CommitTransactionCommand (xact.c:2676) ==2049==by 0x79013E: finish_xact_command (postgres.c:2408) ==2049==by 0x78DE97: exec_simple_query (postgres.c:1062) ==2049==by 0x791FDD: PostgresMain (postgres.c:4010) ==2049==by 0x71B13B: BackendRun (postmaster.c:4113) ==2049==by 0x71A86D: BackendStartup (postmaster.c:3787) ==2049==by 0x71714C: ServerLoop (postmaster.c:1566) ==2049==by 0x716804: PostmasterMain (postmaster.c:1219) ==2049==by 0x679405: main (main.c:219) ==2049== Uninitialised value was created by a stack allocation ==2049==at 0x4FE16C: StartPrepare (twophase.c:942) ==2049== ==2049== Syscall param write(buf) points to uninitialised byte(s) ==2049==at 0x5C69640: __write_nocancel (syscall-template.S:81) ==2049==by 0x4FE6AE: EndPrepare (twophase.c:1064) ==2049==by 0x4F231B: PrepareTransaction (xact.c:2217) ==2049==by 0x4F2A38: CommitTransactionCommand (xact.c:2676) ==2049==by 0x79013E: finish_xact_command (postgres.c:2408) ==2049==by 0x78DE97: exec_simple_query (postgres.c:1062) ==2049==by 0x791FDD: PostgresMain (postgres.c:4010) ==2049==by 0x71B13B: BackendRun (postmaster.c:4113) ==2049==by 0x71A86D: BackendStartup (postmaster.c:3787) ==2049==by 0x71714C: ServerLoop (postmaster.c:1566) ==2049==by 0x716804: PostmasterMain (postmaster.c:1219) ==2049==by 0x679405: main (main.c:219) ==2049== Address 0x64694ed is 1,389 bytes inside a block of size 8,192 alloc'd ==2049==at 0x4C27B8F: malloc (vg_replace_malloc.c:298) ==2049==by 0x8E766E: AllocSetAlloc (aset.c:853) ==2049==by 0x8E8E04: MemoryContextAllocZero (mcxt.c:627) ==2049==by 0x8A54D3: AtStart_Inval (inval.c:704) ==2049==by 0x4F1DFC: StartTransaction (xact.c:1841) ==2049==by 0x4F28D1: StartTransactionCommand (xact.c:2529) ==2049==by 0x7900A7: start_xact_command (postgres.c:2383) ==2049==by 0x78DAF4: exec_simple_query (postgres.c:860) ==2049==by 0x791FDD: PostgresMain (postgres.c:4010) ==2049==by 0x71B13B: BackendRun (postmaster.c:4113) ==2049==by 0x71A86D: BackendStartup (postmaster.c:3787) ==2049==by 0x71714C: ServerLoop (postmaster.c:1566) ==2049== Uninitialised value was created by a stack allocation ==2049==at 0x4FE16C: StartPrepare (twophase.c:942) It's probably just padding - twophase.c:1063 is the CRC32 computation of the record data. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] No toast table for pg_shseclabel but for pg_seclabel
On Fri, Jul 4, 2014 at 10:53:15AM -0400, Tom Lane wrote: Kohei KaiGai kai...@kaigai.gr.jp writes: Here is no other reason than what Alvaro mentioned in the upthread. We intended to store security label of SELinux (less than 100bytes at most), so I didn't think it leads any problem actually. On the other hands, pg_seclabel was merged in another development cycle. We didn't have deep discussion about necessity of toast table of pg_seclabel. I added its toast table mechanically. So maybe we should get rid of the toast table for pg_seclabel. One less catalog table for a feature that hardly anyone is using seems like a fine idea to me ... Is this still an open item? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] Performance regression: 9.2+ vs. ScalarArrayOpExpr vs. ORDER BY
Uh, did this ever get addressed?
---
On Sun, Jul 6, 2014 at 08:56:00PM +0100, Andrew Gierth wrote:
Tom == Tom Lane t...@sss.pgh.pa.us writes:
I've experimented with the attached patch, which detects when this
situation might have occurred and does another pass to try and
build ordered scans without the SAOP condition. However, the
results may not be quite ideal, because at least in some test
queries (not all) the scan with the SAOP included in the
indexquals is being costed higher than the same scan with the SAOP
moved to a Filter, which seems unreasonable.
Tom I'm not convinced that that's a-priori unreasonable, since the
Tom SAOP will result in multiple index scans under the hood.
Tom Conceivably we ought to try the path with and with SAOPs all the
Tom time.
OK, here's a patch that always retries on lower SAOP clauses, assuming
that a SAOP in the first column is always applicable - or is even that
assumption too strong?
--
Andrew (irc:RhodiumToad)
diff --git a/src/backend/optimizer/path/indxpath.c
b/src/backend/optimizer/path/indxpath.c
index 42dcb11..cfcfbfc 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -50,7 +50,8 @@ typedef enum
{
SAOP_PER_AM,/* Use ScalarArrayOpExpr if
amsearcharray */
SAOP_ALLOW, /* Use
ScalarArrayOpExpr for all indexes */
- SAOP_REQUIRE/* Require ScalarArrayOpExpr to
be used */
+ SAOP_REQUIRE, /* Require ScalarArrayOpExpr to
be used */
+ SAOP_SKIP_LOWER /* Require lower
ScalarArrayOpExpr to be eliminated */
} SaOpControl;
/* Whether we are looking for plain indexscan, bitmap scan, or either */
@@ -118,7 +119,8 @@ static void get_index_paths(PlannerInfo *root, RelOptInfo
*rel,
static List *build_index_paths(PlannerInfo *root, RelOptInfo *rel,
IndexOptInfo *index, IndexClauseSet *clauses,
bool useful_predicate,
- SaOpControl saop_control, ScanTypeControl
scantype);
+ SaOpControl saop_control, ScanTypeControl
scantype,
+ bool *saop_retry);
static List *build_paths_for_OR(PlannerInfo *root, RelOptInfo *rel,
List *clauses, List *other_clauses);
static List *generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel,
@@ -734,6 +736,7 @@ get_index_paths(PlannerInfo *root, RelOptInfo *rel,
{
List *indexpaths;
ListCell *lc;
+ bool saop_retry = false;
/*
* Build simple index paths using the clauses. Allow ScalarArrayOpExpr
@@ -742,7 +745,23 @@ get_index_paths(PlannerInfo *root, RelOptInfo *rel,
indexpaths = build_index_paths(root, rel,
index,
clauses,
index-predOK,
-SAOP_PER_AM,
ST_ANYSCAN);
+SAOP_PER_AM,
ST_ANYSCAN, saop_retry);
+
+ /*
+ * If we allowed any ScalarArrayOpExprs on an index with a useful sort
+ * ordering, then try again without them, since otherwise we miss
important
+ * paths where the index ordering is relevant.
+ */
+ if (saop_retry)
+ {
+ indexpaths = list_concat(indexpaths,
+
build_index_paths(root, rel,
+
index, clauses,
+
index-predOK,
+
SAOP_SKIP_LOWER,
+
ST_ANYSCAN,
+
NULL));
+ }
/*
* Submit all the ones that can form plain IndexScan plans to add_path.
(A
@@ -779,7 +798,7 @@ get_index_paths(PlannerInfo *root, RelOptInfo *rel,
indexpaths = build_index_paths(root, rel,
index, clauses,
false,
-
SAOP_REQUIRE, ST_BITMAPSCAN);
+
SAOP_REQUIRE,
Re: [HACKERS] No toast table for pg_shseclabel but for pg_seclabel
Bruce Momjian br...@momjian.us writes: On Fri, Jul 4, 2014 at 10:53:15AM -0400, Tom Lane wrote: So maybe we should get rid of the toast table for pg_seclabel. One less catalog table for a feature that hardly anyone is using seems like a fine idea to me ... Is this still an open item? I haven't done anything about it ... regards, tom lane -- 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] Shapes on the regression test for polygon
On Wed, Jul 23, 2014 at 06:12:59PM -0400, Robert Haas wrote: On Wed, Jul 23, 2014 at 4:06 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Mon, Jul 21, 2014 at 10:52 AM, Emre Hasegeli e...@hasegeli.com wrote: The first two shapes on src/test/regress/sql/polygon.sql do not make sense to me. Well, I think the number of tabs that makes them look best depends on your tab-stop setting. At present, I find that with 8-space tabs things seem to line up pretty well, whereas with your patch, 4-space tabs line up well. Perhaps we should expand tabs to spaces in those ascii-art diagrams? Personally, though, my vote would be to just leave it the way it is. I don't think there's really a problem here in need of solving. I concur with doubting that changing the actual regression test cases is a good thing to do, at least not without more analysis. But the pictures make no sense with the wrong tab setting is a pretty simple issue, and one that I can see biting people. AFAICT, the pictures make no sense with the right tab setting, either. The possibility that someone might use the wrong tab setting is just icing on the cake. I extracted Emre's diagram adjustments from the patch and applied it, and no tabs now. Emre, I assume your regression changes did not affect the diagram contents. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] No toast table for pg_shseclabel but for pg_seclabel
On Sat, Oct 11, 2014 at 5:40 PM, Tom Lane t...@sss.pgh.pa.us wrote: Bruce Momjian br...@momjian.us writes: On Fri, Jul 4, 2014 at 10:53:15AM -0400, Tom Lane wrote: So maybe we should get rid of the toast table for pg_seclabel. One less catalog table for a feature that hardly anyone is using seems like a fine idea to me ... Is this still an open item? I haven't done anything about it ... If the final decision is get rid the toast table for pg_seclabel and as I've time then I did it. Patch attached. Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog: http://fabriziomello.github.io Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello Github: http://github.com/fabriziomello diff --git a/src/include/catalog/toasting.h b/src/include/catalog/toasting.h index a4af551..c3128b8 100644 --- a/src/include/catalog/toasting.h +++ b/src/include/catalog/toasting.h @@ -51,7 +51,6 @@ DECLARE_TOAST(pg_constraint, 2832, 2833); DECLARE_TOAST(pg_description, 2834, 2835); DECLARE_TOAST(pg_proc, 2836, 2837); DECLARE_TOAST(pg_rewrite, 2838, 2839); -DECLARE_TOAST(pg_seclabel, 3598, 3599); DECLARE_TOAST(pg_statistic, 2840, 2841); DECLARE_TOAST(pg_trigger, 2336, 2337); -- 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] split builtins.h to quote.h
On Sat, Oct 11, 2014 at 10:44:39AM -0300, Alvaro Herrera wrote: Started a new thread to raise awareness. Ref: this comes from http://www.postgresql.org/message-id/cab7npqr1ivd5r_qn_ngmkbolqmagbosj4wnpo8eybnn6we_...@mail.gmail.com Thanks. You can assume I'm -1 on every header split proposal not driving a substantial compile duration improvement: http://www.postgresql.org/message-id/flat/20130917163056.ga327...@tornado.leadboat.com -- 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] Remove comment about manually flipping attnotnull
On Thu, Jul 24, 2014 at 12:51:09PM -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: catalogs.sgml has this comment: This represents a not-null constraint. It is possible to change this column to enable or disable the constraint. Someone on irc just took that as permission to do so manually... Did anything especially bad happen? Obviously, turning it on wouldn't have verified that the column contains no nulls, but hopefully anyone who's bright enough to do manual catalog updates would realize that. I think things would generally have worked otherwise, in particular sinval signalling would have happened (IIRC). That comment has been there since 1efd7330c/2000-11-29, in the the initial commit adding catalogs.sgml. Does somebody object to just removing the second part of the comment in all branches? Seems like a reasonable idea, in view of the fact that we're thinking about adding pg_constraint entries for NOT NULL, in which case frobbing attnotnull by itself would definitely be a bad thing. Change applied to head. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] split builtins.h to quote.h
On Sat, Oct 11, 2014 at 05:19:27PM -0400, Noah Misch wrote: On Sat, Oct 11, 2014 at 10:44:39AM -0300, Alvaro Herrera wrote: Started a new thread to raise awareness. Ref: this comes from http://www.postgresql.org/message-id/cab7npqr1ivd5r_qn_ngmkbolqmagbosj4wnpo8eybnn6we_...@mail.gmail.com Thanks. You can assume I'm -1 on every header split proposal not driving a substantial compile duration improvement: http://www.postgresql.org/message-id/flat/20130917163056.ga327...@tornado.leadboat.com Uh, I think clarity is also a reasonable reason to split headers. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] multixact optimization patches
On Tue, Jul 29, 2014 at 03:56:19PM -0400, Alvaro Herrera wrote: I have just pushed two optimization patches for multixacts to HEAD only. I hesitate to backpatch them to 9.3 right away, but will consider doing so if people think differently. I also have a patch that supposedly fixes the performance regression reported in bug #8470, but it's considerably more obscure so I'm not pushing it right now. It's attached. I'd need to spend some more time with it to ensure it doesn't break other cases before pushing. I know some people is interested in this fix and thought I'd throw it out there to gather some input. Since it affects much of the same code as the two patches I just pushed, using it in 9.3 requires cherry-picking those patches, but they apply without conflicts so it should be easy. Wow, this is some very complex code. Are you closer to a decision on it? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] No toast table for pg_shseclabel but for pg_seclabel
On 2014-10-11 18:19:05 -0300, Fabrízio de Royes Mello wrote: On Sat, Oct 11, 2014 at 5:40 PM, Tom Lane t...@sss.pgh.pa.us wrote: Bruce Momjian br...@momjian.us writes: On Fri, Jul 4, 2014 at 10:53:15AM -0400, Tom Lane wrote: So maybe we should get rid of the toast table for pg_seclabel. One less catalog table for a feature that hardly anyone is using seems like a fine idea to me ... Is this still an open item? I haven't done anything about it ... If the final decision is get rid the toast table for pg_seclabel and as I've time then I did it. I still think this the wrong direction. I really fail to see why we want to restrict security policies to some rather small size. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] split builtins.h to quote.h
On 2014-10-11 17:19:27 -0400, Noah Misch wrote: On Sat, Oct 11, 2014 at 10:44:39AM -0300, Alvaro Herrera wrote: Started a new thread to raise awareness. Ref: this comes from http://www.postgresql.org/message-id/cab7npqr1ivd5r_qn_ngmkbolqmagbosj4wnpo8eybnn6we_...@mail.gmail.com Thanks. You can assume I'm -1 on every header split proposal not driving a substantial compile duration improvement: I don't know. Isn't it, from a aesthetic POV, wrong to have all that stuff in builtins.h? The stuff split of really doesn't seem to belong there? I personally wouldn't object plaing a #include for the splitof file into builtin.h to address backward compat concerns. Would imo still be an improvement. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] split builtins.h to quote.h
* Bruce Momjian (br...@momjian.us) wrote: On Sat, Oct 11, 2014 at 05:19:27PM -0400, Noah Misch wrote: On Sat, Oct 11, 2014 at 10:44:39AM -0300, Alvaro Herrera wrote: Started a new thread to raise awareness. Ref: this comes from http://www.postgresql.org/message-id/cab7npqr1ivd5r_qn_ngmkbolqmagbosj4wnpo8eybnn6we_...@mail.gmail.com Thanks. You can assume I'm -1 on every header split proposal not driving a substantial compile duration improvement: http://www.postgresql.org/message-id/flat/20130917163056.ga327...@tornado.leadboat.com Uh, I think clarity is also a reasonable reason to split headers. I've only glanced at the actual patch, but I agree with Bruce. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] pg_dump refactor patch to remove global variables
On Sat, Oct 11, 2014 at 2:56 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Here's the complete patch in case anyone is wondering. Hi, Your patch doesn't apply to master: $ git apply ~/Downloads/pg_dump_refactor_globals.6.diff /home/fabrizio/Downloads/pg_dump_refactor_globals.6.diff:10: trailing whitespace. static void flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables); /home/fabrizio/Downloads/pg_dump_refactor_globals.6.diff:19: trailing whitespace. getSchemaData(Archive *fout, DumpOptions *dopt, int *numTablesPtr) /home/fabrizio/Downloads/pg_dump_refactor_globals.6.diff:28: trailing whitespace. tblinfo = getTables(fout, dopt, numTables); /home/fabrizio/Downloads/pg_dump_refactor_globals.6.diff:37: trailing whitespace. extinfo = getExtensions(fout, dopt, numExtensions); /home/fabrizio/Downloads/pg_dump_refactor_globals.6.diff:42: trailing whitespace. funinfo = getFuncs(fout, dopt, numFuncs); error: patch failed: src/bin/pg_dump/common.c:64 error: src/bin/pg_dump/common.c: patch does not apply error: patch failed: src/bin/pg_dump/dumputils.h:19 error: src/bin/pg_dump/dumputils.h: patch does not apply error: patch failed: src/bin/pg_dump/parallel.c:89 error: src/bin/pg_dump/parallel.c: patch does not apply error: patch failed: src/bin/pg_dump/parallel.h:19 error: src/bin/pg_dump/parallel.h: patch does not apply error: patch failed: src/bin/pg_dump/pg_backup.h:25 error: src/bin/pg_dump/pg_backup.h: patch does not apply error: patch failed: src/bin/pg_dump/pg_backup_archiver.c:107 error: src/bin/pg_dump/pg_backup_archiver.c: patch does not apply error: patch failed: src/bin/pg_dump/pg_backup_archiver.h:29 error: src/bin/pg_dump/pg_backup_archiver.h: patch does not apply error: patch failed: src/bin/pg_dump/pg_backup_custom.c:41 error: src/bin/pg_dump/pg_backup_custom.c: patch does not apply error: patch failed: src/bin/pg_dump/pg_backup_db.c:217 error: src/bin/pg_dump/pg_backup_db.c: patch does not apply error: patch failed: src/bin/pg_dump/pg_backup_db.h:16 error: src/bin/pg_dump/pg_backup_db.h: patch does not apply error: patch failed: src/bin/pg_dump/pg_backup_directory.c:71 error: src/bin/pg_dump/pg_backup_directory.c: patch does not apply error: patch failed: src/bin/pg_dump/pg_backup_null.c:35 error: src/bin/pg_dump/pg_backup_null.c: patch does not apply error: patch failed: src/bin/pg_dump/pg_backup_tar.c:48 error: src/bin/pg_dump/pg_backup_tar.c: patch does not apply error: patch failed: src/bin/pg_dump/pg_dump.c:81 error: src/bin/pg_dump/pg_dump.c: patch does not apply error: patch failed: src/bin/pg_dump/pg_dump.h:16 error: src/bin/pg_dump/pg_dump.h: patch does not apply error: patch failed: src/bin/pg_dump/pg_dumpall.c:57 error: src/bin/pg_dump/pg_dumpall.c: patch does not apply error: patch failed: src/bin/pg_dump/pg_restore.c:423 error: src/bin/pg_dump/pg_restore.c: patch does not apply Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog: http://fabriziomello.github.io Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello Github: http://github.com/fabriziomello
Re: [HACKERS] No toast table for pg_shseclabel but for pg_seclabel
* Andres Freund (and...@2ndquadrant.com) wrote: On 2014-10-11 18:19:05 -0300, Fabrízio de Royes Mello wrote: On Sat, Oct 11, 2014 at 5:40 PM, Tom Lane t...@sss.pgh.pa.us wrote: Bruce Momjian br...@momjian.us writes: On Fri, Jul 4, 2014 at 10:53:15AM -0400, Tom Lane wrote: So maybe we should get rid of the toast table for pg_seclabel. One less catalog table for a feature that hardly anyone is using seems like a fine idea to me ... Is this still an open item? I haven't done anything about it ... If the final decision is get rid the toast table for pg_seclabel and as I've time then I did it. I still think this the wrong direction. I really fail to see why we want to restrict security policies to some rather small size. I agree with this. There's no ability to store multiple labels for the same object and provider with multiple rows (which is fine by itself), and so that means security providers with multiple overlapping labels for the same object need to combine them together and store them together. While I agree that individual labels don't tend to get very long, when you combine overlapping ones, they could get long enough to need toasting. Admittedly, you could complicate the system by defining those labels as new labels, but we are likely working with an external authorization system and it's a lot less trouble to attach multiple labels to the given object than to ask everyone else to change because PG ran out of room in the text column because it can't TOAST it.. Then there's the other discussion about using the security labels structure for more than just security labels, which could end up with a lot of other use-cases where the label is even larger. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] split builtins.h to quote.h
On Sat, Oct 11, 2014 at 11:43:46PM +0200, Andres Freund wrote: On 2014-10-11 17:19:27 -0400, Noah Misch wrote: On Sat, Oct 11, 2014 at 10:44:39AM -0300, Alvaro Herrera wrote: Started a new thread to raise awareness. Ref: this comes from http://www.postgresql.org/message-id/cab7npqr1ivd5r_qn_ngmkbolqmagbosj4wnpo8eybnn6we_...@mail.gmail.com Thanks. You can assume I'm -1 on every header split proposal not driving a substantial compile duration improvement: I don't know. Isn't it, from a aesthetic POV, wrong to have all that stuff in builtins.h? The stuff split of really doesn't seem to belong there? Yes, the status quo is aesthetically wrong. Still, any clarity improvement from this split is vaporous. The cost of breaking module builds is real. I personally wouldn't object plaing a #include for the splitof file into builtin.h to address backward compat concerns. Would imo still be an improvement. Agreed. If the patch preserved compatibility by having builtins.h include quote.h, I would not object. nm -- 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] split builtins.h to quote.h
* Noah Misch (n...@leadboat.com) wrote: On Sat, Oct 11, 2014 at 11:43:46PM +0200, Andres Freund wrote: I personally wouldn't object plaing a #include for the splitof file into builtin.h to address backward compat concerns. Would imo still be an improvement. Agreed. If the patch preserved compatibility by having builtins.h include quote.h, I would not object. That seems reasonable to me also- though I'd caveat it as for now and make sure to make a note of the reason it's included in the comments... Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Is analyze_new_cluster.sh still useful?
On Mon, Aug 4, 2014 at 10:17:40AM +0200, Christoph Berg wrote: Re: Bruce Momjian 2014-07-29 20140729094234.gc13...@momjian.us On Fri, Jun 20, 2014 at 05:15:05PM +0200, Christoph Berg wrote: Another nitpick here: What pg_upgrade outputs doesn't even work on most systems, you need to ./analyze_new_cluster.sh or sh analyze_new_cluster.sh. Well, the output is: Optimizer statistics are not transferred by pg_upgrade so, once you start the new server, consider running: analyze_new_cluster.sh Running this script will delete the old cluster's data files: delete_old_cluster.sh It is not really telling you _how_ to run them. Would adding a ./ prefix help? I think it would help in hinting the user that these are not system-wide commands in $PATH but rather scripts in the current directory. There's also a case for prefixing them with the full path, Debian's pg_upgradecluster wrapper drops these scripts in a new subdirectory in /var/log/postgresql/ along with the log files. Possibly other automation frameworks do likewise. (Though the frameworks will likely make delete_old_cluster.sh redundant.) I have applied a patch to 9.5 to output ./ as a prefix for Unix script file names. While this also works on Windows, it is likely to be confusing. The new Unix output is: Upgrade Complete Optimizer statistics are not transferred by pg_upgrade so, once you start the new server, consider running: ./analyze_new_cluster.sh Running this script will delete the old cluster's data files: ./delete_old_cluster.sh -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] Looked at TODO:Considering improving performance of computing CHAR() value lengths
On Mon, Aug 4, 2014 at 11:48:38AM +0300, Heikki Linnakangas wrote: The biggest issue with the previously discussed patches was the lack of performance testing. People posted results of various micro-benchmarks that showed different aspects, but no-one comprehensively covered all the interesting cases. I'd like to see a simple suite of performance tests that show: Any progress on this? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] [REVIEW] pg_last_xact_insert_timestamp
On Wed, Aug 13, 2014 at 12:41:24PM +0900, Fujii Masao wrote: On Mon, Aug 11, 2014 at 8:27 PM, Fujii Masao masao.fu...@gmail.com wrote: On Mon, Aug 11, 2014 at 4:46 PM, Andres Freund and...@2ndquadrant.com wrote: Hi, On 2011-10-04 20:52:59 +0900, Fujii Masao wrote: *** a/src/backend/access/transam/xact.c --- b/src/backend/access/transam/xact.c *** *** 1066,1071 RecordTransactionCommit(void) --- 1066,1074 (void) XLogInsert(RM_XACT_ID, XLOG_XACT_COMMIT_COMPACT, rdata); } + + /* Save timestamp of latest transaction commit record */ + pgstat_report_xact_end_timestamp(xactStopTimestamp); } Perhaps that pgstat_report() should instead be combined with the pgstat_report_xact_timestamp(0) in CommitTransaction()? Then the number of changecount increases and cacheline references would stay the same. The only thing that'd change would be a single additional assignment. Sounds good suggestion. I attached the updated version of the patch. I changed pgstat_report_xx functions like Andres suggested. While reading the patch again, I found it didn't handle the COMMIT/ABORT PREPARED case properly. According to the commit e74e090, now pg_last_xact_replay_timestamp() returns the timestamp of COMMIT/ABORT PREPARED. pg_last_xact_insert_timestamp() is mainly expected to be used to calculate the replication delay, so it also needs to return that timestam. But the patch didn't change 2PC code at all. We need to add pgstat_report_xact_end_timestamp() into FinishPreparedTransaction(), RecordTransactionCommitPrepared() or RecordTransactionAbortPrepared(). Done. Is this going to be applied? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] [PATCH] PostgreSQL 9.4 mmap(2) performance regression on FreeBSD...
On Tue, Aug 12, 2014 at 07:08:06PM -0400, Robert Haas wrote: On Tue, Aug 12, 2014 at 12:59 PM, Andres Freund and...@2ndquadrant.com wrote: On 2014-08-12 09:42:30 -0700, Sean Chittenden wrote: One of the patches that I've been sitting on and am derelict in punting upstream is the attached mmap(2) flags patch for the BSDs. Is there any chance this can be squeezed in to the PostreSQL 9.4 release? The patch is trivial in size and is used to add one flag to mmap(2) calls in dsm_impl.c. Alan Cox (FreeBSD alc, not Linux) and I went back and forth regarding PostgreSQL's use of mmap(2) and determined that the following is correct and will prevent a likely performance regression in PostgreSQL 9.4. In PostgreSQL 9.3, all mmap(2) calls were called with the flags MAP_ANON | MAP_SHARED, whereas in PostgreSQL 9.4 this is not the case. The performancewise important call to mmap will still use that set of flags, no? That's the one backing shared_buffers. The mmap backend for *dynamic* shared memory (aka dsm) is *NOT* supposed to be used on common platforms. Both posix and sysv shared memory will be used before falling back to the mmap() backend. Hmm, yeah. This might still be a good thing to do (because what do we lose?) but it shouldn't really be an issue in practice. Is there a reason this was not applied? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] pg_dump refactor patch to remove global variables
Fabrízio de Royes Mello wrote: On Sat, Oct 11, 2014 at 2:56 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Here's the complete patch in case anyone is wondering. Hi, Your patch doesn't apply to master: I think your email system mangled it. I can download it from archives and apply it just fine: $ wget http://www.postgresql.org/message-id/attachment/35216/pg_dump_refactor_globals.6.diff -O - | patch -p1 --2014-10-11 20:44:52-- http://www.postgresql.org/message-id/attachment/35216/pg_dump_refactor_globals.6.diff Resolviendo www.postgresql.org (www.postgresql.org)... 87.238.57.232, 174.143.35.230, 217.196.149.50, ... Conectando con www.postgresql.org (www.postgresql.org)[87.238.57.232]:80... conectado. Petición HTTP enviada, esperando respuesta... 200 OK Longitud: no especificado [text/x-diff] Grabando a: “STDOUT” [ = ] 140.127 105K/s en 1,3s 2014-10-11 20:44:55 (105 KB/s) - escritos a stdout [140127] patching file src/bin/pg_dump/common.c patching file src/bin/pg_dump/dumputils.h patching file src/bin/pg_dump/parallel.c patching file src/bin/pg_dump/parallel.h patching file src/bin/pg_dump/pg_backup.h patching file src/bin/pg_dump/pg_backup_archiver.c patching file src/bin/pg_dump/pg_backup_archiver.h patching file src/bin/pg_dump/pg_backup_custom.c patching file src/bin/pg_dump/pg_backup_db.c patching file src/bin/pg_dump/pg_backup_db.h patching file src/bin/pg_dump/pg_backup_directory.c patching file src/bin/pg_dump/pg_backup_null.c patching file src/bin/pg_dump/pg_backup_tar.c patching file src/bin/pg_dump/pg_dump.c patching file src/bin/pg_dump/pg_dump.h patching file src/bin/pg_dump/pg_dumpall.c patching file src/bin/pg_dump/pg_restore.c -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] B-Tree support function number 3 (strxfrm() optimization)
On Mon, Sep 29, 2014 at 10:34 PM, Peter Geoghegan p...@heroku.com wrote: single sortsupport state patch. You probably noticed that I posted an independently useful patch to make all tuplesort cases use sortsupport [1] - currently, both the B-Tree and CLUSTER cases do not use the sortsupport infrastructure more or less for no good reason. That patch was primarily written to make abbreviated keys work for all cases, though. I think that making heap tuple sorts based on a text attribute much faster is a very nice thing, but in practice most OLTP or web applications are not all that sensitive to the amount of time taken to sort heap tuples. However, the length of time it takes to build indexes is something that most busy production applications are very sensitive to, since of course apart from the large consumption of system resources often required, however long the sort takes is virtually the same amount of time as however long we hold a very heavy, disruptive relation-level ShareLock. Obviously the same applies to CLUSTER, but more so, since it must acquire an AccessExclusiveLock on the relation to be reorganized. I think almost everyone will agree that making B-Tree builds much faster is the really compelling case here, because that's where slow sorts cause by far the most pain for users in the real world. Attached patch, when applied, accelerates all tuplesort cases using abbreviated keys, building on previous work here, as well as the patch posted to that other thread. Exact instructions are in the commit message of 0004-* (i.e. where to find the pieces I haven't posted here). I also attach a minor bitrot fix commit/patch. Performance is improved for B-Tree index builds by a great deal, too. The improvements are only slightly less than those seen for comparable heap tuple sorts (that is, my earlier test cases that had client overhead removed). With larger sorts, that difference tends to get lost in the noise easily. I'm very encouraged by this. I think that being able to build B-Tree indexes on text attributes very significantly faster than previous versions of PostgreSQL is likely to be a significant feature for PostgreSQL 9.5. After all, external sorts are where improvements are most noticeable [2] - they're so much faster with this patch that they're actually sometimes faster than internal sorts *with* abbreviated keys. This would something that I found quite surprising. [1] http://www.postgresql.org/message-id/CAM3SWZTfKZHTUiWDdHg+6tcYuMsdHoC=bmuaivgmp9athj4...@mail.gmail.com [2] http://www.postgresql.org/message-id/cam3swzqvjcgme6ube-ydipu0n5bo7rmz31zrhmskdduynej...@mail.gmail.com -- Peter Geoghegan From 72070126e707cf356ddcb3de60cbdb14658ed077 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan p...@heroku.com Date: Fri, 10 Oct 2014 22:26:21 -0700 Subject: [PATCH 4/4] Make B-Tree/CLUSTER sortsupport use abbreviation This commit is intended to be applied on top of several others. First, apply the abbreviated key support patch -- the revision posted here: cam3swzt+v3llrbscyuj9jovumdpbq6bupf9wofbfuywgwuh...@mail.gmail.com. Then, apply the bitrot-fixing commit that was attached alongside this patch. In addition, the B-Tree/CLUSTER sortsupport patch must then be applied. Get it from: CAM3SWZTfKZHTUiWDdHg+6tcYuMsdHoC=bmuaivgmp9athj4...@mail.gmail.com Finally, apply this patch. B-Tree sortsupport with abbreviated keys (for text) shows benefits (and costs) that are similar to the heavily tested/benchmarked MinimalTuple case. There is additional overhead when building a B-Tree index as compared to heap tuplesorts (with no client overhead), but even still the vast majority of system time is spent actually sorting index tuples. Therefore, it is not expected that the addition of B-Tree/CLUSTER support will influence the review of abbreviated keys much either way. --- src/backend/access/nbtree/nbtsort.c | 3 + src/backend/utils/sort/tuplesort.c | 385 +++- 2 files changed, 298 insertions(+), 90 deletions(-) diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c index a6c44a7..1ccc9fb 100644 --- a/src/backend/access/nbtree/nbtsort.c +++ b/src/backend/access/nbtree/nbtsort.c @@ -720,6 +720,9 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2) strategy = (scanKey-sk_flags SK_BT_DESC) != 0 ? BTGreaterStrategyNumber : BTLessStrategyNumber; + /* Abbreviation is not supported here */ + sortKey-abbrev_state = ABBREVIATED_KEYS_NO; + PrepareSortSupportFromIndexRel(wstate-index, strategy, sortKey); } diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c index 1d57de7..c6a18dc 100644 --- a/src/backend/utils/sort/tuplesort.c +++ b/src/backend/utils/sort/tuplesort.c @@ -453,6 +453,7 @@ struct Tuplesortstate static Tuplesortstate *tuplesort_begin_common(int workMem, bool randomAccess); static void puttuple_common(Tuplesortstate *state, SortTuple *tuple); +static
Re: [HACKERS] pg_dump refactor patch to remove global variables
On Sat, Oct 11, 2014 at 10:15 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Fabrízio de Royes Mello wrote: On Sat, Oct 11, 2014 at 2:56 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Here's the complete patch in case anyone is wondering. Hi, Your patch doesn't apply to master: I think your email system mangled it. I can download it from archives and apply it just fine: $ wget http://www.postgresql.org/message-id/attachment/35216/pg_dump_refactor_globals.6.diff -O - | patch -p1 --2014-10-11 20:44:52-- http://www.postgresql.org/message-id/attachment/35216/pg_dump_refactor_globals.6.diff Resolviendo www.postgresql.org (www.postgresql.org)... 87.238.57.232, 174.143.35.230, 217.196.149.50, ... Conectando con www.postgresql.org (www.postgresql.org)[87.238.57.232]:80... conectado. Petición HTTP enviada, esperando respuesta... 200 OK Longitud: no especificado [text/x-diff] Grabando a: “STDOUT” [ = ] 140.127 105K/s en 1,3s 2014-10-11 20:44:55 (105 KB/s) - escritos a stdout [140127] patching file src/bin/pg_dump/common.c patching file src/bin/pg_dump/dumputils.h patching file src/bin/pg_dump/parallel.c patching file src/bin/pg_dump/parallel.h patching file src/bin/pg_dump/pg_backup.h patching file src/bin/pg_dump/pg_backup_archiver.c patching file src/bin/pg_dump/pg_backup_archiver.h patching file src/bin/pg_dump/pg_backup_custom.c patching file src/bin/pg_dump/pg_backup_db.c patching file src/bin/pg_dump/pg_backup_db.h patching file src/bin/pg_dump/pg_backup_directory.c patching file src/bin/pg_dump/pg_backup_null.c patching file src/bin/pg_dump/pg_backup_tar.c patching file src/bin/pg_dump/pg_dump.c patching file src/bin/pg_dump/pg_dump.h patching file src/bin/pg_dump/pg_dumpall.c patching file src/bin/pg_dump/pg_restore.c Yeah... my gmail mangled the attached file. Thanks and sorry by the noise. I'll see the changes. Regards. -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog: http://fabriziomello.github.io Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello Github: http://github.com/fabriziomello
Re: [HACKERS] psql output change in 9.4
On 10/11/14 8:25 PM, Bruce Momjian wrote:
On Mon, Aug 11, 2014 at 02:28:45PM -0400, Robert Haas wrote:
On Mon, Aug 11, 2014 at 1:52 PM, Tom Lane t...@sss.pgh.pa.us wrote:
Robert Haas robertmh...@gmail.com writes:
On Fri, Aug 8, 2014 at 9:34 PM, Peter Eisentraut pete...@gmx.net wrote:
What is the point of that change?
I think the output could justly be criticized for making it
insufficiently clear that the parenthesized text is, in fact, the name
of the pset parameter.
Quite; that wasn't apparent to me either.
We could write something like:
Border style (parameter border) is 1.
How about
Border style (\pset border) is 1.
That would look just fine as a response to \a or \x, but I'm not sure
it would look as good as a response to \pset, which prints out that
line for every parameter (why does every line say \pset when the
command I just typed is \pset?). However, I can certainly live with
it if others prefer that to what I suggested.
I went with quoting the pset variable:
test= \a
Output format (format) is aligned.
test= \x
Expanded display (expanded) is on.
Patch attached. I think this would be for 9.5 only, at this point.
Funny, I was *just* working on that, too. I propose a patch that
reverts the output to how it was in 9.3 (without anything in
parentheses), and implements the output of \pset without any arguments
separately, thus:
# \a
Output format is unaligned.
# \pset
border 2
columns0
expanded auto
fieldsep '|'
fieldsep_zero off
footer on
format unaligned
linestyle unicode
null ''
numericlocale off
pager 1
recordsep '\n'
recordsep_zero off
tableattr
title
tuples_onlyoff
(This is also symmetric with what \set outputs.)
On closer examination, the change in 9.4, besides having the aesthetic
issues discussed earlier, also created some outright incorrect output by
mixing together fieldsep/fieldsep_zero and recordsep/recordsep_zero.
These issues become much clearer if you separate the case of this is
what you just set from these are all the current settings.
commit 42b78a38970808611133031c9e6b30466fdd84b4
Author: Peter Eisentraut pete...@gmx.net
Date: Sun Oct 12 00:08:52 2014 -0400
Fix \pset without arguments
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 0d9b677..d8c477a 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -69,6 +69,7 @@ static void minimal_error_message(PGresult *res);
static void printSSLInfo(void);
static bool printPsetInfo(const char *param, struct printQueryOpt *popt);
+static char *pset_value_string(const char *param, struct printQueryOpt *popt);
#ifdef WIN32
static void checkWin32Codepage(void);
@@ -1050,15 +1051,19 @@ exec_command(const char *cmd,
int i;
static const char *const my_list[] = {
-border, columns, expanded, fieldsep,
+border, columns, expanded, fieldsep, fieldsep_zero,
footer, format, linestyle, null,
-numericlocale, pager, recordsep,
+numericlocale, pager, recordsep, recordsep_zero,
tableattr, title, tuples_only,
NULL
};
for (i = 0; my_list[i] != NULL; i++)
-printPsetInfo(my_list[i], pset.popt);
+ {
+char *val = pset_value_string(my_list[i], pset.popt);
+printf(%-14s %s\n, my_list[i], val);
+free(val);
+ }
success = true;
}
@@ -2199,10 +2204,6 @@ process_file(char *filename, bool single_txn, bool use_relative_path)
-/*
- * do_pset
- *
- */
static const char *
_align2string(enum printFormat in)
{
@@ -2237,6 +2238,10 @@ _align2string(enum printFormat in)
}
+/*
+ * do_pset
+ *
+ */
bool
do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
{
@@ -2447,80 +2452,69 @@ printPsetInfo(const char *param, struct printQueryOpt *popt)
/* show border style/width */
if (strcmp(param, border) == 0)
- {
- if (!popt-topt.border)
- printf(_(Border style (%s) unset.\n), param);
- else
- printf(_(Border style (%s) is %d.\n), param,
- popt-topt.border);
- }
+ printf(_(Border style is %d.\n), popt-topt.border);
/* show the target width for the wrapped format */
else if (strcmp(param, columns) == 0)
{
if (!popt-topt.columns)
- printf(_(Target width (%s) unset.\n), param);
+ printf(_(Target width is unset.\n));
else
- printf(_(Target width (%s) is %d.\n), param,
- popt-topt.columns);
+ printf(_(Target width is %d.\n), popt-topt.columns);
}
/* show expanded/vertical mode */
else if (strcmp(param, x) == 0 || strcmp(param, expanded) == 0 || strcmp(param, vertical) == 0)
{
if (popt-topt.expanded == 1)
- printf(_(Expanded display (%s) is on.\n), param);
+ printf(_(Expanded display is on.\n));
else if (popt-topt.expanded == 2)
- printf(_(Expanded display (%s) is used automatically.\n), param);
+ printf(_(Expanded display is used automatically.\n));
else
- printf(_(Expanded display (%s) is off.\n),
Re: [HACKERS] split builtins.h to quote.h
On Sun, Oct 12, 2014 at 7:09 AM, Stephen Frost sfr...@snowman.net wrote: * Noah Misch (n...@leadboat.com) wrote: On Sat, Oct 11, 2014 at 11:43:46PM +0200, Andres Freund wrote: I personally wouldn't object plaing a #include for the splitof file into builtin.h to address backward compat concerns. Would imo still be an improvement. Agreed. If the patch preserved compatibility by having builtins.h include quote.h, I would not object. That seems reasonable to me also- though I'd caveat it as for now and make sure to make a note of the reason it's included in the comments... This seems like a consensus. I updated the patch with the following things: - Declaration of quote.h in builtins.h, with a comment on top of builtins.h. - Removal of declaration of builtins.h where it is not necessary anymore, replaced by quote.h. I thought there would be more places, but a lot of files still depend on the cstring and format_type. Perhaps this could be as well separated, keeping only the function declarations of the type Datum foo(PG_FUNCTION_ARGS) in builtins.h... Regards, -- Michael From 02b45db20b9893f4517f96b084fb898bb0c41bfc Mon Sep 17 00:00:00 2001 From: Michael Paquier mich...@otacoo.com Date: Sat, 11 Oct 2014 22:28:05 +0900 Subject: [PATCH] Move all quote-related functions into a single header quote.h Note that this impacts as well contrib modules, and mainly external modules particularly for quote_identifier. --- contrib/dblink/dblink.c | 1 + contrib/postgres_fdw/deparse.c| 1 + contrib/postgres_fdw/postgres_fdw.c | 1 + contrib/test_decoding/test_decoding.c | 1 + contrib/worker_spi/worker_spi.c | 2 +- src/backend/catalog/namespace.c | 1 + src/backend/catalog/objectaddress.c | 1 + src/backend/commands/explain.c| 1 + src/backend/commands/extension.c | 1 + src/backend/commands/matview.c| 2 +- src/backend/commands/trigger.c| 1 + src/backend/commands/tsearchcmds.c| 1 + src/backend/parser/parse_utilcmd.c| 1 + src/backend/utils/adt/format_type.c | 1 + src/backend/utils/adt/quote.c | 110 ++ src/backend/utils/adt/regproc.c | 1 + src/backend/utils/adt/ri_triggers.c | 1 + src/backend/utils/adt/ruleutils.c | 109 + src/backend/utils/adt/varlena.c | 1 + src/backend/utils/cache/ts_cache.c| 1 + src/backend/utils/misc/guc.c | 1 + src/include/utils/builtins.h | 10 ++-- src/include/utils/quote.h | 28 + src/pl/plpgsql/src/pl_gram.y | 1 + src/pl/plpython/plpy_plpymodule.c | 1 + 25 files changed, 164 insertions(+), 116 deletions(-) create mode 100644 src/include/utils/quote.h diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c index d77b3ee..cbbc513 100644 --- a/contrib/dblink/dblink.c +++ b/contrib/dblink/dblink.c @@ -56,6 +56,7 @@ #include utils/guc.h #include utils/lsyscache.h #include utils/memutils.h +#include utils/quote.h #include utils/rel.h #include utils/tqual.h diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c index 2045774..0009cd3 100644 --- a/contrib/postgres_fdw/deparse.c +++ b/contrib/postgres_fdw/deparse.c @@ -50,6 +50,7 @@ #include parser/parsetree.h #include utils/builtins.h #include utils/lsyscache.h +#include utils/quote.h #include utils/syscache.h diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 4c49776..feb41f5 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -36,6 +36,7 @@ #include utils/guc.h #include utils/lsyscache.h #include utils/memutils.h +#include utils/quote.h PG_MODULE_MAGIC; diff --git a/contrib/test_decoding/test_decoding.c b/contrib/test_decoding/test_decoding.c index fdbd313..b168fc3 100644 --- a/contrib/test_decoding/test_decoding.c +++ b/contrib/test_decoding/test_decoding.c @@ -25,6 +25,7 @@ #include utils/builtins.h #include utils/lsyscache.h #include utils/memutils.h +#include utils/quote.h #include utils/rel.h #include utils/relcache.h #include utils/syscache.h diff --git a/contrib/worker_spi/worker_spi.c b/contrib/worker_spi/worker_spi.c index 328c722..ec6c9fa 100644 --- a/contrib/worker_spi/worker_spi.c +++ b/contrib/worker_spi/worker_spi.c @@ -37,7 +37,7 @@ #include fmgr.h #include lib/stringinfo.h #include pgstat.h -#include utils/builtins.h +#include utils/quote.h #include utils/snapmgr.h #include tcop/utility.h diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c index 5eb8fd4..7f8f54b 100644 --- a/src/backend/catalog/namespace.c +++ b/src/backend/catalog/namespace.c @@ -53,6 +53,7 @@ #include utils/inval.h #include utils/lsyscache.h #include utils/memutils.h +#include utils/quote.h #include utils/syscache.h diff --git a/src/backend/catalog/objectaddress.c
