Re: [HACKERS] Possible SSL improvements for a newcomer to tackle

2017-10-03 Thread Adrien Nayrat 
On 10/03/2017 05:47 PM, Nico Williams wrote:
> +1, but it's trickier than you might think.  I can connect you with
> Viktor Dukhovni, who has implemented DANE for OpenSSL, and done yeoman's
> work getting DANE for SMTP working.

I really appreciate, but I know it is very tricky :). I do not pretend to work
on this feature, I really do not have sufficient knowledge :/.

-- 
Adrien NAYRAT



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] Possible SSL improvements for a newcomer to tackle

2017-10-03 Thread Adrien Nayrat 
Hi,

On 10/03/2017 06:15 AM, Zeus Kronion wrote:
> 2) I was surprised to learn the following from the docs:
> 
>> By default, PostgreSQL will not perform any verification of the server
> certificate. This means that it is possible to spoof the server identity (for
> example by modifying a DNS record or by taking over the server IP address)
> without the client knowing. In order to prevent spoofing, SSL certificate
> verification must be used.
> 
> Is there a technical reason to perform no verification by default? Wouldn't a
> safer default be desirable?

If you want to verify server's certificate you should use DANE [1] + DNSSEC [2]
? (I am not an SSL expert too)

If I understand correctly, you can store your certificate in a DNS record
(TLSA). Then the client can check the certificate. You must trust your DNS
server (protection against spoofing), that's why you have to use DNSSEC.



1: https://en.wikipedia.org/wiki/DNS-based_Authentication_of_Named_Entities
2: https://en.wikipedia.org/wiki/Domain_Name_System_Security_Extensions

-- 
Adrien NAYRAT



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] Improving DISTINCT with LooseScan node

2017-09-18 Thread Adrien Nayrat 
On 09/17/2017 07:43 PM, Dmitriy Sarafannikov wrote:
[...]

It seems related to this thread? :
https://www.postgresql.org/message-id/flat/5037A9C5.4030701%40optionshouse.com#5037a9c5.4030...@optionshouse.com

And this wiki page : https://wiki.postgresql.org/wiki/Loose_indexscan


Regards,


-- 
Adrien NAYRAT

http://dalibo.com - http://dalibo.org



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] PG 10 release notes

2017-09-12 Thread Adrien Nayrat 
On 09/12/2017 01:50 AM, Bruce Momjian wrote:
> I don't know, but I suggest you read this email thread from April to get
> an idea of how performance items are handled:
> 
>   
> https://www.postgresql.org/message-id/flat/20170425013144.GA7513%40momjian.us#20170425013144.ga7...@momjian.us

Oh, sorry, I missed your explanation :

https://www.postgresql.org/message-id/20170425033742.GG7513%40momjian.us
> I remember seeing those and those are normally details I do not put in
> the release notes as there isn't a clear user experience change except
> "Postgres is faster".  Yeah, a bummer, and I can change my filter, but
> it would require discussion.


>From an user experience, IMHO, it is a huge improvement. I thought it could be
mentioned in "General Performance section". But I do not know your habits.

If it was not mentioned in previous releases, I understand we do not mention in
v10's releases notes.

Regards,



-- 
Adrien NAYRAT




signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] PG 10 release notes

2017-09-09 Thread Adrien Nayrat 
On 07/13/2017 04:36 PM, Adrien Nayrat wrote:
> Hello hackers,
> 
> From: Peter Geoghegan <p...@bowt.ie>
>> Date: Wed, 5 Jul 2017 15:19:57 -0700
>> Subject: Re: [BUGS] BUG #14722: Segfault in tuplesort_heap_siftup, 32 bit 
>> overflow
>> On pgsql-b...@postgresql.org
> 
> On 07/06/2017 12:19 AM, Peter Geoghegan wrote:
>> In Postgres 10, tuplesort external sort run merging became much faster
>> following commit 24598337c8d. It might be noticeable if such a machine
>> were using Postgres 10 [...]
> 
> Should-we mention this improvement in release notes?
> 
> Regards,
> 

Hello,

After seeing theses slides (especially 52) :
https://speakerdeck.com/peterg/sort-hash-pgconfus-2017

