Re: [HACKERS] pageinspect: Hash index support

2017-01-28 Thread Ashutosh Sharma
Hi,

> Please include tests in your patch.  I have posted a sample test suite
> in
> ,
> which you could use.
>
> Also, as mentioned before, hash_metapage_info result fields spares and
> mapp should be arrays of integer, not a text string.

I have added the test-case in the v9 patch shared upthread and also
corrected datatypes for spares and mapp fields in a metapage. Now
these two fields are being shown as an array of integers rather than
text.

https://www.postgresql.org/message-id/CAE9k0Pke046HKYfuJGcCtP77NyHrun7hBV-v20a0TW4CUg4H%2BA%40mail.gmail.com

--
With Regards,
Ashutosh Sharma
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] pageinspect: Hash index support

2017-01-28 Thread Ashutosh Sharma
> The heap tuple is on page 3407872 at line pointer offset 12584?
> That's an awfully but not implausibly big page number (about 26GB into
> the relation) and an awfully and implausibly large line pointer offset
> (how do we fit 12584 index tuples into an 8192-byte page?).  Also, the
> fact that this example has two index entries with the same hash code
> pointing at the same heap TID seems wrong; wouldn't that result in
> index scans returning duplicates?  I think what's actually happening
> here is that (3407872,12584) is actually the attempt to interpret some
> chunk of memory containing the text representation of a TID as an
> actual TID.  When I print those numbers as hex, I get 0x343128, and
> those are the digits "4" and "1" and an opening parenthesis ")" as
> ASCII, so that might fit this theory.

I too had a similar observations when debugging hash_page_items() and
I think we were trying to represent tid as a text rather than tid
itself which was not correct. Attched patch fixes this bug. Below is
what i observed and it is somewhat similar to your explanation in the
above comment:

(gdb) p PageGetItemId(uargs->page, uargs->offset)
$2 = (ItemIdData *) 0x1d84754

(gdb) p *(ItemIdData *) 0x1d84754
$3 = {lp_off = 1664, lp_flags = 1, lp_len = 16}

(gdb) p *(IndexTuple) (page + 1664)
$4 = {t_tid = {ip_blkid = {bi_hi = 0, bi_lo = 839}, ip_posid = 137},
t_info = 16}

(gdb) p BlockIdGetBlockNumber(&(itup->t_tid.ip_blkid))
$5 = 839

(gdb) p (itup->t_tid.ip_posid)
$6 = 137

(gdb) p CStringGetTextDatum(psprintf("(%u,%u)", 839, 137))
$7 = 30959896

(gdb) p DatumGetCString(30959896)
$8 = 0x1d86918 "4"

(gdb) p (char *)0x1d86918
$23 = 0x1d86918 "4"

>
> The code that tries to extract the hashcode from the item also looks
> suspicious.  I'm not 100% sure whether it's wrong or just
> strangely-written, but it doesn't make much sense to extract the item
> contents, then using sprintf() to turn that into a hex string of
> bytes, then parse the hex string using strtol() to get an integer
> back.  I think what it ought to be doing is getting a pointer to the
> index tuple and then calling _hash_get_indextuple_hashkey.
>

I think there is nothing wrong with the hashcode being shown but i do
agree that it is a bit lengthly method to find a hashcode considering
that we already have a extern function _hash_get_indextuple_hashkey()
that can be used to find the hashcode. I have corrected this in the
attached patch.

> Another minor complaint about hash_page_items is that it doesn't
> pfree(uargs->page).  I'm not sure it's necessary to pfree anything
> here, but if we're going to do it I think we might as well be
> consistent with the btree case.  Anyway it can hardly make sense to
> free the 8-byte structure and not the 8192-byte page to which it's
> pointing.
>

In case of btree, we are copying entire page into uargs->page.


memcpy(uargs->page, BufferGetPage(buffer), BLCKSZ);


But in our case we have a raw_page and uargs->page contains pointer to
the page so no need to pfree uargs->page separately.

