Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Magnus Hagander
On Fri, Nov 4, 2011 at 03:27, Fujii Masao masao.fu...@gmail.com wrote:
 On Thu, Nov 3, 2011 at 3:18 AM, Scott Mead sco...@openscg.com wrote:
 ISTM that we're all for:
    creating a new column: state
    renaming current_query = query
    State will display RUNNING, IDLE, IDLE in transaction, etc...
    query will display the last query that was executed.

 The greater/less-than-sign is still required in the State display?

+1 for removing that. I think the only reason for them to be there in
the first place was to decrease the chance of matching up against a
real query - but that's not going to happen if they are in a separate
field...

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.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] Further plans to refactor xlog.c

2011-11-04 Thread Fujii Masao
On Fri, Nov 4, 2011 at 2:16 PM, Fujii Masao masao.fu...@gmail.com wrote:
 On Fri, Nov 4, 2011 at 3:14 AM, Simon Riggs si...@2ndquadrant.com wrote:
 Next steps in refactoring are bigger steps, but not huge ones.

 I propose this

 * everything to do with XLOG rmgr into a file called xlogrmgr.c
 Thats xlog_redo() and most everything to do with checkpoints

 * everything to do with reading WAL files into a file called xlogread.c
 That will allow us to put pg_xlogdump into core

 * possibly some more stuff into xlogboot.c

 The above actions will reduce xlog.c to about 7000 lines, about 4000
 lines smaller than when I started. That sounds like it could go
 further, but it moves out most of the areas of recent growth by
 focusing on the purpose of that code.

 An obvious split would seem to be move all recovery-side code into its
 own file. That seems quite likely to take a lot of time, break
 something, as well as requiring us to share XLogCtl, all of which
 personally I would rather avoid.

 Fujii's work is likely to remove another few hundred lines as well.

 That seems enough to me OK?

 Additionally what about moving all built-in functions defined in xlog.c
 to xlogfuncs.c?

Oh, you've already posted the patch which does that.
http://archives.postgresql.org/message-id/CA+U5nMK=ybzczkdvj8kojfsz+d9lfmxvw+928nhu4x1hbyh...@mail.gmail.com

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

-- 
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] DeArchiver process

2011-11-04 Thread Simon Riggs
On Fri, Nov 4, 2011 at 2:36 AM, Fujii Masao masao.fu...@gmail.com wrote:

 If we introduce walrestore process, pg_standby seems no longer useful.
 We should get rid of it?

Removing things too quickly can cause problems. There's no harm done
by keeping it a while longer.

I agree it should go, just want to be absolutely clear that its no
longer needed for any use case.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services

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


[HACKERS] Storing original rows before update or delete

2011-11-04 Thread Miroslav Šimulčík
Hi.

I'm working on transactiontime temporal support for postgresql 9.0.4. Each
original table with transactiontime support has associated history table,
where original row is stored before each update or delete operation on it.
Both original and history tables have internal timestamp columns for
storing the period of validity of row versions. History tables are internal
and I want to restrict any DML operation on it, so nobody can change
history of operations made on original table. Problem is, that I don't know
where to implement the mechanism for storing original rows to history
tables. Rewrite rules are not suitable, because they are not working with
inheritance and I can't use triggers, because inserts to history tables are
restricted. Can you point me to place in postgresql backend where can i
implement this and maybe give me some hints about how to do it correctly?

Thank you.

Best regards
Miroslav Simulcik


Re: [HACKERS] Storing original rows before update or delete

2011-11-04 Thread Szymon Guz
On 4 November 2011 10:20, Miroslav Šimulčík simulcik.m...@gmail.com wrote:

 Hi.

 I'm working on transactiontime temporal support for postgresql 9.0.4. Each
 original table with transactiontime support has associated history table,
 where original row is stored before each update or delete operation on it.
 Both original and history tables have internal timestamp columns for
 storing the period of validity of row versions. History tables are internal
 and I want to restrict any DML operation on it, so nobody can change
 history of operations made on original table. Problem is, that I don't know
 where to implement the mechanism for storing original rows to history
 tables. Rewrite rules are not suitable, because they are not working with
 inheritance and I can't use triggers, because inserts to history tables are
 restricted. Can you point me to place in postgresql backend where can i
 implement this and maybe give me some hints about how to do it correctly?

 Thank you.

 Best regards
 Miroslav Simulcik



Hi,
use triggers with security definer so the owner of the triggers and the
history table can insert into it, but normal user cannot - this user only
is able to change data in original table, and triggers will copy the data,
but will be executed using the first user credentials.
http://www.postgresql.org/docs/9.0/interactive/sql-createfunction.html

On the other hand: superuser always can delete data from a table, so you
cannot stop him from doing that.

regards
Szymon


Re: [HACKERS] Further plans to refactor xlog.c

2011-11-04 Thread Simon Riggs
On Fri, Nov 4, 2011 at 5:26 AM, Fujii Masao masao.fu...@gmail.com wrote:

 Additionally what about moving all built-in functions defined in xlog.c
 to xlogfuncs.c?

 Oh, you've already posted the patch which does that.
 http://archives.postgresql.org/message-id/CA+U5nMK=ybzczkdvj8kojfsz+d9lfmxvw+928nhu4x1hbyh...@mail.gmail.com

Committed

-- 
 Simon Riggs   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] Storing original rows before update or delete

2011-11-04 Thread Miroslav Šimulčík
If I restrict any DML operation to internal tables (history tables) in
phase of parse analyze, nobody can do such operation regardless of user
rights. However, triggers can't be used in this case.

2011/11/4 Szymon Guz mabew...@gmail.com



 On 4 November 2011 10:20, Miroslav Šimulčík simulcik.m...@gmail.comwrote:

 Hi.

 I'm working on transactiontime temporal support for postgresql 9.0.4.
 Each original table with transactiontime support has associated history
 table, where original row is stored before each update or delete operation
 on it. Both original and history tables have internal timestamp columns for
 storing the period of validity of row versions. History tables are internal
 and I want to restrict any DML operation on it, so nobody can change
 history of operations made on original table. Problem is, that I don't know
 where to implement the mechanism for storing original rows to history
 tables. Rewrite rules are not suitable, because they are not working with
 inheritance and I can't use triggers, because inserts to history tables are
 restricted. Can you point me to place in postgresql backend where can i
 implement this and maybe give me some hints about how to do it correctly?

 Thank you.

 Best regards
 Miroslav Simulcik



 Hi,
 use triggers with security definer so the owner of the triggers and the
 history table can insert into it, but normal user cannot - this user only
 is able to change data in original table, and triggers will copy the data,
 but will be executed using the first user credentials.
 http://www.postgresql.org/docs/9.0/interactive/sql-createfunction.html

 On the other hand: superuser always can delete data from a table, so you
 cannot stop him from doing that.

 regards
 Szymon




Re: [HACKERS] Term positions in GIN fulltext index

2011-11-04 Thread Yoann Moreau

On 03/11/11 19:19, Florian Pflug wrote:

There's a difference between values of type tsvector, and what GIN indices
on columns or expressions of type tsvector store.


I was wondering what was the point about storing the tsvector in the 
table, I now understand. I then should use the GIN index to rank my 
documents, and work on the stored tsvectors for positions.



As I pointed out above, you'll first need to make sure to store the result of
to_tsvector in a columns. Then, what you need seems to be a functions that
takes a tsvector value and returns the contained lexems as individual rows.

Postgres doesn't seem to contain such a function currently (don't believe that,
though - go and recheck the documentation. I don't know all thousands of 
built-in
functions by heart). But it's easy to add one. You could either use PL/pgSQL
to parse the tsvector's textual representation, or write a C function. If you
go the PL/pgSQL route, regexp_split_to_table() might come in handy.


This seems easier to program than what I was thinking about, I'm going 
to do that. But I'm wondering about size of database with the GIN index 
plus the tsvector column, and performance about parsing the whole 
tsvectors for each document I need positions from (as I need them for a 
very few terms).


Maybe some external fulltext engine managing lexemes and positions would 
be more efficient for my purpose. I'll try some different things and let 
you know the results.


Thanks all for your help
Regards,
Yoann Moreau


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


[HACKERS] a tsearch issue

2011-11-04 Thread Pavel Stehule
Hello

I found a interesting issue when I checked a tsearch prefix searching.

We use a ispell based dictionary

CREATE TEXT SEARCH DICTIONARY cspell
   (template=ispell, dictfile = czech, afffile=czech, stopwords=czech);
CREATE TEXT SEARCH CONFIGURATION cs (copy=english);
ALTER TEXT SEARCH CONFIGURATION cs
   ALTER MAPPING FOR word, asciiword WITH cspell, simple;

Then I created a table

postgres=# create table n(a varchar);
CREATE TABLE
postgres=# insert into n values('Stěhule'),('Chromečka');
INSERT 0 2
postgres=# select * from n;
 a
───
 Stěhule
 Chromečka