I noticed several commits which improves performance of hash tables. Commit's
messages mentions performance improvements for bitmap scans, hash aggregation
and (according to Peter Geoghegan's conference) hash join :



commit b30d3ea824c5ccba43d3e942704f20686e7dbab8
Author: Andres Freund <and...@anarazel.de>
Date:   Fri Oct 14 16:05:30 2016 -0700

Add a macro templatized hashtable.
[...]
In queries where these use up a large fraction of the time, this
has been measured to lead to performance improvements of over 100%.



commit 75ae538bc3168bf44475240d4e0487ee2f3bb376
Author: Andres Freund <and...@anarazel.de>
Date:   Fri Oct 14 16:05:30 2016 -0700

Use more efficient hashtable for tidbitmap.c to speed up bitmap scans.
[...]
For bitmap scan heavy queries speedups of over 100% have been measured.



commit 5dfc198146b49ce7ecc8a1fc9d5e171fb75f6ba5
Author: Andres Freund <and...@anarazel.de>
Date:   Fri Oct 14 17:22:51 2016 -0700

Use more efficient hashtable for execGrouping.c to speed up hash 
aggregation.
[...]
Improvements of over 120% have been   measured.



Should we mention it ?

Regards,

-- 
Adrien NAYRAT

http://dalibo.com - http://dalibo.org



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] ALTER INDEX .. SET STATISTICS ... behaviour

2017-09-04 Thread Adrien Nayrat 
On 09/04/2017 06:16 PM, Alexander Korotkov wrote:
> Looks good for me.  I've integrated those changes in the patch.
> New revision is attached.

Thanks, I changed status to "ready for commiter".

-- 
Adrien NAYRAT

http://dalibo.com - http://dalibo.org



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] ALTER INDEX .. SET STATISTICS ... behaviour

2017-09-04 Thread Adrien Nayrat 
On 08/30/2017 02:26 PM, Alexander Korotkov wrote:
> Patch rebased to current master is attached.

Hello,

I reviewed this patch. It works as expected but I have few remarks :

  * There is a warning during compilation :

gram.y: In function ‘base_yyparse’:
gram.y:2090:6: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]
  AlterTableCmd *n = makeNode(AlterTableCmd);
  ^

If I understand we should declare AlterTableCmd *n [...] before "if"?

  * I propose to add "Stats target" information in psql verbose output command
\d+ (patch attached with test)

\d+ tmp_idx
 Index "public.tmp_idx"
 Column |   Type   | Definition | Storage | Stats target
+--++-+--
 a  | integer  | a  | plain   |
 expr   | double precision | (d + e)| plain   | 1000
 b  | cstring  | b  | plain   |
btree, for table "public.tmp"


  * psql completion is missing (added to previous patch)


Regards,

-- 
Adrien NAYRAT