> In hash_bimap_info(), we go to the trouble of creating a raw page but
> never do anything with it.  I guess the idea here is just to error out
> if the supplied page number is not an overflow page, but there are no
> comments explaining that.  Anyway, I'm not sure it's a very good idea,
> because it means that if you try to write a query to interrogate the
> status of all the bitmap pages, it's going to read all of the overflow
> pages to which they point, which makes the operation a LOT more
> expensive.  I think it would be better to leave this part out; if the
> user wants to know which pages are actually overflow pages, they can
> use hash_page_type() to figure it out.  Let's make it the job of this
> function just to check the available/free status of the page in the
> bitmap, without reading the bitmap itself.

Point taken. I am now checking the status of an overflow page without
reading bitmap page.

>
> In doing that, I think it should either return (bitmapblkno,
> bitmapbit, bit) or just bit.  Returning bitmapblkno but not bitmapbit
> seems strange.  Also, I think it should return bit as a bool, not
> int4.
>

The new bitmap_info() now returns bitmapblkno, bitmapbit and bitstatus as bool.

> Another general note is that, in general, if you use the
> BlahGetDatum() function to construct a datum, the SQL type should be
> match the macro you picked - e.g. if the SQL type is int4, the macro
> used should be Int32GetDatum(), not UInt32GetDatum() or anything else.
> If the SQL type is bool, you should be using BoolGetDatum().

Sorry to mention but I didn't find any SQL datatype equivalent to
uint32 or uint16 in 'C'. So, currently i am using int4 for uint16 and
int8 for uint32.


> Apart from the above, I did a little work to improve the reporting
> when a page of the wrong type is verified.  Updated patch with those
> changes attached.

okay. Thanks. I have done changes on top of this patch.

--
With Regards,
Ashutosh 

Re: [HACKERS] One-shot expanded output in psql using \G

2017-01-28 Thread Cat
On Fri, Jan 27, 2017 at 11:03:05AM -0500, Stephen Frost wrote:
> Well, I did get the impression that you weren't thinking about that,
> which is actually kind of surpirsing to me.  Lots of things work on "the
> current query buffer", which is the last query (successful or not, to be
> clear..):
> 
> \crosstabview
> \e
> \g
> \gexec
> \gset
> \p
> \w
> \watch
> 
> It's not entirely clear to me why the docs sometimes say "current query
> buffer" and somtimes say "current query input buffer".

I would expect the "current query input buffer" to be what is used to
enter the next query. When you hit enter that goes into the "current query
buffer" (as the entered query is now officially complete and thus becomes
the current query) and then "current query input buffer" is cleared for
the next action to be dealt with, be it a query or psql command.

-- 
  "A search of his car uncovered pornography, a homemade sex aid, women's 
  stockings and a Jack Russell terrier."
- 
http://www.dailytelegraph.com.au/news/wacky/indeed/story-e6frev20-118083480


-- 
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_hba_file_settings view patch

2017-01-28 Thread Tom Lane
I wrote:
> I spent awhile hacking on this, and made a lot of things better, but
> I'm still very unhappy about the state of the comments.

I made another pass over this, working on the comments and the docs,
and changing the view name to "pg_hba_file_rules".  I think this version
is committable if people are satisfied with that name.

One loose end is what to do about testing.  I did not much like the
proposed TAP tests.  We could just put "select count(*) > 0 from
pg_hba_file_rules" into the main regression tests, which would provide
some code coverage there, if not very much guarantee that what the view
outputs is sane.

regards, tom lane

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 086fafc..204b8cf 100644
*** a/doc/src/sgml/catalogs.sgml
--- b/doc/src/sgml/catalogs.sgml
***
*** 7809,7814 
--- 7809,7819 
   
  
   
+   pg_hba_file_rules
+   summary of client authentication configuration file contents
+  
+ 
+  
pg_indexes
indexes
   
***
*** 8408,8413 
--- 8413,8526 
  
   
  