(2 rows)

and I tested a prefix searching:

I found a following issue

postgres=# select * from n where to_tsvector('cs', a) @@
to_tsquery('cs','Stě:*') ;
 a
───
(0 rows)

I expected one row. The problem is in transformation of word 'Stě'

postgres=# select * from ts_debug('cs','Stě:*') ;
─[ RECORD 1 ]┬──
alias│ word
description  │ Word, all letters
token│ Stě
dictionaries │ {cspell,simple}
dictionary   │ cspell
lexemes  │ {sto}
─[ RECORD 2 ]┼──
alias│ blank
description  │ Space symbols
token│ :*
dictionaries │ {}
dictionary   │ [null]
lexemes  │ [null]


Ispell disctionary cannot to work well with a first n chars from word.
I don't know what is correct solution of this problem.

Minimally note in prefix search, so this cannot work well with *spell
dictionaries - or description of this issue.

Regards

Pavel Stehue

-- 
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] DeArchiver process

2011-11-04 Thread Dimitri Fontaine
Fujii Masao masao.fu...@gmail.com writes:
 If we introduce walrestore process, pg_standby seems no longer useful.

pg_standby is one possible restore_command, right?  I had understood
that walrestore would be the process that cares for running that
command, not another implementation of it.

That said, I would really like us to provide a default restore command,
so if you had any intend of handling the restoring command in the
walrestore process, by all means, go ahead :)

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

-- 
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] DeArchiver process

2011-11-04 Thread Simon Riggs
On Fri, Nov 4, 2011 at 11:08 AM, Dimitri Fontaine
dimi...@2ndquadrant.fr wrote:
 Fujii Masao masao.fu...@gmail.com writes:
 If we introduce walrestore process, pg_standby seems no longer useful.

 pg_standby is one possible restore_command, right?  I had understood
 that walrestore would be the process that cares for running that
 command, not another implementation of it.

Yes, that was the idea.

 That said, I would really like us to provide a default restore command,
 so if you had any intend of handling the restoring command in the
 walrestore process, by all means, go ahead :)

A different proposal, I think. Not no, just not here and now.

-- 
 Simon Riggs   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] Term positions in GIN fulltext index

2011-11-04 Thread Florian Pflug
On Nov4, 2011, at 11:15 , Yoann Moreau wrote:
 On 03/11/11 19:19, Florian Pflug wrote:
 Postgres doesn't seem to contain such a function currently (don't believe 
 that,
 though - go and recheck the documentation. I don't know all thousands of 
 built-in
 functions by heart). But it's easy to add one. You could either use PL/pgSQL
 to parse the tsvector's textual representation, or write a C function. If you
 go the PL/pgSQL route, regexp_split_to_table() might come in handy.
 
 This seems easier to program than what I was thinking about, I'm going to do 
 that.
 But I'm wondering about size of database with the GIN index plus the tsvector 
 column,
 and performance about parsing the whole tsvectors for each document I need 
 positions
 from (as I need them for a very few terms).

AFAICS, the internal storage layout of tsvector should allow you to extract an
individual lexem's positions quite efficiently (with time complexity log(N) 
where
N is the number of lexems in the tsvector). Doing so will require you to 
implement
your function in C though - any solution that works from a tsvector's textual
representation will obviously have time complexity N.

best regards,
Florian Pflug


-- 
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] psql expanded auto

2011-11-04 Thread Noah Misch
On Tue, Nov 01, 2011 at 06:22:47AM +0200, Peter Eisentraut wrote:
 I wrote:
  I have often found myself wanting that psql automatically switch between
  normal and \x mode depending on the width of the output.  Would others
  find this useful?
  
  Attached is a crude demo patch.  Enable with \pset expanded auto.
 
 Here is a finalized patch for this.  The first hunk of the patch is the
 documentation change, so you can see there how it's supposed to work.
 Let me know what you think.

+1.  I'm anticipating liking this enough to put it in .psqlrc.


Perhaps this message should change to just Target width is 120., since it now
applies to more than just the wrapped format:

[local] test=# \pset columns 120
Target width for wrapped format is 120.

Similarly, psql documentation for \pset columns and the COLUMNS environment
variable should note expanded auto in addition to the wrapped format.


For \pset format wrapped, we only use $COLUMNS when the output is a tty.  I'm
thinking it's best, although not terribly important, to apply the same rule to
this feature.

Thanks,
nm

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


Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Marti Raudsepp
On Wed, Nov 2, 2011 at 20:18, Scott Mead sco...@openscg.com wrote:
    State will display RUNNING, IDLE, IDLE in transaction, etc...

While we're already breaking everything, we could remove the waiting
column and use a state with value 'waiting' instead.

Also, returning these as text seems a little lame. Should there be an
enum type for that?

Or we could return single-character mnemonics like some pg_catalog
tables do, and substitute them in the view.

Regards,
Marti

-- 
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] Support for foreign keys with arrays

2011-11-04 Thread Gabriele Bartolini
This patch adds basic support of arrays in foreign keys, by allowing to 
define a referencing column as an array of elements having the same type 
as the referenced column in the referenced table.
Every NOT NULL element in the referencing array is matched against the 
referenced table.



Example:

CREATE TABLE pt (
  id INTEGER PRIMARY KEY,
  ...
);

CREATE TABLE ft (
  id SERIAL PRIMARY KEY,
  pids INTEGER[] REFERENCES pt,
  ...
);


This patch is for discussion and has been built against HEAD.
It compiles and passes all regressions tests (including specific ones - 
see the src/test/regress/sql/foreign_key.sql file).
Empty arrays, multi-dimensional arrays, duplicate elements and NULL 
values are allowed.


We had to enforce some limitations, due to the lack (yet) of a clear and 
universally accepted behaviour and strategy.
For example, consider the ON DELETE action on the above tables: in case 
of delete of a record in the 'pt' table, should we remove the whole row 
or just the values from the array?

We hope we can start a discussion from here.

Current limitations:

* Only arrays of the same type as the primary key in the referenced 
table are supported

* multi-column foreign keys are not supported (only single column)
* Only RESTRICT and NO ACTION methods for referential integrity 
enforcement are currently supported


TODO:
* Improve check for empty arrays, which might interfere with SSI (see below)
* Verify interaction with serializable transactions

AUTHORS:
* Gabriele Bartolini gabriele.bartol...@2ndquadrant.it
* Marco Nenciarini marco.nenciar...@2ndquadrant.it

Cheers,
Gabriele (and Marco)

--
 Gabriele Bartolini - 2ndQuadrant Italia
 PostgreSQL Training, Services and Support
 gabriele.bartol...@2ndquadrant.it | www.2ndQuadrant.it

*** a/doc/src/sgml/ddl.sgml
--- b/doc/src/sgml/ddl.sgml
***
*** 764,769  CREATE TABLE order_items (
--- 764,796 
  the last table.
 /para
  
+para
+ Another option you have with foreign keys is to use a referencing column
+ which is an array of elements with the same type as the referenced column
+ in the related table. This feature, also known as firsttermforeign key 
arrays/firstterm,
+ is described in the following example:
+ 
+ programlisting
+ CREATE TABLE countries (
+ country_id integer PRIMARY KEY,
+ name text,
+ ...
+ );
+ 
+ CREATE TABLE people (
+ person_id integer PRIMARY KEY,
+ first_name text,
+ last_name text,
+ ...
+ citizenship_ids integer[] REFERENCES countries
+ );
+ /programlisting
+ 
+ The above example lists in an array the citizenships held by
+ a person and enforces referential integrity checks.
+ 
+/para
+ 
 indexterm
  primaryCASCADE/primary
  secondaryforeign key action/secondary
***
*** 852,857  CREATE TABLE order_items (
--- 879,891 
 /para
  
 para
+ When working with foreign key arrays, you are currently limited
+ to literalRESTRICT/literal and literalNO ACTION/literal
+ options, as the default behaviour for the other cases is not
+ clearly and universally determined yet.
+/para
+ 
+para
  More information about updating and deleting data is in xref
  linkend=dml.
 /para
*** a/doc/src/sgml/ref/create_table.sgml
--- b/doc/src/sgml/ref/create_table.sgml
***
*** 576,581  CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] 
TABLE [ IF NOT EXI
--- 576,587 
   /para
  
   para
+   If the referencing column is an array of elements of the same type as
+   the referenced column in the referenced table, the value of each element
+   of the array will be matched against some row of the referenced table.
+  /para
+ 
+  para
A value inserted into the referencing column(s) is matched against the
values of the referenced table and referenced columns using the
given match type.  There are three match types: literalMATCH
***
*** 634,640  CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] 
TABLE [ IF NOT EXI
   para
Delete any rows referencing the deleted row, or update the
value of the referencing column to the new value of the
!   referenced column, respectively.
   /para
  /listitem
 /varlistentry
--- 640,647 
   para
Delete any rows referencing the deleted row, or update the
value of the referencing column to the new value of the
!   referenced column, respectively. Foreign key arrays are not
!   supported by this action (as the behaviour is not easily 
determined).
   /para
  /listitem
 /varlistentry
***
*** 643,649  CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] 
TABLE [ IF NOT EXI
  termliteralSET NULL/literal/term
  listitem
   para
!   Set the referencing column(s) 

Re: [HACKERS] psql expanded auto

2011-11-04 Thread Peter Geoghegan
On 17 December 2010 22:12, Peter Eisentraut pete...@gmx.net wrote:
 I have often found myself wanting that psql automatically switch between
 normal and \x mode depending on the width of the output.  Would others
 find this useful?

+1

Sounds like a very good idea.


-- 
Peter Geoghegan       http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and 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] Show sequences owned by

2011-11-04 Thread Magnus Hagander
The attached patch makes the \d output for psql on a sequence show
which table/column owns the sequence. The table already showed the
dependency the other way through the default value, but going from
sequence back to table was not possible.

Comments/reviews?

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index d5466f8..21e9171 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1609,6 +1609,42 @@ describeOneTableDetails(const char *schemaname,
 			PQclear(result);
 		}
 	}
