[HACKERS] how to use eclipse when debugging postgreSQL backend

2009-09-27 Thread 노홍찬
Hello hackers,

 

 

I really appreciate Mr. Cecchet’s effort to establish the wiki page
(working_with_eclipse, http://archives.postgresql.org/pgsql-hackers/2008-
10/msg00312.php).

 

I set up my PostgreSQL development environment with Eclipse, following the
page’s instructions.

 

However, I’m stuck to the situation that I cannot debug the modified
backend source of PostgreSQL 

 

since the gdb incorporated with Eclipse doesn’t support the debugging of
the forked child processes.

 

 

When I start debugging process by using the project default, it can only
debug the postmaster process,

 

since the postmaster process forks child processes each of which is
actually postgres backend process responsible for the response to each
client process (psql).

 

What I’m trying to debug is the storage-related part of the backend
source, so the gdb should be able to access the forked processes.

 

I tried several ways like making the default option of gdb to be ‘set
follow-fork-mode child’, but those tries didn’t work.

 

For sure, there is another option, giving up using eclipse when debugging
and using the console-mode gdb, 

 

but I prefer using graphical development environment and that’s the reason
why I chose to use eclipse as a default development tool in the very first
start.

 

(This try might be regarded as silly though, I might be supposed to get
familiar to console development environment,

 

but if someone went through the same situation before and he or she can
help me, it will be very time-saving)

 

 

If someone can let me know how to set Eclipse or several steps needed to
access the forked process while Elipse debugging, 

 

It will be very much helpful to me.

 

Thank you for reading this

 

 

- Best Regards
  Hongchan
  (  falls...@cs.yonsei.ac.kr, (02)2123-
7757) -

 

 



Re: [HACKERS] [PATCH] Largeobject access controls

2009-09-27 Thread Jaime Casanova
2009/9/24 KaiGai Kohei :
> The attached patch is revised from the previous revision at the following 
> points:
>
> - The "largeobject_compat_acl" is renamed to "largeobject_check_acl".
>  Its default is on, and turning it off means the largeobject stuff
>  performs in compatible mode for the v8.4.x or prior releases.
> - Notification messages were eliminated at the compatible mode.
>  It always allows to bypass ACL checks without any warnings.
>

a few minor points:

+ For example, the lo_import() and
+ lo_export need superuser privileges independent
+ from this setting, as if the prior version doing.

that should read "as prior versions were doing"?

and you're still using pg_largeobject_meta in some comments in
src/include/catalog/pg_largeobject_metadata.h

besides that it looks good to me...
i will mark the patch as "ready for committer"

-- 
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

-- 
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] CREATE LIKE INCLUDING COMMENTS and STORAGES

2009-09-27 Thread Brendan Jurd
2009/9/28 Itagaki Takahiro :
> Brendan Jurd  wrote:
>> patching file src/bin/psql/sql_help.c
>> Hunk #1 FAILED at 3.
>> Hunk #2 FAILED at 1279.
>> 2 out of 2 hunks FAILED -- saving rejects to file src/bin/psql/sql_help.c.rej
>
> Oops, sql_help.c is an automatic generated file. Please ignore the part.
>
>

With the sql_help.c changes removed, the patch applied fine and
testing went well.

I noticed only the following in the new documentation in CREATE TABLE:

diff --git a/doc/src/sgml/ref/create_table.sgml
b/doc/src/sgml/ref/create_table.sgml
index 6417007..9ea8a49 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -299,7 +299,7 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ]
TABLE http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CREATE LIKE INCLUDING COMMENTS and STORAGES

2009-09-27 Thread Itagaki Takahiro

Brendan Jurd  wrote:

> patching file src/bin/psql/sql_help.c
> Hunk #1 FAILED at 3.
> Hunk #2 FAILED at 1279.
> 2 out of 2 hunks FAILED -- saving rejects to file src/bin/psql/sql_help.c.rej
> 
> I have attached the rejects file.

Oops, sql_help.c is an automatic generated file. Please ignore the part.


Regards,
---
ITAGAKI Takahiro
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] CREATE LIKE INCLUDING COMMENTS and STORAGES

2009-09-27 Thread Brendan Jurd
2009/9/28 Itagaki Takahiro :
> Thank you for reviewing.
> I merged your fix and add INCLUDING ALL option to the new patch.
> I changed InhRelation.options to be a bitmap of CreateStmtLikeOption.
> INCLUDING just adds bits, and EXCLUDING drops bits.

I had two hunks fail trying to apply your new patch to the latest (git) HEAD:

patching file doc/src/sgml/ref/create_table.sgml
patching file src/backend/access/common/tupdesc.c
patching file src/backend/catalog/pg_constraint.c
patching file src/backend/commands/comment.c
patching file src/backend/commands/tablecmds.c
patching file src/backend/nodes/copyfuncs.c
patching file src/backend/nodes/equalfuncs.c
patching file src/backend/parser/gram.y
patching file src/backend/parser/parse_utilcmd.c
patching file src/bin/psql/sql_help.c
Hunk #1 FAILED at 3.
Hunk #2 FAILED at 1279.
2 out of 2 hunks FAILED -- saving rejects to file src/bin/psql/sql_help.c.rej
patching file src/include/catalog/pg_constraint.h
patching file src/include/commands/comment.h
patching file src/include/nodes/parsenodes.h
patching file src/include/parser/kwlist.h
patching file src/test/regress/expected/inherit.out
patching file src/test/regress/sql/inherit.sql

I have attached the rejects file.

Cheers,
BJ


sql_help.c.rej
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] operator exclusion constraints

2009-09-27 Thread Jeff Davis
On Sun, 2009-09-27 at 22:40 -0400, Robert Haas wrote:
> Apparently, CommitFest
> no longer means a time when people put aside their own patches to
> review those of others; it seems now to mean a time when 87% of the
> patch authors either continue development or ignore the CommitFest
> completely.

Well, I'm not completely innocent here, either. I also spent time making
progress on my patch, both in terms of code and discussion so that I
would at least have enough information to get it ready during the next
development cycle.

We don't have a clear "design review" stage that allows developers an
opportunity to get thorough feedback during the development cycle, so
the commitfest is also the design review stage by default. I got some
comments when I posted my design on 8/16, but it didn't really get
hashed out until a month later when the commitfest was underway.

The ideal is to propose, design, implement, and then submit for the
commitfest. The reality is that the commitfest is often the first time
the developer gets thorough feedback on the design. So, as a developer,
I'm hesitant to polish a patch before the commitfest because I know
significant changes will be required. Hopefully that didn't waste too
much of Brendan's time.

That's just an observation from my experience with my patch. I know it's
easy to point at little inefficiencies from afar, so I'm not suggesting
we change our process. Overall, real progress is being made for the
project in general and my patch in particular, so I'm more than willing
to set my minor frustrations aside as long as that continues.

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] operator exclusion constraints