+  
+   pg_hba_file_rules
+ 
+   
+pg_hba_file_rules
+   
+ 
+   
+The view pg_hba_file_rules provides a summary of
+the contents of the client authentication configuration
+file, pg_hba.conf.  A row appears in this view for each
+non-empty, non-comment line in the file, with annotations indicating
+whether the rule could be applied successfully.
+   
+ 
+   
+This view can be helpful for checking whether planned changes in the
+authentication configuration file will work, or for diagnosing a previous
+failure.  Note that this view reports on the current contents
+of the file, not on what was last loaded by the server.
+   
+ 
+   
+By default, the pg_hba_file_rules view can be read
+only by superusers.
+   
+ 
+   
+pg_hba_file_rules Columns
+ 
+   
+
+ 
+  Name
+  Type
+  Description
+ 
+
+
+ 
+  line_number
+  integer
+  
+   Line number of this rule in pg_hba.conf
+  
+ 
+ 
+  type
+  text
+  Type of connection
+ 
+ 
+  database
+  text[]
+  List of database name(s) to which this rule applies
+ 
+ 
+  user_name
+  text[]
+  List of user and group name(s) to which this rule applies
+ 
+ 
+  address
+  text
+  
+   Host name or IP address, or one
+   of all, samehost,
+   or samenet, or null for local connections
+  
+ 
+ 
+  netmask
+  text
+  IP address mask, or null if not applicable
+ 
+ 
+  auth_method
+  text
+  Authentication method
+ 
+ 
+  options
+  text[]
+  Options specified for authentication method, if any
+ 
+ 
+  error
+  text
+  
+   If not null, an error message indicating why this
+   line could not be processed
+  
+ 
+
+   
+   
+ 
+   
+Usually, a row reflecting an incorrect entry will have values for only
+the line_number and error fields.
+   
+ 
+   
+See  for more information about
+client authentication configuration.
+   
+  
+ 
   
pg_indexes
  
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index dda5891..231fc40 100644
*** a/doc/src/sgml/client-auth.sgml
--- b/doc/src/sgml/client-auth.sgml
*** hostnossl  database
  
+   
+
+ The preceding statement is not true on Microsoft Windows: there, any
+ changes in the pg_hba.conf file are immediately
+ applied by subsequent new connections.
+
+   
+ 
+   
+The system view
+pg_hba_file_rules
+can be helpful for pre-testing changes to the pg_hba.conf
+file, or for diagnosing problems if loading of the file did not have the
+desired effects.  Rows in the view with
+non-null error fields indicate problems in the
+corresponding lines of the file.
+   
+  

 
  To connect to a particular database, a user must not only pass the
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 4dfedf8..28be27a 100644
*** a/src/backend/catalog/system_views.sql
--- b/src/backend/catalog/system_views.sql
*** CREATE VIEW pg_file_settings AS
*** 459,464 
--- 459,470 
  REVOKE ALL on pg_file_settings FROM PUBLIC;
  REVOKE EXECUTE ON FUNCTION pg_show_all_file_settings() FROM PUBLIC;
  
+ CREATE VIEW pg_hba_file_rules AS
+SELECT * FROM pg_hba_file_rules() AS A;
+ 
+ REVOKE ALL on pg_hba_file_rules FROM PUBLIC;
+ REVOKE EXECUTE ON FUNCTION pg_hba_file_rules() FROM PUBLIC;
+ 
  CREATE VIEW pg_timezone_abbrevs AS
  SELECT * FROM pg_timezone_abbrevs();
  
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index bbe0a88..7a0f1ce 100644
*** a/src/backend/libpq/hba.c
--- b/src/backend/libpq/hba.c
***
*** 25,39 
--- 25,44 
  #include 
  

Re: [HACKERS] proposal: EXPLAIN ANALYZE formatting

2017-01-28 Thread Tom Lane
Peter Eisentraut  writes:
> What I liked about it is that it makes it a bit easier to compare the
> estimate and actual for each node.

Hmm, you'd have to make some effort to line up those fields, if you
wanted that to be a thing.

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] proposal: EXPLAIN ANALYZE formatting

2017-01-28 Thread Peter Eisentraut
On 1/28/17 3:24 PM, Gavin Flower wrote:
> How about have a GUC to control the formatting of how it is displayed?
> 
> Could also include maximum line width (default 'infinite'), and word 
> wrapping rules, ...

You can already configure that in psql.

You can also use the yaml format if you want to use less horizontal
space.  (I have used that in presentations.)

What I liked about it is that it makes it a bit easier to compare the
estimate and actual for each node.

-- 
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] proposal: EXPLAIN ANALYZE formatting

2017-01-28 Thread Gavin Flower

On 29/01/17 05:31, Pavel Stehule wrote:



2017-01-28 17:09 GMT+01:00 Tom Lane >:


Pavel Stehule > writes:
> Now EXPLAIN ANALYZE produce too wide rows for usage in presentations