+	else if (tableinfo.relkind == 'S')
+	{
+		/* Footer information about a sequence */
+		PGresult   *result = NULL;
+
+		/* Get the column that owns this sequence */
+		printfPQExpBuffer(buf, SELECT quote_ident(nspname) || '.' ||
+		  \n   quote_ident(relname) || '.' ||
+		  \n   quote_ident(attname)
+		  \nFROM pg_class
+		  \nINNER JOIN pg_depend ON pg_class.oid=pg_depend.refobjid
+		  \nINNER JOIN pg_namespace ON pg_namespace.oid=pg_class.relnamespace
+		  \nINNER JOIN pg_attribute ON (
+		  \n pg_attribute.attrelid=pg_class.oid AND
+		  \n pg_attribute.attnum=pg_depend.refobjsubid)
+		  \nWHERE classid='pg_class'::regclass
+		  \n AND objid=%s
+		  \n AND deptype='a',
+		  oid);
+
+		result = PSQLexec(buf.data, false);
+		if (!result)
+			goto error_return;
+		else if (PQntuples(result)  1)
+		{
+			PQclear(result);
+			goto error_return;
+		}
+		else if (PQntuples(result) == 1)
+		{
+			printfPQExpBuffer(buf, _(Owned by: %s),
+			  PQgetvalue(result, 0, 0));
+			printTableAddFooter(cont, buf.data);
+		}
+		PQclear(result);
+	}
 	else if (tableinfo.relkind == 'r' || tableinfo.relkind == 'f')
 	{
 		/* Footer information about a table */

-- 
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] DeArchiver process

2011-11-04 Thread Robert Haas
On Fri, Nov 4, 2011 at 5:15 AM, Simon Riggs si...@2ndquadrant.com wrote:
 On Fri, Nov 4, 2011 at 2:36 AM, Fujii Masao masao.fu...@gmail.com wrote:
 If we introduce walrestore process, pg_standby seems no longer useful.
 We should get rid of it?

 Removing things too quickly can cause problems. There's no harm done
 by keeping it a while longer.

 I agree it should go, just want to be absolutely clear that its no
 longer needed for any use case.

I agree that it would be premature to remove pg_standby at this point.
 But how about changing the default value of standby_mode from off
to on in 9.2?  I think most new installations are probably using
that, rather than pg_standby, and changing the default would give
people a gentle push in what now seems to be the preferred direction.

-- 
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] Storing original rows before update or delete

2011-11-04 Thread Robert Haas
2011/11/4 Miroslav Šimulčík simulcik.m...@gmail.com:
 If I restrict any DML operation to internal tables (history tables) in phase
 of parse analyze, nobody can do such operation regardless of user rights.

There is absolutely zero point in trying to prevent the superuser from
doing whatever they want.  The superuser has many, many ways of
bypassing whatever controls you put in place - e.g. loading custom C
code into the backend, whacking around the system catalogs, etc.

 However, triggers can't be used in this case.

Which is another reason not to do it that way.

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


[HACKERS] Show statistics target in \d+

2011-11-04 Thread Magnus Hagander
The attached patch adds a column for statistics target when viewing
tables in psql using \d+.

Comments/reivews?

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index d5466f8..426d934 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1280,6 +1280,7 @@ describeOneTableDetails(const char *schemaname,
 	if (verbose)
 	{
 		appendPQExpBuffer(buf, ,\n  a.attstorage);
+		appendPQExpBuffer(buf, ,\n  a.attstattarget);
 		/*
 		 * In 9.0+, we have column comments for: relations, views, composite
 		 * types, and foreign tables (c.f. CommentObject() in comment.c).
@@ -1374,6 +1375,8 @@ describeOneTableDetails(const char *schemaname,
 	if (verbose)
 	{
 		headers[cols++] = gettext_noop(Storage);
+		if (tableinfo.relkind == 'r')
+			headers[cols++] = gettext_noop(Statistics);
 		/* Column comments, if the relkind supports this feature. */
 		if (tableinfo.relkind == 'r' || tableinfo.relkind == 'v' ||
 			tableinfo.relkind == 'c' || tableinfo.relkind == 'f')
@@ -1473,10 +1476,18 @@ describeOneTableDetails(const char *schemaname,
 		(storage[0] == 'e' ? external :
 		 ???,
 			  false, false);
+
+			/* Statistics target, if the relkind supports this feature */
+			if (tableinfo.relkind == 'r')
+			{
+printTableAddCell(cont, PQgetvalue(res, i, firstvcol + 1),
+  false, false);
+			}
+
 			/* Column comments, if the relkind supports this feature. */
 			if (tableinfo.relkind == 'r' || tableinfo.relkind == 'v' ||
 tableinfo.relkind == 'c' || tableinfo.relkind == 'f')
-printTableAddCell(cont, PQgetvalue(res, i, firstvcol + 1),
+printTableAddCell(cont, PQgetvalue(res, i, firstvcol + 2),
   false, false);
 		}
 	}

-- 
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] IDLE in transaction introspection

2011-11-04 Thread Tom Lane
Marti Raudsepp ma...@juffo.org writes:
 While we're already breaking everything, we could remove the waiting
 column and use a state with value 'waiting' instead.

-1 ... I think it's useful to see the underlying state as well as the
waiting flag.  Also, this would represent breakage of part of the API
that doesn't need to be broken.

 Also, returning these as text seems a little lame. Should there be an
 enum type for that?

Perhaps, but we don't really use enum types in any other system views,
so inventing one here would be out of character.

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] Show sequences owned by

2011-11-04 Thread Robert Haas
On Fri, Nov 4, 2011 at 9:01 AM, Magnus Hagander mag...@hagander.net wrote:
 The attached patch makes the \d output for psql on a sequence show
 which table/column owns the sequence. The table already showed the
 dependency the other way through the default value, but going from
 sequence back to table was not possible.

 Comments/reviews?

Seems reasonable.

-- 
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] heap_page_prune comments

2011-11-04 Thread Robert Haas
On Thu, Nov 3, 2011 at 8:27 PM, Jim Nasby j...@nasby.net wrote:
 On Nov 2, 2011, at 11:27 AM, Robert Haas wrote:
 The following comment - or at least the last sentence thereof -
 appears to be out of date.

        /*
         * XXX Should we update the FSM information of this page ?
         *
         * There are two schools of thought here. We may not want to update 
 FSM
         * information so that the page is not used for unrelated
 UPDATEs/INSERTs
         * and any free space in this page will remain available for further
         * UPDATEs in *this* page, thus improving chances for doing HOT 
 updates.
         *
         * But for a large table and where a page does not receive
 further UPDATEs
         * for a long time, we might waste this space by not updating the FSM
         * information. The relation may get extended and fragmented further.
         *
         * One possibility is to leave fillfactor worth of space in this 
 page
         * and update FSM with the remaining space.
         *
         * In any case, the current FSM implementation doesn't accept
         * one-page-at-a-time updates, so this is all academic for now.
         */

 The simple fix here is just to delete that last sentence, but does
 anyone think we ought to do change the behavior, now that we have the
 option to do so?

 The fillfactor route seems to make the most sense here... it certainly seems 
 to be the least surprising behavior.

Seems a little hackish, though: we'd be reporting an amount of
freespace that we've deliberately set to an incorrect value.  I'm
almost thinking we should report the freespace that's actually
available, on the theory that Bload Is Bad (TM).

Maybe some testing is in order.

-- 
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] IDLE in transaction introspection

2011-11-04 Thread Magnus Hagander
On Fri, Nov 4, 2011 at 14:42, Tom Lane t...@sss.pgh.pa.us wrote:
 Marti Raudsepp ma...@juffo.org writes:
 While we're already breaking everything, we could remove the waiting
 column and use a state with value 'waiting' instead.

 -1 ... I think it's useful to see the underlying state as well as the
 waiting flag.  Also, this would represent breakage of part of the API
 that doesn't need to be broken.