2009-09-27 Thread Robert Haas
On Sun, Sep 27, 2009 at 10:13 PM, Jeff Davis  wrote:
> On Sun, 2009-09-27 at 21:38 -0400, Robert Haas wrote:
>> In that case, I think we should target this for the next CommitFest.
>> Especially given the number and complexity of the patches remaining
>> for this CommitFest, I feel very uncomfortable with the idea of
>> waiting another week for a new patch version, and then possibly still
>> needing further changes before it is finally committed.   While we
>> allow patches to be resubmitted for the same CommitFest, this is
>> intended to be for minor adjustments, not significant rewrites.
>
> OK, I expected that to be the case. I got significant feedback at the
> beginning of this commitfest that required some substantial language
> changes. I did find this commitfest extremely productive for my feature.

Excellent, glad to hear it.

> Right now I'm trying to provide some useful feedback to Paval for his
> patch.

Thanks, I deeply appreciate that.  I believe that there are 29 people
who submitted patches for this CommitFest, and that 4 of them are
reviewing, yourself included.  Furthermore, patches and feature
proposals from people who are not themselves helping with the
CommitFest have continued to roll in during this CommitFest.
Personally, I find this quite objectionable.  Apparently, CommitFest
no longer means a time when people put aside their own patches to
review those of others; it seems now to mean a time when 87% of the
patch authors either continue development or ignore the CommitFest
completely.

Fortunately, a number of very competent people who did NOT submit
patches nevertheless volunteered to help review, so we may be OK.  But
I am not sure this is a very sustainable solution.  If everyone who
submitted a pach for this CF had also reviewed one, every patch would
now have a review and there would even be enough reviewers for major
patches to have two each.  Instead, we are still struggling to get
every patch looked at once.

...Robert

-- 
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] CREATE LIKE INCLUDING COMMENTS and STORAGES

2009-09-27 Thread Itagaki Takahiro

Brendan Jurd  wrote:

> I am doing an initial review of your patch.

Thank you for reviewing.
I merged your fix and add INCLUDING ALL option to the new patch.
I changed InhRelation.options to be a bitmap of CreateStmtLikeOption.
INCLUDING just adds bits, and EXCLUDING drops bits.

Now this patch adds:
* CREATE TABLE LIKE ... INCLUDING COMMENTS (for columns and constraints)
* CREATE TABLE LIKE ... INCLUDING STORAGE
* CREATE TABLE LIKE ... INCLUDING ALL

> I think I'm failing to understand why this would be an issue.  Why
> would the user be specifying columns in the CREATE TABLE statement
> that already exist in the table they are cloning?

Without inline-STORAGE syntax, we cannot resolve conflictions of
storage parameters unless we can define tables without STORAGE
and then re-add options with ALTER TABLE.

There might be ToDo items:
* Make INCLUDING COMMENTS also copy comments on indexes.
* Add syntax to define storage options inline like
  CREATE TABLE tbl (col text STORAGE MAIN).

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center



create-including_20090928.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] TODO item: Allow more complex user/database default GUC settings

2009-09-27 Thread Tom Lane
Robert Haas  writes:
> That seems to me to be just confusing the issue.  Now the table name
> and the view name are just totally different with no obvious
> connection between them.  We have enough nonsense of this type already
> (e.g. pg_stats vs. pg_statistic; pg_authid vs. pg_roles vs.
> pg_shadow).  I think we need to settle on a system for handling
> problems of this type and document it in the fine manual or perhaps a
> README somewhere, and stick with it.  Inventing random unconnected
> names is just craziness.

Actually, to the extent that we have any convention at all, it's
to use plurals for system view names (pg_tables, pg_views, etc)
and singular for underlying catalogs (pg_index).  The only exception
to this on the catalog side is pg_auth_members, which is arguably
misnamed.  (pg_inherits is sort of an exception, but it's weird in a
different way: its name is a verb not a noun.)  The apparent exceptions
on the view side (pg_group, pg_shadow, pg_user) are actually views that
are backward compatible substitutes for former catalogs, so they are not
really intentional exceptions.

Now it's also the case that we've tended to use tech-speak names
for catalogs (eg, pg_class, pg_namespace not pg_table, pg_schema)
and so that gives us an additional degree of separation between
those names and the more user-facing names chosen for views.

What we seem to be lacking in this case is a good tech-speak
option for the underlying catalog name.  I'm still not happy
with having a catalog and a view that are exactly the same
except for "s", especially since as Alvaro notes that won't
lead to desirable tab-completion behavior.  OTOH, we have
survived with pg_index vs pg_indexes, so maybe it wouldn't
kill us.

BTW, have we thought much about the simplest possible solution,
which is to not have the view?  How badly do we need it?  Seems
like dropping the functionality into a psql \d command might be
a viable alternative.

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] operator exclusion constraints

2009-09-27 Thread Jeff Davis
On Sun, 2009-09-27 at 21:38 -0400, Robert Haas wrote:
> In that case, I think we should target this for the next CommitFest.
> Especially given the number and complexity of the patches remaining
> for this CommitFest, I feel very uncomfortable with the idea of
> waiting another week for a new patch version, and then possibly still
> needing further changes before it is finally committed.   While we
> allow patches to be resubmitted for the same CommitFest, this is
> intended to be for minor adjustments, not significant rewrites.

OK, I expected that to be the case. I got significant feedback at the
beginning of this commitfest that required some substantial language
changes. I did find this commitfest extremely productive for my feature.

Right now I'm trying to provide some useful feedback to Paval for his
patch.

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] Unicode UTF-8 table formatting for psql text output

2009-09-27 Thread Robert Haas
On Sun, Sep 27, 2009 at 9:24 PM, Selena Deckelmann
 wrote:
> Hi!
>
> On Wed, Sep 23, 2009 at 2:16 AM, Roger Leigh  wrote:
>> On Fri, Sep 18, 2009 at 11:30:05AM -0700, Selena Deckelmann wrote:
>>> Brad says:
>>>
>>>        The patched code compiles without any additional warnings.
>>> Lint gripes about a trailing ',' in 'typedef enum printTextRule' in
>>> print.h.  Other additional lint seem to be false positives.  The
>>> regression tests pass against the new patch.
>>
>> I've attached a new patch which tidies up those extra commas, plus
>> a patch showing the changes from the previous patch.
>
> Great!  Thank you.
>
> Brad -- can you review this patch? Is it ready for a committer?

Brad already marked it that way on the CommitFest application, so I
think that probably means yes.  :-)

...Robert

-- 
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] TODO item: Allow more complex user/database default GUC settings

2009-09-27 Thread Robert Haas
On Fri, Sep 25, 2009 at 8:05 PM, Alvaro Herrera
 wrote:
> Alvaro Herrera escribió:
>
>> I think it can be solved by splitting OptRoleElem in a set of
>> productions for ALTER and a superset of that for ALTER.  I'll go try
>> that.
>
> Right, that works.  Updated patch attached; should solve the issues
> raised in the thread.  I renamed the catalog pg_db_role_setting as
> suggested by Tom.
>
> I have updated the pg_user and pg_roles definitions so that they include
> the settings for the role, but only those that are not specific to any
> database.
>
> I have also added a view, whose only purpose is to convert the role and
> database OIDs into names.  It's been named pg_db_role_settings, but if
> anyone has a better suggestion I'm all ears.

Bernd,

Can you review this new version and mark this as Ready for Committer
if it looks OK, or else respond with comments and set it back to
Waiting on Author?

Thanks,

...Robert

-- 
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] TODO item: Allow more complex user/database default GUC settings

2009-09-27 Thread Robert Haas
On Sat, Sep 26, 2009 at 11:44 PM, Alvaro Herrera
 wrote:
> Robert Haas escribió:
>
>> The problem of having both a table and a closely related view is,
>> IME, one that comes up a lot. I think you just need to pick a
>> convention and stick with it.  Mine is to append "_view" to the
>> table name.
>
> That would make the difference clear, but since what the user normally
> wants to see is the view, it seems a poor solution to make the view the
> more difficult one to type (and the one that isn't tab-completed first
> in psql).  I'd go with naming the view pg_db_role_setting and append
> "_internal" to the catalog or something similar, except that we don't
> have any catalog with such a bad name yet and I don't want to start.
>
> Maybe name the table pg_configuration?

That seems to me to be just confusing the issue.  Now the table name
and the view name are just totally different with no obvious
connection between them.  We have enough nonsense of this type already
(e.g. pg_stats vs. pg_statistic; pg_authid vs. pg_roles vs.
pg_shadow).  I think we need to settle on a system for handling
problems of this type and document it in the fine manual or perhaps a
README somewhere, and stick with it.  Inventing random unconnected
names is just craziness.

Now, if you/others don't like my _view convention; that's fine.  Just
pick something else.  Really, I don't believe the tab-completion thing
is much of a problem, you just type underscore-tab and you're there.
But I am 100% OK with whatever we pick, as long as it is something
easy to remember that we have a chance of being able to apply
consistently.

...Robert

-- 
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] new version of PQconnectdb was:(Re: [HACKERS] Determining client_encoding from client locale)

2009-09-27 Thread Robert Haas
On Wed, Sep 23, 2009 at 3:26 PM, Tom Lane  wrote:
> Jaime Casanova  writes:
>> i extracted the functions to connect that Heikki put on psql in his
>> patch for determining client_encoding from client locale and put it in
>> libpq so i follow the PQconnectdbParams(* params[]) approach.
>
> I got around to looking at the actual patch a bit.  I hadn't understood
> why it was so small, but now I do: it's implemented as a wrapper around
> PQconnectdb.  That is, it takes the given keywords and values, and
> builds a conninfo string, which PQconnectdb then has to pull apart
> again.  This seems, well, dumb.  I'll admit that the wasted cycles are
> probably not much compared to all the other costs of establishing a
> connection.  But it means that you're still exposed to all the other
> limitations of PQconnectdb, eg any possible bugs or restrictions in the
> quoting/escaping logic.  It seems to me that a non-bogus patch for this
> would involve refactoring the code within PQconnectdb so that the
> keywords and values could be plugged in directly without the escaping
> and de-escaping logic.  It doesn't look that hard to pull apart
> conninfo_parse into two or three functions that would support this.
>
> Another reason why it needs refactoring is that this way doesn't expose
> all the functionality that ought to be available; in particular not the
> ability to do an async connection while passing the parameters in this
> style.  You need analogs to PQconnectStart and PQconnectPoll too.
> (Actually I guess the existing PQconnectPoll would work fine, but you
> definitely need an analog to PQconnectStart.)

Based on this review, it sounds like this patch will need a major
rewrite before it can be seriously reviewed.  Given that, I think it
makes sense to postpone this to the next CommitFest, so I am going to
mark it as Returned with Feedback.

...Robert

-- 
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] operator exclusion constraints

2009-09-27 Thread Robert Haas
On Sun, Sep 27, 2009 at 5:47 PM, Jeff Davis  wrote:
> Attached is a new patch. I ran it through filterdiff, but in case that
> didn't work for some reason, I attached a gzipped version of the
> original unified diff produced by git.
>
>  * Changed underlying algorithm to match Tom's suggestion: do the second
> index lookup after already inserting into the first.
>  * Language changes, including the latest " CHECK WITH "
> idea from Tom, seconded by Robert Haas.
>  * automatically builds index for you, no need to create it separately,
> just specify the index AM (or let it default to btree)
>  * Only one constraint per index is allowed, and the index is treated
> entirely as an internal implementation detail.
>  * Support for predicates (partial constraints/partial index)
>  * Support for expressions
>  * Support for other index options, like WITH list and USING INDEX
> TABLESPACE
>  * Docs updated and improved
>  * Tests updated
>  * Full recheck support (the previous recheck mechanism didn't work for
> expressions)
>  * Make information_schema ignore operator exclusion constraints
>  * error message improvements
>
> When testing/reviewing, use the documentation from CREATE TABLE, but use
> the ALTER TABLE variant instead. Right now the CREATE TABLE variant
> doesn't work (see below).
>
> There is still a significant TODO list:
>
>  * CREATE TABLE -- right now, it only works for ALTER TABLE, and the
> docs are lying. Coming soon.
>
>  * psql - haven't updated it to keep up with the language changes
>
>  * pg_dump
>
>  * LIKE
>
>  * Inheritance
>
>  * Enforce on existing tuples when the constraint is created -- This is
> intertwined with inheritance, I think, and I am still working on that.
> Obviously, this is an important TODO item to get the patch ready for
> commit.
>
>  * Deferrability (optional for now) -- I need the trigger to be able to
> perform the check as well. It looks like it has most of the information
> necessary, but I'm trying to determine where would be the cleanest place
> to export the constraint checking function so that it can be called by
> the trigger as well as ExecInsertIndexTuples and the bulk checker (that
> checks existing tuples at the time the constraint is added).
>
>  * GIN support (optional for now) -- I need to create a gingettuple
> method. It would have to be a wrapper around gingetbitmap, and would not
> be any more efficient than gingetbitmap, but it would allow my patch to
> work for GIN indexes.
>
> I think I've made some progress this commitfest, both in terms of
> decisions made (thanks to everyone who provided input and direction),
> and also in terms of code. I would still appreciate more review during
> this commitfest, but realistically, it will still be at least another
> week before I can say that I'm done with all open items.

In that case, I think we should target this for the next CommitFest.
Especially given the number and complexity of the patches remaining
for this CommitFest, I feel very uncomfortable with the idea of
waiting another week for a new patch version, and then possibly still
needing further changes before it is finally committed.   While we
allow patches to be resubmitted for the same CommitFest, this is
intended to be for minor adjustments, not significant rewrites.