> What do you think about possibility to implement >>optional<<
alternative
> formatting.
> Now:
>   node name (estimation) (actual)
> Alternative:
>   node name (estimation)
>(actual)

Seems like that would make a difference in only a tiny minority of
situations.  In a deeply nested plan you'll have trouble no matter
what, and it's not uncommon that the node name line isn't the widest
thing anyway.


It is related to presentation where you have to use large type - and 
where usually don't present complex nested plans, or you present only 
fragments.


A output of EXPLAIN is usually ok - EXPLAIN ANALYZE does a overflow

This feature is in nice to have category - probably interesting for 
lectures or presenters only - can helps and doesn't need lot of work. 
So I am ask for community opinion.


The result should not be exactly how I proposed - any form what is 
more friendly for tiny monitor (projectors) is welcome


Regards

Pavel


regards, tom lane



How about have a GUC to control the formatting of how it is displayed?

Could also include maximum line width (default 'infinite'), and word 
wrapping rules, ...



Cheers,
Gavin



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Add support to COMMENT ON CURRENT DATABASE

2017-01-28 Thread Fabrízio de Royes Mello
On Sat, Jan 28, 2017 at 4:26 PM, Fabrízio de Royes Mello <
fabriziome...@gmail.com> wrote:
>
>
>
> On Fri, Jan 27, 2017 at 8:53 PM, Peter Eisentraut <
peter.eisentr...@2ndquadrant.com> wrote:
> >
> > On 1/26/17 1:20 PM, Fabrízio de Royes Mello wrote:
> > > Ok, but doing in that way the syntax would be:
> > >
> > > COMMENT ON DATABASE CURRENT_DATABASE IS 'comment';
> >
> > Yes, that's right.  We also have ALTER USER CURRENT_USER already.
> >
>
> Ok, but if we make CURRENT_DATABASE reserved wouldn't it lead us to a
conflict with current_database() function?
>

I did what you suggested making CURRENT_DATABASE reserved but I got the
following error during the bootstrap:


The files belonging to this database system will be owned by user
"fabrizio".
This user must also own the server process.

The database cluster will be initialized with locale "en_US.UTF-8".
The default database encoding has accordingly been set to "UTF8".
The default text search configuration will be set to "english".

Data page checksums are disabled.

creating directory /tmp/master5432 ... ok
creating subdirectories ... ok
selecting default max_connections ... 100
selecting default shared_buffers ... 128MB
selecting dynamic shared memory implementation ... posix
creating configuration files ... ok
running bootstrap script ... ok
performing post-bootstrap initialization ... 2017-01-28 16:19:07.994 BRST
[18252] FATAL:  syntax error at or near "current_database" at character 120
2017-01-28 16:19:07.994 BRST [18252] STATEMENT:
/*
 * 5.2
 * INFORMATION_SCHEMA_CATALOG_NAME view
 */

CREATE VIEW information_schema_catalog_name AS
SELECT CAST(current_database() AS sql_identifier) AS catalog_name;

child process exited with exit code 1
initdb: removing data directory "/tmp/master5432"
pg_ctl: directory "/tmp/master5432" does not exist
psql: could not connect to server: No such file or directory
Is the server running locally and accepting
connections on Unix domain socket "/tmp/.s.PGSQL.5432"?
psql: could not connect to server: No such file or directory
Is the server running locally and accepting
connections on Unix domain socket "/tmp/.s.PGSQL.5432"?


If I use CURRENT_CATALOG instead this error doesn't occurs of course...


--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello


Re: [HACKERS] Add support to COMMENT ON CURRENT DATABASE

2017-01-28 Thread Fabrízio de Royes Mello
On Fri, Jan 27, 2017 at 8:53 PM, Peter Eisentraut <
peter.eisentr...@2ndquadrant.com> wrote:
>
> On 1/26/17 1:20 PM, Fabrízio de Royes Mello wrote:
> > Ok, but doing in that way the syntax would be:
> >
> > COMMENT ON DATABASE CURRENT_DATABASE IS 'comment';
>
> Yes, that's right.  We also have ALTER USER CURRENT_USER already.
>

Ok, but if we make CURRENT_DATABASE reserved wouldn't it lead us to a
conflict with current_database() function?

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello


Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands: \quit_if, \quit_unless)