http://dalibo.com - http://dalibo.org
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index f6049cc9e5..6fb9bdd063 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1742,6 +1742,7 @@ describeOneTableDetails(const char *schemaname,
 	{
 		headers[cols++] = gettext_noop("Storage");
 		if (tableinfo.relkind == RELKIND_RELATION ||
+			tableinfo.relkind == RELKIND_INDEX ||
 			tableinfo.relkind == RELKIND_MATVIEW ||
 			tableinfo.relkind == RELKIND_FOREIGN_TABLE ||
 			tableinfo.relkind == RELKIND_PARTITIONED_TABLE)
@@ -1841,6 +1842,7 @@ describeOneTableDetails(const char *schemaname,
 
 			/* Statistics target, if the relkind supports this feature */
 			if (tableinfo.relkind == RELKIND_RELATION ||
+tableinfo.relkind == RELKIND_INDEX ||
 tableinfo.relkind == RELKIND_MATVIEW ||
 tableinfo.relkind == RELKIND_FOREIGN_TABLE ||
 tableinfo.relkind == RELKIND_PARTITIONED_TABLE)
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 1583cfa998..baa739a8c0 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1644,7 +1644,10 @@ psql_completion(const char *text, int start, int end)
    "UNION SELECT 'ALL IN TABLESPACE'");
 	/* ALTER INDEX  */
 	else if (Matches3("ALTER", "INDEX", MatchAny))
-		COMPLETE_WITH_LIST4("OWNER TO", "RENAME TO", "SET", "RESET");
+		COMPLETE_WITH_LIST5("ALTER COLUMN", "OWNER TO", "RENAME TO", "SET", "RESET");
+	/* ALTER INDEX  ALTER COLUMN  */
+	else if (Matches6("ALTER", "INDEX", MatchAny, "ALTER", "COLUMN", MatchAny))
+		COMPLETE_WITH_CONST("SET STATISTICS");
 	/* ALTER INDEX  SET */
 	else if (Matches4("ALTER", "INDEX", MatchAny, "SET"))
 		COMPLETE_WITH_LIST2("(", "TABLESPACE");
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 64160a8b4d..0f36423163 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -103,6 +103,15 @@ ALTER INDEX tmp_idx ALTER COLUMN 1 SET STATISTICS 1000;
 ERROR:  cannot alter statistics on non-expression column "a" of index "tmp_idx"
 HINT:  Alter statistics on table column instead.
 ALTER INDEX tmp_idx ALTER COLUMN 2 SET STATISTICS 1000;
+\d+ tmp_idx
+ Index "public.tmp_idx"
+ Column |   Type   | Definition | Storage | Stats target 
++--++-+--
+ a  | integer  | a  | plain   | 
+ expr   | double precision | (d + e)| plain   | 1000
+ b  | cstring  | b  | plain   | 
+btree, for table "public.tmp"
+
 ALTER INDEX tmp_idx ALTER COLUMN 3 SET STATISTICS 1000;
 ERROR:  cannot alter statistics on non-expression column "b" of index "tmp_idx"
 HINT:  Alter statistics on table column instead.
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 064adb4640..8450f2463e 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2324,10 +2324,10 @@ DROP TABLE array_gin_test;
 CREATE INDEX gin_relopts_test ON array_index_op_test USING gin (i)
   WITH (FASTUPDATE=on, GIN_PENDING_LIST_LIMIT=128);
 \d+ gin_relopts_test
- Index "public.gin_relopts_test"
- Column |  Type   | Definition | Storage 
-+-++-
- i  | integer | i  | plain
+Index "public.gin_relopts_test"
+ Column |  Type   | Definition | Storage | Stats target 
++-++-+--
+

Re: [HACKERS] auto_explain : log queries with wrong estimation

2017-08-28 Thread Adrien Nayrat 
On 08/24/2017 03:08 PM, Maksim Milyutin wrote:
[...]
> 
> AFAICS you want to introduce two additional per-node variables:
>  - auto_explain_log_estimate_ratio that denotes minimum ratio (>= 1) between
> real value and planned one. I would add 'min' prefix before 'ratio'.
>  - auto_explain_log_estimate_min_rows - minimum absolute difference between
> those two values. IMHO this name is somewhat poor, the suffix 'min_diff_rows'
> looks better.
> If real expressions (ratio and diff) exceed these threshold values both, you 
> log
> this situation. I'm right?

Yes, you're totaly right! I wonder if "ratio" is fine or if I should use 
"factor"?

[...]
> 
> Instrumentation is initialized only with analyze (log_analyze is true)[1]

Good, I didn't notice instrumentation can be enabled in auto_explain's hook. I
added these lines and it works :

if (auto_explain_log_estimate_ratio || auto_explain_log_estimate_min_rows)
{
 queryDesc->instrument_options |= INSTRUMENT_ROWS;
}

But I need to undestand how instrumentation works.

Thanks for your answer. I will continue my work, actually my patch is not
functionnal.


-- 
Adrien NAYRAT




signature.asc
Description: OpenPGP digital signature

[HACKERS] auto_explain : log queries with wrong estimation

2017-08-24 Thread Adrien Nayrat 
Hi hackers,


I try to made a patch to auto_explain in order to log queries with wrong 
estimation.

I compare planned row id : queryDesc->planstate->plan->plan_rows

Vs ntuples : queryDesc->planstate->instrument->ntuples;

If I understand, instrumentation is used only with explain. So my patch works
only with explain (and segfault without).


Is there a simple way to get ntuples?

Attached a naive patch.

Thanks :)

-- 
Adrien NAYRAT

http://dalibo.com - http://dalibo.org
diff --git a/contrib/auto_explain/auto_explain.c 
b/contrib/auto_explain/auto_explain.c
index edcb91542a..165fe8e4ae 100644
--- a/contrib/auto_explain/auto_explain.c
+++ b/contrib/auto_explain/auto_explain.c
@@ -30,6 +30,8 @@ static bool auto_explain_log_timing = true;
 static int auto_explain_log_format = EXPLAIN_FORMAT_TEXT;
 static bool auto_explain_log_nested_statements = false;
 static double auto_explain_sample_rate = 1;
+static int auto_explain_log_estimate_ratio = -1;
+static int auto_explain_log_estimate_min_rows = -1;
 
 static const struct config_enum_entry format_options[] = {
{"text", EXPLAIN_FORMAT_TEXT, false},
@@ -52,7 +54,9 @@ static ExecutorEnd_hook_type prev_ExecutorEnd = NULL;
 static bool current_query_sampled = true;
 
 #define auto_explain_enabled() \
-   (auto_explain_log_min_duration >= 0 && \
+   (auto_explain_log_min_duration >= 0 || \
+auto_explain_log_estimate_ratio >= 0 || \
+auto_explain_log_estimate_min_rows >= 0 && \
 (nesting_level == 0 || auto_explain_log_nested_statements))
 
 void   _PG_init(void);
@@ -176,6 +180,31 @@ _PG_init(void)
 NULL,
 NULL);
 
+   DefineCustomIntVariable("auto_explain.estimate_ratio",
+"Planned / returned 
row ratio.",
+NULL,
+
_explain_log_estimate_ratio,
+-1,
+-1, INT_MAX / 1000,
+PGC_SUSET,
+0,
+NULL,
+NULL,
+NULL);
+
+   DefineCustomIntVariable("auto_explain.estimate_min_rows",
+"Planned / returned 
min rows.",
+NULL,
+
_explain_log_estimate_min_rows,
+-1,
+-1, INT_MAX / 1000,
+PGC_SUSET,
+0,
+NULL,
+NULL,
+NULL);
+
+
EmitWarningsOnPlaceholders("auto_explain");
 
/* Install hooks. */
@@ -309,6 +338,30 @@ explain_ExecutorEnd(QueryDesc *queryDesc)
if (queryDesc->totaltime && auto_explain_enabled() && 
current_query_sampled)
{
double  msec;
+   int estimate_ratio = -1;
+   int estimate_rows = -1;
+   int plan_rows = 
queryDesc->planstate->plan->plan_rows;
+   int ntuples = 
queryDesc->planstate->instrument->ntuples;
+
+   if (plan_rows == 0 && ntuples == 0)
+   {
+   estimate_ratio = 1;
+   }
+   else if ( ntuples > plan_rows && plan_rows > 0)
+   {
+   estimate_ratio = ntuples / plan_rows;
+   }
+   else if ( ntuples > 0)
+   {
+   estimate_ratio = plan_rows / ntuples;
+   }
+   else
+   {
+   estimate_ratio = INT_MAX;
+   }
+
+   estimate_rows = abs(ntuples - plan_rows);
+
 
/*
 * Make sure stats accumulation is done.  (Note: it's okay if 
several
@@ -318,7 +371,10 @@ explain_ExecutorEnd(QueryDesc *queryDesc)
 
/* Log plan if duration is exceeded. */
msec = queryDesc->totaltime->total * 1000.0;
-   if (msec >= auto_explain_

Re: [HACKERS] PATCH: multivariate histograms and MCV lists

2017-08-17 Thread Adrien Nayrat 
On 08/14/2017 12:48 AM, Tomas Vondra wrote:
> Hi all,
> 
> For PostgreSQL 10 we managed to get the basic CREATE STATISTICS bits in
> (grammar, infrastructure, and two simple types of statistics). See:
> 
> https://commitfest.postgresql.org/13/852/
> 
> This patch presents a rebased version of the remaining parts, adding more
> complex statistic types (MCV lists and histograms), and hopefully some
> additional improvements.
> 
> The code was rebased on top of current master, and I've made various
> improvements to match how the committed parts were reworked. So the basic idea
> and shape remains the same, the tweaks are mostly small.
> 
> 
> regards
> 
> 
> 
> 

Hello,

There is no check of "statistics type/kind" in pg_stats_ext_mcvlist_items and
pg_histogram_buckets.

select stxname,stxkind from pg_statistic_ext ;
  stxname  | stxkind
---+-
 stts3 | {h}
 stts2 | {m}

So you can call :

SELECT * FROM pg_mcv_list_items((SELECT oid FROM pg_statistic_ext WHERE stxname
= 'stts3'));

SELECT * FROM pg_histogram_buckets((SELECT oid FROM pg_statistic_ext WHERE
stxname = 'stts2'), 0);

Both crashes.

Unfotunately, I don't have the knowledge to produce a patch :/

Small fix in documentation, patch attached.


Thanks!

-- 
Adrien NAYRAT

http://dalibo.com - http://dalibo.org
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 3a86577b0a..a4ab48cc81 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -6445,7 +6445,9 @@ SCRAM-SHA-256$iteration count:salt<
 An array containing codes for the enabled statistic types;
 valid values are:
 d for n-distinct statistics,
-f for functional dependency statistics
+f for functional dependency statistics,
+m for mcv statistics,
+h for histogram statistics
   
  
 
diff --git a/doc/src/sgml/planstats.sgml b/doc/src/sgml/planstats.sgml
index 8857fc7542..9faa7ee393 100644
--- a/doc/src/sgml/planstats.sgml
+++ b/doc/src/sgml/planstats.sgml
@@ -653,7 +653,7 @@ Statistics objects:
 pg_mcv_list_items set-returning function.
 
 
-SELECT * FROM pg_mcv_list_items((SELECT oid FROM pg_statistic_ext WHERE staname = 'stts2'));
+SELECT * FROM pg_mcv_list_items((SELECT oid FROM pg_statistic_ext WHERE stxname = 'stts2'));
  index | values  | nulls | frequency
 ---+-+---+---
  0 | {0,0}   | {f,f} |  0.01
@@ -783,7 +783,7 @@ EXPLAIN ANALYZE SELECT * FROM t WHERE a = 1 AND b = 1;
 using a function called pg_histogram_buckets.
 
 
-test=# SELECT * FROM pg_histogram_buckets((SELECT oid FROM pg_statistic_ext WHERE staname = 'stts3'), 0);
+test=# SELECT * FROM pg_histogram_buckets((SELECT oid FROM pg_statistic_ext WHERE stxname = 'stts3'), 0);
  index | minvals | maxvals | nullsonly | mininclusive | maxinclusive | frequency | density  | bucket_volume 
 ---+-+-+---+--+--+---+--+---
  0 | {0,0}   | {3,1}   | {f,f} | {t,t}| {f,f}|  0.01 | 1.68 |  0.005952


signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] [PATCH v3] pg_progress() SQL function to monitor progression of long running SQL queries/utilities

2017-08-02 Thread Adrien Nayrat 
On 08/01/2017 06:35 PM, Remi Colinet wrote:
> Does anyone use these formats (XML, JSON, YAML) for EXPLAIN output?

Yes : http://tatiyants.com/pev/#/plans :)


-- 
Adrien NAYRAT

http://dalibo.com - http://dalibo.org



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] PG 10 release notes

2017-07-14 Thread Adrien Nayrat 
On 07/13/2017 04:36 PM, Adrien Nayrat wrote:
> Hello hackers,
> 
> From: Peter Geoghegan <p...@bowt.ie>
>> Date: Wed, 5 Jul 2017 15:19:57 -0700
>> Subject: Re: [BUGS] BUG #14722: Segfault in tuplesort_heap_siftup, 32 bit 
>> overflow
>> On pgsql-b...@postgresql.org
> 
> On 07/06/2017 12:19 AM, Peter Geoghegan wrote:
>> In Postgres 10, tuplesort external sort run merging became much faster
>> following commit 24598337c8d. It might be noticeable if such a machine
>> were using Postgres 10 [...]
> 
> Should-we mention this improvement in release notes?
> 
> Regards,
> 

Patch attached.

Magic sorting is not my cup of tea, so I only put "Improve external sort". Maybe
we should add an explanation.



Regards,

-- 
Adrien NAYRAT

http://dalibo.com - http://dalibo.org
diff --git a/doc/src/sgml/release-10.sgml b/doc/src/sgml/release-10.sgml
index debaa80..e654c17 100644
--- a/doc/src/sgml/release-10.sgml
+++ b/doc/src/sgml/release-10.sgml
@@ -856,6 +856,16 @@
 
   
 
+   
+Improve external sort (Peter Geoghegan)
+   
+
+  
+
+  
+



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] PG 10 release notes

2017-07-13 Thread Adrien Nayrat 
Hello hackers,

From: Peter Geoghegan <p...@bowt.ie>
> Date: Wed, 5 Jul 2017 15:19:57 -0700
> Subject: Re: [BUGS] BUG #14722: Segfault in tuplesort_heap_siftup, 32 bit 
> overflow
> On pgsql-b...@postgresql.org

On 07/06/2017 12:19 AM, Peter Geoghegan wrote:
> In Postgres 10, tuplesort external sort run merging became much faster
> following commit 24598337c8d. It might be noticeable if such a machine
> were using Postgres 10 [...]

Should-we mention this improvement in release notes?

Regards,

-- 
Adrien NAYRAT

http://dalibo.com - http://dalibo.org



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] Dumping database creation options and ACLs