So I'm going to mark this Returned with Feedback.

...Robert

-- 
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] Unicode UTF-8 table formatting for psql text output

2009-09-27 Thread Selena Deckelmann
Hi!

On Wed, Sep 23, 2009 at 2:16 AM, Roger Leigh  wrote:
> On Fri, Sep 18, 2009 at 11:30:05AM -0700, Selena Deckelmann wrote:
>> Brad says:
>>
>>        The patched code compiles without any additional warnings.
>> Lint gripes about a trailing ',' in 'typedef enum printTextRule' in
>> print.h.  Other additional lint seem to be false positives.  The
>> regression tests pass against the new patch.
>
> I've attached a new patch which tidies up those extra commas, plus
> a patch showing the changes from the previous patch.

Great!  Thank you.

Brad -- can you review this patch? Is it ready for a committer?

Thanks,
-selena


-- 
http://chesnok.com/daily - me
http://endpoint.com - work

-- 
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] handlers for inline code

2009-09-27 Thread Tom Lane
Andrew Dunstan  writes:
> I'm looking at creating a plperl inline code handler. I'd like to modify 
> the new InlineCodeBlock structure slightly by adding a "trusted" flag 
> and having the calling code in src/backend/commands/functioncmds.c fill 
> it in. This will save every language handler that implements both 
> trusted and untrusted variants from having to fetch the property. I 
> hope  that seems reasonable.

Hm, I wonder if we should just have ExecuteDoStmt keep the pin on the
syscache entry a bit longer and pass the whole tuple?  Offhand I can't
see any other fields of the tuple that are likely to be wanted besides
the trusted flag, so I agree with your proposal; but I wonder if I'm
(again) not looking far enough ahead.

regards, tom lane

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


[HACKERS] handlers for inline code

2009-09-27 Thread Andrew Dunstan


I'm looking at creating a plperl inline code handler. I'd like to modify 
the new InlineCodeBlock structure slightly by adding a "trusted" flag 
and having the calling code in src/backend/commands/functioncmds.c fill 
it in. This will save every language handler that implements both 
trusted and untrusted variants from having to fetch the property. I 
hope  that seems reasonable.


cheers

andrew

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

2009-09-27 Thread Robert Haas
On Sun, Sep 27, 2009 at 4:54 PM, Peter Eisentraut  wrote:
> On Sun, 2009-09-27 at 16:15 -0400, Tom Lane wrote:
>> Peter Eisentraut  writes:
>> > Then why not send everything to syslog and have syslog filter it to the
>> > places you want to?  That is what syslog is for, after all.
>>
>> We send all syslog output with the same identifier/priority/facility,
>> so there's not a lot of hope of getting syslog to do any useful
>> filtering (at least not with the versions of syslog I'm familiar with).
>
> Time to upgrade then. ;-)  For example, the default syslog in Fedora has
> been rsyslog since Fedora 8, and that one can do a lot more than just
> filter by identifier/priority/facility.  syslog-ng is another popular
> example for a more featureful syslog.

Presumably csvlog would be good for these sorts of things too, no?
The whole point is it's machine-readable.

...Robert

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

2009-09-27 Thread Peter Eisentraut
On Sun, 2009-09-27 at 16:15 -0400, Tom Lane wrote:
> Peter Eisentraut  writes:
> > Then why not send everything to syslog and have syslog filter it to the
> > places you want to?  That is what syslog is for, after all.
> 
> We send all syslog output with the same identifier/priority/facility,
> so there's not a lot of hope of getting syslog to do any useful
> filtering (at least not with the versions of syslog I'm familiar with).

Time to upgrade then. ;-)  For example, the default syslog in Fedora has
been rsyslog since Fedora 8, and that one can do a lot more than just
filter by identifier/priority/facility.  syslog-ng is another popular
example for a more featureful syslog.


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

2009-09-27 Thread Tom Lane
Peter Eisentraut  writes:
> On Fri, 2009-09-25 at 14:58 -0600, Joshua Tolley wrote:
>> Actually the thing I want is to be able to send some stuff to syslog,
>> and some to a file, and other stuff to another file. This patch
>> doesn't do all that, but lays the necessary groundwork.

> Then why not send everything to syslog and have syslog filter it to the
> places you want to?  That is what syslog is for, after all.

We send all syslog output with the same identifier/priority/facility,
so there's not a lot of hope of getting syslog to do any useful
filtering (at least not with the versions of syslog I'm familiar with).

Possibly it'd be worth trying to make that more configurable, but
that would require a lot of the same infrastructure and complexity
we're arguing about now.  And it'd still be no help to Windows users.

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

2009-09-27 Thread Peter Eisentraut
On Fri, 2009-09-25 at 14:58 -0600, Joshua Tolley wrote:
> Actually the thing I want is to be able to send some stuff to syslog,
> and some
> to a file, and other stuff to another file. This patch doesn't do all
> that,
> but lays the necessary groundwork.

Then why not send everything to syslog and have syslog filter it to the
places you want to?  That is what syslog is for, after all.


-- 
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.conf: samehost and samenet [REVIEW]

2009-09-27 Thread Stef Walter
Robert Haas wrote:
>> Attached patch contains a fix.
> 
> So is this one Ready for Committer?

Not yet. Two more things to do. Will work on them early next week:

 * On Solaris the ioctl used only returns IPv4 addresses.
 * Don't use hard coded buffers on win32 and ioctl.

Cheers,

Stef




-- 
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] Join optimization for inheritance tables

2009-09-27 Thread Josh Berkus
On 9/26/09 3:00 PM, Emmanuel Cecchet wrote:
> Hi Simon,
> 
> Thanks for the insight. I might take that as a long term project. I have
> to discuss that with my colleagues at Aster. It would certainly help to
> put together a wiki page with the key insights on the design of such
> implementation to be able to better scope the project and agree on what
> is the right approach for this.

In the meantime, can you look at the possibility I suggested?  If you
could produce a patch which would implement the most common and least
complex case of partition joining: where the constraints on two
partitions match exactly (or are mathematically provable subsets) with a
small patch, it would probably work for 8.5.

-- 
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.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] pg_hba.conf: samehost and samenet [REVIEW]

2009-09-27 Thread Robert Haas
On Thu, Sep 24, 2009 at 8:32 PM, Stef Walter  wrote:
> Magnus Hagander wrote:
>> On Mon, Sep 21, 2009 at 20:12, Stef Walter  wrote:
>> This patch does not build on Windows, the error is:
>> ip.obj : error LNK2019: unresolved external symbol __imp__wsaio...@36 
>> referenced
>>  in function _pg_foreach_ifaddr
>> ip.obj : error LNK2019: unresolved external symbol __imp__wsasock...@24 
>> referenc
>> ed in function _pg_foreach_ifaddr
>> .\Release\libpq\libpq.dll : fatal error LNK1120: 2 unresolved externals
>>
>>
>> I don't have time to investigate this further right now, so if
>> somebody else want to dig into why that is happening that would be
>> helpful :)
>
> Seems there are two windows build systems. Once I discovered the MSVC
> one, and got it working, I added the required ws2 library (already used
> by other components of postgresql).
>
> Attached patch contains a fix.