2017-01-28 Thread Corey Huinker
>
>
> I'm wondering what pg would do on "EXISTS(SELECT 1 FROM customer)" if
> there are many employees. EXPLAIN suggests a seq_scan, which seems bad.
> With "(SELECT COUNT(*) FROM pgbench_accounts) <> 0" pg seems to generate an
> index only scan on a large table, so maybe it is a better way to do it. It
> seems strange that there is no better way to do that in a relational tool,
> the relational model being an extension of set theory... and there is no
> easy way (?) to check whether a set is empty.
>

I believe that the scan stops on the first row it finds, because the
EXITS() clause is met. But it's not relevant to the documentation, I simply
wanted to demonstrate some results that couldn't be resolved at parse time,
so that the \if tests were meaningful. If the query example is distracting
from the point of the documentation, we should change it.


>
> """If an EOF is reached on the main file or an
> \include-ed file before all
> \if-\endif are matched, then psql
> will raise an error."""
>
> In sentence above: "before all" -> "before all local"? "then" -> ""?
>

+1

>
> "other options booleans" -> "other booleans of options"? or "options'
> booleans" maybe, but that is for people?
>

+1

>
> "unabigous" -> "unambiguous"
>

+1


>
> I think that the three paragraph explanation about non evaluation could be
> factor into one, maybe something like: """Lines within false branches are
> not evaluated in any way: queries are not sent to the server, non
> conditional commands are not evaluated but bluntly ignored, nested if
> expressions in such branches are also not evaluated but are tallied to
> check for nesting."""
>
> I would suggest to merge elif/else constraints by describing what is
> expected rather than what is not expected. """An \if command may contain
> any number of \elif clauses and may end with one \else clause""".
>

I'll give it another shot, as I forgot to mention the non-evaluation of
expressions in dead branches.



>
>
> ## CODE
>
> In "read_boolean_expression":
>
>  + if (value)
>
> "if (value != NULL)" is usually used, I think.
>
>  + if (value == NULL)
>  +   return false; /* not set -> assume "off" */
>
> This is dead code, because value has been checked to be non NULL a few
> lines above.
>
>  + (pg_strncasecmp(value, "on", (len > 2 ? len : 2)) == 0)
>  + (pg_strncasecmp(value, "off", (len > 2 ? len : 2)) == 0)
>
> Hmmm, not easy to parse. Maybe it deserves a comment?
> "check at least two chars to distinguish on & off"
>
> ",action" -> ", action" (space after commas).
>
> The "result" is not set on errors, but maybe it should be set to false
> anyway and explicitely, instead of relying on some prior initialization?
> Or document that the result is not set on errors.
>