2017-07-10 Thread Adrien Nayrat 
On 07/03/2017 05:16 PM, Rafael Martinez wrote:
> We have a discussion about this some time ago and we created a wiki page
> where we tried to write down some ideas/proposals and links to threads
> discussing the subject:
> 
> https://wiki.postgresql.org/wiki/Pg_dump_improvements

Thanks for this link! I'll look at this.


On 07/03/2017 04:58 PM, Robert Haas wrote:
> Note that some progress has been made on the CURRENT_DATABASE thing:
>
>
https://www.postgresql.org/message-id/caf3+xm+xsswcwqzmp1cjj12gpz8dxhcm9_ft1y-0fvzxi9p...@mail.gmail.com
>
> I tend to favor that approach myself, although one point in favor of
> your suggestion is that adding another flag to pg_dumpall is a heck of
> a lot less work to get to some kind of solution to this issue.

Thanks, I'll look. Even if my approach is simple, the question is "Do we want
another flag in pg_dumpall? Is it the role of pg_dumpall?".


Regards,

-- 
Adrien NAYRAT

http://dalibo.com - http://dalibo.org



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] Dumping database creation options and ACLs

2017-06-29 Thread Adrien Nayrat 
On 12/08/2014 04:21 PM, Ronan Dunklau wrote:
> Hello.
> 
> As of now, the only way to restore database options and ACLs is to use 
> pg_dumpall without the globals options. The often recommended pg_dumpall -g + 
> individual dumps of the target databases doesn't restore those.
> 
> Since pg_dump/pg_restore offer the ability to create the database, it should 
> do 
> so with the correct owner, options and database ACLs. 
> 
> There was some discussion about those issues a while ago (see 
> http://www.postgresql.org/message-id/11646.1272814...@sss.pgh.pa.us for 
> example). As I understand it, the best way to handle that would be to push 
> these modifications in pg_dump, but it is unclear how it should be done with 
> regards to restoring to a different database.
> 
> In the meantime, it would be great to add an option to pg_dumpall allowing to 
> dump this information. We could add the db creation in the output of 
> pg_dumpall -g,  and add a specific --createdb-only option (similar to --roles-
> only and --tablespaces-only).
> 
> Would such a patch be welcome ?
> 
> 
> 