I guess with the changes that showed different thing like fastpath,
that makes sense. But if we just mapped the states that are there
today straight off, is there any case where waiting can be true, when
we're either idle or idle in transaction? I think not..


 Also, returning these as text seems a little lame. Should there be an
 enum type for that?

 Perhaps, but we don't really use enum types in any other system views,
 so inventing one here would be out of character.

Yeha, that seems inconsistent. Using a single character might work -
but it's not particularly user-friendly to do that in the view itself.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.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] Show statistics target in \d+

2011-11-04 Thread Cédric Villemain
2011/11/4 Magnus Hagander mag...@hagander.net:
 The attached patch adds a column for statistics target when viewing
 tables in psql using \d+.

 Comments/reivews?

Interesting, can the ouput be clear on the value being a default or an
explicit stat target ? (not mandatory but I believe I would like to
have it only when the stat target is jnot the default)


 --
  Magnus Hagander
  Me: http://www.hagander.net/
  Work: http://www.redpill-linpro.com/


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





-- 
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation

-- 
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] Show statistics target in \d+

2011-11-04 Thread Magnus Hagander
On Fri, Nov 4, 2011 at 14:53, Cédric Villemain
cedric.villemain.deb...@gmail.com wrote:
 2011/11/4 Magnus Hagander mag...@hagander.net:
 The attached patch adds a column for statistics target when viewing
 tables in psql using \d+.

 Comments/reivews?

 Interesting, can the ouput be clear on the value being a default or an
 explicit stat target ? (not mandatory but I believe I would like to
 have it only when the stat target is jnot the default)

It shows -1 when it's the default.

We could map that to the string default if we want, or just NULL, I
guess. Not sure which is best?

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.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] Show statistics target in \d+

2011-11-04 Thread Robert Haas
On Fri, Nov 4, 2011 at 9:34 AM, Magnus Hagander mag...@hagander.net wrote:
 The attached patch adds a column for statistics target when viewing
 tables in psql using \d+.

 Comments/reivews?

Statistics doesn't seem like a very clear name for the column header.

-- 
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] Show sequences owned by

2011-11-04 Thread Stephen Frost
Magnus,

* Magnus Hagander (mag...@hagander.net) wrote:
 The attached patch makes the \d output for psql on a sequence show
 which table/column owns the sequence. The table already showed the
 dependency the other way through the default value, but going from
 sequence back to table was not possible.

Seems reasonable.

 Comments/reviews?

Not sure if that 'goto error_return;' handles this correctly, but it
would seem like you're missing the possibility that a sequence isn't
owned by any table/column..?  Or that it could be depended upon by more
than one table/column?  Both of those happen and are perfectly valid
situations for a sequence to be in..

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Show statistics target in \d+

2011-11-04 Thread Magnus Hagander
On Fri, Nov 4, 2011 at 15:06, Robert Haas robertmh...@gmail.com wrote:
 On Fri, Nov 4, 2011 at 9:34 AM, Magnus Hagander mag...@hagander.net wrote:
 The attached patch adds a column for statistics target when viewing
 tables in psql using \d+.

 Comments/reivews?

 Statistics doesn't seem like a very clear name for the column header.

Got any ideas for a better one? Statistics Target seemed too long to
me, and Statstarget not necessarily very user friendly?


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.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] IDLE in transaction introspection

2011-11-04 Thread Robert Haas
On Fri, Nov 4, 2011 at 9:48 AM, Magnus Hagander mag...@hagander.net wrote:
 On Fri, Nov 4, 2011 at 14:42, Tom Lane t...@sss.pgh.pa.us wrote:
 Marti Raudsepp ma...@juffo.org writes:
 While we're already breaking everything, we could remove the waiting
 column and use a state with value 'waiting' instead.

 -1 ... I think it's useful to see the underlying state as well as the
 waiting flag.  Also, this would represent breakage of part of the API
 that doesn't need to be broken.

 I guess with the changes that showed different thing like fastpath,
 that makes sense. But if we just mapped the states that are there
 today straight off, is there any case where waiting can be true, when
 we're either idle or idle in transaction? I think not..

Maybe if we get a sinval reset while we're just sitting there?  Not sure.

In any case, I agree with Tom.  I think that most people will be happy
to have a query column and a state column rather than a
current_query column that amalgamates the two, but I see no reason
to tinker with the waiting column if the changes we want to make don't
otherwise require it.

 Also, returning these as text seems a little lame. Should there be an
 enum type for that?

 Perhaps, but we don't really use enum types in any other system views,
 so inventing one here would be out of character.

 Yeha, that seems inconsistent. Using a single character might work -
 but it's not particularly user-friendly to do that in the view itself.

+1 for keeping it as text, but loosing the angle brackets.

-- 
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] Show statistics target in \d+

2011-11-04 Thread Robert Haas
On Fri, Nov 4, 2011 at 10:09 AM, Magnus Hagander mag...@hagander.net wrote:
 On Fri, Nov 4, 2011 at 15:06, Robert Haas robertmh...@gmail.com wrote:
 On Fri, Nov 4, 2011 at 9:34 AM, Magnus Hagander mag...@hagander.net wrote:
 The attached patch adds a column for statistics target when viewing
 tables in psql using \d+.

 Comments/reivews?

 Statistics doesn't seem like a very clear name for the column header.

 Got any ideas for a better one? Statistics Target seemed too long to
 me, and Statstarget not necessarily very user friendly?

I would prefer Stats Target to Statistics, because I think it's more clear.

-- 
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] Show statistics target in \d+

2011-11-04 Thread Josh Kupershmidt
On Fri, Nov 4, 2011 at 10:05 AM, Magnus Hagander mag...@hagander.net wrote:
 On Fri, Nov 4, 2011 at 14:53, Cédric Villemain
 Interesting, can the ouput be clear on the value being a default or an
 explicit stat target ? (not mandatory but I believe I would like to
 have it only when the stat target is jnot the default)

 It shows -1 when it's the default.

 We could map that to the string default if we want, or just NULL, I
 guess. Not sure which is best?

I thought -1 was just fine. The ALTER TABLE doc page is clear that -1
means the system default, and having another value may just confuse
users.

Josh

-- 
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] Show statistics target in \d+

2011-11-04 Thread Stephen Frost
* Magnus Hagander (mag...@hagander.net) wrote:
 On Fri, Nov 4, 2011 at 15:06, Robert Haas robertmh...@gmail.com wrote:
  Comments/reivews?
 
  Statistics doesn't seem like a very clear name for the column header.
 
 Got any ideas for a better one? Statistics Target seemed too long to
 me, and Statstarget not necessarily very user friendly?

I don't think we need to kill ourselves over making it super
user-friendly.  If they don't know what it is/means, they can go read
the docs. :)  I agree w/ Robert that 'Statistics' is a bad name.  I'd
suggest 'stat_target'.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Scott Mead
On Fri, Nov 4, 2011 at 9:48 AM, Magnus Hagander mag...@hagander.net wrote:

 On Fri, Nov 4, 2011 at 14:42, Tom Lane t...@sss.pgh.pa.us wrote:
  Marti Raudsepp ma...@juffo.org writes:
  While we're already breaking everything, we could remove the waiting
  column and use a state with value 'waiting' instead.
 
  -1 ... I think it's useful to see the underlying state as well as the
  waiting flag.  Also, this would represent breakage of part of the API
  that doesn't need to be broken.

 I guess with the changes that showed different thing like fastpath,
 that makes sense. But if we just mapped the states that are there
 today straight off, is there any case where waiting can be true, when
 we're either idle or idle in transaction? I think not..


   Leave the waiting column and display 'WAITING' if st_watiting = 1 seems
to be the clearest solution.  I can see people getting confused by waiting
= 't' and state='RUNNING'.




  Also, returning these as text seems a little lame. Should there be an
  enum type for that?
 
  Perhaps, but we don't really use enum types in any other system views,
  so inventing one here would be out of character.

 Yeha, that seems inconsistent. Using a single character might work -
 but it's not particularly user-friendly to do that in the view itself.


 I'll nuke the '', which is definitely an improvement, anything more
complex seems like it'll require fairly wordy documentation.

--
  Scott Mead
   OpenSCG http://www.openscg.com



 --
  Magnus Hagander
  Me: http://www.hagander.net/
  Work: http://www.redpill-linpro.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] Show sequences owned by

2011-11-04 Thread Magnus Hagander
On Fri, Nov 4, 2011 at 15:09, Stephen Frost sfr...@snowman.net wrote:
 Magnus,

 * Magnus Hagander (mag...@hagander.net) wrote:
 The attached patch makes the \d output for psql on a sequence show
 which table/column owns the sequence. The table already showed the
 dependency the other way through the default value, but going from
 sequence back to table was not possible.

 Seems reasonable.

 Comments/reviews?

 Not sure if that 'goto error_return;' handles this correctly, but it
 would seem like you're missing the possibility that a sequence isn't
 owned by any table/column..?  Or that it could be depended upon by more
 than one table/column?  Both of those happen and are perfectly valid
 situations for a sequence to be in..