This is code lifted from variable.c's ParseVariableBool().  When the other
patch for "psql hooks" is committed (the one that detects when the string
wasn't a valid boolean), this code will go away and we'll just use
ParseVariableBool() again.


>
> if command:
>
>   if (is active) {
> success = ...
> if (success) {
>   ...
> }
>   }
>   if (success) {
> ...
>   }
>
> The second test on success may not rely on the value set above. That looks
> very strange. ISTM that the state should be pushed whether parsing
> succeeded or not. Moreover, it results in:
>
>   \if ERROR
>  \echo X
>   \else
>  \echo Y
>   \endif
>
> having both X & Y printed and error reported on else and endif. I think
> that an expression error should just put the stuff in ignore state.
>

Not just false, but ignore the whole if-endif? interesting. I hadn't
thought of that. Can do.


>
>
> On "else" when in state ignored, ISTM that it should remain in state
> ignore, not switch to else-false.
>

That's how I know if this is the first "else" I encountered. I could split
the if-state back into a struct of booleans if you think that makes more
sense.



>
>
> Comment about "IFSTATE_FALSE" talks about the state being true, maybe a
> copy-paste error?
>

Yes.


>
> In comments: "... variables the branch" -> "variables if the branch"
>

Yes.


>
> The "if_endifs_balanced" variable is not very useful. Maybe just the test
> and error reporting in the right place:
>
>  if (... && !psqlscan_branch_empty(scan_state))
>psql_error("found EOF before closing \\endif(s)\n");
>

+1
I think I got the idea at some point that psql_error broke out of the
current execution block.


> History saving: I'm wondering whether all read line should be put into
> history, whether executed or not.
>

Good question. I gave it some thought and I decided it shouldn't.  First,
because history is a set of statements that were attempted, and those
statements were not. But perhaps more importantly, because the statement
could have contained an expandable variable, and since that variable would
not be evaluated the SQL stored would be subtly altered from the original
intent, perhaps in ways that might drastically alter the meaning of the
statement. A 

Re: [HACKERS] One-shot expanded output in psql using \G

2017-01-28 Thread Daniel Verite
Christoph Berg wrote:

> A workaround is to submit queries using "\x\g\x", but that's ugly,
> clutters the output with toggle messages, and will forget that "\x
> auto" was set.
> 
> Mysql's CLI client is using \G for this purpose, and adding the very
> same functionality to psql fits nicely into the set of existing
> backslash commands: \g sends the query buffer, \G will do exactly the
> same as \g (including parameters), but forces expanded output just for
> this query.

+1 for the functionality but should we choose to ignore the comparison
to mysql, I'd suggest \gx for the name.


Best regards,
-- 
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite


-- 
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: EXPLAIN ANALYZE formatting

2017-01-28 Thread Pavel Stehule
2017-01-28 17:58 GMT+01:00 Tom Lane :

> Pavel Stehule  writes:
> > 2017-01-28 17:09 GMT+01:00 Tom Lane :
> >> Seems like that would make a difference in only a tiny minority of
> >> situations.  In a deeply nested plan you'll have trouble no matter
> >> what, and it's not uncommon that the node name line isn't the widest
> >> thing anyway.
>
> > It is related to presentation where you have to use large type - and
> where
> > usually don't present complex nested plans, or you present only
> fragments.
>
> Sure, but then you're whacking around the text anyway while you put it
> into your slides.  I doubt anyone would have trouble understanding your
> slides if you break up the lines like that, whether or not it's exactly
> what you'd get out of EXPLAIN itself.
>

I cannot to break lines when I use psql and mirrored screen.

Regards

Pavel


>
> regards, tom lane
>


Re: [HACKERS] proposal: EXPLAIN ANALYZE formatting

2017-01-28 Thread Tom Lane
Pavel Stehule  writes:
> 2017-01-28 17:09 GMT+01:00 Tom Lane :
>> Seems like that would make a difference in only a tiny minority of
>> situations.  In a deeply nested plan you'll have trouble no matter
>> what, and it's not uncommon that the node name line isn't the widest
>> thing anyway.

> It is related to presentation where you have to use large type - and where
> usually don't present complex nested plans, or you present only fragments.

Sure, but then you're whacking around the text anyway while you put it
into your slides.  I doubt anyone would have trouble understanding your
slides if you break up the lines like that, whether or not it's exactly
what you'd get out of EXPLAIN itself.

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] pageinspect: Hash index support

2017-01-28 Thread Ashutosh Sharma
Hi,

Please find my reply inline.

> In hash_bimap_info(), we go to the trouble of creating a raw page but
> never do anything with it.  I guess the idea here is just to error out
> if the supplied page number is not an overflow page, but there are no
> comments explaining that.  Anyway, I'm not sure it's a very good idea,
> because it means that if you try to write a query to interrogate the
> status of all the bitmap pages, it's going to read all of the overflow
> pages to which they point, which makes the operation a LOT more
> expensive.  I think it would be better to leave this part out; if the
> user wants to know which pages are actually overflow pages, they can
> use hash_page_type() to figure it out.

Yes, the intention was to ensure that user only passes overflow page
as an input to this function. I think if we wan't to avoid creating a
raw page then we may need to find some other way to verify if it is an
overflow page or not, may be we can make use of hash_check_page().

Let's make it the job of this
> function just to check the available/free status of the page in the
> bitmap, without reading the bitmap itself.
>

okay, In that case I think we can check the previous block number that
the overflow page is pointing to in order to identify if it is free or
in-use. AFAIU, if an overflow page is free it's prev block number will
be Invalid. This way, we may not have to read bitmap page. Now the
question here is, we also have bucket pages where previous block
number is always Invalid but before checking this we ensure that we
are only dealing with an overflow page.Please let me know if you feel
we do have some better option than this to identify the status of
overflow page without reading bitmap.