Hello,


As reported by Ronan there's no other option than using pg_dumpall to restore
database options and ACLs.

So, we use this trick to stop pg_dumpall before \connect and then use 
pg_restore:

pg_dumpall -s | sed -rn '/^\\connect/{q}; p' > database+grants.sql


Of course, it is not graceful as we just need results of pg_dumpall -g and what
the dumpCreateDB() function outputs.

What do you think about adding an option like --createdb-only (as suggested by
Ronan) for this?  I'm not fully satisfied with this name though, I'll be happy
if you have a better suggestion.

Attached a naive patch.

-- 
Adrien NAYRAT

http://dalibo.com - http://dalibo.org
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index b14bb8e..35fa22d 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -68,6 +68,7 @@ static bool dosync = true;
 
 static int	binary_upgrade = 0;
 static int	column_inserts = 0;
+static int	createdb_only = 0;
 static int	disable_dollar_quoting = 0;
 static int	disable_triggers = 0;
 static int	if_exists = 0;
@@ -121,6 +122,7 @@ main(int argc, char *argv[])
 		{"attribute-inserts", no_argument, _inserts, 1},
 		{"binary-upgrade", no_argument, _upgrade, 1},
 		{"column-inserts", no_argument, _inserts, 1},
+		{"createdb-only", no_argument, _only, 1},
 		{"disable-dollar-quoting", no_argument, _dollar_quoting, 1},
 		{"disable-triggers", no_argument, _triggers, 1},
 		{"if-exists", no_argument, _exists, 1},