If there is noone owning it at all, it just falls through the if/else
block and ignores it if that happens (PQntuples() returns 0).

Is there really a case for multiple sequences to own it? How would you
go about making that happen? ALTER SEQUENCE.. OWNED BY.. only takes
one table, afaics?

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.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] Show statistics target in \d+

2011-11-04 Thread Kevin Grittner
Magnus Hagander mag...@hagander.net wrote:
 
  Interesting, can the ouput be clear on the value being a default
 or an explicit stat target ? (not mandatory but I believe I would
 like to have it only when the stat target is jnot the default)
 
 It shows -1 when it's the default.
 
 We could map that to the string default if we want, or just
 NULL, I guess. Not sure which is best?
 
I think it would be most useful for it to show as blank, so that
overrides stand out.  NULL seems the best way to do that.
 
-Kevin

-- 
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] Show statistics target in \d+

2011-11-04 Thread Cédric Villemain
2011/11/4 Josh Kupershmidt schmi...@gmail.com:
 On Fri, Nov 4, 2011 at 10:05 AM, Magnus Hagander mag...@hagander.net wrote:
 On Fri, Nov 4, 2011 at 14:53, Cédric Villemain
 Interesting, can the ouput be clear on the value being a default or an
 explicit stat target ? (not mandatory but I believe I would like to
 have it only when the stat target is jnot the default)

 It shows -1 when it's the default.

 We could map that to the string default if we want, or just NULL, I
 guess. Not sure which is best?

 I thought -1 was just fine. The ALTER TABLE doc page is clear that -1
 means the system default, and having another value may just confuse
 users.

yes, it looks good to me.


 Josh




-- 
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation

-- 
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] Show sequences owned by

2011-11-04 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 The attached patch makes the \d output for psql on a sequence show
 which table/column owns the sequence. The table already showed the
 dependency the other way through the default value, but going from
 sequence back to table was not possible.

 Comments/reviews?

The join conditions are far from adequate.  You can *not* just check the
objid, you *must* check classid (and refclassid) to avoid being fooled
by duplicate OIDs in different system catalogs.  You've also not held
to psql's normal conventions about fully qualifying names to avoid
making assumptions about the search_path.

regards, tom lane

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


Re: [HACKERS] pg_dump --exclude-table-data

2011-11-04 Thread Robert Haas
On Wed, Nov 2, 2011 at 6:02 PM, Andrew Dunstan and...@dunslane.net wrote:
 On 09/02/2011 03:15 PM, Josh Berkus wrote:

 OK, this seems to have some pluses and no negative comments, so it seems
 worth going forward. Do we want an equivalent pg_restore option?

 I'm not sure it's *as* important for pg_restore, since I can easily use
  a manifest to avoid restoring data for a single table.  So I guess it's
 a question of how hard is it to add it?


 The short answer is more work than I want to put in to this. pg_restore
 doesn't have any of pg_dump's infrastructure for handling table name
 patterns, nor for excluding tables. So I think all that would remain a TODO.
 (A good beginner project, maybe).

 A slightly updated patch is attached, the main change being that I removed
 use of a short option and only support the long name option. -D didn't
 seem sufficiently mnemonic to me. I'll add this to the November commitfest,
 but I'd like to get it committed ASAP as it will simplify setting up the
 -pre and -post data patches.

Instead of:

do NOT dump data for the named table(s)

How about:

dump only schema for the named table(s)

I'm also a bit concerned about the relationship between this and the
existing -s option.  It seems odd that you use --schema-only to get
the behavior database-wide, and --exclude-table-data to get it for
just one table.  Is there some way we can make that a bit more
consistent?

-- 
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] Show statistics target in \d+

2011-11-04 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 The attached patch adds a column for statistics target when viewing
 tables in psql using \d+.

 Comments/reivews?

Isn't this going to show -1 most of the time?  Seems rather useless,
not to mention confusing to people who don't know what that means.

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] Show sequences owned by

2011-11-04 Thread Magnus Hagander
On Fri, Nov 4, 2011 at 15:19, Tom Lane t...@sss.pgh.pa.us wrote:
 Magnus Hagander mag...@hagander.net writes:
 The attached patch makes the \d output for psql on a sequence show
 which table/column owns the sequence. The table already showed the
 dependency the other way through the default value, but going from
 sequence back to table was not possible.

 Comments/reviews?

 The join conditions are far from adequate.  You can *not* just check the
 objid, you *must* check classid (and refclassid) to avoid being fooled

Uh, it does check classid. Or are you saying it's checked the wrong way?

But it's not checking refclassid, that's true - and should be fixed.

 by duplicate OIDs in different system catalogs.  You've also not held
 to psql's normal conventions about fully qualifying names to avoid
 making assumptions about the search_path.

Will fix.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.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] Term positions in GIN fulltext index

2011-11-04 Thread Yoann Moreau

On 04/11/11 12:15, Florian Pflug wrote:

AFAICS, the internal storage layout of tsvector should allow you to extract an
individual lexem's positions quite efficiently (with time complexity log(N) 
where
N is the number of lexems in the tsvector). Doing so will require you to 
implement
your function in C though - any solution that works from a tsvector's textual
representation will obviously have time complexity N.

best regards,
Florian Pflug

I'll do a pl/pgsql function first, I need to test it with other parts of 
the project. But I will look for more efficient algorithms for a C 
function as soon as possible if we still decide to use the postgresql 
fulltext engine.


Regards,
Yoann Moreau

--
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] removing =(text, text) in 9.2

2011-11-04 Thread Robert Haas
On Thu, Nov 3, 2011 at 10:18 AM, Dimitri Fontaine
dimi...@2ndquadrant.fr wrote:
 Robert Haas robertmh...@gmail.com writes:
 Hmm, I was kind of expecting that to be wrong at least in some minor way.

 +/* contrib/hstore/hstore-1.0-1.1.sql */
 +
 +-- complain if script is sourced in psql, rather than via CREATE EXTENSION
 +\echo Use ALTER EXTENSION hstore to load this file. \quit

 You could mention ALTER EXTENSION hstore UPDATE TO 1.1; in this comment,
 I think.

 +++ b/contrib/hstore/hstore--1.1.sql
 @@ -0,0 +1,524 @@
 +/* contrib/hstore/hstore--1.0.sql */

 That needs a comment update too.

Thanks for the review.  New version attached.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


hstore-drop-arrow-v3.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] Show sequences owned by

2011-11-04 Thread Stephen Frost
* Magnus Hagander (mag...@hagander.net) wrote:
 If there is noone owning it at all, it just falls through the if/else
 block and ignores it if that happens (PQntuples() returns 0).

Ah, right, but 'result' is still non-zero, ok.

 Is there really a case for multiple sequences to own it? How would you
 go about making that happen? ALTER SEQUENCE.. OWNED BY.. only takes
 one table, afaics?

I just noticed it was pulling from pg_depend and we could be creating
multiple dependencies on a single sequence by having two tables use it
as a default value.  If that situation doesn't cause a problem for this,
then that's fine. :)  Couldn't remember if we distinguished 'owned by'
from 'dependend upon' for seqeunces.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 I guess with the changes that showed different thing like fastpath,
 that makes sense. But if we just mapped the states that are there
 today straight off, is there any case where waiting can be true, when
 we're either idle or idle in transaction? I think not..

I'm not totally sure about that.  I know that it's possible to see idle
waiting in the ps display, but that comes from getting blocked in parse
analysis before the command type has been determined.  The
pg_stat_activity string is set a bit earlier, so *maybe* it's impossible
there.  Still, why wire such an assumption into the structure of the
view?  Robert's point about sinval catchup is another good one, though
I don't remember what that does to the pg_stat_activity display.

BTW, a quick grep shows that there are four not two special values of
the activity string today:

src/backend/tcop/postgres.c: 3792: 
pgstat_report_activity(IDLE in transaction (aborted));
src/backend/tcop/postgres.c: 3797: 
pgstat_report_activity(IDLE in transaction);
src/backend/tcop/postgres.c: 3805: 
pgstat_report_activity(IDLE);
src/backend/tcop/postgres.c: 3925: 
pgstat_report_activity(FASTPATH function call);

It's also worth considering whether the autovacuum: that's prepended
by autovac_report_activity() ought to be folded into the state instead
of continuing to put something that's not valid SQL into the query
string.

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] Show sequences owned by

2011-11-04 Thread Magnus Hagander
On Fri, Nov 4, 2011 at 15:29, Stephen Frost sfr...@snowman.net wrote:
 * Magnus Hagander (mag...@hagander.net) wrote:
 If there is noone owning it at all, it just falls through the if/else
 block and ignores it if that happens (PQntuples() returns 0).

 Ah, right, but 'result' is still non-zero, ok.