> In doing that, I think it should either return (bitmapblkno,
> bitmapbit, bit) or just bit.  Returning bitmapblkno but not bitmapbit
> seems strange.  Also, I think it should return bit as a bool, not
> int4.

It would be good to return bitmap bit number as well along with the
bitmap block number. Also, returning bit as bool would look good. I
will do that.

I am also working on other review comments and will share the updated
patch asap. Thanks.


-- 
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_hba_file_settings view patch

2017-01-28 Thread Tom Lane
I wrote:
> I'm still not very happy about the choice of view name ...

After looking over this thread again, I think that we should go with
pg_file_hba_rules or perhaps pg_hba_file_rules.  I see that options
like that were discussed and rejected earlier, but I feel the arguments
against were based on false premises.  I think we need "file" in the
name because:

1. It makes the analogy to the pg_file_settings view clearer.

2. It emphasizes that what you see in the view is the contents of
the disk files, not necessarily the active rules.

3. It leaves the door open to use "pg_hba_rules" as the name of some
future view that *does* show the active rules, analogously to pg_settings
which does show the active GUC settings.

I realize that there's no very convenient way to implement a true
active-auth-rules view right now, but it's not hard to see how that
could be fixed if we were motivated to do so.  One simple way would
be for the postmaster, any time it had successfully loaded the hba
file, to write out some representation of the parsed data into an
"active auth rules" file.  I doubt anyone would bother if the only
application were an active-rules view, but there's at least one
other reason to do this, which is that we could make the HBA stuff
work the same on Windows as it does elsewhere.  Right now, since
new EXEC_BACKEND backends must read the HBA files for themselves,
Windows does not have the property that pg_hba.conf is read only
at SIGHUP --- break the file with some fat-fingered editing, and
new connections begin to fail instantly.  But if new backends
always read a postmaster-written file, then the behavior would be
the same as it is on Unix.

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] proposal: EXPLAIN ANALYZE formatting

2017-01-28 Thread Pavel Stehule
2017-01-28 17:09 GMT+01:00 Tom Lane :

> Pavel Stehule  writes:
> > Now EXPLAIN ANALYZE produce too wide rows for usage in presentations
>
> > What do you think about possibility to implement >>optional<< alternative
> > formatting.
> > Now:
> >   node name (estimation) (actual)
> > Alternative:
> >   node name (estimation)
> >(actual)
>
> Seems like that would make a difference in only a tiny minority of
> situations.  In a deeply nested plan you'll have trouble no matter
> what, and it's not uncommon that the node name line isn't the widest
> thing anyway.
>

It is related to presentation where you have to use large type - and where
usually don't present complex nested plans, or you present only fragments.

A output of EXPLAIN is usually ok - EXPLAIN ANALYZE does a overflow

This feature is in nice to have category - probably interesting for
lectures or presenters only - can helps and doesn't need lot of work. So I
am ask for community opinion.

The result should not be exactly how I proposed - any form what is more
friendly for tiny monitor (projectors) is welcome

Regards

Pavel


>
> regards, tom lane
>


Re: [HACKERS] Removing link-time cross-module refs in contrib

2017-01-28 Thread Tom Lane
Noah Misch  writes:
> ... adding $(pkglibdir) to rpath is obsolete, now that this ceased to link to
> hstore explicitly.

OK...

> For consistency with longstanding src/pl/plpython practice, $(python_libspec)
> should always have an accompanying $(python_additional_libs).  This matters on
> few configurations.

Good point.

> I propose to clean up both points as attached.  Tested on AIX, which ceases to
> be a special case.

Looks reasonable to the naked eye.  I have not tested it, but presumably
if there is any problem the buildfarm would soon tell us.

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] proposal: EXPLAIN ANALYZE formatting

2017-01-28 Thread Tom Lane
Pavel Stehule  writes:
> Now EXPLAIN ANALYZE produce too wide rows for usage in presentations

> What do you think about possibility to implement >>optional<< alternative
> formatting.
> Now:
>   node name (estimation) (actual)
> Alternative:
>   node name (estimation)
>(actual)

Seems like that would make a difference in only a tiny minority of
situations.  In a deeply nested plan you'll have trouble no matter
what, and it's not uncommon that the node name line isn't the widest
thing anyway.

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] proposal: EXPLAIN ANALYZE formatting