@@ -504,13 +506,13 @@ main(int argc, char *argv[])
 		 */
 		if (output_clean)
 		{
-			if (!globals_only && !roles_only && !tablespaces_only)
+			if (!globals_only && !roles_only && !tablespaces_only && !createdb_only)
 dropDBs(conn);
 
-			if (!roles_only && !no_tablespaces)
+			if (!roles_only && !no_tablespaces && !createdb_only)
 dropTablespaces(conn);
 
-			if (!tablespaces_only)
+			if (!tablespaces_only && !createdb_only)
 dropRoles(conn);
 		}
 
@@ -518,7 +520,7 @@ main(int argc, char *argv[])
 		 * Now create objects as requested.  Be careful that option logic here
 		 * is the same as for drops above.
 		 */
-		if (!tablespaces_only)
+		if (!tablespaces_only && !createdb_only)
 		{
 			/* Dump roles (users) */
 			dumpRoles(conn);
@@ -531,7 +533,7 @@ main(int argc, char *argv[])
 		}
 
 		/* Dump tablespaces */
-		if (!roles_only && !no_tablespaces)
+		if (!roles_only && !no_tablespaces && !createdb_only)
 			dumpTablespaces(conn);
 
 		/* Dump CREATE DATABASE commands */
@@ -539,14 +541,14 @@ main(int argc, char *argv[])
 			dumpCreateDB(conn);
 
 		/* Dump role/database settings */