So is this one Ready for Committer?

...Robert

-- 
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] Linux LSB init script

2009-09-27 Thread Robert Haas
On Mon, Sep 21, 2009 at 3:20 AM, Peter Eisentraut  wrote:
> On Sun, 2009-09-20 at 22:54 -0400, Robert Haas wrote:
>> It seems like there is some support for what this patch is trying to
>> do, but much disagreement about the details of how to get there.
>> Where do we go from here?
>
> I think the next step would be to outline what changes would be
> necessary in pg_ctl to implement LSB behavior.  And then decide case by
> case whether it should become the default, an option, or is not
> appropriate for pg_ctl.
>
> Kevin apparently sort of agreed to do that when he came back.

Given the lack of progress here, I'm going to move this one to
"Returned with Feedback" for now.  I think Kevin is busy with his
non-PostgreSQL life, and there's always next CommitFest.

...Robert

-- 
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] Issues for named/mixed function notation patch

2009-09-27 Thread Robert Haas
On Sun, Sep 27, 2009 at 1:46 PM, Pavel Stehule  wrote:
> 2009/9/27 Robert Haas :
>> On Sun, Sep 27, 2009 at 12:37 AM, Pavel Stehule  
>> wrote:
 "However, a named variadic argument can only be called the way shown in
 the example above. The VARIADIC keyword must not be specified and a
 variadic notation of all arguments is not supported. To use variadic
 argument lists you must use positional notation instead."

 What is the intended behavior? I think we should always require VARIADIC
 to be specified regardless of using named notation.

>>>
>>> maybe we could to support variadic named parameters in future - then
>>> using VARIADIC keyword should be necessary - like
>>>
>>> foo(10 AS p1, 20 AS p1, 30 AS p3) is equalent of
>>> foo(VARIADIC ARRAY[10,20] AS p1, 30 AS p3)
>>
>> Pavel,
>>
>> This doesn't make sense to me, FWIW.  I don't think we should allow
>> parameters to be specified more than once.  It's hard for me to
>> imagine how that could be useful.
>
> ook I thing, so this should be little bit unclean too. I though why we
> need VARIADIC keyword mandatory for named notation. When we could
> specify only unique names, then we could use only one "packed"
> variadic parameter - and then VARIADIC keyword isn't necessary.
>
> Is this idea correct? I thing, so there are not problem ensure an
> using VARIADIC keyword in this context - but, personally I don't feel,
> so there it have to be. But I don't thing, so this is important
> (minimally for me) - I'll accept others opinions.

Sorry, I'm having trouble understanding what you're driving at here.
I think we should just not allow named notation to be combined with
VARIADIC, at least for a first version of this feature, either when
defining a function or when calling one.  We can consider relaxing
that restriction at a later date if we can agree on what the semantics
should be.

...Robert

-- 
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] operator exclusion constraints [was: generalized index constraints]

2009-09-27 Thread Robert Haas
On Sun, Sep 27, 2009 at 1:08 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Wed, Sep 23, 2009 at 1:47 PM, Tom Lane  wrote:
>>> I think that USING is just about as content-free as WITH in this
>>> particular example --- it doesn't give you any hint about what the
>>> purpose of the operator is.
>
>> USING might be just as content-free as WITH, but USING OPERATOR seems
>> clearly better, at least IMO.
>
> It's not enough better to justify the conflict with USING opclass, IMO.
>
> An idea that just struck me is CHECK WITH, ie
>
>        EXCLUSION (expr CHECK WITH operator)

I don't like that as well as USING OPERATOR, but I like it far better
than any of the single-word choices, so maybe it's a reasonable
compromise.

...Robert

-- 
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] SELECT ... FOR UPDATE [WAIT integer | NOWAIT] for 8.5

2009-09-27 Thread Tom Lane
Robert Haas  writes:
> As to #1, personally, I think it's quite useful.  The arguments that
> have been made that lock_timeout is redundant with statement_timeout
> don't seem to me to have much merit.
> ...
> As to #2, I was initially thinking dedicated syntax would be better
> because I hate "SET guc = value; do thing; SET guc = previous_value;".
>  But now I'm realizing that there's every reason to suppose that
> SELECT FOR UPDATE will not be the only case where we want to do this -
> so I think a GUC is the only reasonable choice.

Yeah.  I believe that a reasonable argument can be made for being able
to limit lock waits separately from total execution time, but it is
*not* clear to me why SELECT FOR UPDATE per-tuple waits should be the
one single solitary place where that is useful.  IIRC I was against the
SELECT FOR UPDATE NOWAIT syntax to begin with, because of exactly this
same reasoning.

> But that having been
> said, I think some kind of syntax to set a GUC for just one statement
> would be way useful, per discussions downthread.  However, that seems
> like it can and should be a separate pach.

Worth looking at.  We do already have SET LOCAL, and the per-function
GUC settings, but that may not be sufficient.

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] Issues for named/mixed function notation patch

2009-09-27 Thread Pavel Stehule
2009/9/27 Robert Haas :
> On Sun, Sep 27, 2009 at 12:37 AM, Pavel Stehule  
> wrote:
>>> "However, a named variadic argument can only be called the way shown in
>>> the example above. The VARIADIC keyword must not be specified and a
>>> variadic notation of all arguments is not supported. To use variadic
>>> argument lists you must use positional notation instead."
>>>
>>> What is the intended behavior? I think we should always require VARIADIC
>>> to be specified regardless of using named notation.
>>>
>>
>> maybe we could to support variadic named parameters in future - then
>> using VARIADIC keyword should be necessary - like
>>
>> foo(10 AS p1, 20 AS p1, 30 AS p3) is equalent of
>> foo(VARIADIC ARRAY[10,20] AS p1, 30 AS p3)
>
> Pavel,
>
> This doesn't make sense to me, FWIW.  I don't think we should allow
> parameters to be specified more than once.  It's hard for me to
> imagine how that could be useful.

ook I thing, so this should be little bit unclean too. I though why we
need VARIADIC keyword mandatory for named notation. When we could
specify only unique names, then we could use only one "packed"
variadic parameter - and then VARIADIC keyword isn't necessary.

Is this idea correct? I thing, so there are not problem ensure an
using VARIADIC keyword in this context - but, personally I don't feel,
so there it have to be. But I don't thing, so this is important
(minimally for me) - I'll accept others opinions.

Regards
Pavel