Yes, that's a regular libpq result set...

 Is there really a case for multiple sequences to own it? How would you
 go about making that happen? ALTER SEQUENCE.. OWNED BY.. only takes
 one table, afaics?

 I just noticed it was pulling from pg_depend and we could be creating
 multiple dependencies on a single sequence by having two tables use it
 as a default value.  If that situation doesn't cause a problem for this,
 then that's fine. :)  Couldn't remember if we distinguished 'owned by'
 from 'dependend upon' for seqeunces.

I tried that now to be sure, and to confirm, this is the scenario:
CREATE TABLE seqtest (a SERIAL PRIMARY KEY);
CREATE TABLE seqtest2 (a int NOT NULL DEFAULT
nextval('seqtest_a_seq'::regclass);

In this case, we end up with just one entry in pg_depend, which refers
to seqtest.a.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.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] Show sequences owned by

2011-11-04 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 On Fri, Nov 4, 2011 at 15:19, Tom Lane t...@sss.pgh.pa.us wrote:
 The join conditions are far from adequate.  You can *not* just check the
 objid, you *must* check classid (and refclassid) to avoid being fooled

 Uh, it does check classid. Or are you saying it's checked the wrong way?

Oh, sheesh, not enough caffeine.  I was expecting to see it written as
part of the ON condition --- I always think of objid and classid as
being two parts of the join key for pg_depend queries.  You should write
it as classid='pg_catalog.pg_class'::pg_catalog.regclass, but at least
it's there.

 But it's not checking refclassid, that's true - and should be fixed.

Yeah.

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] warning in pg_upgrade

2011-11-04 Thread Robert Haas
On Thu, Nov 3, 2011 at 3:45 PM, Kevin Grittner
kevin.gritt...@wicourts.gov wrote:
 Kevin Grittner kevin.gritt...@wicourts.gov wrote:
 Untested patch attached for purposes of discussion.

 I got in a little testing on it -- not only does this patch
 eliminate the compile-time warning, but if you try to run pg_upgrade
 when another session has removed your current working directory, you
 get a reasonable message instead of the program attempting to
 proceed with undefined (potential garbage) for a working directory.

Committed.  Also fixed another compiler warning that popped up for me.

-- 
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] IDLE in transaction introspection

2011-11-04 Thread Robert Haas
On Fri, Nov 4, 2011 at 10:34 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert's point about sinval catchup is another good one, though
 I don't remember what that does to the pg_stat_activity display.

My thought was that sinval catchup might require acquiring a relation
lock (e.g. on pg_class), and that might block waiting for a lock.

Not sure it's possible, but even if it can't happen today, it doesn't
seem impossible that we might want to let it happen in the future.

-- 
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] Show sequences owned by

2011-11-04 Thread Tom Lane
Stephen Frost sfr...@snowman.net writes:
 I just noticed it was pulling from pg_depend and we could be creating
 multiple dependencies on a single sequence by having two tables use it
 as a default value.  If that situation doesn't cause a problem for this,
 then that's fine. :)  Couldn't remember if we distinguished 'owned by'
 from 'dependend upon' for seqeunces.

Yeah, we do, via the deptype.  The check for deptype = 'a' is the
correct thing here.

Still, I'm not terribly comfortable with having multiple matches be
treated as a reason to fail the entire \d command.  It'd likely be
better to just not add a footer if you get an unexpected number of
matches.

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] heap_page_prune comments

2011-11-04 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 Seems a little hackish, though: we'd be reporting an amount of
 freespace that we've deliberately set to an incorrect value.  I'm
 almost thinking we should report the freespace that's actually
 available, on the theory that Bload Is Bad (TM).

IIRC, this code is following the very longstanding precedent of
RelationGetBufferForTuple.

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] Show sequences owned by

2011-11-04 Thread Magnus Hagander
On Fri, Nov 4, 2011 at 15:44, Tom Lane t...@sss.pgh.pa.us wrote:
 Stephen Frost sfr...@snowman.net writes:
 I just noticed it was pulling from pg_depend and we could be creating
 multiple dependencies on a single sequence by having two tables use it
 as a default value.  If that situation doesn't cause a problem for this,
 then that's fine. :)  Couldn't remember if we distinguished 'owned by'
 from 'dependend upon' for seqeunces.

 Yeah, we do, via the deptype.  The check for deptype = 'a' is the
 correct thing here.

 Still, I'm not terribly comfortable with having multiple matches be
 treated as a reason to fail the entire \d command.  It'd likely be
 better to just not add a footer if you get an unexpected number of
 matches.

Ok.

Updated patch attached that does this, and the proper schema qualifications.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index d5466f8..746f18e 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1609,6 +1609,43 @@ describeOneTableDetails(const char *schemaname,
 			PQclear(result);
 		}
 	}
+	else if (tableinfo.relkind == 'S')
+	{
+		/* Footer information about a sequence */
+		PGresult   *result = NULL;
+
+		/* Get the column that owns this sequence */
+		printfPQExpBuffer(buf, SELECT quote_ident(nspname) || '.' ||
+		  \n   quote_ident(relname) || '.' ||
+		  \n   quote_ident(attname)
+		  \nFROM pg_catalog.pg_class
+		  \nINNER JOIN pg_catalog.pg_depend ON pg_class.oid=pg_depend.refobjid
+		  \nINNER JOIN pg_catalog.pg_namespace ON pg_namespace.oid=pg_class.relnamespace
+		  \nINNER JOIN pg_catalog.pg_attribute ON (
+		  \n pg_attribute.attrelid=pg_class.oid AND
+		  \n pg_attribute.attnum=pg_depend.refobjsubid)
+		  \nWHERE classid='pg_catalog.pg_class'::regclass
+		  \n AND refclassid='pg_catalog.pg_class'::regclass
+		  \n AND objid=%s
+		  \n AND deptype='a',
+		  oid);
+
+		result = PSQLexec(buf.data, false);
+		if (!result)
+			goto error_return;
+		else if (PQntuples(result) == 1)
+		{
+			printfPQExpBuffer(buf, _(Owned by: %s),
+			  PQgetvalue(result, 0, 0));
+			printTableAddFooter(cont, buf.data);
+		}
+		/*
+		 * If we get no rows back, don't show anything (obviously).
+		 * We should never get more than one row back, but if we do,
+		 * just ignore it and don't print anything.
+		 */
+		PQclear(result);
+	}
 	else if (tableinfo.relkind == 'r' || tableinfo.relkind == 'f')
 	{
 		/* Footer information about a table */

-- 
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] Show statistics target in \d+

2011-11-04 Thread Magnus Hagander
On Fri, Nov 4, 2011 at 15:23, Tom Lane t...@sss.pgh.pa.us wrote:
 Magnus Hagander mag...@hagander.net writes:
 The attached patch adds a column for statistics target when viewing
 tables in psql using \d+.

 Comments/reivews?

 Isn't this going to show -1 most of the time?  Seems rather useless,
 not to mention confusing to people who don't know what that means.

It will, but in the cases where it doesn't, it'll be very useful and
good to have thrown in your face.

Would you find it better if we showed blank (NULL) when it was -1?


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.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] Show sequences owned by

2011-11-04 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 Updated patch attached that does this, and the proper schema qualifications.

I'd schema-qualify the quote_ident and regclass names too.

Also, just as a matter of style, I think it'd be better to assign short
aliases to the table names (pg_catalog.pg_class c etc) and use those.
I forget what the letter of the SQL standard is about whether an
un-aliased schema-qualified table name can be referenced in the query
without schema-qualifying the reference, but I'm pretty sure that not
doing so is at least frowned on.

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] Show statistics target in \d+

2011-11-04 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 Would you find it better if we showed blank (NULL) when it was -1?

Yeah, I would.  Seems less confusing.

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] heap_page_prune comments

2011-11-04 Thread Robert Haas
On Fri, Nov 4, 2011 at 10:46 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 Seems a little hackish, though: we'd be reporting an amount of
 freespace that we've deliberately set to an incorrect value.  I'm
 almost thinking we should report the freespace that's actually
 available, on the theory that Bload Is Bad (TM).

 IIRC, this code is following the very longstanding precedent of
 RelationGetBufferForTuple.

I don't understand the analogy - that function isn't freeing any
space, just searching for a block that already has some.  And it does
update the free space map if the free space map is found to be out of
date, whereas this function does not.

-- 
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] IDLE in transaction introspection

2011-11-04 Thread Marti Raudsepp
On Fri, Nov 4, 2011 at 15:42, Tom Lane t...@sss.pgh.pa.us wrote:
 Marti Raudsepp ma...@juffo.org writes:
 While we're already breaking everything, we could remove the waiting
 column and use a state with value 'waiting' instead.

 -1 ... I think it's useful to see the underlying state as well as the
 waiting flag.  Also, this would represent breakage of part of the API
 that doesn't need to be broken.