-		if (!tablespaces_only && !roles_only)
+		if (!tablespaces_only && !roles_only && !createdb_only)
 		{
 			if (server_version >= 9)
 dumpDbRoleConfig(conn);
 		}
 	}
 
-	if (!globals_only && !roles_only && !tablespaces_only)
+	if (!globals_only && !roles_only && !tablespaces_only && !createdb_only)
 		dumpDatabases(conn);
 
 	PQfinish(conn);
@@ -594,6 +596,7 @@ help(void)
 	printf(_("  -x, --no-privileges  do not dump privileges (grant/revoke)\n"));
 	printf(_("  --binary-upgrade for use by upgrade utilities only\n"));
 	printf(_("  --column-inserts dump data as INSERT commands with column names\n"));
+	printf(_("  --createdb-only  CREATE and ACL databases commands\n"));
 	printf(_("  --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
 	printf(_("  --disable-triggers   disable triggers during data-only restore\n"));
 	printf(_("  --if-exists  use IF EXISTS when dropping objects\n"));


signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] On How To Shorten the Steep Learning Curve Towards PG Hacking...

2017-03-28 Thread Adrien Nayrat 
On 03/27/2017 02:00 PM, Kang Yuzhe wrote:
> 1. Prepare Hands-on with PG internals
> 
>  For example, a complete Hands-on  with SELECT/INSERT SQL Standard PG 
> internals.
> The point is the experts can pick one fairly complex feature and walk it from
> Parser to Executor in a hands-on manner explaining step by step every 
> technical
> detail.
Hi,

Bruce Momjian has made several presentations about Postgres Internal :
http://momjian.us/main/presentations/internals.html


Regards
-- 
Adrien NAYRAT




signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] New design for FK-based join selectivity estimation

2016-12-13 Thread Adrien Nayrat 
Hi hackers,

The commit 100340e2dcd05d6505082a8fe343fb2ef2fa5b2a introduce an
estimation error :

create table t3 as select j from generate_series(1,1)
i,generate_series(1,100) j ;
create table t4 as select j from generate_series(1,100) j ;
create unique index ON t4(j);
alter table t3 add constraint fk foreign key (j) references t4(j);
analyze;

9.5.5
explain analyze select * from t3 where j in (select * from t4 where j<10);
 QUERY PLAN


 Hash Semi Join  (cost=2.36..18053.61 rows=9 width=4) (actual
time=0.217..282.325 rows=9 loops=1)
   Hash Cond: (t3.j = t4.j)
   ->  Seq Scan on t3  (cost=0.00..14425.00 rows=100 width=4)
(actual time=0.112..116.063 rows=100 loops=1)
   ->  Hash  (cost=2.25..2.25 rows=9 width=4) (actual time=0.083..0.083
rows=9 loops=1)
 Buckets: 1024  Batches: 1  Memory Usage: 9kB
 ->  Seq Scan on t4  (cost=0.00..2.25 rows=9 width=4) (actual
time=0.019..0.074 rows=9 loops=1)
   Filter: (j < 10)
   Rows Removed by Filter: 91
 Planning time: 0.674 ms
 Execution time: 286.043 ms

On 9.6 HEAD

explain analyze select * from t3 where j in (select * from t4 where j<10);
QUERY PLAN

---
 Hash Semi Join  (cost=2.36..18053.61 rows=100 width=4) (actual
time=0.089..232.327 rows=9 loops=1)
   Hash Cond: (t3.j = t4.j)
   ->  Seq Scan on t3  (cost=0.00..14425.00 rows=100 width=4)
(actual time=0.047..97.926 rows=100 loops=1)
   ->  Hash  (cost=2.25..2.25 rows=9 width=4) (actual time=0.032..0.032
rows=9 loops=1)
 Buckets: 1024  Batches: 1  Memory Usage: 9kB
 ->  Seq Scan on t4  (cost=0.00..2.25 rows=9 width=4) (actual
time=0.008..0.030 rows=9 loops=1)
   Filter: (j < 10)
   Rows Removed by Filter: 91
 Planning time: 0.247 ms
 Execution time: 235.295 ms