>
>>> I'm still reviewing the code.
>
> Jeff,
>
> When will you be able to post this review?
>
> Thanks,
>
> ...Robert
>

-- 
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] SELECT ... FOR UPDATE [WAIT integer | NOWAIT] for 8.5

2009-09-27 Thread Robert Haas
On Mon, Sep 21, 2009 at 6:07 AM, Boszormenyi Zoltan  wrote:
> Jeff Janes írta:
>> On Thu, Sep 3, 2009 at 6:47 AM, Boszormenyi Zoltan > > wrote:
>>
>>     Boszormenyi Zoltan írta:
>>     > Alvaro Herrera írta:
>>     >
>>     >> Boszormenyi Zoltan wrote:
>>     >>
>>     >>
>>     >>
>>     >>> The vague consensus for syntax options was that the GUC
>>     >>> 'lock_timeout' and WAIT [N] extension (wherever NOWAIT
>>     >>> is allowed) both should be implemented.
>>     >>>
>>     >>> Behaviour would be that N seconds timeout should be
>>     >>> applied to every lock that the statement would take.
>>     >>>
>>     >>>
>>     >> In
>>     http://archives.postgresql.org/message-id/291.1242053...@sss.pgh.pa.us
>>     >> Tom argues that lock_timeout should be sufficient.  I'm not
>>     sure what
>>     >> does WAIT [N] buy
>>
>>
>> I disagree with Tom on this point.  *If* I was trying to implement  a
>> server policy, then sure, it should not be done by embedding the
>> timeout in the SQL statement.  But I don't think they want this to
>> implement a server policy.  (And if we do, why would we thump the poor
>> victims that are waiting on the lock, rather than the rogue who
>> decided to take a lock and then camp out on it?)  The use case for
>> WAIT [N] is not a server policy, but a UI policy.  I have two ways to
>> do this task.  The preferred way needs to lock a row, but waiting for
>> it may take too long.  So if I can't get the lock within a reasonable
>> time, I fall back on a less-preferred but still acceptable way of
>> doing the task, one that doesn't need the lock.  If we move to a new
>> server, the appropriate value for the time out does not change,
>> because the appropriate level is the concern of the UI and the end
>> users, not the database server.  This wouldn't be scattered all over
>> the application, either.  In my experience, if you have an application
>> that could benefit from this, you might have 1 or 2 uses for WAIT [N]
>> out of 1,000+ statements in the application.  (From my perspective, if
>> there were to be a WAIT [N] option, it could plug into the
>> statement_timeout mechanism rather than the proposed lock_timeout
>> mechanism.)
>>
>> I think that if the use case for a GUC is to set it, run a single very
>> specific statement, and then unset it, that is pretty clear evidence
>> that this should not be a GUC in the first place.
>>
>> Maybe I am biased in this because I am primarily thinking about how I
>> would use such a feature, rather than how Hans-Juergen intends to use
>> it, and maybe those uses differ.  Hans-Juergen, could you describe
>> your use case a little bit more?   Who do is going to be getting these
>> time-out errors, the queries run by the web-app, or longer running
>> back-office queries?  And when they do get an error, what will they do
>> about it?
>
> Our use case is to port a huge set of Informix apps,
> that use SET LOCK MODE TO WAIT N;
> Apparently Tom Lane was on the opinion that
> PostgreSQL won't need anything more in that regard.
>
> In case the app gets an error, the query (transaction)
> can be retried, the "when" can be user controlled.
>
> I tried to argue on the SELECT ... WAIT N part as well,
> but for our purposes currently the GUC is enough.
>
>>     > Okay, we implemented only the lock_timeout GUC.
>>     > Patch attached, hopefully in an acceptable form.
>>     > Documentation included in the patch, lock_timeout
>>     > works the same way as statement_timeout, takes
>>     > value in milliseconds and 0 disables the timeout.
>>     >
>>     > Best regards,
>>     > Zoltán Böszörményi
>>     >
>>
>>     New patch attached. It's only regenerated for current CVS
>>     so it should apply cleanly.
>>
>>
>>
>> In addition to the previously mentioned seg-fault issues when
>> attempting to use this feature (confirmed in another machine, linux,
>> 64 bit, and --enable-cassert does not offer any help), I have some
>> more concerns about the patch.  From the docs:
>>
>> doc/src/sgml/config.sgml
>>
>>         Abort any statement that tries to lock any rows or tables and
>> the lock
>>         has to wait more than the specified number of milliseconds,
>> starting
>>         from the time the command arrives at the server from the client.
>>         If log_min_error_statement is set to
>> ERROR or
>>         lower, the statement that timed out will also be logged.
>>         A value of zero (the default) turns off the limitation.
>>
>> This suggests that all row locks will have this behavior.  However, my
>> experiments show that row locks attempted to be taken for ordinary
>> UPDATE commands do not time out.  If this is only intended to apply to
>> SELECT  FOR UPDATE, that should be documented here.  It is
>> documented elsewhere that this applies to SELECT...FOR UPDATE, but it
>> is not documented that this the only row-locks it applies to.
>>
>> "from the time the command arrives at the server".  I am pretty sure
>> th

Re: [HACKERS] Issues for named/mixed function notation patch

2009-09-27 Thread Robert Haas
On Sun, Sep 27, 2009 at 12:37 AM, Pavel Stehule  wrote:
>> "However, a named variadic argument can only be called the way shown in
>> the example above. The VARIADIC keyword must not be specified and a
>> variadic notation of all arguments is not supported. To use variadic
>> argument lists you must use positional notation instead."
>>
>> What is the intended behavior? I think we should always require VARIADIC
>> to be specified regardless of using named notation.
>>
>
> maybe we could to support variadic named parameters in future - then
> using VARIADIC keyword should be necessary - like
>
> foo(10 AS p1, 20 AS p1, 30 AS p3) is equalent of
> foo(VARIADIC ARRAY[10,20] AS p1, 30 AS p3)

Pavel,

This doesn't make sense to me, FWIW.  I don't think we should allow
parameters to be specified more than once.  It's hard for me to
imagine how that could be useful.

>> I'm still reviewing the code.

Jeff,

When will you be able to post this review?

Thanks,

...Robert

-- 
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] GRANT ON ALL IN schema

2009-09-27 Thread Petr Jelinek

Robert Haas napsal(a):

Abhijit,

If this patch looks good now, can you mark it Ready for Committer in
the CommitFest app?  If there are any remaining issues, please post a
further review.
  


I believe he'll be out for two more days.

--
Regards
Petr Jelinek (PJMODOS)


--
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] operator exclusion constraints [was: generalized index constraints]

2009-09-27 Thread Tom Lane
Robert Haas  writes:
> On Wed, Sep 23, 2009 at 1:47 PM, Tom Lane  wrote:
>> I think that USING is just about as content-free as WITH in this
>> particular example --- it doesn't give you any hint about what the
>> purpose of the operator is.