Well the waiting column can stay. My concern is that listing lock-wait
backends as 'running' will be misleading for users. pg_stat_activity
is a pretty common starting point for debugging problems and if
there's a new column that says a query is 'running', then I'm afraid
the current waiting 't' and 'f' values will be too subtle for users to
notice. (I find that it's too subtle already now, if you don't know
what you're looking for).

Regards,
Marti

-- 
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] unite recovery.conf and postgresql.conf

2011-11-04 Thread Bruce Momjian
Josh Berkus wrote:
  We can always do nothing, which is a safe and viable option.
 
 Not really, no.  The whole recovery.conf thing is very broken and
 inhibits adoption of PostgreSQL because our users can't figure it out.
 
 You've made it pretty clear that you're personally comfortable with how
 replication configuration works now, and aren't really interested in any
 changes.  That's certainly a valid viewpoint, but the users and
 contributors who find the API horribly unusable also have a valid
 viewpoint.  You don't automatically win arguments because you're on the
 side of backwards compatibility.
 
 When we released binary replication in 9.0, I thought everyone knew that
 it was a first cut and that we'd be making some dramatic changes --
 including ones which broke things -- over the next few versions.  There
 was simply no way for us to know real user requirements until the
 feature was in the field and being deployed in production.  We would
 discover some things which really didn't work and that we had to break
 and remake.  And we have.
 
 Now you are arguing for premature senescence, where our first API
 becomes our only API now and forever.  That's a road to project death.

Agreed.  This thread has already expended too much of our valuable time,
in my opinion.

I think we have enough agreement that we need a new API, so let's design
one.  If we can add some backward-compatibility here, great, but let's
not have that driving the discussion.  Replication is already complex
enough that having two ways to set this up just adds confusion. 
Replication/PITR does not affect SQL or applications --- it affects
admin scripts and tools, so they are just going to have to adjust.  

We are not going to make everyone happy, so let's just move forward ---
if people want to pout in the corner, I really don't care.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

-- 
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] TOAST versus VACUUM, or missing chunk number 0 for toast value identified

2011-11-04 Thread Bruce Momjian
Tom Lane wrote:
 Alvaro Herrera alvhe...@commandprompt.com writes:
  Excerpts from Alvaro Herrera's message of vie oct 28 16:47:13 -0300 2011:
  BTW we had previous discussions about dropping pg_database's toast
  table.  Maybe this is a good time to do it, even if there's no risk of
  this bug (or the hypothetical circularity detoasting problem) showing up
  there.
 
 No objection from me.
 
  Oh, something unrelated I just remembered: we have
  pg_database.datlastsysoid which seems unused.  Perhaps we should remove
  that column for cleanliness.
 
 I have a vague recollection that some client-side code uses this to
 figure out what's the dividing line between user and system OIDs.
 pg_dump used to need to know that number, and it can still be useful
 with casts and other things that are hard to tell whether they're built-in.
 While in principle people could use FirstNormalObjectId instead, that
 could come back to bite us if we ever have to increase that constant
 in future releases.  I'm inclined to leave this one alone.

Maybe add a C comment?

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

-- 
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] Range Types - typo + NULL string constructor

2011-11-04 Thread Jeff Davis
On Thu, 2011-11-03 at 10:37 +0200, Heikki Linnakangas wrote:
 Looking at the most recent patch, I don't actually see any GiST support 
 for the empty and non-empty operators (!? and ?). I don't see how those 
 could be accelerated with GiST, anyway; I think if you want to use an 
 index for those operators, you might as well create a partial or 
 functional index on empty(x).
 
 So I'm actually inclined to remove not only the nonempty function, but 
 also the ? and !? operators. They don't seem very useful, and ? and !? 
 don't feel very intuitive to me, anyway. I'll just leave the empty(x) 
 function.

That's fine with me. At best, they were only marginally useful.

Regards,
Jeff Davis


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


Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

2011-11-04 Thread Jeff Davis
On Wed, 2011-11-02 at 21:29 +0200, Heikki Linnakangas wrote:
  +   else if (lower1.infinite || upper1.infinite)
  +   length1 = 1.0/0.0;
 
 That seems wrong. I take it that the point is to set length1 to infinity?

I reworked this in commit (on my private repo, of course):
6197fbffb00f729feba8082136801cdef5ac850e

For the archives, it's essentially taking the difference on the left
side of the range, and the difference on the right side of the range,
and adding them together. There are just a lot of special cases for
infinite boundaries, empty ranges, and the lack of a subtype_diff
function.

I think it's a little closer to what Alexander intended, which I think
is an improvement. It should now be able to recognize that expanding
[10,) into [0,) has a penalty of 10.

 PS. I note the docs still refer to subtype_float. I'll fix that before 
 committing.

Thank you. The only change I found strange was the test that used \c to
reconnect; but I can't say that my solution was any better.

Regards,
Jeff Davis



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


Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

2011-11-04 Thread Jeff Davis
On Wed, 2011-11-02 at 22:59 +0200, Heikki Linnakangas wrote:
 This seems to be coming from the selectivity estimation function. The 
 selectivity function for @ is scalargtsel, which is usually used for 
 scalar  and =. That doesn't seem right. But what do we store in the 
 statistics for range types in the first place, and what would be the 
 right thing to do for selectivity estimation?

I'll have to think more about that, and it depends on the operator. It
seems like an easier problem for contains a point than contains
another range or overlaps with another range.

Right now I don't have a very good answer, and even for the contains a
point case I'll have to think about the representation in pg_statistic.

Regards,
Jeff Davis


-- 
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] heap_page_prune comments

2011-11-04 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Fri, Nov 4, 2011 at 10:46 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 IIRC, this code is following the very longstanding precedent of
 RelationGetBufferForTuple.

 I don't understand the analogy - that function isn't freeing any
 space, just searching for a block that already has some.  And it does
 update the free space map if the free space map is found to be out of
 date, whereas this function does not.

No, I'm talking about what it does at the very bottom, when it's had to
add a new block to the relation:

 * XXX should we enter the new page into the free space map immediately,
 * or just keep it for this backend's exclusive use in the short run
 * (until VACUUM sees it)?  Seems to depend on whether you expect the
 * current backend to make more insertions or not, which is probably a
 * good bet most of the time.  So for now, don't add it to FSM yet.

Now, heap_page_prune is in a slightly different place, because it
doesn't actually know whether the current backend is going to make an
insertion or update in the page.  If it did know that was going to
happen, then the analogy would be exact.

In any case, the comment in heap_page_prune is ignoring the probability
that VACUUM will eventually visit the page and then update the FSM.
That ought to be factored into any discussion of what to do here.

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] heap vacuum cleanup locks

2011-11-04 Thread Robert Haas
On Thu, Nov 3, 2011 at 12:57 PM, Simon Riggs si...@2ndquadrant.com wrote:
 On Thu, Nov 3, 2011 at 2:22 PM, Robert Haas robertmh...@gmail.com wrote:
 On Thu, Nov 3, 2011 at 9:52 AM, Simon Riggs si...@2ndquadrant.com wrote:
 On Thu, Nov 3, 2011 at 1:26 PM, Robert Haas robertmh...@gmail.com wrote:

 I think that should be OK, but:

 - It looks to me like you haven't done anything about the second heap
 pass.  That should probably get a similar fix.

 I was assuming this worked with Pavan's patch to remove second pass.

 It's not entirely certain that will make it into 9.2, so I would
 rather get this done first.  If you want I can pick up what you've
 done and send you back a version that addresses this.

 OK, that seems efficient. Thanks.

Here's a new version.  I fixed the second pass as discussed (which
turned out to be trivial).  To address the concern about relpages, I
moved this pre-existing line to after we get the buffer lock:

+   vacrelstats-scanned_pages++;

That appears to do the right thing.

I found it kind of confusing that lazy_scan_page_for_wraparound()
returns with the pin either held or not held depending on the return
value, so I rearranged things very slightly so that it doesn't need to
do that.  I'm wondering whether we should rename that function to
something like lazy_check_needs_freeze().

I tested this out and discovered that VACUUM FREEZE doesn't set the
for_wraparound flag.  On further review, I think that we should
probably conditionalize the behavior on the scan_all flag that
lazy_vacuum_rel sets, rather than for_wraparound.  Otherwise, there's
no way for the user to manually force relfrozenxid to advance, which
doesn't seem good.  I haven't made that change in this version,
though.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


vacuum_skip_busy_pages.v2.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] IDLE in transaction introspection

