Re: [HACKERS] SCRAM authentication, take three
On Tue, Feb 7, 2017 at 11:28 AM, Michael Paquierwrote: > Yes, I am actively working on this one now. I am trying to come up > first with something in the shape of an extension to begin with, and > get a patch out of it. That will be more simple for testing. For now > the work that really remains in the patches attached on this thread is > to get the internal work done, all the UTF8-related routines being > already present in scram-common.c to work on the strings. It took me a couple of days... And attached is the prototype implementing SASLprep(), or NFKC if you want for UTF-8 strings. This extension is pretty useful to check the validity of any normalization forms defined in the Unicode specs, and is as well on my github: https://github.com/michaelpq/pg_plugins/tree/master/pg_sasl_prepare In short, at build what this extension does is fetching UnicodeData.txt, and it builds a conversion table including the fields necessary for NFKC: - The utf code of a character. - The combining class of the character. - The decomposition sets of characters. I am aware of the fact that the implemention of this extension is the worst possible, there are many bytes wasted, and the resulting shared library gets at 2.4MB. Now as an example of how normalization forms work that's a great study, and with that I am now aware of what needs to be done to reduce the size of the conversion tables. This extension has two conversion functions for UTF-8 string <=> integer array (as UTF-8 characters are on 4 bytes with pg_wchar): =# SELECT array_to_utf8('{50064}'); array_to_utf8 --- Ð (1 row) =# SELECT utf8_to_array('ÐÐÐ'); utf8_to_array - {50064,50064,50064} (1 row) Then using an integer array in input SASLprep() can be done using pg_sasl_prepare(), which returns to caller a decomposed recursively set, with reordering done using the combining class integer array from the conversion table generated wiht UnicodeData.txt. Lookups at the conversion tables are done using bsearch(), so that's, at least I guess, fast. I have implemented as well a function to query the whole conversion as a SRF (character number, combining class and decomposition), which is useful for analysis: =# select count(*) from utf8_conv_table(); count --- 30590 (1 row) Now using this module I have arrived to the following conclusions to put to a minimum the size of the conversion tables, without much impacting lookup performance: - There are 24k characters with a combining class of 0 and no decomposition for a total of 30k characters, those need to be dropped from the conversion table. - Most characters have a single, or double decomposition, one has a decomposition of 18 characters. So we need to create two sets of conversion tables: -- A base table, with the character number (4 bytes), the combining class (1 byte) and the size of the decomposition (1 byte). -- A set of decomposition tables, classified by size. So when decomposing a character, we check first the size of the decomposition, then get the set from the correct table. Now regarding the shape of the implementation for SCRAM, we need one thing: a set of routines in src/common/ to build decompositions for a given UTF-8 string with conversion UTF8 string <=> pg_wchar array, the decomposition and the reordering. The extension attached roughly implements that. What we can actually do as well is have in contrib/ a module that does NFK[C|D] using the base APIs in src/common/. Using arrays of pg_wchar (integers) to manipulate the characters, we can validate and have a set of regression tests that do *not* have to print non-ASCII characters. Let me know if this plan looks good, now I think that I have enough material to get SASLprep done cleanly, with a minimum memory footprint. Heikki, others, how does that sound for you? -- Michael pg_sasl_prepare.tar.gz Description: GNU Zip compressed data -- 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] Proposal : For Auto-Prewarm.
On Tue, Feb 7, 2017 at 1:31 AM, Mithun Cywrote: > SEGFAULT was the coding mistake I have called the C-language function > directly without initializing the functioncallinfo. Thanks for > raising. Below patch fixes same. It would be nice if this had an option to preload only internal B-tree pages into shared_buffers. They're usually less than 1% of the total pages in a B-Tree, and are by far the most frequently accessed. It's reasonable to suppose that much of the random I/O incurred when warming the cache occurs there. Now, prewarming those will incur random I/O, often completely random I/O, but by and large it would be a matter of swallowing that cost sooner, through using your tool, rather than later, during the execution of queries. -- 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] Proposal : For Auto-Prewarm.
Hello, On Wed, Feb 8, 2017 at 3:40 PM, Amit Kapilawrote: > On Tue, Feb 7, 2017 at 5:14 PM, Mithun Cy > wrote: > > On Tue, Feb 7, 2017 at 12:24 PM, Amit Kapila > wrote: > >> On Tue, Feb 7, 2017 at 11:53 AM, Beena Emerson > wrote: > >>> Are 2 workers required? > >>> > >> > >> I think in the new design there is a provision of launching the worker > >> dynamically to dump the buffers, so there seems to be a need of > >> separate workers for loading and dumping the buffers. However, there > >> is no explanation in the patch or otherwise when and why this needs a > >> pair of workers. Also, if the dump interval is greater than zero, > >> then do we really need to separately register a dynamic worker? > > > > We have introduced a new value -1 for pg_prewarm.dump_interval this > > means we will not dump at all, At this state, I thought auto > > pg_prewarm process need not run at all, so I coded to exit the auto > > pg_prewarm immediately. But If the user decides to start the auto > > pg_prewarm to dump only without restarting the server, I have > > introduced a launcher function "launch_pg_prewarm_dump" to restart the > > auto pg_prewarm only to dump. Since now we can launch worker only to > > dump, I thought we can redistribute the code between two workers, one > > which only does prewarm (load only) and another dumps periodically. > > This helped me to modularize and reuse the code. So once load worker > > has finished its job, it registers a dump worker and then exists. > > But if max_worker_processes is not enough to launch the "auto > > pg_prewarm dump" bgworker > > We throw an error > > 2017-02-07 14:51:59.789 IST [50481] ERROR: registering dynamic > > bgworker "auto pg_prewarm dump" failed c > > 2017-02-07 14:51:59.789 IST [50481] HINT: Consider increasing > > configuration parameter "max_worker_processes". > > > > Now thinking again instead of such error and then correcting same by > > explicitly launching the auto pg_prewarm dump bgwroker through > > launch_pg_prewarm_dump(), I can go back to original design where there > > will be one worker which loads and then dumps periodically. And > > launch_pg_prewarm_dump will relaunch dump only activity of that > > worker. Does this sound good? > > > > Won't it be simple if you consider -1 as a value to just load library? > For *_interval = -1, it will neither load nor dump. > > +1 That is what I thought was the behaviour we decided upon for -1. -- Thank you, Beena Emerson Have a Great Day!
Re: [HACKERS] Parallel Index Scans
On Wed, Feb 8, 2017 at 10:33 PM, Amit Kapilawrote: > I had some offlist discussion with Robert about the above point and we > feel that keeping only heap pages for parallel computation might not > be future proof as for parallel index only scans there might not be > any heap pages. So, it is better to use separate GUC for parallel > index (only) scans. We can have two guc's > min_parallel_table_scan_size (8MB) and min_parallel_index_scan_size > (512kB) for computing parallel scans. The parallel sequential scan > and parallel bitmap heap scans can use min_parallel_table_scan_size as > a threshold to compute parallel workers as we are doing now. For > parallel index scans, both min_parallel_table_scan_size and > min_parallel_index_scan_size can be used for threshold; We can > compute parallel workers both based on heap_pages to be scanned and > index_pages to be scanned and then keep the minimum of those. This > will help us to engage parallel index scans when the index pages are > lower than threshold but there are many heap pages to be scanned and > will also allow keeping a maximum cap on the number of workers based > on index scan size. What about parallel CREATE INDEX? The patch currently uses min_parallel_relation_size as an input into the optimizer's custom cost model. I had wondered if that made sense. Note that another such input is the projected size of the final index. That's the thing that increases at logarithmic intervals as there is a linear increase in the number of workers assigned to the operation (so it's not the size of the underlying table). -- 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] Parallel Index Scans
On Sat, Feb 4, 2017 at 7:14 AM, Amit Kapilawrote: > On Sat, Feb 4, 2017 at 5:54 AM, Robert Haas wrote: >> On Wed, Feb 1, 2017 at 12:58 AM, Amit Kapila wrote: > >> On balance, I'm somewhat inclined to think that we ought to base >> everything on heap pages, so that we're always measuring in the same >> units. That's what Dilip's patch for parallel bitmap heap scan does, >> and I think it's a reasonable choice. However, for parallel index >> scan, we might want to also cap the number of workers to, say, >> index_pages/10, just so we don't pick an index scan that's going to >> result in a very lopsided work distribution. >> > > I guess in the above context you mean heap_pages or index_pages that > are expected to be *fetched* during index scan. > > Yet another thought is that for parallel index scan we use > index_pages_fetched, but use either a different GUC > (min_parallel_index_rel_size) with a relatively lower default value > (say equal to min_parallel_relation_size/4 = 2MB) or directly use > min_parallel_relation_size/4 for parallel index scans. > I had some offlist discussion with Robert about the above point and we feel that keeping only heap pages for parallel computation might not be future proof as for parallel index only scans there might not be any heap pages. So, it is better to use separate GUC for parallel index (only) scans. We can have two guc's min_parallel_table_scan_size (8MB) and min_parallel_index_scan_size (512kB) for computing parallel scans. The parallel sequential scan and parallel bitmap heap scans can use min_parallel_table_scan_size as a threshold to compute parallel workers as we are doing now. For parallel index scans, both min_parallel_table_scan_size and min_parallel_index_scan_size can be used for threshold; We can compute parallel workers both based on heap_pages to be scanned and index_pages to be scanned and then keep the minimum of those. This will help us to engage parallel index scans when the index pages are lower than threshold but there are many heap pages to be scanned and will also allow keeping a maximum cap on the number of workers based on index scan size. guc_parallel_index_scan_v1.patch - Change name of existing min_parallel_relation_size to min_parallel_table_scan_size and added a new guc min_parallel_index_scan_size with default value of 512kB. This patch also adjusted the computation in compute_parallel_worker based on two guc's. compute_index_pages_v2.patch - This function extracts the computation of index pages to be scanned in a separate function and used it in existing code. You will notice that I have pulled up the logic of conversion of clauses to indexquals from create_index_path to build_index_paths as that is required to compute the number of index and heap pages to be scanned by scan in patch parallel_index_opt_exec_support_v8.patch. This doesn't impact any existing functionality. parallel_index_scan_v7 - patch to parallelize btree scans, nothing is changed from previous version (just rebased on latest head). parallel_index_opt_exec_support_v8.patch - This contain changes to compute parallel workers using both heap and index pages that need to be scanned. Patches guc_parallel_index_scan_v1.patch and compute_index_pages_v2.patch are independent patches. Both the patches are required by parallel index scan patches. The current set of patches handles all the reported comments. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com guc_parallel_index_scan_v1.patch Description: Binary data compute_index_pages_v2.patch Description: Binary data parallel_index_scan_v7.patch Description: Binary data parallel_index_opt_exec_support_v8.patch Description: Binary data -- 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] Declarative partitioning - another take
About 0001-Check-partition-strategy-in-ATExecDropNotNull.patch, following test is already covered in alter_table.sql @ Line # 1918, instead of this kindly add test for list_partition: 77 +-- cannot drop NOT NULL constraint of a range partition key column 78 +ALTER TABLE range_parted ALTER a DROP NOT NULL; 79 + Regards, Amul -- 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] WAL consistency check facility
On Thu, Feb 9, 2017 at 2:26 AM, Robert Haaswrote: > On Wed, Feb 8, 2017 at 1:25 AM, Kuntal Ghosh > wrote: >> Thank you Robert for the review. Please find the updated patch in the >> attachment. > > I have committed this patch after fairly extensive revisions: > Thank you, Robert, for the above corrections and commit. Thanks to Michael Paquier, Peter Eisentraut, Amit Kapila, Álvaro Herrera, and Simon Riggs for taking their time to complete the patch. It was a great learning experience for me. -- Thanks & Regards, Kuntal Ghosh 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] CREATE COLLATION IF NOT EXISTS
Here is a patch to complete the implementation of CREATE COLLATION IF NOT EXISTS. The meat of this was already implemented for pg_import_system_collations; this just exposes it in the SQL command. If we go ahead with ICU, then creating collations by hand will become more common, so this could be useful in practice. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services >From aa6f7563b0893961bbc5d82c8496c2bbea990f3c Mon Sep 17 00:00:00 2001 From: Peter EisentrautDate: Wed, 8 Feb 2017 22:51:09 -0500 Subject: [PATCH] Add CREATE COLLATION IF NOT EXISTS clause The core of the functionality was already implemented when pg_import_system_collations was added. This just exposes it as an option in the SQL command. --- doc/src/sgml/ref/create_collation.sgml | 15 +-- src/backend/commands/collationcmds.c | 4 ++-- src/backend/nodes/copyfuncs.c| 1 + src/backend/nodes/equalfuncs.c | 1 + src/backend/parser/gram.y| 20 src/backend/tcop/utility.c | 3 ++- src/include/commands/collationcmds.h | 2 +- src/include/nodes/parsenodes.h | 1 + src/test/regress/expected/collate.linux.utf8.out | 4 src/test/regress/sql/collate.linux.utf8.sql | 2 ++ 10 files changed, 47 insertions(+), 6 deletions(-) diff --git a/doc/src/sgml/ref/create_collation.sgml b/doc/src/sgml/ref/create_collation.sgml index d757cdfb43..c09e5bd6d4 100644 --- a/doc/src/sgml/ref/create_collation.sgml +++ b/doc/src/sgml/ref/create_collation.sgml @@ -18,12 +18,12 @@ -CREATE COLLATION name ( +CREATE COLLATION [ IF NOT EXISTS ] name ( [ LOCALE = locale, ] [ LC_COLLATE = lc_collate, ] [ LC_CTYPE = lc_ctype ] ) -CREATE COLLATION name FROM existing_collation +CREATE COLLATION [ IF NOT EXISTS ] name FROM existing_collation @@ -48,6 +48,17 @@ Parameters + IF NOT EXISTS + + + Do not throw an error if a collation with the same name already exists. + A notice is issued in this case. Note that there is no guarantee that + the existing collation is anything like the one that would have been created. + + + + + name diff --git a/src/backend/commands/collationcmds.c b/src/backend/commands/collationcmds.c index e165d4b2a6..919cfc6a06 100644 --- a/src/backend/commands/collationcmds.c +++ b/src/backend/commands/collationcmds.c @@ -37,7 +37,7 @@ * CREATE COLLATION */ ObjectAddress -DefineCollation(ParseState *pstate, List *names, List *parameters) +DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_exists) { char *collName; Oid collNamespace; @@ -137,7 +137,7 @@ DefineCollation(ParseState *pstate, List *names, List *parameters) GetDatabaseEncoding(), collcollate, collctype, - false); + if_not_exists); if (!OidIsValid(newoid)) return InvalidObjectAddress; diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 30d733e57a..ab96d71a8f 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3104,6 +3104,7 @@ _copyDefineStmt(const DefineStmt *from) COPY_NODE_FIELD(defnames); COPY_NODE_FIELD(args); COPY_NODE_FIELD(definition); + COPY_SCALAR_FIELD(if_not_exists); return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 55c73b7292..94789feb08 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1210,6 +1210,7 @@ _equalDefineStmt(const DefineStmt *a, const DefineStmt *b) COMPARE_NODE_FIELD(defnames); COMPARE_NODE_FIELD(args); COMPARE_NODE_FIELD(definition); + COMPARE_SCALAR_FIELD(if_not_exists); return true; } diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index cf97be512d..7c025f247d 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -5606,6 +5606,16 @@ DefineStmt: n->definition = $4; $$ = (Node *)n; } + | CREATE COLLATION IF_P NOT EXISTS any_name definition +{ + DefineStmt *n = makeNode(DefineStmt); + n->kind = OBJECT_COLLATION; + n->args = NIL; + n->defnames = $6; + n->definition = $7; + n->if_not_exists = true; + $$ = (Node *)n; +} | CREATE COLLATION any_name FROM any_name { DefineStmt *n = makeNode(DefineStmt); @@ -5615,6 +5625,16 @@ DefineStmt: n->definition = list_make1(makeDefElem("from", (Node *) $5, @5)); $$ = (Node *)n; } + | CREATE COLLATION IF_P NOT EXISTS any_name FROM any_name +{ + DefineStmt *n = makeNode(DefineStmt); + n->kind = OBJECT_COLLATION; + n->args = NIL; + n->defnames = $6; + n->definition = list_make1(makeDefElem("from", (Node *) $8, @8)); + n->if_not_exists = true; + $$ =
Re: [HACKERS] Declarative partitioning - another take
On 2017/02/08 21:20, amul sul wrote: > Regarding following code in ATExecDropNotNull function, I don't see > any special check for RANGE partitioned, is it intended to have same > restriction for LIST partitioned too, I guess not? > > /* > * If the table is a range partitioned table, check that the column is not > * in the partition key. > */ > if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) > { > PartitionKey key = RelationGetPartitionKey(rel); > int partnatts = get_partition_natts(key), > i; > > for (i = 0; i < partnatts; i++) > { > AttrNumber partattnum = get_partition_col_attnum(key, i); > > if (partattnum == attnum) > ereport(ERROR, > (errcode(ERRCODE_INVALID_TABLE_DEFINITION), > errmsg("column \"%s\" is in range partition key", > colName))); > } > } Good catch! Seems like an oversight, attached fixes it. Thanks, Amit >From b84ac63b6b4acd19a09e8507cf199db127c06d71 Mon Sep 17 00:00:00 2001 From: amitDate: Thu, 9 Feb 2017 10:31:58 +0900 Subject: [PATCH] Check partition strategy in ATExecDropNotNull We should prevent dropping the NOT NULL constraint on a column only if the column is in the *range* partition key (as the error message also says). Add a regression test while at it. Reported by: Amul Sul Patch by: Amit Langote Report: https://postgr.es/m/CAAJ_b95g5AgkpJKbLajAt8ovKub874-B9X08PiOqHvVfMO2mLw%40mail.gmail.com --- src/backend/commands/tablecmds.c | 22 +- src/test/regress/expected/alter_table.out | 3 +++ src/test/regress/sql/alter_table.sql | 3 +++ 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 37a4c4a3d6..f33aa70da6 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -5593,18 +5593,22 @@ ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode) if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { PartitionKey key = RelationGetPartitionKey(rel); - int partnatts = get_partition_natts(key), - i; - for (i = 0; i < partnatts; i++) + if (get_partition_strategy(key) == PARTITION_STRATEGY_RANGE) { - AttrNumber partattnum = get_partition_col_attnum(key, i); + int partnatts = get_partition_natts(key), + i; - if (partattnum == attnum) -ereport(ERROR, - (errcode(ERRCODE_INVALID_TABLE_DEFINITION), - errmsg("column \"%s\" is in range partition key", -colName))); + for (i = 0; i < partnatts; i++) + { +AttrNumber partattnum = get_partition_col_attnum(key, i); + +if (partattnum == attnum) + ereport(ERROR, + (errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("column \"%s\" is in range partition key", + colName))); + } } } diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index d8e7b61294..e6d45fbf9c 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -3330,6 +3330,9 @@ ALTER TABLE list_parted2 DROP COLUMN b; ERROR: cannot drop column named in partition key ALTER TABLE list_parted2 ALTER COLUMN b TYPE text; ERROR: cannot alter type of column named in partition key +-- cannot drop NOT NULL constraint of a range partition key column +ALTER TABLE range_parted ALTER a DROP NOT NULL; +ERROR: column "a" is in range partition key -- cleanup: avoid using CASCADE DROP TABLE list_parted, part_1; DROP TABLE list_parted2, part_2, part_5, part_5_a; diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 1f551ec53c..12edb8b3ba 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -2189,6 +2189,9 @@ ALTER TABLE part_2 INHERIT inh_test; ALTER TABLE list_parted2 DROP COLUMN b; ALTER TABLE list_parted2 ALTER COLUMN b TYPE text; +-- cannot drop NOT NULL constraint of a range partition key column +ALTER TABLE range_parted ALTER a DROP NOT NULL; + -- cleanup: avoid using CASCADE DROP TABLE list_parted, part_1; DROP TABLE list_parted2, part_2, part_5, part_5_a; -- 2.11.0 -- 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] Logical replication existing data copy
On 2017-02-08 23:25, Petr Jelinek wrote: 0001-Use-asynchronous-connect-API-in-libpqwalreceiver-v2.patch 0002-Always-initialize-stringinfo-buffers-in-walsender-v2.patch 0003-Fix-after-trigger-execution-in-logical-replication-v2.patch 0004-Add-RENAME-support-for-PUBLICATIONs-and-SUBSCRIPTION-v2.patch 0001-Logical-replication-support-for-initial-data-copy-v4.patch test 'object_address' fails, see atachment. That's all I found in a quick first trial. thanks, Erik Rijkers *** /home/aardvark/pg_stuff/pg_sandbox/pgsql.logical_replication/src/test/regress/expected/object_address.out 2017-02-09 00:51:30.345519608 +0100 --- /home/aardvark/pg_stuff/pg_sandbox/pgsql.logical_replication/src/test/regress/results/object_address.out 2017-02-09 00:54:11.884715532 +0100 *** *** 38,43 --- 38,45 TO SQL WITH FUNCTION int4recv(internal)); CREATE PUBLICATION addr_pub FOR TABLE addr_nsp.gentable; CREATE SUBSCRIPTION addr_sub CONNECTION '' PUBLICATION bar WITH (DISABLED, NOCREATE SLOT); + ERROR: could not connect to the publisher: FATAL: no pg_hba.conf entry for replication connection from host "[local]", user "aardvark", SSL off + -- test some error cases SELECT pg_get_object_address('stone', '{}', '{}'); ERROR: unrecognized object type "stone" *** *** 409,463 pg_identify_object_as_address(classid, objid, subobjid) ioa(typ,nms,args), pg_get_object_address(typ, nms, ioa.args) as addr2 ORDER BY addr1.classid, addr1.objid, addr1.subobjid; !type| schema | name| identity | ?column? ! ---++---+--+-- ! default acl || | for role regress_addr_user in schema public on tables| t ! default acl || | for role regress_addr_user on tables | t ! type | pg_catalog | _int4 | integer[]| t ! type | addr_nsp | gencomptype | addr_nsp.gencomptype | t ! type | addr_nsp | genenum | addr_nsp.genenum | t ! type | addr_nsp | gendomain | addr_nsp.gendomain | t ! function | pg_catalog | | pg_catalog.pg_identify_object(pg_catalog.oid,pg_catalog.oid,integer) | t ! aggregate | addr_nsp | | addr_nsp.genaggr(integer)| t ! sequence | addr_nsp | gentable_a_seq| addr_nsp.gentable_a_seq | t ! table | addr_nsp | gentable | addr_nsp.gentable| t ! table column | addr_nsp | gentable | addr_nsp.gentable.b | t ! index | addr_nsp | gentable_pkey | addr_nsp.gentable_pkey | t ! view | addr_nsp | genview | addr_nsp.genview | t ! materialized view | addr_nsp | genmatview| addr_nsp.genmatview | t ! foreign table | addr_nsp | genftable | addr_nsp.genftable | t ! foreign table column | addr_nsp | genftable | addr_nsp.genftable.a | t ! role || regress_addr_user | regress_addr_user| t ! server|| addr_fserv| addr_fserv | t ! user mapping || | regress_addr_user on server integer | t ! foreign-data wrapper || addr_fdw | addr_fdw | t ! access method || btree | btree| t ! operator of access method || | operator 1 (integer, integer) of pg_catalog.integer_ops USING btree | t ! function of access method || | function 2 (integer, integer) of pg_catalog.integer_ops USING btree | t ! default value |
Re: [HACKERS] Backport of pg_statistics typos fix
On Wed, 8 Feb 2017 14:54:17 -0500 Robert Haaswrote: > On Tue, Feb 7, 2017 at 4:22 AM, Yugo Nagata wrote: > > I found typos "pg_statistics" in REL9_6_STABLE, but that has been > > fixed in the master branch. > > > > Fix typo: pg_statistics -> pg_statistic > > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=5a366b4ff4ceceb9793fcc13c3f097ee0d32c56d;hp=f7c62462402972b13d10e43f104ca0c0fecb6d08 > > > > I think it would be better to backport this to other branches. > > We usually leave such decisions to the discretion of the committer, > because back-porting such changes takes time and sometimes it just > isn't that important. Nobody's likely to be confused by a few > instances of writing pg_statistics rather than pg_statistic. > Personally, I favor not back-porting such things in most cases, > because I think patches that get back-ported should be strictly > limited to bug fixes, and typos in code comments aren't bug fixes. > But not everyone has the same opinion on this. What's your reason for > wanting it back-ported? I agree typos in code comments aren't bug fixes and not need to get back-ported. However, there are typos also in the document. The scalarltsel function retrieves the histogram for unique1 from - pg_statistics. For manual queries it is more + pg_statistic. For manual queries it is more convenient to look in the simpler pg_stats view: I think this might be a document bug, but if nobody cares of it, I also don't mind. Thanks, > > BTW, looking at that commit, this change looks to have adjusted this > from being wrong to still being wrong: > > -Allow pg_statistics to be reset by calling > pg_stat_reset() (Christopher) > +Allow pg_statistic to be reset by calling > pg_stat_reset() (Christopher) > > It's true that pg_stat_reset() doesn't reset the nonexistent > pg_statistics table. But it doesn't reset pg_statistic either. IIUC, > it resets the data gathered by the statistics collector, which is > something else altogether. > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company -- Yugo Nagata -- 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] Improve OR conditions on joined columns.
Jim Nasbywrites: > AFAICT this can be transformed into a UNION (not all) if dim.id is > unique. Does the upper planner pathification make this any easier? What I did in 9.6 is a first step. The next step, I think, is to replace prepunion.c with something that can consider more than one implementation path for a union. Although ... actually, that may not be the bottleneck for what you're after. The issue here is not so much discovering a clever plan for a union as realizing that the query could be cast as a union in the first place. Maybe it'd be better to imagine this as something closer to planagg.c, that is it knows how to apply a specific high-level optimization that might or might not be a win, so it builds a path describing that and sees if it looks cheaper than a path done the normal way. The fact that we could even build a path describing a union is something that wasn't there before 9.6, but maybe there's enough infrastructure for that now. > There's another transform using arrays that's possible as well (see > attached example); I believe that would work regardless of uniqueness. That doesn't look terribly promising for automated application. And I think it's really dependent on the exact shape of the OR clause, which is an unpleasant limitation. Considering the amount of work this will take to do at all, you'd want it to be pretty general I think. I'm imagining something that would look for an OR in which each clause referenced only one rel, then if it can identify a way to re-unique-ify the result, split into a UNION with an arm for each rel used in the OR. The nature of the conditions in each OR arm don't seem to matter ... though probably you'd have to insist on there not being volatile conditions anywhere in the query, because they'd get evaluated too many times. 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] Press Release Draft - 2016-02-09 Cumulative Update
On 2/7/17 9:37 AM, Jonathan S. Katz wrote: Below is the draft of the press release for the update this Thursday: Thanks for the work on this! 11 There existed a race condition if CREATE INDEX CONCURRENTLY was called on a column that had not been indexed before, then rows that were updated by transactions running at the same time as the CREATE INDEX CONCURRENTLY command could have been indexed incorrectly. I think that'd read better as 11 There existed a race condition /where/ if CREATE INDEX CONCURRENTLY was called on a column that had not been indexed before, then rows that were updated by transactions running at the same time as the CREATE INDEX CONCURRENTLY command /may not/ have been indexed incorrectly. Also, maybe we should mention that there's no way to test for this, and make a stronger suggestion to redo any affected indexes? 20 These release contains several fixes to improve the stability of visible data and WAL logging that we wish to highlight here. I think this sentence can just go. If we want to keep it, IMHO this is a better alternative: "This release contains several improvements to the stability of data visibility and WAL logging." 22 Prior to this release, data could be prematurely pruned by a vacuum operation when a special snapshot used for catalog scans was presently available. ... vacuum operation even though a special catalog scan snapshot was in use. BTW, I don't know what came out of the discussion of git references in release notes, but I'd find it useful to be able to at least get a complete list. Not hard for me to do that since I know git and our naming scheme, but maybe we should include directions for doing so? -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com 855-TREBLE2 (855-873-2532) -- 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] Logical replication existing data copy
Hi, here is updated patch. Note that it's rebased on top of logical replication improvements patches [1] (which still apply fine to my surprise). It will probably need another rebase once patches from Masahiko Sawada and Fujii Masao get in. [1] https://www.postgresql.org/message-id/42655eb4-6b2e-b35b-c184-509efd6eba10%402ndquadrant.com -- Petr Jelinek 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] contrib modules and relkind check
On Mon, Feb 6, 2017 at 4:01 AM, Amit Langotewrote: > On 2017/01/24 15:35, Amit Langote wrote: > > On 2017/01/24 15:11, Michael Paquier wrote: > >> On Tue, Jan 24, 2017 at 2:14 PM, Amit Langote > >> wrote: > >>> Some contrib functions fail to fail sooner when relations of > unsupported > >>> relkinds are passed, resulting in error message like one below: > >>> > >>> create table foo (a int); > >>> create view foov as select * from foo; > >>> select pg_visibility('foov', 0); > >>> ERROR: could not open file "base/13123/16488": No such file or > directory > >>> > >>> Attached patch fixes that for all such functions I could find in > contrib. > >>> > >>> It also installs RELKIND_PARTITIONED_TABLE as unsupported in a couple > of > >>> places (in pageinspect and pgstattuple). > >> > >> I have spent some time looking at your patch, and did not find any > >> issues with it, nor did I notice code paths that were not treated or > >> any other contrib modules sufferring from the same deficiencies that > >> you may have missed. Nice work. > > > > Thanks for the review, Michael! > > Added to the next CF, just so someone can decide to pick it up later. > > https://commitfest.postgresql.org/13/988/ > > Thanks, > Amit > Is this still needing a reviewer? If so, here it goes: Patch applies. make check-pgstattuple-recurse, check-pg_visibility-recurse, check-pageinspect-recurse all pass. Code is quite clear. It does raise two questions: 1. should have these tests named in core functions, like maybe: relation_has_visibility(Relation) relation_has_storage(Relation) and/or corresponding void functions check_relation_has_visibility() and check_relation_has_storage() which would raise the appropriate error message when the boolean test fails. Then again, this might be overkill since we don't add new relkinds very often. 2. Is it better stylistically to have an AND-ed list of != tests, or a negated list of OR-ed equality tests, and should the one style be changed to conform to the other? No new regression tests. I think we should create a dummy partitioned table for each contrib and show that it fails.
Re: [HACKERS] Press Release Draft - 2016-02-09 Cumulative Update
On 2/8/17 2:51 PM, Alvaro Herrera wrote: I always have a bit of mixed feelings with these kind of string manipulations on dynamic SQL. It may look a bit nasty, but locking tables for long periods (or being without an important index for a period) is much worse in production scenarios. I think posting a recipe in the wiki is a great idea (especially if it handles corner cases like constraints). I'm not so keen on trying to code it entirely in psql though. I think it'd be a lot cleaner to have a plpgsql function that uses format() to construct the appropriate commands to run and then spit that out as text. Users can either cut and paste that, or they can \gset it to a variable that they then execute, or they can capture the output to a file which they execute. The big advantage to this is by default you see what commands would be run, but you can still fully automate if you want to without much extra effort. -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com 855-TREBLE2 (855-873-2532) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Improve OR conditions on joined columns.
I've a client interested enough in $SUBJECT that they're willing to offer a bounty on it. An example of the pain is (working example attached): create temp view denorm as select f.*, d1.t t1, d2.t t2 from fact f left join dim d1 on f1=d1.id left join dim d2 on f2=d2.id ; -- Fast explain analyze select count(*) from denorm where 1 in (f1,f2); explain analyze select count(*) from denorm where '1' in (t1); -- Slow explain analyze select count(*) from denorm where '1' in (t1,t2); They currently work around this by doing a union: select ... from denorm where t1 = '1' union select ... from denorm where t2 = '1' ... or depending on needs using IN instead of =. AFAICT this can be transformed into a UNION (not all) if dim.id is unique. Does the upper planner pathification make this any easier? There's another transform using arrays that's possible as well (see attached example); I believe that would work regardless of uniqueness. Just to be clear; the OR by itself is not a problem (as shown by the first fast query); it's the OR with the JOIN that's a problem. -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com 855-TREBLE2 (855-873-2532) create temp table fact as select generate_series(1,999) f1, generate_series(1,999) f2; insert into fact select s,null from generate_series(1,999) s; insert into fact select null,s from generate_series(1,999) s; create index f_f1 on fact(f1); create index f_f2 on fact(f2); analyze fact; create temp table dim as select s, s::text t from generate_series(1,999) s; alter table dim add primary key(s), add unique (t); create temp view denorm as select f.*, d1.t t1, d2.t t2, array[f1,f2] ident_ids from fact f left join dim d1 on f1=d1.s left join dim d2 on f2=d2.s ; -- Fast explain analyze select count(*) from denorm where 1 in (f1,f2); explain analyze select count(*) from denorm where '1' in (t1); -- Slow explain analyze select count(*) from denorm where '1' in (t1,t2); CREATE FUNCTION ident_ids( idents text[] ) RETURNS int[] STABLE LANGUAGE sql AS $$ SELECT array( SELECT s FROM dim WHERE t = ANY(idents) ) $$; CREATE FUNCTION ident_ids( idents text ) RETURNS int[] STABLE LANGUAGE sql AS $$ SELECT ident_ids( ('{' || idents || '}')::text[] ) $$; explain analyze select count(*) from denorm where ident_ids && ident_ids('42,84,128'); \echo ERROR OK here on version >= 10 create index fact__f_array on fact using gin ((array[f1,f2]) _int4_ops); -- pre 10.0 \echo ERROR OK here on version < 10 create index fact__f_array on fact using gin ((array[f1,f2]) array_ops); -- 10.0 explain analyze select count(*) from denorm where ident_ids && ident_ids('42,84,128'); -- 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] WIP: About CMake v2
Hi, On 2017-02-08 16:52:19 -0500, Tom Lane wrote: > For my own purposes, the only thing that I find seriously annoying about > the status quo is the amount of time required to run "configure". For > me, that step is usually comparable to or even more than the time to > do the build proper, because (a) ccache and (b) multiple CPUs. > configure isn't parallelizable, and there's probably nothing that > can be done about that. I use autoconf caching feature to make that a bit less painful (plus some scripting about when to scrap the cache file...). I find that seriously annoying too. > If CMake offers a substantial improvement > in configuration time then that would be attractive. Otherwise I'd > just as soon see us put the effort into making the MSVC scripts more > robust and able to pull more data from the makefiles. Some of the build-tooling in cmake is quite nice, I have to admit. I've e.g. grown to like using ninja instead of make to build the resulting files. Primarily in projects that take longer than pg to compile - a clean build in llvm with ninja takes like 0.1 seconds. Being more able to rely on things working on windows when doing them on linux does seem like an advantage to me - fiddlin with Mkvcbuild.pm is quite annoying. - Andres -- 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] WIP: About CMake v2
On 2/8/17 3:52 PM, Tom Lane wrote: For my own purposes, the only thing that I find seriously annoying about the status quo is the amount of time required to run "configure". For me, that step is usually comparable to or even more than the time to do the build proper, because (a) ccache and (b) multiple CPUs. configure isn't parallelizable, and there's probably nothing that can be done about that. If CMake offers a substantial improvement in configuration time then that would be attractive. Otherwise I'd just as soon see us put the effort into making the MSVC scripts more robust and able to pull more data from the makefiles. FWIW, I've had good luck adding -C to configure to cache the output. I'd guess it's at least 10x faster on my laptop. Obviously doesn't help if you're doing where you're testing something that alters config output. In those cases I'll either edit config.cache and delete the relevant lines or just temporarily move it out of the way (or just nuke it...). -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com 855-TREBLE2 (855-873-2532) -- 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] WIP: About CMake v2
Peter Eisentrautwrites: > On 2/8/17 6:21 AM, Yuriy Zhuravlev wrote: >> Support two build systems it's not big deal really. I have been working >> on this past year without any big troubles. >> Also we have second perl build system... > The perl/msvc build system pulls in information from the makefiles. So > when you add a file or something basic like that, you don't have to > update it. So it's really more like 1.5 build systems. Really it's more like 1.1 build systems, in that the MSVC scripts do that just well enough that you *usually* don't have to think about them. But then when they fail, and you have to figure out why, it can be a pain. For my own purposes, the only thing that I find seriously annoying about the status quo is the amount of time required to run "configure". For me, that step is usually comparable to or even more than the time to do the build proper, because (a) ccache and (b) multiple CPUs. configure isn't parallelizable, and there's probably nothing that can be done about that. If CMake offers a substantial improvement in configuration time then that would be attractive. Otherwise I'd just as soon see us put the effort into making the MSVC scripts more robust and able to pull more data from the makefiles. 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] WIP: About CMake v2
Hi, On 2017-01-30 10:26:18 -0500, Peter Eisentraut wrote: > On 1/30/17 1:28 AM, Andres Freund wrote: > > Given that fact, I just don't buy why it's a good idea to not also > > replace autoconf initially. > > Well, I find it a bit scary. If you do the big switch all at once, then > you will have to dedicate the following 3 months to fixing complaints > from developers and build farmers. That'll be the case just as well if we spread it out over two cycles, except that we'll have it in multiple phases, and we run into the danger of a half-done conversion. I'd rather not change systems at all than run into the danger of that. > > Either we're able to properly test it - i.e. it runs all tests - on *nix or > > we're not. > > That would work if there were a single entry point into the build > system. But in practice there are many, and every one of them is > someone's favorite. It's unlikely that we will be able to enumerate all > of them during patch review. Not sure what you mean with "entry point"? - Andres -- 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] WIP: About CMake v2
On 2/8/17 6:21 AM, Yuriy Zhuravlev wrote: > Support two build systems it's not big deal really. I have been working > on this past year without any big troubles. > Also we have second perl build system... The perl/msvc build system pulls in information from the makefiles. So when you add a file or something basic like that, you don't have to update it. So it's really more like 1.5 build systems. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, 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] Documentation improvements for partitioning
On Fri, Feb 3, 2017 at 4:15 AM, Amit Langotewrote: > Here are some patches to improve the documentation about partitioned > tables: > > 0001: Adds some details about partition_bound_spec to the CREATE TABLE > page, especially: > > - a note about inclusivity of range partition bounds, > - a note about the UNBOUNDED literal in case of range partitioning, > - a note about the NULL literal in case of list partitioning, > > I wonder if the above "note" should be added under the Notes section or > are they fine to be added as part of the description of PARTITION OF > clause. Also: > > - in syntax synopsis, it appears now that any expression is OK to be used >for individual bound datum, but it's not true. Only literals are >allowed. So fixed that. > - added an example showing how to create partitions of a range >partitioned table with multiple columns in the partition key > - added PARTITION BY and PARTITION OF (FOR VALUES) as PostgreSQL >extensions in the compatibility section > > > 0002: Adds details about partitioned tables to the DDL chapter (ddl.sgml) > > - a new section named "Partitioned Tables" right next to the >Inheritance and Partitioning sections is created. > - examples are added to the existing Partitioning section using the new >partitioned tables. Old text about implementing table partitioning >using inheritance is kept, sort of as a still supported older >alternative. > > 0003: Add partitioning keywords to keywords.sgml > > This is all I have for now. Any feedback is greatly appreciated. Adding > this to the next CF. > > Thanks, > Amit > Patch applies. Overall this looks really good. It goes a long way towards stating some of the things I had to learn through experimentation. I had to read a really long way into the patch before finding a blurb that I felt wasn't completely clear: + + + INSERT statements with ON CONFLICT + clause are currently not allowed on partitioned tables, that is, + cause error when specified. + Here's some other tries at saying the same thing, none of which are completely satisfying: ...ON CONFLICT clause are currently not allowed on partitioned tables and will cause an error? ...ON CONFLICT clause are currently not allowed on partitioned tables and will instead cause an error? ...ON CONFLICT clause will currently cause an error if used on a partitioned table? As far as additional issues to cover, this bit: + + + One cannot drop a NOT NULL constraint on a + partition's column, if the constraint is present in the parent table. + + Maybe we should add something about how one would go about dropping a NOT NULL constraint (parent first then partitions?) In reviewing this patch, do all our target formats make word spacing irrelevant? i.e. is there any point in looking at the number of spaces after a period, etc? A final note, because I'm really familiar with partitioning on Postgres and other databases, documentation which is clear to me might not be to someone less familiar with partitioning. Maybe we want another reviewer for that?
Re: [HACKERS] Cannot shutdown subscriber after DROP SUBSCRIPTION
On Sat, Feb 4, 2017 at 3:11 PM, Petr Jelinekwrote: > That was the reason why DropSubscription didn't release the lock in the > first place. It was supposed to be released at the end of the > transaction though. Holding an LWLock until end-of-transaction is a phenomenally bad idea, both because you lose interruptibility and because of the deadlock risk. -- 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] WAL consistency check facility
On Wed, Feb 8, 2017 at 1:25 AM, Kuntal Ghoshwrote: > Thank you Robert for the review. Please find the updated patch in the > attachment. I have committed this patch after fairly extensive revisions: * Rewrote the documentation to give some idea what the underlying mechanism of operation of the feature is, so that users who choose to enable this will hopefully have some understanding of what they've turned on. * Renamed "char *page" arguments to "char *pagedata" and "Page page", because tempPage doesn't seem to be to be any better a name than page_norm. * Moved bufmask.c to src/backend/access/common, because there's no code in src/backend/storage/buffer that knows anything about the format of pages; that is the job of AMs, hence src/backend/access. * Improved some comments in bufmask.c * Removed consistencyCheck_is_enabled in favor of determining which RMs support masking by the presence of absence of an rm_mask function. * Removed assertion in checkXLogConsistency that consistency checking is enabled for the RM; that's trivially false if wal_consistency_checking is not the same on the master and the standby. (Note that quite apart from the issue of whether this function should exist at all, adding it to a header file after the closing #endif guard is certainly not right.) * Changed checkXLogConsistency to use RBM_NORMAL_NO_LOG instead of RBM_NORMAL. I'm not sure if there are any cases where this makes a difference, but it seems safer. * Changed checkXLogConsistency to skip pages whose LSN is newer than that of the record. Without this, if you shut down recovery and restart it, it complains of inconsistent pages and dies. (I'm not sure this is the only scenario that needs to be covered; it would be smart to do more testing of restarting the standby.) * Made wal_consistency_checking a developer option instead of a WAL option. Even though it CAN be used in production, we don't particularly want to encourage that; enabling WAL consistency checking has a big performance cost and makes your system more fragile not less -- a WAL consistency failure causes your standby to die a hard death. (Maybe there should be a way to suppress consistency checking on the standby -- but I think not just by requiring wal_consistency_checking on both ends. Or maybe we should just downgrade the FATAL to WARNING; blowing up the standby irrevocably seems like poor behavior.) * Coding style improvement in check_wal_consistency_checking. * Removed commas in messages added to pg_xlogdump; those didn't look good to me, on further review. * Comment improvements in xlog_internal.h and xlogreader.h I also bumped XLOG_PAGE_MAGIC (which is normally done by the committer, not the patch author, so I wasn't expecting that to be in the patch as submitted). -- 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] Patch: Avoid precision error in to_timestamp().
I wrote: > I wonder if we could make things better just by using rint() rather than > a naive cast-to-integer. The cast will truncate not round, and I think > that might be what's mostly biting you. Does this help for you? > #ifdef HAVE_INT64_TIMESTAMP > - result = seconds * USECS_PER_SEC; > + result = rint(seconds * USECS_PER_SEC); > #else I poked around looking for possible similar issues elsewhere, and found that a substantial majority of the datetime-related code already uses rint() when trying to go from float to int representations. As far as I can find, this function and make_interval() are the only ones that fail to do so. So I'm now thinking that this is a clear oversight, and both those places need to be patched to use rint(). 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] Press Release Draft - 2016-02-09 Cumulative Update
Tobias Bussmann wrote: > But I could put this > snippet as a "REINDEX CONCURRENTLY" workaround into the Administrative > Snippets category of the wiki, if there are no further objections > about the way it works. Sounds like a good idea. There are further complications: * you can't DROP indexes belonging to constraints, so this recipe doesn't work for them. One useful trick is to create the index first, then ADD CONSTRAINT USING INDEX. * For unique constraints referenced by FKs, the above doesn't work either. One thing you can do is create a second index and swap the relfilenode underneath. This is a nasty, dirty, dangerous, unsupported trick, but it can save people's neck at times. > I always have a bit of mixed feelings with these kind of string > manipulations on dynamic SQL. It may look a bit nasty, but locking tables for long periods (or being without an important index for a period) is much worse in production scenarios. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, 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] Press Release Draft - 2016-02-09 Cumulative Update
Am 08.02.2017 um 20:17 schrieb Alvaro Herrera: > Note that this is likely to fail if the original index name is close to > the 63 chars limit. Perhaps it's enough to add substring() when > computing index_name_tmp. (You could just not use :'index_name' there > and rely on the random md5 only, actually). Watch out for UNIQUE too. thank you for your valuable input! Here is a version that should take both into account - the query also could be simplified a bit: \set index_name 'my_bad_index' \set table_schema 'public' SELECT 'tmp_'||md5(random()::text) AS index_name_tmp \gset SELECT replace(pg_get_indexdef((quote_ident(:'table_schema')||'.'||quote_ident(:'index_name'))::regclass), ' '||quote_ident(:'index_name')||' ON', ' CONCURRENTLY '||:'index_name_tmp'||' ON') \gexec DROP INDEX CONCURRENTLY :"table_schema".:"index_name"; ALTER INDEX :"table_schema".:"index_name_tmp" RENAME TO :"index_name"; > FWIW for previous problems we've documented them in wiki pages along > with suggested solutions, and added a link to that wiki page in the > announce. Perhaps one thing to do is create a wiki page for this one > too (not volunteering myself). I'm not even remotely into the details of the CIC issue, so I'm not the right one to create a page on that topic. But I could put this snippet as a "REINDEX CONCURRENTLY" workaround into the Administrative Snippets category of the wiki, if there are no further objections about the way it works. I always have a bit of mixed feelings with these kind of string manipulations on dynamic SQL. Best, Tobias -- 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] possibility to specify template database for pg_regress
Hi 2017-02-08 8:33 GMT+01:00 Pavel Stehule: > > > 2017-02-08 8:30 GMT+01:00 Michael Paquier : > >> On Wed, Feb 8, 2017 at 4:24 PM, Pavel Stehule >> wrote: >> > What is sense for list of databases? >> >> ECPG uses it for example, see 0992259. >> >> > Some option --template can be great - with backpatch if it is possible. >> >> That's not really complicated to patch... That could be a nice task >> for a starter. >> > > Today I am doing some training - I can look on it at evening > here is a patch Regards Pavel > > Regards > > Pavel > > >> -- >> Michael >> > > diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index d4d00d9..354b918 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -68,6 +68,7 @@ const char *pretty_diff_opts = "-w -C3"; /* options settable from command line */ _stringlist *dblist = NULL; +_stringlist *templatelist = NULL; bool debug = false; char *inputdir = "."; char *outputdir = "."; @@ -1907,7 +1908,7 @@ drop_database_if_exists(const char *dbname) } static void -create_database(const char *dbname) +create_database(const char *dbname, const char *template) { _stringlist *sl; @@ -1917,10 +1918,12 @@ create_database(const char *dbname) */ header(_("creating database \"%s\""), dbname); if (encoding) - psql_command("postgres", "CREATE DATABASE \"%s\" TEMPLATE=template0 ENCODING='%s'%s", dbname, encoding, + psql_command("postgres", "CREATE DATABASE \"%s\" TEMPLATE=\"%s\" ENCODING='%s'%s", + dbname, template, encoding, (nolocale) ? " LC_COLLATE='C' LC_CTYPE='C'" : ""); else - psql_command("postgres", "CREATE DATABASE \"%s\" TEMPLATE=template0%s", dbname, + psql_command("postgres", "CREATE DATABASE \"%s\" TEMPLATE=\"%s\"%s", + dbname, template, (nolocale) ? " LC_COLLATE='C' LC_CTYPE='C'" : ""); psql_command(dbname, "ALTER DATABASE \"%s\" SET lc_messages TO 'C';" @@ -1995,6 +1998,7 @@ help(void) printf(_(" --outputdir=DIR place output files in DIR (default \".\")\n")); printf(_(" --schedule=FILE use test ordering schedule from FILE\n")); printf(_("(can be used multiple times to concatenate)\n")); + printf(_(" --template=DB use template DB (default \"template0\")\n")); printf(_(" --temp-instance=DIR create a temporary instance in DIR\n")); printf(_(" --use-existinguse an existing installation\n")); printf(_("\n")); @@ -2041,6 +2045,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc {"launcher", required_argument, NULL, 21}, {"load-extension", required_argument, NULL, 22}, {"config-auth", required_argument, NULL, 24}, + {"template", required_argument, NULL, 25}, {NULL, 0, NULL, 0} }; @@ -2154,6 +2159,16 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc case 24: config_auth_datadir = pg_strdup(optarg); break; + case 25: + +/* + * If a default template was specified, we need to remove it + * before we add the specified one. + */ +free_stringlist(); +split_to_stringlist(optarg, ",", ); +break; + default: /* getopt_long already emitted a complaint */ fprintf(stderr, _("\nTry \"%s -h\" for more information.\n"), @@ -2454,8 +2469,25 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc */ if (!use_existing) { - for (sl = dblist; sl; sl = sl->next) - create_database(sl->str); + if (templatelist != NULL) + { + _stringlist *tl; + + for (sl = dblist, tl = templatelist; sl; sl = sl->next, tl = tl->next) + { +if (tl != NULL) + create_database(sl->str, tl->str); +else +{ + fprintf(stderr, _("%s: the template list is shorter than database list\n"), + progname); + exit(2); +} + } + } + else + for (sl = dblist; sl; sl = sl->next) +create_database(sl->str, "template0"); for (sl = extraroles; sl; sl = sl->next) create_role(sl->str, dblist); } -- 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] Backport of pg_statistics typos fix
On Tue, Feb 7, 2017 at 4:22 AM, Yugo Nagatawrote: > I found typos "pg_statistics" in REL9_6_STABLE, but that has been > fixed in the master branch. > > Fix typo: pg_statistics -> pg_statistic > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=5a366b4ff4ceceb9793fcc13c3f097ee0d32c56d;hp=f7c62462402972b13d10e43f104ca0c0fecb6d08 > > I think it would be better to backport this to other branches. We usually leave such decisions to the discretion of the committer, because back-porting such changes takes time and sometimes it just isn't that important. Nobody's likely to be confused by a few instances of writing pg_statistics rather than pg_statistic. Personally, I favor not back-porting such things in most cases, because I think patches that get back-ported should be strictly limited to bug fixes, and typos in code comments aren't bug fixes. But not everyone has the same opinion on this. What's your reason for wanting it back-ported? BTW, looking at that commit, this change looks to have adjusted this from being wrong to still being wrong: -Allow pg_statistics to be reset by calling pg_stat_reset() (Christopher) +Allow pg_statistic to be reset by calling pg_stat_reset() (Christopher) It's true that pg_stat_reset() doesn't reset the nonexistent pg_statistics table. But it doesn't reset pg_statistic either. IIUC, it resets the data gathered by the statistics collector, which is something else altogether. -- 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] Adding the optional clause 'AS' in CREATE TRIGGER
On Tue, Feb 7, 2017 at 3:11 AM, Okano, Naokiwrote: > On Wednesday, November 16, 2016 4:31 PM Okano Naoki wrote: >> > But in any case it would be a serious mistake to do this without first >> > implementing CREATE OR REPLACE TRIGGER. I think that's an entirely >> > separate >> > proposal and you would be well advised to treat it as such. >> I see. There are more problems than I expected... >> Let me start with 'OR REPLACE' clause. > I tried to cretae a patch for CREATE OR REPLACE TRIGGER. > An example of execution is shown below. > > example) > 1.define a new trigger > CREATE TRIGGER regular_trigger >AFTER UPDATE ON table_name >REFERENCING OLD TABLE AS oldtable_1 NEW TABLE AS newtable_1 >FOR EACH STATEMENT >EXECUTE PROCEDURE func_1(); > 2.redinfe a trigger in single command > CREATE OR REPLACE TRIGGER regular_trigger >AFTER UPDATE OR DELETE ON table_name >REFERENCING OLD TABLE AS oldtable_2 NEW TABLE AS newtable_2 >FOR EACH ROW >EXECUTE PROCEDURE func_2(); > > If the named trigger does not exist. > a new trigger is also created by using OR REPLACE clause. > A regular trigger cannot be replaced by a constraint trigger and vice varsa. > because a constraint trigger has a different role from regular triger. > > Please give me feedback. You should add this to the next CommitFest so it doesn't get missed. https://commitfest.postgresql.org/ -- 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] Press Release Draft - 2016-02-09 Cumulative Update
Tobias Bussmann wrote: > Am 07.02.2017 um 18:44 schrieb Alvaro Herrera: > > 80 CREATE INDEX CONCURRENTLY bad_index_name ON table_name > > (column_name); /* replace names with your original index definition */ > > I was thinking if we could replace that "replace names with your original > index definition" with something more fancy using pg_get_indexdef in that > recipe. I ended up with quite a "REINDEX CONCURRENTLY" monster: > > \set index_name 'my_bad_index' > \set table_schema 'public' > SELECT :'index_name'||'_'||left(md5(random()::text), 5) AS index_name_tmp > \gset > SELECT > replace(replace(pg_get_indexdef((quote_ident(:'table_schema')||'.'||quote_ident(:'index_name'))::regclass), > 'INDEX '||quote_ident(:'index_name'), 'INDEX > '||quote_ident(:'index_name_tmp')), 'CREATE INDEX', 'CREATE INDEX > CONCURRENTLY') \gexec > DROP INDEX CONCURRENTLY :"table_schema".:"index_name"; > ALTER INDEX :"table_schema".:"index_name_tmp" RENAME TO :"index_name"; > > Probably not useable as a recipe in such an announcement but it was fun to > build and to see what is actually possible with some psql magic :) Note that this is likely to fail if the original index name is close to the 63 chars limit. Perhaps it's enough to add substring() when computing index_name_tmp. (You could just not use :'index_name' there and rely on the random md5 only, actually). Watch out for UNIQUE too. FWIW for previous problems we've documented them in wiki pages along with suggested solutions, and added a link to that wiki page in the announce. Perhaps one thing to do is create a wiki page for this one too (not volunteering myself). Probably too late to add the link to the press release now, since it's already out as "final". -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, 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] Press Release Draft - 2016-02-09 Cumulative Update
Am 07.02.2017 um 18:44 schrieb Alvaro Herrera: > 80 CREATE INDEX CONCURRENTLY bad_index_name ON table_name (column_name); > /* replace names with your original index definition */ I was thinking if we could replace that "replace names with your original index definition" with something more fancy using pg_get_indexdef in that recipe. I ended up with quite a "REINDEX CONCURRENTLY" monster: \set index_name 'my_bad_index' \set table_schema 'public' SELECT :'index_name'||'_'||left(md5(random()::text), 5) AS index_name_tmp \gset SELECT replace(replace(pg_get_indexdef((quote_ident(:'table_schema')||'.'||quote_ident(:'index_name'))::regclass), 'INDEX '||quote_ident(:'index_name'), 'INDEX '||quote_ident(:'index_name_tmp')), 'CREATE INDEX', 'CREATE INDEX CONCURRENTLY') \gexec DROP INDEX CONCURRENTLY :"table_schema".:"index_name"; ALTER INDEX :"table_schema".:"index_name_tmp" RENAME TO :"index_name"; Probably not useable as a recipe in such an announcement but it was fun to build and to see what is actually possible with some psql magic :) Tobias -- 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_bsd_indent: implement -lps ("leave preprocessor space")
On 2017-02-07 23:30:44 -0500, Tom Lane wrote: > Piotr Stefaniakwrites: > > this is a patch that Andres asked me for. It makes pg_bsd_indent leave > > preprocessor space alone, as in this example: > > > #if 0 > > # if 0 > > # if 0 > > # error > > # endif > > # endif > > #else > > # line 7 > > #endif For context: I'd asked Piotr how dificult it'd be to add this. > Um ... but the point of pgindent is to standardize spacing, not to let > people invent their own style. If you wanted to have a discussion about > whether pgindent should force preprocessor directives to look like the > above, we could talk about that. But I do not want to be reading code that > looks like the above in one place and code that does not ten lines away. I don't think that's something easily done in an automatic manner. Because you'd e.g. obviously not want to indent everything within include guards. I don't really buy the danger of large divergances in code nearby - this seems mostly useful when writing a bit more complicated macros, and I don't think they'll be that frequently added in existing files. I do think this makes the nesting for #ifdefs a *lot* more readable, and we have plenty of cases where it's currently really hard to discern what "branch" one is currently reading. Allowing to opt-in into the "newer" formatting in places where it makes sense, seems reasonable to me. Regards, Andres -- 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] multivariate statistics (v19)
On 8 February 2017 at 16:09, David Fetterwrote: > Combinations are n!/(k! * (n-k)!), so computing those is more > along the lines of: > > unsigned long long > choose(unsigned long long n, unsigned long long k) { > if (k > n) { > return 0; > } > unsigned long long r = 1; > for (unsigned long long d = 1; d <= k; ++d) { > r *= n--; > r /= d; > } > return r; > } > > which greatly reduces the chance of overflow. > Hmm, but that doesn't actually prevent overflows, since it can overflow in the multiplication step, and there is no protection against that. In the algorithm I presented, the inputs and the intermediate result are kept below INT_MAX, so the multiplication step cannot overflow the 64-bit integer, and it will only raise an overflow error if the actual result won't fit in a 32-bit int. Actually a crucial part of that, which I failed to mention previously, is the first step replacing k with min(k, n-k). This is necessary for inputs like (100,99), which should return 100, and which must be computed as 100 choose 1, not 100 choose 99, otherwise it will overflow internally before getting to the final result. Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pg_basebackup -R
I just tried out pg_basebackup -R and got the following recovery.conf file: standby_mode = 'on' primary_conninfo = 'user=rhaas passfile=''/home/rhaas/.pgpass'' port=5432 sslmode=disable sslcompression=1 target_session_attrs=any' This seems fairly random to me. pg_basebackup explains: * Do not emit this setting if: - the setting is "replication", * "dbname" or "fallback_application_name", since these would be * overridden by the libpqwalreceiver module anyway. - not set or * empty. ...which looks like it got clubbed by pgindent, but anyway I'm not sure that's the best algorithm. passfile and target_session_attrs are both new in v10 and have non-empty default values, yet neither of them seems like something that you necessarily want in your automatically-created recovery.conf file. It seems like it would be more correct to leave out parameters that are set to the default value, rather than those that are set to an empty value. -- 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] Patch: Avoid precision error in to_timestamp().
=?UTF-8?Q?Erik_Nordstr=C3=B6m?=writes: > I stumbled upon a precision issue with the to_timestamp() function that > causes it to return unexpected timestamp values. For instance, the query > SELECT to_timestamp(1486480176.236538) returns the timestamp "2017-02-07 > 16:09:36.236537+01", which is off by one microsecond. Looking at the source > code, the issue seems to be that the conversion is unnecessarily done using > imprecise floating point calculations. Since the target timestamp has > microsecond precision, and is internally represented by a 64-bit integer > (on modern platforms), it is better to first convert the given floating > point value to a microsecond integer and then doing the epoch conversion, > rather than doing the conversion using floating point and finally casting > to an integer/timestamp. This change would introduce overflow failures near the end of the range of valid inputs. Maybe it's worth doing anyway and we should just tighten the range bound tests right above what you patched, but I'm a bit skeptical. Float inputs are going to be inherently imprecise anyhow. I wonder if we could make things better just by using rint() rather than a naive cast-to-integer. The cast will truncate not round, and I think that might be what's mostly biting you. Does this help for you? #ifdef HAVE_INT64_TIMESTAMP - result = seconds * USECS_PER_SEC; + result = rint(seconds * USECS_PER_SEC); #else 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] Idea on how to simplify comparing two sets
On Wed, Feb 8, 2017 at 10:30 AM, Tom Lanewrote: > David Fetter writes: > > On Wed, Feb 08, 2017 at 11:22:56AM -0500, Tom Lane wrote: > >> Yes. I think a new set-operation keyword would inevitably have to > >> be fully reserved --- UNION, INTERSECT, and EXCEPT all are --- which > >> means that you'd break every application that has used that word as > >> a table, column, or function name. > > > I've long wanted a SYMMETRIC DIFFERENCE join type, that being the only > > elementary set operation not included in join types, but nobody at the > > SQL standards committee seems to have cared enough to help. > > I wonder whether you could shoehorn it in with no new reserved word > by spelling it "EXCEPT SYMMETRIC", which could be justified by the > precedent of BETWEEN SYMMETRIC. But not sure what to do with > duplicate rows (ie, if LHS has two occurrences of row X and RHS > has one occurrence, do you output X?) > Without SYMMETRIC its defined to return: max(m-n,0) with SYMMETRIC I'd think that would just change to: abs(m-n) Then you still have to apply ALL or DISTINCT on the above result. David J.
Re: [HACKERS] Idea on how to simplify comparing two sets
David Fetterwrites: > On Wed, Feb 08, 2017 at 11:22:56AM -0500, Tom Lane wrote: >> Yes. I think a new set-operation keyword would inevitably have to >> be fully reserved --- UNION, INTERSECT, and EXCEPT all are --- which >> means that you'd break every application that has used that word as >> a table, column, or function name. > I've long wanted a SYMMETRIC DIFFERENCE join type, that being the only > elementary set operation not included in join types, but nobody at the > SQL standards committee seems to have cared enough to help. I wonder whether you could shoehorn it in with no new reserved word by spelling it "EXCEPT SYMMETRIC", which could be justified by the precedent of BETWEEN SYMMETRIC. But not sure what to do with duplicate rows (ie, if LHS has two occurrences of row X and RHS has one occurrence, do you output X?) 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] Idea on how to simplify comparing two sets
On Wed, Feb 08, 2017 at 11:22:56AM -0500, Tom Lane wrote: > Robert Haaswrites: > > On Wed, Feb 8, 2017 at 4:24 AM, Pantelis Theodosiou > > wrote: > >> I'm not advocating it but I don't see how introducing new SQL keywords > >> breaks backwards compatibility. > > > It does at least a little bit. > > Yes. I think a new set-operation keyword would inevitably have to > be fully reserved --- UNION, INTERSECT, and EXCEPT all are --- which > means that you'd break every application that has used that word as > a table, column, or function name. I've long wanted a SYMMETRIC DIFFERENCE join type, that being the only elementary set operation not included in join types, but nobody at the SQL standards committee seems to have cared enough to help. > Generally speaking, we try very darn hard not to introduce new > reserved words that are not called out as reserved in the SQL > standard. (And even for those, we've sometimes made the grammar > jump through hoops so as not to reserve a word that we didn't > reserve previously.) We just never know what new keywords the standards committee will dream up, or what silliness they'll introduce in the grammar :/ Best, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com 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
Re: [HACKERS] [PATCH] configure-time knob to set default ssl ciphers
Alvaro Herrerawrites: > Tom Lane wrote: >> If we did have code for multiple libraries, perhaps some people would >> want to compile all the variants at once; in which case overloading a >> single option to be used for all the libraries would be a problem. > Hmm, I don't think our abstraction would allow for compiling more than > one at a time. ISTM that all that work has been considering that you'd > choose at most one at compile time. Very possibly it'll end up that way. But I'm not eager to pre-judge that decision, especially if we're doing it only to support a system-specific hack that could be handled in other ways. 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] [PATCH] configure-time knob to set default ssl ciphers
Tom Lane wrote: > Daniel Gustafssonwrites: > > Since we hopefully will support more SSL libraries than OpenSSL at some > > point, > > and we don’t want a torrent of configure options, wouldn’t this be better as > > --with-server-ciphers=STRING or something similar? > > One of the reasons I'm not very excited about exposing this as a configure > option is exactly that I'm not sure what happens when we get multiple TLS > library support. The cipher list we've got at the moment seems like it > is probably OpenSSL-specific (but maybe not?). Maybe the list of ciphers is not OpenSSL-specific, but the *syntax* most likely is. Particularly the abbreviations such as !eNULL and !MD5, etc. > If we did have code for multiple libraries, perhaps some people would > want to compile all the variants at once; in which case overloading a > single option to be used for all the libraries would be a problem. Hmm, I don't think our abstraction would allow for compiling more than one at a time. ISTM that all that work has been considering that you'd choose at most one at compile time. I'm not sure it's useful to have more than one anyway. If you choose one SSL implementation at configure time, it's on your head to specify a ssl-ciphers that that implementation accepts (of course, we would choose a working default if you don't specify one.) (I was going to suggest --with-ssl-ciphers but the protocol is called TLS nowadays, so maybe not a great idea.) -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, 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] GSoC 2017
2017-02-08 17:06 GMT+01:00 Robert Haas: > On Mon, Feb 6, 2017 at 6:51 AM, Ruben Buchatskiy wrote: > > 2017-01-10 12:53 GMT+03:00 Alexander Korotkov >: > >> 1. What project ideas we have? > > > > We would like to propose a project on rewriting PostgreSQL executor from > > > > traditional Volcano-style [1] to so-called push-based architecture as > > implemented in > > > > Hyper [2][3] and VitesseDB [4]. The idea is to reverse the direction of > data > > flow > > > > control: instead of pulling up tuples one-by-one with ExecProcNode(), we > > suggest > > > > pushing them from below to top until blocking operator (e.g. > Aggregation) is > > > > encountered. There’s a good example and more detailed explanation for > this > > approach in [2]. > > I think this very possibly a good idea but extremely unlikely to be > something that a college student or graduate student can complete in > one summer. More like an existing expert developer and a year of > doing not much else. > +1 Pavel > > -- > 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] [PATCH] configure-time knob to set default ssl ciphers
Daniel Gustafssonwrites: > Since we hopefully will support more SSL libraries than OpenSSL at some point, > and we don’t want a torrent of configure options, wouldn’t this be better as > --with-server-ciphers=STRING or something similar? One of the reasons I'm not very excited about exposing this as a configure option is exactly that I'm not sure what happens when we get multiple TLS library support. The cipher list we've got at the moment seems like it is probably OpenSSL-specific (but maybe not?). If we did have code for multiple libraries, perhaps some people would want to compile all the variants at once; in which case overloading a single option to be used for all the libraries would be a problem. This would all be a lot clearer if we already had that code, but since we don't, I'm inclined to be conservative about exposing new features that make assumptions about how it will be. 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
[HACKERS] Patch: Avoid precision error in to_timestamp().
Hello hackers, I stumbled upon a precision issue with the to_timestamp() function that causes it to return unexpected timestamp values. For instance, the query SELECT to_timestamp(1486480176.236538) returns the timestamp "2017-02-07 16:09:36.236537+01", which is off by one microsecond. Looking at the source code, the issue seems to be that the conversion is unnecessarily done using imprecise floating point calculations. Since the target timestamp has microsecond precision, and is internally represented by a 64-bit integer (on modern platforms), it is better to first convert the given floating point value to a microsecond integer and then doing the epoch conversion, rather than doing the conversion using floating point and finally casting to an integer/timestamp. I am attaching a patch that fixes the above issue. Regards, Erik to_timestamp_fix.patch Description: Binary data -- 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] Idea on how to simplify comparing two sets
Robert Haaswrites: > On Wed, Feb 8, 2017 at 4:24 AM, Pantelis Theodosiou > wrote: >> I'm not advocating it but I don't see how introducing new SQL keywords >> breaks backwards compatibility. > It does at least a little bit. Yes. I think a new set-operation keyword would inevitably have to be fully reserved --- UNION, INTERSECT, and EXCEPT all are --- which means that you'd break every application that has used that word as a table, column, or function name. Generally speaking, we try very darn hard not to introduce new reserved words that are not called out as reserved in the SQL standard. (And even for those, we've sometimes made the grammar jump through hoops so as not to reserve a word that we didn't reserve previously.) 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] [PATCH] configure-time knob to set default ssl ciphers
> On 08 Feb 2017, at 13:31, Pavel Raiskupwrote: > > On Wednesday, February 8, 2017 1:29:19 PM CET Pavel Raiskup wrote: >> On Wednesday, February 8, 2017 1:05:08 AM CET Tom Lane wrote: >>> Peter Eisentraut writes: On 2/7/17 11:21 AM, Tom Lane wrote: > A compromise that might be worth considering is to introduce > #define PG_DEFAULT_SSL_CIPHERS "HIGH:MEDIUM:+3DES:!aNULL" > into pg_config_manual.h, which would at least give you a reasonably > stable target point for a long-lived patch. >>> You'd still need to patch postgresql.conf.sample somehow. >>> >>> Right. The compromise position that I had in mind was to add the >>> #define in pg_config_manual.h and teach initdb to propagate it into >>> the installed copy of postgresql.conf, as we've done with other GUCs >>> with platform-dependent defaults, such as backend_flush_after. >>> >>> That still leaves the question of what to do with the SGML docs. >>> We could add some weasel wording to the effect that the default might >>> be platform-specific, or we could leave the docs alone and expect the >>> envisioned Red Hat patch to patch config.sgml along with >>> pg_config_manual.h. >> >> Thanks for quickt feedback. Just to not give up too early, I'm attaching >> 2nd iteration. I'm fine to fallback to pg_config_manual.h solution though, >> if this is considered too bad. >> >> I tried to fix the docs now (crucial part indeed) so we are not that >> "scrict" and there's some space for per-distributor change of ssl_ciphers >> default. >> >> From the previous mail: >>> I'm not really sure that we want to carry around that much baggage for a >>> single-system hack. >> >> Accepted, but still I'm giving a chance. OpenSSL maintainers predict this >> (or >> something else in similar fashion) is going to be invented in OpenSSL >> upstream. >> So there's still some potential in ./configure option. > > Argh :( ! Attaching now and sorry. Since we hopefully will support more SSL libraries than OpenSSL at some point, and we don’t want a torrent of configure options, wouldn’t this be better as --with-server-ciphers=STRING or something similar? + --with-openssl-be-ciphers=STRING + Replace the default list of server-supported ciphers Each SSL implementation would then be responsible for handling it appropriately. cheers ./daniel -- 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] multivariate statistics (v19)
On Wed, Feb 08, 2017 at 03:23:25PM +, Dean Rasheed wrote: > On 6 February 2017 at 21:26, Alvaro Herrerawrote: > > Tomas Vondra wrote: > >> On 02/01/2017 11:52 PM, Alvaro Herrera wrote: > > > >> > Nearby, some auxiliary functions such as n_choose_k and > >> > num_combinations are not documented. What it is that they do? I'd > >> > move these at the end of the file, keeping the important entry points > >> > at the top of the file. > >> > >> I'd say n-choose-k is pretty widely known term from combinatorics. The > >> comment would essentially say just 'this is n-choose-k' which seems rather > >> pointless. So as much as I dislike the self-documenting code, this actually > >> seems like a good case of that. > > > > Actually, we do have such comments all over the place. I knew this as > > "n sobre k", so the english name doesn't immediately ring a bell with me > > until I look it up; I think the function comment could just say > > "n_choose_k -- this function returns the binomial coefficient". > > One of the things you have to watch out for when writing code to > compute binomial coefficients is integer overflow, since the numerator > and denominator get large very quickly. For example, the current code > will overflow for n=13, k=12, which really isn't that large. > > This can be avoided by computing the product in reverse and using a > larger datatype like a 64-bit integer to store a single intermediate > result. The point about multiplying the terms in reverse is that it > guarantees that each intermediate result is an exact integer (a > smaller binomial coefficient), so there is no need to track separate > numerators and denominators, and you avoid huge intermediate > factorials. Here's what that looks like in psuedo-code: > > binomial(int n, int k): > # Save computational effort by using the symmetry of the binomial > # coefficients > k = min(k, n-k); > > # Compute the result using binomial(n, k) = binomial(n-1, k-1) * n / k, > # starting from binomial(n-k, 0) = 1, and computing the sequence > # binomial(n-k+1, 1), binomial(n-k+2, 2), ... > # > # Note that each intermediate result is an exact integer. > int64 result = 1; > for (int i = 1; i <= k; i++) > { > result = (result * (n-k+i)) / i; > if (result > INT_MAX) Raise overflow error > } > return (int) result; > > > Note also that I think num_combinations(n) is just an expensive way of > calculating 2^n - n - 1. Combinations are n!/(k! * (n-k)!), so computing those is more along the lines of: unsigned long long choose(unsigned long long n, unsigned long long k) { if (k > n) { return 0; } unsigned long long r = 1; for (unsigned long long d = 1; d <= k; ++d) { r *= n--; r /= d; } return r; } which greatly reduces the chance of overflow. Best, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com 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
Re: [HACKERS] drop support for Python 2.3
On 02/07/2017 11:49 PM, Tom Lane wrote: > Peter Eisentrautwrites: >> I would like to propose that we drop support for Python 2.3. >> ... >> We do have buildfarm coverage on prairiedog. However, that runs a >10 >> year old operating system, so I think it is not representing real usage. > I have no particular objection to dropping 2.3 support, but should we > make some effort to fail gracefully (ie, with a relevant error message) > on older versions? I would guess that the effect of your patch will be > to produce quite opaque failures. We seem to be computing python_version > in configure, so it shouldn't be that hard to check. > >> - It's unlikely that Python 2.3 is still used in practice. Python 2.4 >> is in RHEL 5, which is the typically the oldest mainstream OS we look at. > Hm, is there anything running 2.4 in the buildfarm? If we're going to > claim support for 2.4, we'd be well advised to test it. with top as (select distinct on (sysname) sysname, snapshot from build_status_recent_500 where branch = 'HEAD' order by sysname, snapshot desc ) select * from top where exists (select 1 from build_status_log l where l.sysname = top.sysname and l.snapshot = top.snapshot and l.branch = 'HEAD' and l.log_stage = 'config.log' and l.log_text ~ 'python2\.4'); This returns no rows. Maybe we need to set up a Centos5 or RHEL 5 animal. cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, 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] GSoC 2017
On Mon, Feb 6, 2017 at 6:51 AM, Ruben Buchatskiywrote: > 2017-01-10 12:53 GMT+03:00 Alexander Korotkov : >> 1. What project ideas we have? > > We would like to propose a project on rewriting PostgreSQL executor from > > traditional Volcano-style [1] to so-called push-based architecture as > implemented in > > Hyper [2][3] and VitesseDB [4]. The idea is to reverse the direction of data > flow > > control: instead of pulling up tuples one-by-one with ExecProcNode(), we > suggest > > pushing them from below to top until blocking operator (e.g. Aggregation) is > > encountered. There’s a good example and more detailed explanation for this > approach in [2]. I think this very possibly a good idea but extremely unlikely to be something that a college student or graduate student can complete in one summer. More like an existing expert developer and a year of doing not much else. -- 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] chomp PQerrorMessage() in backend uses
Peter Eisentrautwrites: > Here is a patch to systematically trim the trailing newlines off > PQerrorMessage() results in backend uses (dblink, postgres_fdw, > libpqwalreceiver). +1 > I noticed that there are some inconsistent assumptions about whether > PQerrorMessage() can ever return NULL. From the code, I think that > should not be possible, but some code appears to be prepared for it. > Other code is not. What is correct? libpq.sgml doesn't specify, so it's hard to argue that either position is "correct". I don't mind resolving the ambiguity via a documentation change though. I'd want to see it also cover other corner cases like what if there hasn't been an error on the connection. 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] Parallel bitmap heap scan
On Wed, Feb 8, 2017 at 8:58 AM, Dilip Kumarwrote: > On Wed, Feb 8, 2017 at 7:01 PM, Robert Haas wrote: >> You can store whatever you want in SH_TYPE's private_data member. >> SH_ALLOCATE and SH_FREE both get a pointer to the SH_TYPE, so they >> have access to that. Hmm, but there's no way to get that set in >> SH_CREATE before SH_ALLOCATE is called. Maybe we need to add a >> private_data argument to SH_CREATE. execGrouping.c could use that >> instead of frobbing private_data directly: >> >> -hashtable->hashtab = tuplehash_create(tablecxt, nbuckets); >> -hashtable->hashtab->private_data = hashtable; >> +hashtable->hashtab = tuplehash_create(tablecxt, nbuckets, hashtable); > > Okay, will go ahead as you suggested. Patch attached for the same. Looks good to me. If nobody has further ideas here, I'll push this and your previous patch tomorrow. -- 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] pageinspect: Hash index support
On Wed, Feb 8, 2017 at 9:25 AM, Ashutosh Sharmawrote: >>> 1) Check if an overflow page is a new page. If so, read a bitmap page >>> to confirm if a bit corresponding to this overflow page is clear or >>> not. For this, I would first add Assert statement to ensure that the >>> bit is clear and if it is, then set the statusbit as false indicating >>> that the page is free. >>> >>> 2) In case if an overflow page is not new, first verify if it is >>> really an overflow page and if so, check if the bit corresponding to >>> it in the bitmap page is SET. If so, set the statusbit as true; If >>> not, we would see an assertion failure happening. >> >> I think this is complicated and not what anybody wants. I think you >> should do exactly what I said above: return true if the bit is set in >> the bitmap, and false if it isn't. Full stop. Don't read or do >> anything with the underlying page. Only read the bitmap page. > > Okay, As per the inputs from you, I have modified hash_bitmap_info() > and have tried to keep the things simple. Attached is the patch that > has this changes. Please have a look and let me know if you feel it is > not yet in the right shape. Thanks. Well, this changes the function signature and I don't see any advantage in doing that. Also, it still doesn't read the code that reads the overflow page. Any correct patch for this problem needs to include the following hunk: -buf = ReadBufferExtended(indexRel, MAIN_FORKNUM, (BlockNumber) ovflblkno, - RBM_NORMAL, NULL); -LockBuffer(buf, BUFFER_LOCK_SHARE); -_hash_checkpage(indexRel, buf, LH_PAGE_TYPE); -page = BufferGetPage(buf); -opaque = (HashPageOpaque) PageGetSpecialPointer(page); - -if (opaque->hasho_flag != LH_OVERFLOW_PAGE) -ereport(ERROR, -(errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("page is not an overflow page"), - errdetail("Expected %08x, got %08x.", -LH_OVERFLOW_PAGE, opaque->hasho_flag))); - -if (BlockNumberIsValid(opaque->hasho_prevblkno)) -bit = true; - -UnlockReleaseBuffer(buf); And then, instead, you need to add some code to set bit based on the bitmap page, like what you have: +mapbuf = _hash_getbuf(indexRel, bitmapblkno, HASH_READ, LH_BITMAP_PAGE); +mappage = BufferGetPage(mapbuf); +freep = HashPageGetBitmap(mappage); + +if (ISSET(freep, bitmapbit)) +bit = 1; Except I would write that last line as... bit = ISSET(freep, bitmapbit) != 0 ...instead of using an if statement. I'm sort of confused as to why the idea of not reading the underlying page is so hard to understand. -- 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] DROP SUBSCRIPTION and ROLLBACK
On 08/02/17 07:40, Masahiko Sawada wrote: > On Wed, Feb 8, 2017 at 9:01 AM, Michael Paquier >wrote: >> On Wed, Feb 8, 2017 at 1:30 AM, Fujii Masao wrote: >>> On Wed, Feb 8, 2017 at 12:26 AM, Petr Jelinek >>> wrote: For example what happens if apply crashes during the DROP SUBSCRIPTION/COMMIT and is not started because the delete from catalog is now visible so the subscription is no longer there? >>> >>> Another idea is to treat DROP SUBSCRIPTION in the same way as VACUUM, i.e., >>> make it emit an error if it's executed within user's transaction block. >> >> It seems to me that this is exactly Petr's point: using >> PreventTransactionChain() to prevent things to happen. > > Agreed. It's better to prevent to be executed inside user transaction > block. And I understood there is too many failure scenarios we need to > handle. > >>> Also DROP SUBSCRIPTION should call CommitTransactionCommand() just >>> after removing the entry from pg_subscription, then connect to the publisher >>> and remove the replication slot. >> >> For consistency that may be important. > > Agreed. > > Attached patch, please give me feedback. > This looks good (and similar to what initial patch had btw). Works fine for me as well. Remaining issue is, what to do about CREATE SUBSCRIPTION then, there are similar failure scenarios there, should we prevent it from running inside transaction as well? -- Petr Jelinek 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] drop support for Python 2.3
Hi, On Wed, 2017-02-08 at 09:16 -0500, Peter Eisentraut wrote: > It appears that we don't have anything running 2.4. A RHEL/CentOS 5 > system with standard components would be a good addition to the build farm. I have CentOS 5 instances running on buildfarm. I'll register them via buildfarm.pg.org soon. Regards, -- Devrim Gündüz EnterpriseDB: http://www.enterprisedb.com PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer Twitter: @DevrimGunduz , @DevrimGunduzTR signature.asc Description: This is a digitally signed message part
Re: [HACKERS] multivariate statistics (v19)
On 6 February 2017 at 21:26, Alvaro Herrerawrote: > Tomas Vondra wrote: >> On 02/01/2017 11:52 PM, Alvaro Herrera wrote: > >> > Nearby, some auxiliary functions such as n_choose_k and >> > num_combinations are not documented. What it is that they do? I'd >> > move these at the end of the file, keeping the important entry points >> > at the top of the file. >> >> I'd say n-choose-k is pretty widely known term from combinatorics. The >> comment would essentially say just 'this is n-choose-k' which seems rather >> pointless. So as much as I dislike the self-documenting code, this actually >> seems like a good case of that. > > Actually, we do have such comments all over the place. I knew this as > "n sobre k", so the english name doesn't immediately ring a bell with me > until I look it up; I think the function comment could just say > "n_choose_k -- this function returns the binomial coefficient". > One of the things you have to watch out for when writing code to compute binomial coefficients is integer overflow, since the numerator and denominator get large very quickly. For example, the current code will overflow for n=13, k=12, which really isn't that large. This can be avoided by computing the product in reverse and using a larger datatype like a 64-bit integer to store a single intermediate result. The point about multiplying the terms in reverse is that it guarantees that each intermediate result is an exact integer (a smaller binomial coefficient), so there is no need to track separate numerators and denominators, and you avoid huge intermediate factorials. Here's what that looks like in psuedo-code: binomial(int n, int k): # Save computational effort by using the symmetry of the binomial # coefficients k = min(k, n-k); # Compute the result using binomial(n, k) = binomial(n-1, k-1) * n / k, # starting from binomial(n-k, 0) = 1, and computing the sequence # binomial(n-k+1, 1), binomial(n-k+2, 2), ... # # Note that each intermediate result is an exact integer. int64 result = 1; for (int i = 1; i <= k; i++) { result = (result * (n-k+i)) / i; if (result > INT_MAX) Raise overflow error } return (int) result; Note also that I think num_combinations(n) is just an expensive way of calculating 2^n - n - 1. Regards, Dean -- 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] One-shot expanded output in psql using \gx
On Wed, Feb 08, 2017 at 03:52:40PM +0100, Christoph Berg wrote: > Re: David Fetter 2017-02-07 <20170207051659.gc3...@fetter.org> > > On Mon, Feb 06, 2017 at 08:54:13PM +0100, Christoph Berg wrote: > > > The majority of voices here was in favor of using \gx, so here is > > > another version of the same patch which implements that. > > > > Patch is useful, and works as documented. > > > > Maybe it could get a test or two in src/test/regress/*/psql.* > > Good point. The new version tests \g and \gx with a new query, and > re-running it on the last query buffer. > > ! /* \g [filename] -- send query, optionally with output to file/pipe */ > ! else if (strcmp(cmd, "g") == 0) > { > char *fname = psql_scan_slash_option(scan_state, > >OT_FILEPIPE, NULL, false); > --- 910,920 > free(fname); > } > > ! /* > ! * \g [filename] -- send query, optionally with output to file/pipe > ! * \gx [filename] -- same as \g, with expanded mode forced > ! */ > ! else if (strcmp(cmd, "g") == 0 || strcmp(cmd, "gx") == 0) > { > char *fname = psql_scan_slash_option(scan_state, > >OT_FILEPIPE, NULL, false); > *** exec_command(const char *cmd, > *** 924,929 > --- 927,934 > pset.gfname = pg_strdup(fname); > } > free(fname); > + if (strcmp(cmd, "gx") == 0) > + pset.g_expanded = true; > status = PSQL_CMD_SEND; > } Would you be open to saving the next person some work by doing something similar to how \d is done, namely looking for an 'x' modifier after g without regard to how far after? As of this writing, the \d version starts at line 398 in master. Best, David. -- David Fetterhttp://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com 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
Re: [HACKERS] One-shot expanded output in psql using \gx
Re: David Fetter 2017-02-07 <20170207051659.gc3...@fetter.org> > On Mon, Feb 06, 2017 at 08:54:13PM +0100, Christoph Berg wrote: > > The majority of voices here was in favor of using \gx, so here is > > another version of the same patch which implements that. > > Patch is useful, and works as documented. > > Maybe it could get a test or two in src/test/regress/*/psql.* Good point. The new version tests \g and \gx with a new query, and re-running it on the last query buffer. Christoph -- Senior Berater, Tel.: +49 2166 9901 187 credativ GmbH, HRB Mönchengladbach 12080, USt-ID-Nummer: DE204566209 Trompeterallee 108, 41189 Mönchengladbach Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer pgp fingerprint: 5C48 FE61 57F4 9179 5970 87C6 4C5A 6BAB 12D2 A7AE diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml new file mode 100644 index ae58708..e0302ea *** a/doc/src/sgml/ref/psql-ref.sgml --- b/doc/src/sgml/ref/psql-ref.sgml *** Tue Oct 26 21:40:57 CEST 1999 *** 1891,1896 --- 1891,1908 + \gx [ filename ] + \gx [ |command ] + + + \gx is equivalent to \g, but + forces expanded output mode for this query. + + + + + + \gexec diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c new file mode 100644 index f17f610..6e140fe *** a/src/bin/psql/command.c --- b/src/bin/psql/command.c *** exec_command(const char *cmd, *** 910,917 free(fname); } ! /* \g [filename] -- send query, optionally with output to file/pipe */ ! else if (strcmp(cmd, "g") == 0) { char *fname = psql_scan_slash_option(scan_state, OT_FILEPIPE, NULL, false); --- 910,920 free(fname); } ! /* ! * \g [filename] -- send query, optionally with output to file/pipe ! * \gx [filename] -- same as \g, with expanded mode forced ! */ ! else if (strcmp(cmd, "g") == 0 || strcmp(cmd, "gx") == 0) { char *fname = psql_scan_slash_option(scan_state, OT_FILEPIPE, NULL, false); *** exec_command(const char *cmd, *** 924,929 --- 927,934 pset.gfname = pg_strdup(fname); } free(fname); + if (strcmp(cmd, "gx") == 0) + pset.g_expanded = true; status = PSQL_CMD_SEND; } diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c new file mode 100644 index 5349c39..4534bd9 *** a/src/bin/psql/common.c --- b/src/bin/psql/common.c *** PrintQueryTuples(const PGresult *results *** 770,775 --- 770,779 { printQueryOpt my_popt = pset.popt; + /* one-shot expanded output requested via \gx */ + if (pset.g_expanded) + my_popt.topt.expanded = true; + /* write output to \g argument, if any */ if (pset.gfname) { *** sendquery_cleanup: *** 1410,1415 --- 1414,1422 pset.gfname = NULL; } + /* reset \gx's expanded-mode flag */ + pset.g_expanded = false; + /* reset \gset trigger */ if (pset.gset_prefix) { diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c new file mode 100644 index 3e3cab4..2aece7e *** a/src/bin/psql/help.c --- b/src/bin/psql/help.c *** slashUsage(unsigned short int pager) *** 174,179 --- 174,180 fprintf(output, _(" \\copyright show PostgreSQL usage and distribution terms\n")); fprintf(output, _(" \\errverboseshow most recent error message at maximum verbosity\n")); fprintf(output, _(" \\g [FILE] or ; execute query (and send results to file or |pipe)\n")); + fprintf(output, _(" \\gx [FILE] as \\g, but force expanded output\n")); fprintf(output, _(" \\gexec execute query, then execute each value in its result\n")); fprintf(output, _(" \\gset [PREFIX] execute query and store results in psql variables\n")); fprintf(output, _(" \\q quit psql\n")); diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h new file mode 100644 index 195f5a1..70ff181 *** a/src/bin/psql/settings.h --- b/src/bin/psql/settings.h *** typedef struct _psqlSettings *** 91,96 --- 91,97 printQueryOpt popt; char *gfname; /* one-shot file output argument for \g */ + bool g_expanded; /* one-shot expanded output requested via \gx */ char *gset_prefix; /* one-shot prefix argument for \gset */ bool gexec_flag; /* one-shot flag to execute query's results */ bool crosstab_flag; /* one-shot request to crosstab results */ diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c new file mode 100644 index 6e759d0..0bd2ae3 *** a/src/bin/psql/tab-complete.c --- b/src/bin/psql/tab-complete.c *** psql_completion(const char *text, int st *** 1338,1344 "\\dm", "\\dn", "\\do", "\\dO", "\\dp", "\\drds", "\\ds", "\\dS", "\\dt", "\\dT", "\\dv", "\\du", "\\dx", "\\dy", "\\e",
[HACKERS] chomp PQerrorMessage() in backend uses
Here is a patch to systematically trim the trailing newlines off PQerrorMessage() results in backend uses (dblink, postgres_fdw, libpqwalreceiver). I noticed that there are some inconsistent assumptions about whether PQerrorMessage() can ever return NULL. From the code, I think that should not be possible, but some code appears to be prepared for it. Other code is not. What is correct? -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services >From d6835cff36bcc237462febf5ba7d3df7d560329f Mon Sep 17 00:00:00 2001 From: Peter EisentrautDate: Wed, 8 Feb 2017 09:31:35 -0500 Subject: [PATCH] chomp PQerrorMessage() in backend uses PQerrorMessage() returns an error message with a trailing newline, but in backend use (dblink, postgres_fdw, libpqwalreceiver), we want to have the error message without that for emitting via ereport(). To simplify that, add a function pchomp() that returns a pstrdup'ed string with the trailing newline characters removed. --- contrib/dblink/dblink.c| 16 ++-- contrib/dblink/expected/dblink.out | 2 -- contrib/postgres_fdw/connection.c | 14 ++ .../libpqwalreceiver/libpqwalreceiver.c| 30 +++--- src/backend/utils/mmgr/mcxt.c | 14 ++ src/include/utils/palloc.h | 2 ++ 6 files changed, 41 insertions(+), 37 deletions(-) diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c index ac43c458cb..db0a8ba72d 100644 --- a/contrib/dblink/dblink.c +++ b/contrib/dblink/dblink.c @@ -166,7 +166,7 @@ typedef struct remoteConnHashEnt #define DBLINK_RES_INTERNALERROR(p2) \ do { \ - msg = pstrdup(PQerrorMessage(conn)); \ + msg = pchomp(PQerrorMessage(conn)); \ if (res) \ PQclear(res); \ elog(ERROR, "%s: %s", p2, msg); \ @@ -204,7 +204,7 @@ typedef struct remoteConnHashEnt conn = PQconnectdb(connstr); \ if (PQstatus(conn) == CONNECTION_BAD) \ { \ - msg = pstrdup(PQerrorMessage(conn)); \ + msg = pchomp(PQerrorMessage(conn)); \ PQfinish(conn); \ ereport(ERROR, \ (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION), \ @@ -278,7 +278,7 @@ dblink_connect(PG_FUNCTION_ARGS) if (PQstatus(conn) == CONNECTION_BAD) { - msg = pstrdup(PQerrorMessage(conn)); + msg = pchomp(PQerrorMessage(conn)); PQfinish(conn); if (rconn) pfree(rconn); @@ -651,7 +651,7 @@ dblink_send_query(PG_FUNCTION_ARGS) /* async query send */ retval = PQsendQuery(conn, sql); if (retval != 1) - elog(NOTICE, "could not send query: %s", PQerrorMessage(conn)); + elog(NOTICE, "could not send query: %s", pchomp(PQerrorMessage(conn))); PG_RETURN_INT32(retval); } @@ -1087,7 +1087,7 @@ storeQueryResult(volatile storeInfo *sinfo, PGconn *conn, const char *sql) PGresult *res; if (!PQsendQuery(conn, sql)) - elog(ERROR, "could not send query: %s", PQerrorMessage(conn)); + elog(ERROR, "could not send query: %s", pchomp(PQerrorMessage(conn))); if (!PQsetSingleRowMode(conn)) /* shouldn't fail */ elog(ERROR, "failed to set single-row mode for dblink query"); @@ -1366,8 +1366,8 @@ dblink_error_message(PG_FUNCTION_ARGS) DBLINK_INIT; DBLINK_GET_NAMED_CONN; - msg = PQerrorMessage(conn); - if (msg == NULL || msg[0] == '\0') + msg = pchomp(PQerrorMessage(conn)); + if (msg[0] == '\0') PG_RETURN_TEXT_P(cstring_to_text("OK")); else PG_RETURN_TEXT_P(cstring_to_text(msg)); @@ -2709,7 +2709,7 @@ dblink_res_error(PGconn *conn, const char *conname, PGresult *res, * return NULL, not a PGresult at all. */ if (message_primary == NULL) - message_primary = PQerrorMessage(conn); + message_primary = pchomp(PQerrorMessage(conn)); if (res) PQclear(res); diff --git a/contrib/dblink/expected/dblink.out b/contrib/dblink/expected/dblink.out index 5acaaf225a..4b6d26e574 100644 --- a/contrib/dblink/expected/dblink.out +++ b/contrib/dblink/expected/dblink.out @@ -377,7 +377,6 @@ FROM dblink('myconn','SELECT * FROM foo') AS t(a int, b text, c text[]) WHERE t.a > 7; ERROR: could not establish connection DETAIL: missing "=" after "myconn" in connection info string - -- create a named persistent connection SELECT dblink_connect('myconn',connection_parameters()); dblink_connect @@ -604,7 +603,6 @@ FROM dblink('myconn','SELECT * FROM foo') AS t(a int, b text, c text[]) WHERE t.a > 7; ERROR: could not establish connection DETAIL: missing "=" after "myconn" in connection info string - -- create a named persistent connection SELECT dblink_connect('myconn',connection_parameters()); dblink_connect diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c index 7f7a744ac0..c6e3d44515 100644 --- a/contrib/postgres_fdw/connection.c +++ b/contrib/postgres_fdw/connection.c @@ -226,21 +226,11 @@ connect_pg_server(ForeignServer *server, UserMapping
Re: [HACKERS] pageinspect: Hash index support
>> 1) Check if an overflow page is a new page. If so, read a bitmap page >> to confirm if a bit corresponding to this overflow page is clear or >> not. For this, I would first add Assert statement to ensure that the >> bit is clear and if it is, then set the statusbit as false indicating >> that the page is free. >> >> 2) In case if an overflow page is not new, first verify if it is >> really an overflow page and if so, check if the bit corresponding to >> it in the bitmap page is SET. If so, set the statusbit as true; If >> not, we would see an assertion failure happening. > > I think this is complicated and not what anybody wants. I think you > should do exactly what I said above: return true if the bit is set in > the bitmap, and false if it isn't. Full stop. Don't read or do > anything with the underlying page. Only read the bitmap page. > Okay, As per the inputs from you, I have modified hash_bitmap_info() and have tried to keep the things simple. Attached is the patch that has this changes. Please have a look and let me know if you feel it is not yet in the right shape. Thanks. simplify_hash_bitmap_info_pageinspect.patch Description: application/download -- 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] drop support for Python 2.3
On 2/7/17 11:49 PM, Tom Lane wrote: > Hm, is there anything running 2.4 in the buildfarm? If we're going to > claim support for 2.4, we'd be well advised to test it. It appears that we don't have anything running 2.4. A RHEL/CentOS 5 system with standard components would be a good addition to the build farm. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, 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] LWLock optimization for multicore Power machines
Am Dienstag, den 07.02.2017, 16:48 +0300 schrieb Alexander Korotkov: > But win isn't > as high as I observed earlier. And I wonder why absolute numbers are > lower > than in our earlier experiments. We used IBM E880 which is actually > two Did you run your tests on bare metal or were they also virtualized? > nodes with interconnect. However interconnect is not fast enough to > make > one PostgreSQL instance work on both nodes. Thus, used half of IBM > E880 > which has 4 sockets and 32 physical cores. While you use IBM E850 > which is > really single node with 4 sockets and 48 physical cores. Thus, it > seems > that you have lower absolute numbers on more powerful hardware. That > makes > me uneasy and I think we probably don't get the best from this > hardware. > Just in case, do you use SMT=8? Yes, SMT=8 was used. The machine has 4 sockets, 8 Core each, 3.7 GHz clock frequency. The Ubuntu LPAR running on PowerVM isn't using all physical cores, currently 28 cores are assigned (=224 SMT Threads). The other cores are dedicated to the PowerVM hypervisor and a (very) small AIX LPAR. I've done more pgbenches this morning with SMT-4, too, fastest result with master was SMT-4 transaction type: scaling factor: 1000 query mode: prepared number of clients: 100 number of threads: 100 duration: 300 s number of transactions actually processed: 167306423 latency average = 0.179 ms latency stddev = 0.072 ms tps = 557685.144912 (including connections establishing) tps = 557835.683204 (excluding connections establishing) compared with SMT-8: transaction type: scaling factor: 1000 query mode: prepared number of clients: 100 number of threads: 100 duration: 300 s number of transactions actually processed: 173476449 latency average = 0.173 ms latency stddev = 0.059 ms tps = 578250.676019 (including connections establishing) tps = 578412.159601 (excluding connections establishing) and retried with lwlocks-power-3, SMT-4: transaction type: scaling factor: 1000 query mode: prepared number of clients: 100 number of threads: 100 duration: 300 s number of transactions actually processed: 185991995 latency average = 0.161 ms latency stddev = 0.059 ms tps = 619970.030069 (including connections establishing) tps = 620112.263770 (excluding connections establishing) credativ@iicl183:~/git/postgres$ ...and SMT-8 transaction type: scaling factor: 1000 query mode: prepared number of clients: 100 number of threads: 100 duration: 300 s number of transactions actually processed: 185878717 latency average = 0.161 ms latency stddev = 0.047 ms tps = 619591.476154 (including connections establishing) tps = 619655.867280 (excluding connections establishing) Interestingly the lwlocks patch seems to decrease the SMT influence factor. Side note: the system makes around 2 Mio Context Switches during the benchmarks, e.g. awk '{print $12;}' /tmp/vmstat.out cs 10 2153533 2134864 2141623 2126845 2128330 2127454 2145325 2126769 2134492 2130246 2130071 2142660 2136077 2126783 2126107 2125823 2136511 2137752 2146307 2141127 I've also tried a more recent kernel this morning (4.4 vs. 4.8), but this didn't change the picture. Is there anything more i can do? -- 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] Parallel bitmap heap scan
On Wed, Feb 8, 2017 at 7:01 PM, Robert Haaswrote: > You can store whatever you want in SH_TYPE's private_data member. > SH_ALLOCATE and SH_FREE both get a pointer to the SH_TYPE, so they > have access to that. Hmm, but there's no way to get that set in > SH_CREATE before SH_ALLOCATE is called. Maybe we need to add a > private_data argument to SH_CREATE. execGrouping.c could use that > instead of frobbing private_data directly: > > -hashtable->hashtab = tuplehash_create(tablecxt, nbuckets); > -hashtable->hashtab->private_data = hashtable; > +hashtable->hashtab = tuplehash_create(tablecxt, nbuckets, hashtable); Okay, will go ahead as you suggested. Patch attached for the same. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com hash_create_fix.patch Description: Binary data -- 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] Parallel bitmap heap scan
On Wed, Feb 8, 2017 at 1:59 AM, Dilip Kumarwrote: > IIUC, tbm_prepare_shared_iterate will be called only by the leader, > for tbmiterator as well as for the prefetch_iterator. And, > tbm_attach_shared_iterate will be called by each backend and for both > the iterators. That's what I had in mind. > IMHO, tbm_attach_shared_iterate should return TBMIterator with > reference to TBMSharedIterator inside it. The reason behind same is > that we can not keep TBMIterateResult inside TBMSharedIterator > otherwise, results will also go in shared memory but we want to have > local result memory for each worker so that other worker doesn't > disturb it. No, I don't agree. I think TBMSharedIterator should be an unshared structure created by tbm_attach_shared_iterate, which can internally contain backend-private state like a TBMIterateResult, and which can also contain a pointer to the shared-memory structure previously created by tbm_prepare_shared_iterate. That thing needs to be called something other than a TBMSharedIterator, like TBMSharedIterationState or something. -- 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] Idea on how to simplify comparing two sets
On Wed, Feb 8, 2017 at 4:24 AM, Pantelis Theodosiouwrote: > I'm not advocating it but I don't see how introducing new SQL keywords > breaks backwards compatibility. It does at least a little bit. This starts failing: select 1 new_keyword form blah; (now you have to insert AS or quote the keyword) If the new keyword is partially or fully reserved, then anybody who is using that column in a now-impermissible way has to change names of conflicting tables, columns, functions, etc. But of course we do add keywords in every release, where it's warranted by the value of the new feature. I think the problem for this proposed feature is not that adding new keywords is a completely insane thing to do but that (1) there are lots of other good ways to do more or less what is being requested with this new syntax, so it's not clear that we need a new one and (2) there are cases where it might be ambiguous which meaning of IS NOT DISTINCT FROM is met. Joel's answer to #2 is to just prefer the existing meaning wherever it's possible to assign that meaning and use the new meaning only in cases where the existing meaning is impossible, but (2a) if you tried to code it up, you'd probably find that it's quite difficult to make bison generate a grammar that works that way, because bison isn't designed around the idea of giving things a meaning only if they don't already have one and (2b) apparently ambiguities can be confusing to users even if they've been eliminated in the formal grammar. -- 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] Parallel bitmap heap scan
On Wed, Feb 8, 2017 at 4:20 AM, Dilip Kumarwrote: > The new SH_CREATE(MemoryContext ctx, uint32 nelements) don't have any > option to supply arguments to it. Our callback functions need access > to TBM. > > Is it expected that if the user of SH_CREATE who doesn't want to pass > a "MemoryContext" then we can pass arguments instead of ctx? You can store whatever you want in SH_TYPE's private_data member. SH_ALLOCATE and SH_FREE both get a pointer to the SH_TYPE, so they have access to that. Hmm, but there's no way to get that set in SH_CREATE before SH_ALLOCATE is called. Maybe we need to add a private_data argument to SH_CREATE. execGrouping.c could use that instead of frobbing private_data directly: -hashtable->hashtab = tuplehash_create(tablecxt, nbuckets); -hashtable->hashtab->private_data = hashtable; +hashtable->hashtab = tuplehash_create(tablecxt, nbuckets, hashtable); -- 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] Parallel bitmap heap scan
On Wed, Feb 8, 2017 at 5:21 AM, Dilip Kumarwrote: > On Wed, Feb 8, 2017 at 3:44 AM, Robert Haas wrote: +#ifndef SH_USE_NONDEFAULT_ALLOCATOR + >>> >>> That should probably be documented in the file header. >> >> Right. OK, did that and a few other cleanups, and committed. > > I think we need to have prototype for the default allocator outside of > #ifndef SH_USE_NONDEFAULT_ALLOCATOR. Because the file e.g. tidbitmap.c > who wants to use SH_USE_NONDEFAULT_ALLOCATOR will provide the > allocator function definition but it can not have the declaration of > those function as that function take SH_TYPE as input and that will be > only defined once we include the simplehash.h. > > So basically we can not declare prototype before including > simplehash.h for allocator. And, if we don't declare we will get > "implicit declaration warning" because simplehash itself is using > those functions. > > The solution is simplehash.h, should always declare it, and provide > the definitions only if SH_USE_NONDEFAULT_ALLOCATOR is not defined. > Attached patch does that. Makes sense, will commit. -- 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] WIP: [[Parallel] Shared] Hash
> > 0004-hj-refactor-batch-increases-v4.patch: > > Modify the existing hash join code to detect work_mem exhaustion at > the point where chunks are allocated, instead of checking after every > tuple insertion. This matches the logic used for estimating, and more > importantly allows for some parallelism in later patches. The patch has three changes 1. change dense_alloc() to accept respect_workmem argument and use it within the function. 2. Move call to ExecHashIncreaseNumBatches() into dense_alloc() from ExecHashTableInsert() to account for memory before inserting new tuple 3. Check growEnabled before calling ExecHashIncreaseNumBatches(). I think checking growEnabled within ExecHashIncreaseNumBatches() is more easy to maintain that checking at every caller. If someone is to add a caller tomorrow, s/he has to remember to add the check. It might be better to add some comments in ExecHashRemoveNextSkewBucket() explaining why dense_alloc() should be called with respect_work_mem = false? ExecHashSkewTableInsert() does call ExecHashIncreaseNumBatches() after calling ExecHashRemoveNextSkewBucket() multiple times, so it looks like we do expect increase in space used and thus go beyond work_mem for a short while. Is there a way we can handle this case in dense_alloc()? Is it possible that increasing the number of batches changes the bucket number of the tuple being inserted? If so, should we recalculate the bucket and batch of the tuple being inserted? -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database 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] [PATCH] configure-time knob to set default ssl ciphers
On Wednesday, February 8, 2017 1:29:19 PM CET Pavel Raiskup wrote: > On Wednesday, February 8, 2017 1:05:08 AM CET Tom Lane wrote: > > Peter Eisentrautwrites: > > > On 2/7/17 11:21 AM, Tom Lane wrote: > > >> A compromise that might be worth considering is to introduce > > >> #define PG_DEFAULT_SSL_CIPHERS "HIGH:MEDIUM:+3DES:!aNULL" > > >> into pg_config_manual.h, which would at least give you a reasonably > > >> stable target point for a long-lived patch. > > > > > You'd still need to patch postgresql.conf.sample somehow. > > > > Right. The compromise position that I had in mind was to add the > > #define in pg_config_manual.h and teach initdb to propagate it into > > the installed copy of postgresql.conf, as we've done with other GUCs > > with platform-dependent defaults, such as backend_flush_after. > > > > That still leaves the question of what to do with the SGML docs. > > We could add some weasel wording to the effect that the default might > > be platform-specific, or we could leave the docs alone and expect the > > envisioned Red Hat patch to patch config.sgml along with > > pg_config_manual.h. > > Thanks for quickt feedback. Just to not give up too early, I'm attaching > 2nd iteration. I'm fine to fallback to pg_config_manual.h solution though, > if this is considered too bad. > > I tried to fix the docs now (crucial part indeed) so we are not that > "scrict" and there's some space for per-distributor change of ssl_ciphers > default. > > From the previous mail: > > I'm not really sure that we want to carry around that much baggage for a > > single-system hack. > > Accepted, but still I'm giving a chance. OpenSSL maintainers predict this (or > something else in similar fashion) is going to be invented in OpenSSL > upstream. > So there's still some potential in ./configure option. Argh :( ! Attaching now and sorry. Pavel > Thanks! > Pavel > > > It looks like the xxx_flush_after GUCs aren't exactly fully documented > > as to this point, so we have some work to do there too :-( > > > > > regards, tom lane > > > > >From 41f73a910bb7afc2afa12be35a195df317f9447b Mon Sep 17 00:00:00 2001 From: Pavel RaiskupDate: Wed, 18 Jan 2017 13:34:55 +0100 Subject: [PATCH] Allow setting distribution-specific cipher set Fedora OpenSSL maintainers invented a way to specify consolidated, per-system cipher set [1] and it is our packaging policy to comply (if this is a bit meaningful). So for such situations ./configure options comes in handy instead of downstream-patching, per Red Hat bug report [2]. [1] https://fedoraproject.org/wiki/Packaging:CryptoPolicies [2] https://bugzilla.redhat.com/show_bug.cgi?id=1348125 --- configure | 34 +++ configure.in | 10 doc/src/sgml/config.sgml | 3 ++- doc/src/sgml/installation.sgml| 15 src/backend/utils/misc/guc.c | 2 +- src/backend/utils/misc/postgresql.conf.sample | 2 +- src/bin/initdb/initdb.c | 4 src/include/pg_config.h.in| 3 +++ 8 files changed, 70 insertions(+), 3 deletions(-) diff --git a/configure b/configure new file mode 100755 index bb285e4..15fad9e *** a/configure --- b/configure *** with_bsd_auth *** 831,836 --- 831,837 with_ldap with_bonjour with_openssl + with_openssl_be_ciphers with_selinux with_systemd with_readline *** Optional Packages: *** 1521,1526 --- 1522,1529 --with-ldap build with LDAP support --with-bonjour build with Bonjour support --with-openssl build with OpenSSL support + --with-openssl-be-ciphers=STRING + Replace the default list of server-supported ciphers --with-selinux build with SELinux support --with-systemd build with systemd support --without-readline do not use GNU Readline nor BSD Libedit for editing *** fi *** 5712,5717 --- 5715,5751 $as_echo "$with_openssl" >&6; } + pg_be_ciphers=HIGH:MEDIUM:+3DES:!aNULL + + + + # Check whether --with-openssl-be-ciphers was given. + if test "${with_openssl_be_ciphers+set}" = set; then : + withval=$with_openssl_be_ciphers; + case $withval in + yes) + as_fn_error $? "argument required for --with-openssl-be-ciphers option" "$LINENO" 5 + ;; + no) + as_fn_error $? "argument required for --with-openssl-be-ciphers option" "$LINENO" 5 + ;; + *) + pg_be_ciphers=$withval + ;; + esac + + fi + + + + cat >>confdefs.h <<_ACEOF + #define PG_DEFAULT_SSL_CIPHERS
Re: [HACKERS] [PATCH] configure-time knob to set default ssl ciphers
On Wednesday, February 8, 2017 1:05:08 AM CET Tom Lane wrote: > Peter Eisentrautwrites: > > On 2/7/17 11:21 AM, Tom Lane wrote: > >> A compromise that might be worth considering is to introduce > >> #define PG_DEFAULT_SSL_CIPHERS "HIGH:MEDIUM:+3DES:!aNULL" > >> into pg_config_manual.h, which would at least give you a reasonably > >> stable target point for a long-lived patch. > > > You'd still need to patch postgresql.conf.sample somehow. > > Right. The compromise position that I had in mind was to add the > #define in pg_config_manual.h and teach initdb to propagate it into > the installed copy of postgresql.conf, as we've done with other GUCs > with platform-dependent defaults, such as backend_flush_after. > > That still leaves the question of what to do with the SGML docs. > We could add some weasel wording to the effect that the default might > be platform-specific, or we could leave the docs alone and expect the > envisioned Red Hat patch to patch config.sgml along with > pg_config_manual.h. Thanks for quickt feedback. Just to not give up too early, I'm attaching 2nd iteration. I'm fine to fallback to pg_config_manual.h solution though, if this is considered too bad. I tried to fix the docs now (crucial part indeed) so we are not that "scrict" and there's some space for per-distributor change of ssl_ciphers default. >From the previous mail: > I'm not really sure that we want to carry around that much baggage for a > single-system hack. Accepted, but still I'm giving a chance. OpenSSL maintainers predict this (or something else in similar fashion) is going to be invented in OpenSSL upstream. So there's still some potential in ./configure option. Thanks! Pavel > It looks like the xxx_flush_after GUCs aren't exactly fully documented > as to this point, so we have some work to do there too :-( > 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] Declarative partitioning - another take
Hi Amit, Regarding following code in ATExecDropNotNull function, I don't see any special check for RANGE partitioned, is it intended to have same restriction for LIST partitioned too, I guess not? /* * If the table is a range partitioned table, check that the column is not * in the partition key. */ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { PartitionKey key = RelationGetPartitionKey(rel); int partnatts = get_partition_natts(key), i; for (i = 0; i < partnatts; i++) { AttrNumber partattnum = get_partition_col_attnum(key, i); if (partattnum == attnum) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("column \"%s\" is in range partition key", colName))); } } Regards, Amul -- 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] WIP: About CMake v2
2017-01-28 1:50 GMT+03:00 Michael Paquier: > On Fri, Jan 27, 2017 at 11:09 PM, Peter Eisentraut > wrote: > > On 1/24/17 8:37 AM, Tom Lane wrote: > >> Craig Ringer writes: > >>> Personally I think we should aim to have this in as a non default build > >>> mode in pg10 if it can be made ready, and aim to make it default in > pg11 at > >>> least for Windows. > >> > >> AFAIK we haven't committed to accepting this at all, let alone trying > >> to do so on a tight schedule. And I believe there was general agreement > >> that we would not accept it as something to maintain in parallel with > >> the existing makefiles. If we have to maintain two build systems, we > >> have that already. > > > > My preferred scenario would be to replace the Windows build system by > > this first, then refine it, then get rid of Autoconf. > > > > The ideal timeline would be to have a ready patch to commit early in a > > development cycle, then get rid of the Windows build system by the end > > of it. Naturally, this would need buy-in from Windows developers. > > This looks like a really good plan to me. I think it's best plan because when this patch will be in Postgres guys from community can test it for Unix systems too. Support two build systems it's not big deal really. I have been working on this past year without any big troubles. Also we have second perl build system...
Re: [HACKERS] WIP: About CMake v2
> > I don't understand what this has to do with cmake. If this is a > worthwhile improvement for the Windows build, then please explain why, > with a "before" and "after" output and a patch for the existing build > system as well. During the porting process, I meet such situations when I should fix something. It's happening because I build with different way also current build system is trying to avoid many sharp corners. If talk about this situation - without strict mode many "floats" checks don't work correctly. You can read the link above. Besides this option puts by build system. I think we can make a new thread for this approach. (with patch for current perl system) It might also be worth refactoring the existing Autoconf code here to > make this consistent. I do it because it's convenient in CMake. I can change this it's not big deal. Please explain what the circular dependency is. If there is one, we > should also side-port this change. It's an important part. I have a rule for generate rijndael.tbl by gen-rtab who make from rijndael.c (with special define) who include rijndael.tbl . If I generate rijndael.tbl it's to force build gen-rtab and generate rijndael.tbl again. CMake knows about "includes" in files but we can make the wraparound macro to hide include. This patch removes the uuid.h include but doesn't add it anywhere else. How does it work? CMake sends to compiler right place for uuid.h (I mean -I/usr/include and etc for gcc). > Yeah, I think this is how the MSVC stuff effectively works right now as > well. I glad to hear it. 2017-01-03 17:11 GMT+03:00 Peter Eisentraut < peter.eisentr...@2ndquadrant.com>: > On 12/30/16 9:10 AM, Yuriy Zhuravlev wrote: > > cmake_v2_2_c_define.patch > > > > Small chages in c.h . At first it is “#pragma fenv_access (off)” it is > > necessary if we use /fp:strict for MSVC compiler. Without this pragma we > > can’t calc floats for const variables in compiller time (2 * M_PI for > > example). Strict mode important if we want to be close with ieee754 > > float format on MSVC (1.0 / 0.0 = inf for example). Detail info here: > > https://msdn.microsoft.com/en-us/library/e7s85ffb.aspx > > I don't understand what this has to do with cmake. If this is a > worthwhile improvement for the Windows build, then please explain why, > with a "before" and "after" output and a patch for the existing build > system as well. > > > Second change is because I find and set HAVE_INT128 directly from CMake. > > PG_INT128_TYPE used only for autoconfig scripts. > > It might also be worth refactoring the existing Autoconf code here to > make this consistent. > > (My assumption is that if we were to move forward with cmake or any > other build system change, we would have to keep the old one alongside > at least for a little while. So any changes to the C code would need to > be side-ported.) > > > cmake_v2_3_rijndael.patch > > > > First I added special wraparound because here CMake have circular > > dependency (cmake very smart here). Second I removed rijndael.tbl > > because it generated during build process every time. > > Please explain what the circular dependency is. If there is one, we > should also side-port this change. > > > cmake_v2_4_uuid.patch > > > > Another small patch. Right place for uuid.h I find by CMake and not > > necessary this ifdef hell. > > This patch removes the uuid.h include but doesn't add it anywhere else. > How does it work? > > > Questions for discussion: > > > > In generated project by CMake we always have only one enter point. Also > > INSTALL macross support only including to “all” targets. It follows that > > it is impossible build contrib modules separately only with “all” > > target. Here write about this behavior: > > https://cmake.org/cmake/help/v3.7/prop_tgt/EXCLUDE_FROM_ALL.html > > Yeah, I think this is how the MSVC stuff effectively works right now as > well. > > -- > Peter Eisentraut http://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services >
Re: [HACKERS] pg_stat_wal_write statistics view
On Tue, Feb 7, 2017 at 11:47 AM, Haribabu Kommiwrote: > Hi Hackers, > > I just want to discuss adding of a new statistics view that provides > the information of wal writing details as follows > +1. I think it will be useful to observe WAL activity. > postgres=# \d pg_stat_wal_writer > View "pg_catalog.pg_stat_wal_writer" > Column | Type | Collation | Nullable | > Default > ---+--+---+--+- > num_backend_writes | bigint | | > | > num_total_writes | bigint | | | > num_blocks | bigint | | | > total_write_time | bigint| | | > stats_reset | timestamp with time zone | | | > > The columns of the view are > 1. Total number of xlog writes that are called from the backend. > 2. Total number of xlog writes that are called from both backend > and background workers. (This column can be changed to just > display on the background writes). > 3. The number of the blocks that are written. > 4. Total write_time of the IO operation it took, this variable data is > filled only when the track_io_timing GUC is enabled. So, here is *write_time* the total time system has spent in WAL writing before the last reset? I think there should be a separate column for write and sync time. > Or it is possible to integrate the new columns into the existing > pg_stat_bgwriter view also. > I feel separate view is better. -- With Regards, Amit Kapila. 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
Re: [HACKERS] Parallel tuplesort (for parallel B-Tree index creation)
On Wed, Feb 8, 2017 at 8:40 PM, Thomas Munrowrote: > On Tue, Feb 7, 2017 at 5:43 PM, Peter Geoghegan wrote: >> Does anyone have any suggestions on how to tackle this? > > Hmm. One approach might be like this: > > [hand-wavy stuff] Thinking a bit harder about this, I suppose there could be a kind of object called a SharedBufFileManager (insert better name) which you can store in a DSM segment. The leader backend that initialises a DSM segment containing one of these would then call a constructor function that sets an internal refcount to 1 and registers an on_dsm_detach callback for its on-detach function. All worker backends that attach to the DSM segment would need to call an attach function for the SharedBufFileManager to increment a refcount and also register the on_dsm_detach callback, before any chance that an error might be thrown (is that difficult?); failure to do so could result in file leaks. Then, when a BufFile is to be shared (AKA exported, made unifiable), a SharedBufFile object can be initialised somewhere in the same DSM segment and registered with the SharedBufFileManager. Internally all registered SharedBufFile objects would be linked together using offsets from the start of the DSM segment for link pointers. Now when SharedBufFileManager's on-detach function runs, it decrements the refcount in the SharedBufFileManager, and if that reaches zero then it runs a destructor that spins through the list of SharedBufFile objects deleting files that haven't already been deleted explicitly. I retract the pin/unpin and per-file refcounting stuff I mentioned earlier. You could make the default that all files registered with a SharedBufFileManager survive until the containing DSM segment is detached everywhere using that single refcount in the SharedBufFileManager object, but also provide a 'no really delete this particular shared file now' operation for client code that knows it's safe to do that sooner (which would be the case for me, I think). I don't think per-file refcounts are needed. There are a couple of problems with the above though. Firstly, doing reference counting in DSM segment on-detach hooks is really a way to figure out when the DSM segment is about to be destroyed by keeping a separate refcount in sync with the DSM segment's refcount, but it doesn't account for pinned DSM segments. It's not your use-case or mine currently, but someone might want a DSM segment to live even when it's not attached anywhere, to be reattached later. If we're trying to use DSM segment lifetime as a scope, we'd be ignoring this detail. Perhaps instead of adding our own refcount we need a new kind of hook on_dsm_destroy. Secondly, I might not want to be constrained by a fixed-sized DSM segment to hold my SharedBufFile objects... there are cases where I need to shared a number of batch files that is unknown at the start of execution time when the DSM segment is sized (I'll write about that shortly on the Parallel Shared Hash thread). Maybe I can find a way to get rid of that requirement. Or maybe it could support DSA memory too, but I don't think it's possible to use on_dsm_detach-based cleanup routines that refer to DSA memory because by the time any given DSM segment's detach hook runs, there's no telling which other DSM segments have been detached already, so the DSA area may already have partially vanished; some other kind of hook that runs earlier would be needed... Hmm. -- Thomas Munro 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
Re: [HACKERS] Parallel bitmap heap scan
On Wed, Feb 8, 2017 at 3:44 AM, Robert Haaswrote: >>> +#ifndef SH_USE_NONDEFAULT_ALLOCATOR >>> + >> >> That should probably be documented in the file header. > > Right. OK, did that and a few other cleanups, and committed. I think we need to have prototype for the default allocator outside of #ifndef SH_USE_NONDEFAULT_ALLOCATOR. Because the file e.g. tidbitmap.c who wants to use SH_USE_NONDEFAULT_ALLOCATOR will provide the allocator function definition but it can not have the declaration of those function as that function take SH_TYPE as input and that will be only defined once we include the simplehash.h. So basically we can not declare prototype before including simplehash.h for allocator. And, if we don't declare we will get "implicit declaration warning" because simplehash itself is using those functions. The solution is simplehash.h, should always declare it, and provide the definitions only if SH_USE_NONDEFAULT_ALLOCATOR is not defined. Attached patch does that. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com hash_allocate_fix.patch Description: Binary data -- 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] Proposal : For Auto-Prewarm.
On Tue, Feb 7, 2017 at 5:14 PM, Mithun Cywrote: > On Tue, Feb 7, 2017 at 12:24 PM, Amit Kapila wrote: >> On Tue, Feb 7, 2017 at 11:53 AM, Beena Emerson >> wrote: >>> Are 2 workers required? >>> >> >> I think in the new design there is a provision of launching the worker >> dynamically to dump the buffers, so there seems to be a need of >> separate workers for loading and dumping the buffers. However, there >> is no explanation in the patch or otherwise when and why this needs a >> pair of workers. Also, if the dump interval is greater than zero, >> then do we really need to separately register a dynamic worker? > > We have introduced a new value -1 for pg_prewarm.dump_interval this > means we will not dump at all, At this state, I thought auto > pg_prewarm process need not run at all, so I coded to exit the auto > pg_prewarm immediately. But If the user decides to start the auto > pg_prewarm to dump only without restarting the server, I have > introduced a launcher function "launch_pg_prewarm_dump" to restart the > auto pg_prewarm only to dump. Since now we can launch worker only to > dump, I thought we can redistribute the code between two workers, one > which only does prewarm (load only) and another dumps periodically. > This helped me to modularize and reuse the code. So once load worker > has finished its job, it registers a dump worker and then exists. > But if max_worker_processes is not enough to launch the "auto > pg_prewarm dump" bgworker > We throw an error > 2017-02-07 14:51:59.789 IST [50481] ERROR: registering dynamic > bgworker "auto pg_prewarm dump" failed c > 2017-02-07 14:51:59.789 IST [50481] HINT: Consider increasing > configuration parameter "max_worker_processes". > > Now thinking again instead of such error and then correcting same by > explicitly launching the auto pg_prewarm dump bgwroker through > launch_pg_prewarm_dump(), I can go back to original design where there > will be one worker which loads and then dumps periodically. And > launch_pg_prewarm_dump will relaunch dump only activity of that > worker. Does this sound good? > Won't it be simple if you consider -1 as a value to just load library? For *_interval = -1, it will neither load nor dump. -- With Regards, Amit Kapila. 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
Re: [HACKERS] Idea on how to simplify comparing two sets
On Tue, Feb 7, 2017 at 3:13 PM, Joel Jacobsonwrote: > Hi hackers, > > Currently there is no simple way to check if two sets are equal. > > Looks like no RDBMS in the world has a simple command for it. > > You have to do something like: > > ... > > Introducing new SQL keywords is of course not an option, > since it would possibly break backwards compatibility. > > I'm not advocating it but I don't see how introducing new SQL keywords breaks backwards compatibility. Pantelis Theodosiou
Re: [HACKERS] Parallel bitmap heap scan
On Wed, Feb 8, 2017 at 3:44 AM, Robert Haaswrote: >>> +#ifndef SH_USE_NONDEFAULT_ALLOCATOR >>> + >> >> That should probably be documented in the file header. > > Right. OK, did that and a few other cleanups, and committed. The new SH_CREATE(MemoryContext ctx, uint32 nelements) don't have any option to supply arguments to it. Our callback functions need access to TBM. Is it expected that if the user of SH_CREATE who doesn't want to pass a "MemoryContext" then we can pass arguments instead of ctx? something like this ? if (!tbm->dsa) tbm->pagetable = pagetable_create(tbm->mcxt, 128); else tbm->pagetable = pagetable_create((MemoryContext)tbm, 128); And, In allocation function, we can access this context and typecast to tbm? As shown below. static void * pagetable_allocate(pagetable_hash *pagetable, Size size) { TIDBitmap *tbm = pagetable->ctx; So Is it expected to do like I explained above, or we missed to have an arg parameter to SH_CREATE as well as in SH_TYPE structure or there is some other way you have in mind? -- Regards, Dilip Kumar 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
Re: [HACKERS] Idea on how to simplify comparing two sets
On Tue, Feb 7, 2017 at 3:58 PM, Tom Lanewrote: > Joel Jacobson writes: > > Currently there is no simple way to check if two sets are equal. > > Uh ... maybe check whether SELECT set1 EXCEPT SELECT set2 > and SELECT set2 EXCEPT SELECT set1 are both empty? > > regards, tom lane > > > Yes, if the wanted result is true or false, something like this: SELECT EXISTS (TABLE a EXCEPT TABLE b) OR EXISTS (TABLE b EXCEPT TABLE a) ; And if a new operator was added (in the same category as UNION and EXCEPT), it could be: SELECT EXISTS (TABLE a XORSET TABLE b) ; What about using the = and <> operators in sets? Is the following allowed in the standard? SELECT (TABLE a) <> (TABLE b) ;