> USING might be just as content-free as WITH, but USING OPERATOR seems
> clearly better, at least IMO.

It's not enough better to justify the conflict with USING opclass, IMO.

An idea that just struck me is CHECK WITH, ie

EXCLUSION (expr CHECK WITH operator)

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] operator exclusion constraints [was: generalized index constraints]

2009-09-27 Thread Robert Haas
On Wed, Sep 23, 2009 at 1:47 PM, Tom Lane  wrote:
> Jeff Davis  writes:
>> We can either eliminate the USING variant from opt_class (unless it's
>> necessary for some reason or I missed it in the documentation), or we
>> can use another word (e.g. WITH or WITH OPERATOR) if you don't like
>> CHECK.
>
> Hmm ... we don't seem to have documented the USING noise-word, so it
> probably would be safe to remove it; but why take a chance?  I don't
> particularly agree with Peter's objection to CHECK.  There are plenty
> of examples in SQL of the same keyword being used for different purposes
> in nearby places.  Indeed you could make about the same argument to
> object to USING, since it'd still be there in "USING access_method"
> elsewhere in the same command.
>
> I think that USING is just about as content-free as WITH in this
> particular example --- it doesn't give you any hint about what the
> purpose of the operator is.

USING might be just as content-free as WITH, but USING OPERATOR seems
clearly better, at least IMO.

Also, this patch has not been updated in a week, and the clock is
ticking: if we don't have an updated version RSN, we need to move this
to Returned with Feedback and wait until next CommitFest.  That would
be too bad; this is an awesome feature.

Thanks,

...Robert

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


Re: [HACKERS] [PATCH] Largeobject access controls

2009-09-27 Thread Robert Haas
2009/9/24 KaiGai Kohei :
> The attached patch is revised from the previous revision at the following 
> points:

Jaime,

Do you think this is Ready for Committer review at this point?  If so,
please mark it that way; otherwise, what do you think are the
outstanding issues?

...Robert

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


Re: [HACKERS] [PATCH] Reworks for Access Control facilities (r2311)

2009-09-27 Thread Robert Haas
2009/9/24 KaiGai Kohei :
> I noticed that the previous patch (r2311) fails to apply on the CVS HEAD.
> The attached patch is only rebased to the latest CVS HEAD, without any
> other changes.

Stephen,

Are you planning to post a review for this?  We are 12 days into the
CommitFest so we need to give KaiGai some feedback soon.

Thanks,

...Robert

-- 
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] GRANT ON ALL IN schema

2009-09-27 Thread Robert Haas
2009/9/21 Petr Jelinek :
> Abhijit Menon-Sen wrote:
>
> I have not yet been able to do a complete review of this patch, but I am
> posting this because I'll be travelling for a week starting tomorrow. My
> comments are based mostly on reading the patch, and not on any intensive
> testing of the feature. I have left the patch status unchanged at "needs
> review", although I think it's close to "ready for committer".
>
>
> Thanks for your review.
>
> 1. The patch did apply to HEAD and build cleanly, but there are now a
>couple of minor (documentation) conflicts. (Sorry, I would have fixed
>them and reposted a patch, but I'm running out of time right now.)
>
>
> I fixed those conflicts in attached patch.
>
>
>
> *** a/doc/src/sgml/ref/grant.sgml
> --- b/doc/src/sgml/ref/grant.sgml
> [...]
>
> 
> +There is also the possibility of granting permissions to all objects of
> +given type inside one or multiple schemas. This functionality is
> supported
> +for tables, views, sequences and functions and can done by using
> +ALL {TABLES|SEQUENCES|FUNCTIONS} IN SCHEMA schemaname syntax in place
> +of object name.
> +   
> +
> +   
>
>
> 2. Here I suggest the following wording:
>
> 
> You can also grant permissions on all tables, sequences, or
> functions that currently exist within a given schema by specifying
> "ALL {TABLES|SEQUENCES|FUNCTIONS} IN SCHEMA schemaname" in place of
> an object name.
> 
>
> 3. I believe MySQL's "grant all privileges on foo.* to someone" grants
>privileges on all existing objects in foo _but also_ on any objects
>that may be created later. This patch only gives you a way to grant
>privileges only on the objects currently within a schema. I strongly
>prefer this behaviour myself, but I do think the documentation needs
>a brief mention of this fact, to avoid surprising people. That's why
>I added "that currently exist" to (2), above. Maybe another sentence
>that specifically says that objects created later are unaffected is
>in order. I'm not sure.
>
>
> I'll leave the exact wording to commiter, but in the attached patch I
> changed it to say "all existing objects" instead of "all objects".
>
> Except for above two changes and the fact that it's against current head,
> the patch is exactly the same.

Abhijit,

If this patch looks good now, can you mark it Ready for Committer in
the CommitFest app?  If there are any remaining issues, please post a
further review.

Thanks,

...Robert

-- 
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] Using results from INSERT ... RETURNING

2009-09-27 Thread Tom Lane
Robert Haas  writes:
> Heh.  I was actually asking an even stupider question, which is why do
> we need to keep all of them in ANY centrally known data structure?
> What operation do we perform that requires us to find all of the
> exstant TTS?

ExecDropTupleTable is used to release slot-related buffer pins and
tupdesc refcounts at conclusion of a query.  I suppose we could require
the individual plan node End routines to do it instead.  Not sure if
that's an improvement.

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] Using results from INSERT ... RETURNING

2009-09-27 Thread Robert Haas
On Sun, Sep 27, 2009 at 12:40 AM, Tom Lane  wrote:
> Robert Haas  writes:
>> Well, part of the problem is that I've not had a lot of luck trying to
>> understand how the executor really works (what's a tuple table slot
>> and why do we need to know in advance how many of them there are?).
>
> You know, that's actually a really good question.  There is not, as far
> as I can see, much advantage to keeping the Slots in an array.  We could
> perfectly well keep them in a List and eliminate the notational overhead
> of having to count them in advance.  There'd be a bit more palloc
> overhead (for the list cells) but avoiding the counting work would more
> or less make up for that.

Heh.  I was actually asking an even stupider question, which is why do
we need to keep all of them in ANY centrally known data structure?
What operation do we perform that requires us to find all of the
exstant TTS?

...Robert

-- 
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] Hot Standby 0.2.1

2009-09-27 Thread Heikki Linnakangas
TransactionIdIsInProgress() doesn't consult the known-assigned-xids
structure. That's a problem: in the standby, TransactionIdIsInProgress()
can return false for a transaction that is still running in the master.
HeapTupleSatisfies* functions can incorrectly set HEAP_XMIN/XMAX_INVALID
hint bits for xids that commit later on.

-- 
  Heikki Linnakangas
  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] Hot Standby 0.2.1