2017-01-28 Thread Pavel Stehule
2017-01-28 16:22 GMT+01:00 Peter Eisentraut <
peter.eisentr...@2ndquadrant.com>:

> On 1/28/17 2:36 AM, Pavel Stehule wrote:
> > Now EXPLAIN ANALYZE produce too wide rows for usage in presentations
> >
> > What do you think about possibility to implement >>optional<<
> > alternative formatting.
> >
> > Now:
> >
> >   node name (estimation) (actual)
> >
> > Alternative:
> >
> >   node name (estimation)
> >(actual)
>
> I think that could be useful, even outside of presentations.
>

There is another variant with less white space

node name
  (estimation)
  (actual)

Regards

Pavel


>
> --
> Peter Eisentraut  http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>


Re: [HACKERS] proposal: EXPLAIN ANALYZE formatting

2017-01-28 Thread Peter Eisentraut
On 1/28/17 2:36 AM, Pavel Stehule wrote:
> Now EXPLAIN ANALYZE produce too wide rows for usage in presentations
> 
> What do you think about possibility to implement >>optional<<
> alternative formatting.
> 
> Now:
> 
>   node name (estimation) (actual)
> 
> Alternative:
> 
>   node name (estimation)
>(actual)

I think that could be useful, even outside of presentations.

-- 
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] Transactions involving multiple postgres foreign servers

2017-01-28 Thread Peter Eisentraut
On 1/26/17 4:49 AM, Masahiko Sawada wrote:
> Sorry, I attached wrong version patch of pg_fdw_xact_resovler. Please
> use attached patch.

So in some other thread we are talking about renaming "xlog", because
nobody knows what the "x" means.  In the spirit of that, let's find
better names for new functions as well.

-- 
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


[HACKERS] Z-curve spatial index

2017-01-28 Thread Boris Muratshin
Hi hackers,

I've implemented subj algorithm as PostgreSQL extension.
It looks like this realization is such as fast as R-Tree but sufficiently
more simple & compact.
Here (https://habrahabr.ru/post/319810/) or
here (
http://www.google.com/translate?hl=en=UTF8=auto=en=https%3A%2F%2Fhabrahabr.ru%2Fpost%2F319810%2F
)
you can find the algirithm description and some benchmarks in comparison
with GiST R-Tree.

The extension sources are uploaded at
https://github.com/bmuratshin/zcurve/tree/raw-2D
under the BSD license.

Feel free to connect with me if you have some questions.

Regards,
Boris


Re: [HACKERS] Microvacuum support for Hash Index

2017-01-28 Thread Amit Kapila
On Fri, Jan 27, 2017 at 5:15 PM, Ashutosh Sharma  wrote:
>>
>> Don't you think we should try to identify the reason of the deadlock
>> error reported by you up thread [1]?  I know that you and Ashutosh are
>> not able to reproduce it, but still I feel some investigation is
>> required to find the reason.  It is quite possible that the test case
>> is such that the deadlock is expected in rare cases, if that is the
>> case then it is okay.  I have not spent enough time on that to comment
>> whether it is a test or code issue.
>>
>>
>> [1]  - 
>> https://www.postgresql.org/message-id/dc6d7247-050f-4014-8c80-a4ee676eb384%40redhat.com
>>
>
> I am finally able to reproduce the issue using the attached script
> file (deadlock_report). Basically, once I was able to reproduce the
> issue with hash index I also thought of checking it with a non unique
> B-Tree index and was able to reproduce it with B-Tree index as well.
> This certainly tells us that there is nothing wrong at the code level
> rather there is something wrong with the test script which is causing
> this deadlock issue. Well, I have ran pgbench with two different
> configurations and my observations are as follows:
>
> 1) With Primary keys i.e. uinque values: I have never encountered
> deadlock issue with this configuration.
>
> 2) With non unique indexes (be it hash or B-Tree): I have seen
> deadlock many a times with this configuration. Basically when we have
> non-unique values associated with a column then there is a high
> probability that multiple records will get updated with a single
> 'UPDATE' statement and when there are huge number of backends trying
> to do so there is high chance of getting deadlock which i assume is
> expected behaviour in database.
>

I agree with your analysis, surely trying to update multiple rows with
same values from different backends can lead to deadlock.


-- 
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