2011-11-04 Thread Robert Haas
On Fri, Nov 4, 2011 at 12:46 PM, Marti Raudsepp ma...@juffo.org wrote:
 On Fri, Nov 4, 2011 at 15:42, Tom Lane t...@sss.pgh.pa.us wrote:
 Marti Raudsepp ma...@juffo.org writes:
 While we're already breaking everything, we could remove the waiting
 column and use a state with value 'waiting' instead.

 -1 ... I think it's useful to see the underlying state as well as the
 waiting flag.  Also, this would represent breakage of part of the API
 that doesn't need to be broken.

 Well the waiting column can stay. My concern is that listing lock-wait
 backends as 'running' will be misleading for users. pg_stat_activity
 is a pretty common starting point for debugging problems and if
 there's a new column that says a query is 'running', then I'm afraid
 the current waiting 't' and 'f' values will be too subtle for users to
 notice. (I find that it's too subtle already now, if you don't know
 what you're looking for).

Maybe there's a better term than running, like in progress or
something of that sort.

-- 
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] IDLE in transaction introspection

2011-11-04 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 Maybe there's a better term than running, like in progress or
 something of that sort.

active?

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] heap_page_prune comments

2011-11-04 Thread Robert Haas
On Fri, Nov 4, 2011 at 2:17 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Now, heap_page_prune is in a slightly different place, because it
 doesn't actually know whether the current backend is going to make an
 insertion or update in the page.  If it did know that was going to
 happen, then the analogy would be exact.

OK.

 In any case, the comment in heap_page_prune is ignoring the probability
 that VACUUM will eventually visit the page and then update the FSM.
 That ought to be factored into any discussion of what to do here.

True.  Unfortunately, I have no intuition on what the right thing to
do is, here.

-- 
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] IDLE in transaction introspection

2011-11-04 Thread Robert Haas
On Fri, Nov 4, 2011 at 2:28 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 Maybe there's a better term than running, like in progress or
 something of that sort.

 active?

+1.

-- 
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] IDLE in transaction introspection

2011-11-04 Thread Scott Mead
On Fri, Nov 4, 2011 at 2:31 PM, Robert Haas robertmh...@gmail.com wrote:

 On Fri, Nov 4, 2011 at 2:28 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  Robert Haas robertmh...@gmail.com writes:
  Maybe there's a better term than running, like in progress or
  something of that sort.
 
  active?

 +1.

Letting this one 'poll' a bit more before I post the patch, but here's
what I have:

   If waiting == true, then state = WAITING
   else
 state = appropriate state

   I leave the waiting flag in place for posterity.  With this in mind, is
the consensus:

   RUNNING

or

   ACTIVE

 --
  Scott Mead
   OpenSCG http://www.openscg.com


--
 Robert Haas
 EnterpriseDB: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company



Re: [HACKERS] IDLE in transaction introspection

2011-11-04 Thread Tom Lane
Scott Mead sco...@openscg.com writes:
I leave the waiting flag in place for posterity.  With this in mind, is
 the consensus:
RUNNING
 or
ACTIVE

Personally, I'd go for lower case.

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] heap vacuum cleanup locks

2011-11-04 Thread Simon Riggs
On Fri, Nov 4, 2011 at 6:18 PM, Robert Haas robertmh...@gmail.com wrote:

 Here's a new version.  I fixed the second pass as discussed (which
 turned out to be trivial).  To address the concern about relpages, I
 moved this pre-existing line to after we get the buffer lock:

 +               vacrelstats-scanned_pages++;

 That appears to do the right thing.

I think we need to count skipped pages also. I don't like the idea
that vacuum would just report less pages than there are in the table.
We'll just get requests to explain that.

 I found it kind of confusing that lazy_scan_page_for_wraparound()
 returns with the pin either held or not held depending on the return
 value, so I rearranged things very slightly so that it doesn't need to
 do that.  I'm wondering whether we should rename that function to
 something like lazy_check_needs_freeze().

OK

 I tested this out and discovered that VACUUM FREEZE doesn't set the
 for_wraparound flag.  On further review, I think that we should
 probably conditionalize the behavior on the scan_all flag that
 lazy_vacuum_rel sets, rather than for_wraparound.  Otherwise, there's
 no way for the user to manually force relfrozenxid to advance, which
 doesn't seem good.  I haven't made that change in this version,
 though.

Agreed, separate patch.

-- 
 Simon Riggs   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] IDLE in transaction introspection

2011-11-04 Thread Robert Haas
On Fri, Nov 4, 2011 at 2:46 PM, Scott Mead sco...@openscg.com wrote:
    If waiting == true, then state = WAITING
    else
      state = appropriate state

No, I think the state and waiting columns should be completely independent.

-- 
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] Strange behavior on to_tsquery()

2011-11-04 Thread Rodrigo Hjort
2011/11/3 Tom Lane

 I wrote:
  (Just offhand, it rather looks like dict_int and dict_xsyn are both
  assuming that palloc will give back zeroed space, which is bogus...)

 Yeah, this is definitely broken.  Patches committed; thanks for the
 report.

 http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=e3e3087d8717c26cd1c4581ba29274ac214eb816

regards, tom lane


I modified my code by calling *palloc0()* instead and that issue no longer
appears. :D

Thanks, Tom Lane!

-- 
Rodrigo Hjort
www.hjort.co


Re: [HACKERS] heap vacuum cleanup locks

2011-11-04 Thread Robert Haas
On Fri, Nov 4, 2011 at 3:12 PM, Simon Riggs si...@2ndquadrant.com wrote:
 On Fri, Nov 4, 2011 at 6:18 PM, Robert Haas robertmh...@gmail.com wrote:

 Here's a new version.  I fixed the second pass as discussed (which
 turned out to be trivial).  To address the concern about relpages, I
 moved this pre-existing line to after we get the buffer lock:

 +               vacrelstats-scanned_pages++;

 That appears to do the right thing.

 I think we need to count skipped pages also. I don't like the idea
 that vacuum would just report less pages than there are in the table.
 We'll just get requests to explain that.

It's been skipping pages due to the visibility map since 8.4.  This
seems like small potatoes by comparison, but we could add some
counters if you like.

 I found it kind of confusing that lazy_scan_page_for_wraparound()
 returns with the pin either held or not held depending on the return
 value, so I rearranged things very slightly so that it doesn't need to
 do that.  I'm wondering whether we should rename that function to
 something like lazy_check_needs_freeze().

 OK

 I tested this out and discovered that VACUUM FREEZE doesn't set the
 for_wraparound flag.  On further review, I think that we should
 probably conditionalize the behavior on the scan_all flag that
 lazy_vacuum_rel sets, rather than for_wraparound.  Otherwise, there's
 no way for the user to manually force relfrozenxid to advance, which
 doesn't seem good.  I haven't made that change in this version,
 though.

 Agreed, separate patch.

Doing that actually makes the patch simpler, so I went ahead and did
that in the attached version, along with the renaming mentioned above.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


vacuum_skip_busy_pages.v3.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] IDLE in transaction introspection

2011-11-04 Thread Robert Treat
On Fri, Nov 4, 2011 at 3:28 PM, Robert Haas robertmh...@gmail.com wrote:
 On Fri, Nov 4, 2011 at 2:46 PM, Scott Mead sco...@openscg.com wrote:
    If waiting == true, then state = WAITING
    else
      state = appropriate state

 No, I think the state and waiting columns should be completely independent.


If they aren't I don't think we need both columns. +1 for leaving them
independent though.

Robert Treat
play: xzilla.net
work: omniti.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] IDLE in transaction introspection

2011-11-04 Thread Robert Treat
On Fri, Nov 4, 2011 at 10:34 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Magnus Hagander mag...@hagander.net writes:
 I guess with the changes that showed different thing like fastpath,
 that makes sense. But if we just mapped the states that are there
 today straight off, is there any case where waiting can be true, when
 we're either idle or idle in transaction? I think not..

 I'm not totally sure about that.  I know that it's possible to see idle
 waiting in the ps display, but that comes from getting blocked in parse
 analysis before the command type has been determined.  The
 pg_stat_activity string is set a bit earlier, so *maybe* it's impossible
 there.  Still, why wire such an assumption into the structure of the
 view?  Robert's point about sinval catchup is another good one, though
 I don't remember what that does to the pg_stat_activity display.

 BTW, a quick grep shows that there are four not two special values of
 the activity string today:

 src/backend/tcop/postgres.c: 3792:                 
 pgstat_report_activity(IDLE in transaction (aborted));
 src/backend/tcop/postgres.c: 3797:                 
 pgstat_report_activity(IDLE in transaction);
 src/backend/tcop/postgres.c: 3805:                 
 pgstat_report_activity(IDLE);
 src/backend/tcop/postgres.c: 3925:                 
 pgstat_report_activity(FASTPATH function call);

 It's also worth considering whether the autovacuum: that's prepended
 by autovac_report_activity() ought to be folded into the state instead
 of continuing to put something that's not valid SQL into the query
 string.


Well, it would be interesting to see rows for autovacuum or
replication processes showing up in pg_stat_activity with a
corresponding state (autovacuum, walreciever?) and the query field
showing what they are working on, at the risk that we'd need to build
more complex parsing into the various monitoring scripts, but I guess
it's no worse than before (and I guess probably easier on some level).

Robert Treat
play: xzilla.net
work: omniti.com

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