(10 rows)


Estimated row is 10x larger since 100340e2d

Regards,

-- 
Adrien NAYRAT

http://dalibo.com - http://dalibo.org



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] LSN as a recovery target

2016-08-23 Thread Adrien Nayrat 
On 08/23/2016 10:39 AM, Petr Jelinek wrote:
> On 23/08/16 09:33, Michael Paquier wrote:
>> On Tue, Aug 23, 2016 at 12:49 AM, Robert Haas <robertmh...@gmail.com>
>> wrote:
>>> On Mon, Aug 22, 2016 at 8:28 AM, Michael Paquier
>>> <michael.paqu...@gmail.com> wrote:
>>>> On Mon, Aug 22, 2016 at 7:12 PM, Adrien Nayrat
>>>> <adrien.nay...@dalibo.com> wrote:
>>>>> As Julien said, there is nothing to notice that error comes from
>>>>> recovery.conf.
>>>>> My fear would be that an user encounters an error like this. Il
>>>>> will be
>>>>> difficult to link to the recovery.conf.
>>>>
>>>> Thinking a bit wider than that, we may want to know such context for
>>>> normal GUC parameters as well, and that's not the case now. Perhaps
>>>> there is actually a reason why that's not done for GUCs, but it seems
>>>> that it would be useful there as well. That would give another reason
>>>> to move all that under the GUC umbrella.
>>>
>>> Maybe so, but that's been tried multiple times without success.  If
>>> you think an error context is useful here, and I bet it is, I'd say
>>> just add it and be done with it.
>>
>> This has finished by being less ugly than I thought, so I implemented
>> it as attached. Patch 0001 introduces recovery_target_lsn, and patch
>> 0002 sets up an error context callback generating things like that on
>> failure:
>> FATAL:  invalid input syntax for type pg_lsn: "popo"
>> CONTEXT:  line 11 of configuration file "recovery.conf", parameter
>> "recovery_target_lsn"

Good! Message is clear now for recovery_target_lsn and recovery_target_time.

Thanks for your work.

>>
> 
> Looks very reasonable to me (both patches). Thanks for doing that.
> 
> I am inclined to mark this as ready for committer.
> 

+1 everything if fine for me.

-- 
Adrien NAYRAT

http://dalibo.com - http://dalibo.org


-- 
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] LSN as a recovery target

2016-08-22 Thread Adrien Nayrat 
On 08/20/2016 04:16 PM, Michael Paquier wrote:
> On Sat, Aug 20, 2016 at 10:44 AM, Petr Jelinek <p...@2ndquadrant.com> wrote:
>> On 20/08/16 02:13, Michael Paquier wrote:
>>> On Fri, Aug 19, 2016 at 10:47 PM, Adrien Nayrat
>>> <adrien.nay...@dalibo.com> wrote:
>>> Using a PG_TRY/CATCH block the way you do to show to user a different
>>> error message while the original one is actually correct does not
>>> sound like a good idea to me. It complicates the code and the original
>>> pattern matches what is already done for timestamps, where in case of
>>> error you'd get that:
>>> FATAL:  invalid input syntax for type timestamp with time zone: "aa"
>>>
>>> I think that a better solution would be to make clearer in the docs
>>> that pg_lsn is used here. First, in recovery.conf.sample, let's use
>>> pg_lsn and not LSN. Then, in the sgml docs, let's add a reference to
>>> the page of pg_lsn as well:
>>> https://www.postgresql.org/docs/devel/static/datatype-pg-lsn.html
>>>
>>> Now we have that:
>>> +   
>>> +This parameter specifies the LSN of the write-ahead log stream up
>>> to
>>> +which recovery will proceed. The precise stopping point is also
>>> +influenced by .
>>> +   
>>> Perhaps we could just add an additional sentence: "This parameter
>>> value is parsed using the system datatype

Re: [HACKERS] LSN as a recovery target

2016-08-19 Thread Adrien Nayrat 
On 06/09/2016 02:33 PM, Michael Paquier wrote:
> On Wed, May 25, 2016 at 1:32 AM, Michael Paquier
>  wrote:
>> On Tue, May 24, 2016 at 9:29 AM, Alvaro Herrera
>>  wrote:
>>> Christoph Berg wrote:
 Re: Michael Paquier 2016-05-24