2009-09-27 Thread Heikki Linnakangas
The locking in smgr_redo_commit and smgr_redo_abort doesn't look right.
First of all, smgr_redo_abort is not holding XidGenLock and
ProcArrayLock while modifying ShmemVariableCache->nextXid and
ShmemVariableCache->latestCompletedXid, respectively, like
smgr_redo_commit is. Attached patch fixes that.

But I think there's a little race condition in there anyway, as they
remove the finished xids from known-assigned-xids list by calling
ExpireTreeKnownAssignedTransactionIds, and
ShmemVariableCache->latestCompletedXid is updated only after that. We're
not holding ProcArrayLock between those two steps, like we do during
normal transaction commit. I'm not sure what kind of issues that can
lead to, but it seems like it can lead to broken snapshots if someone
calls GetSnapshotData() between those steps.

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com
>From 9d6ef0a3eb901b1d2182b3f2efd5c75d52e88cba Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas 
Date: Sun, 27 Sep 2009 14:51:43 +0300
Subject: [PATCH] Make locking in smgr_redo_abort reflect that in smgr_redo_commit.

---
 src/backend/access/transam/xact.c |   22 --
 1 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index a696857..92a7c43 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -4502,10 +4502,7 @@ xact_redo_commit(xl_xact_commit *xlrec, TransactionId xid,
 		/*
 		 * Release locks, if any. We do this for both two phase and normal
 		 * one phase transactions. In effect we are ignoring the prepare
-		 * phase and just going straight to lock release. This explains
-		 * why the twophase_postcommit_standby_callbacks[] do not invoke
-		 * a special routine to handle locks - that is performed here
-		 * instead.
+		 * phase and just going straight to lock release.
 		 */
 		RelationReleaseRecoveryLockTree(xid, xlrec->nsubxacts, sub_xids);
 	}
@@ -4593,12 +4590,25 @@ xact_redo_abort(xl_xact_abort *xlrec, TransactionId xid)
 	}
 
 	/* Make sure nextXid is beyond any XID mentioned in the record */
+	/* We don't expect anyone else to modify nextXid, hence we
+	 * don't need to hold a lock while checking this. We still acquire
+	 * the lock to modify it, though.
+	 */
 	if (TransactionIdFollowsOrEquals(max_xid,
 	 ShmemVariableCache->nextXid))
 	{
+		LWLockAcquire(XidGenLock, LW_EXCLUSIVE);
 		ShmemVariableCache->nextXid = max_xid;
-		ShmemVariableCache->latestCompletedXid = ShmemVariableCache->nextXid;
-		TransactionIdAdvance(ShmemVariableCache->nextXid);
+		LWLockRelease(XidGenLock);
+	}
+
+	/* Same here, don't use lock to test, but need one to modify */
+	if (TransactionIdFollowsOrEquals(max_xid,
+	 ShmemVariableCache->latestCompletedXid))
+	{
+		LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
+		ShmemVariableCache->latestCompletedXid = max_xid;
+		LWLockRelease(ProcArrayLock);
 	}
 
 	/* Make sure files supposed to be dropped are dropped */
-- 
1.6.3.3


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


Re: [HACKERS] [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server

2009-09-27 Thread Jim Cox
On Sat, Sep 26, 2009 at 6:48 PM, David Fetter  wrote:
> On Sat, Sep 26, 2009 at 11:02:55PM +0300, Peter Eisentraut wrote:
>> On Fri, 2009-09-25 at 16:59 -0400, Tom Lane wrote:
>> > "shakahsha...@gmail.com"  writes:
>> > > From pg_dump/pg_restore section (9.2 of the Todo page on the
>> > > PostgreSQL Wiki), is the following item "Add comments to output
>> > > indicating version of pg_dump and of the database server" simply
>> > > asking for a change to the pg_dump header from:
>> >
>> > I think so, but what's not clear is whether this is a good idea to
>> > do in the default output.  It might only be appropriate in
>> > "verbose" mode, so as not to introduce unnecessary diffs between
>> > logically identical dumps.
>>
>> Well, a diff of the same database made by different (major) versions
>> of pg_dump will already be different in most situations, so adding
>> the pg_dump version number in it is essentially free from this
>> perspective.
>>
>> What is the use case for adding the server version?
>
> There have been cases where pg_restore doesn't fix infelicities.  For
> example, there was a time when it was a good idea to run adddepend
> after the reload.  Knowing what server version the dump came from
> could be handy for this kind of case.
>
>> I can imagine something like wanting to know exactly where the dump
>> came from, but then host name and such would be better.  (And then
>> you can infer the server version from that.)
>
> You can infer the server version until the next upgrade, at which
> point the information is lost.
>
> Cheers,
> David.

Attached s/b a patch for the 8.5 TODO "Add comments to output indicating version
of pg_dump and of the database server" (pg_dump/pg_restore section, 9.2).

In verbose mode, pg_dump version and remote database version now appear
in the "plain" output format.

For example, original verbose output:
--
-- PostgreSQL database dump
--

-- Started on 2009-09-27 11:05:52 UTC

SET statement_timeout = 0;
[...etc...]


New verbose output:
--
-- PostgreSQL database dump
--

--
-- pg_dump version: 8.5devel
--
-- remote database version: 8.1.3 (80103)
--

-- Started on 2009-09-27 11:05:52 UTC

SET statement_timeout = 0;
[...etc...]
Index: src/bin/pg_dump/pg_backup_archiver.c
===
RCS file: /projects/cvsroot/pgsql/src/bin/pg_dump/pg_backup_archiver.c,v
retrieving revision 1.175
diff -c -r1.175 pg_backup_archiver.c
*** src/bin/pg_dump/pg_backup_archiver.c	7 Aug 2009 22:48:34 -	1.175
--- src/bin/pg_dump/pg_backup_archiver.c	27 Sep 2009 11:26:37 -
***
*** 301,306 
--- 301,315 
  	ahprintf(AH, "--\n-- PostgreSQL database dump\n--\n\n");
  
  	if (AH->public.verbose)
+ 	{
+ 		ahprintf(AH, "--\n-- pg_dump version: %s\n", PG_VERSION);
+ 		ahprintf(AH, "--\n-- remote database version: %s (%d)\n"
+ 			,AHX->remoteVersionStr
+ 			,AHX->remoteVersion) ;
+ 		ahprintf(AH, "--\n\n");
+ 	}
+ 
+ 	if (AH->public.verbose)
  		dumpTimestamp(AH, "Started on", AH->createDate);
  
  	if (ropt->single_txn)


-- 
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] Hot Standby on git

2009-09-27 Thread Heikki Linnakangas
Per Simon's request, for the benefit of the archive, here's all the
changes I've done on the patch since he posted the initial version for
review for this commitfest as incremental patches. This is extracted
from my git repository at
git://git.postgresql.org/git/users/heikki/postgres.git.

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com


hs-riggs-branch-20090927.tar.gz
Description: GNU Zip compressed data

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