Re: [HACKERS] pl/perl and utf-8 in sql_ascii databases

[Kyotaro HORIGUCHI]

Hello,

> > I've been stuck in mud trying to plperl work on windows
> > environment. I saw many messages complaining that plperl wouldn't
> > be built to work. For the convenience of those and myself, I
> > describe the process of building postgresql with plperl on
> > Windows with cygwin and VC++ I've done below.
> 
> Hrm, I don't develop on windows here, but out of curiosity, what were
> the messages like?

My memory about that has already become faint.. As far as I
remember, I saw two patterns of crash.

One is caused by gcc-4's stack-protector in cygperl5_10.dll. It
caused crash on "create function", (or create language). Building
postgresql with gcc-4 did not help for me. Finally, I gave up to
use pre-installed dll and built all including perl with GCC-3 to
make it work.

The another is 0xC005 (Access Violation) on 'create language
plperl' for VC10(:-p) vs ActivePerl5.14. This happenend at ERRSV
in plperl_(un)trasted_init(). Replacing ERRSV with
get_sv("@",FALSE) had put down that (but also I don't know if it
works) but finally I had a error "didn't get a CODE reference
from compiling function" on "create function .. language plperl"
which was the sign of dead end for me. I decided to behave well
to use ActivePerl5.12 and VC8 at last. I suppose this is a kind
of so-called "DLL HELL" related to memory allocation. ActivePerl
5.12 links the system's msvcrt.dll but VC links its output with
msvcrxx.dll. MS says memory allocaltion/deallocation across DLL
bounary should cause crash. But I don't know why the pair of
AP5.12 and VC8 results in success.

http://msdn.microsoft.com/en-us/library/ms235460.aspx

badalex> >> - The remainder of the patch whic fixes the easy fixable leakes
badalex> >>   of palloc'ed memory won't be ported into 9.1. This is only for
badalex> >>   9.3dev.
badalex> 
badalex> > What should I do for this?
badalex> 
badalex> Just let the commiter decide? :-)

Agreed.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

== My e-mail address has been changed since Apr. 1, 2012.

-- 
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] enhanced error fields

[Pavel Stehule]

Hello

* fixed typo
* support two new fields: constraint_table and trigger_table
* routine_table, routine_schema, trigger_name, trigger_table,
trigger_schema has value when exception coming from plpgsql.

Regards

Pavel


2012/7/2 Peter Geoghegan :
> On 2 July 2012 15:19, Peter Geoghegan  wrote:
>> On 9 May 2012 14:33, Pavel Stehule  wrote:
>>> here is patch with enhancing ErrorData structure. Now constraints
>>> errors and RI uses these fields
>>
>> So I took a look at the patch eelog-2012-05-09.diff today. All of the
>> following remarks apply to it alone.
>
> I decided to follow through and take a look at
> eelog-plpgsql-2012-05-09.diff today too, while I have all of this
> swapped into my head.
>
> This patch is not an atomic unit - it builds upon the first patch. I
> successfully merged the local feature branch that I'd created for
> eelog-2012-05-09.diff without any merge conflicts, and I can build
> Postgres and get the regression tests to pass (including a couple of
> new ones, for this added functionality for plggsql - the functionality
> is testing exclusively using the new (9.2) "get stacked diagnostics"
> and "raise custom exception 'some_custom_exception' using"
> feature).
>
> Since that feature branch had all my revisions committed, my
> observations about redundancies in the other base patch still stand -
> the 2 functions mentioned did not exist for the benefit of this
> further patch either.
>
> There is a typo here:
>
> +   case PLPGSQL_RAISEOPTION_TRIGGER_SCHEMA:
> +   printf("TRIGGER_SCHENA = ");
> +   break;
> }
>
>
> I'm not sure about this inconsistency within unreserved_keyword:
>
> For routines:
> +   | K_DIAG_ROUTINE_NAME
> +   | K_DIAG_ROUTINE_SCHEMA
> 
>
> For triggers:
> +   | K_DIAG_TRIGGER_NAME
> +   | K_DIAG_TRIGGER_SCHEMA
> 
>
> For tables:
> +   | K_DIAG_SCHEMA_NAME
> .
> . **SNIP**
> .
> +   | K_DIAG_TABLE_NAME
>
> The same inconsistency exists within the anonymous enum that contains
> PLPGSQL_GETDIAG_TABLE_NAME (and other constants), as well as the new
> token keywords within plpgsql's gram.y .
>
> The doc changes need a little work here too.
>
> I'm not sure that I agree with the extensive use of the term "routine"
> in all of these constants - sure, information_schema has a view called
> "routines". But wouldn't it be more appropriate to use a
> Postgres-centric term within our own code?
>
> So, what about the concern about performance taking a hit when plpgsql
> exception blocks are entered as a result of this patch? Well, while I
> think that  an effort to reduce the overhead of PL exception handling
> would be worthwhile, these patches do not appear to alter things
> appreciable (though the overhead *is* measurable):
>
> [peter@peterlaptop eelog]$ ls
> exceptions.sql  test_eelog_outer.sql
>
> Patch (eelog-plpgsql):
>
> [peter@peterlaptop eelog]$ pgbench -T 300 -f exceptions.sql -c 10 -n
> transaction type: Custom query
> scaling factor: 1
> query mode: simple
> number of clients: 10
> number of threads: 1
> duration: 300 s
> number of transactions actually processed: 305756
> tps = 1019.026055 (including connections establishing)
> tps = 1019.090135 (excluding connections establishing)
>
> Master:
>
> [peter@peterlaptop eelog]$ pgbench -T 300 -f exceptions.sql -c 10 -n
> transaction type: Custom query
> scaling factor: 1
> query mode: simple
> number of clients: 10
> number of threads: 1
> duration: 300 s
> number of transactions actually processed: 308376
> tps = 1027.908182 (including connections establishing)
> tps = 1027.977879 (excluding connections establishing)
>
> An archive with simple scripts for repeating this are attached, if
> anyone is interested.
>
> --
> Peter Geoghegan   http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training and Services


eelog-plpgsql-2012-07-06.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] autocomplete - SELECT fx

[Pavel Stehule]

2012/7/6 Josh Kupershmidt :
> On Mon, Jul 2, 2012 at 1:13 AM, Pavel Stehule  wrote:
>
>> I tested Peter's patch and it works well.
>
> I liked it as well. But I'm not sure what should happen with the patch
> now. It seems like it'd be commit-ready with just a tweak or two to
> the query as I noted in my last mail, but Tom did seem opposed[1] to
> the idea in the first thread, and no one else has spoken up recently
> in favor of the idea, so maybe it should just be marked Rejected or
> Returned with Feedback?

Hard to say - it is usable, but on second hand - some user can be
surprised, because it works only for first item in list.

Regards

Pavel

>
> Josh
>
> [1] http://archives.postgresql.org/message-id/13612.1328887227%40sss.pgh.pa.us

-- 
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: add conversion from pg_wchar to multibyte

[Robert Haas]

On Thu, Jul 5, 2012 at 8:46 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Thu, Jul 5, 2012 at 7:11 PM, Tom Lane  wrote:
>>> Hm, several of these routines seem to neglect to advance the "from"
>>> pointer?
>
>> Err... yeah.  That's not a bug I introduced, but I should have caught
>> it... and it does make me wonder how well this code was tested.
>
>> Does the attached look like an appropriate fix?
>
> I'd be inclined to put the from++ and len-- at the bottom of each loop,
> and in that order every time, just for consistency and obviousness.
> But yeah, that's basically what's needed.

OK, I've committed a slightly tweaked version of that patch.

-- 
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] Event Triggers reduced, v1

[Robert Haas]

On Thu, Jul 5, 2012 at 5:31 PM, Dimitri Fontaine  wrote:
> [ new patch ]

Attached is a incremental patch with a bunch of minor cleanups,
including reverts of a few spurious white space changes.  Could you
merge this into your version?

I have some concerns about pg_dump:

1. Can we spell out EvtTriggerInfo, getEvtTriggers, and dumpEvtTrigger
as EventTriggerInfo, getEventTriggers, and dumpEventTrigger?

2. I don't think that this code properly handles older server
versions.  First, the version test in getEvtTriggers checks against
90200 instead of 90300.  Second, when running against a too-old server
version, this is going to try to execute an empty query and then parse
the results.  I think you want to restructure this code so that it
just plain old returns if the server is too old.  See for example
getForeignDataWrappers.

3. The way you're generating evtfname is unsafe if either the schema
or the function name contains SQL characters.  I think this should
probably be casting the function OID to regproc in lieu of rolling its
own formatting code.  Also, the tags should probably be escaped using
quote_literal, just to be future-proof.

4. I think we should aim to generate all the SQL in upper case.  Right
now "CREATE EVENT TRIGGER" and "EXECUTE PROCEDURE" are in upper case
but "when tag in" is in lower case.

In psql, I think we should similarly have listEventTriggers() rather
than listEvtTriggers(); here as in pg_dump I think you should cast the
evtfoid to regproc to get schema-qualification and escaping, in lieu
of the explicit join.

>> In terms of the schema itself, I think we are almost there, but:
>>
>> - I noticed while playing around with this that pg_dump emits a
>> completely empty owner field when dumping an event trigger.  At first
>> I thought that was just an oversight, but then I realized there's a
>> deeper reason for it: pg_event_trigger doesn't have an owner field.  I
>> think it should.  The only other objects in the system that don't have
>> owners are text search parsers and text search templates (and casts,
>> sort of).  It might seem redundant to have an owner even when
>> event-triggers are superuser-only, but we might want to try to relax
>> that restriction later.  Note that foreign data wrappers, which are
>> also superuser-create-only, do have an owner.  (Note that if we give
>> event triggers an owner, then we also need ALTER .. OWNER TO support
>> for them.)
>
> Damn, I had it on my TODO and Álvaro hinted me already, and I kept
> forgetting about it nonetheless. Fixed now.

evtowner seems to have missed the documentation bus.

With respect to the documentation, keep in mind that the synopsis is
going to show up in the command line help for \h.  I'm thinking that
the full list of command tags is too long for that, and that we should
instead rearrange the page so that the list of supported commands is
outside the synopsis.  The synposis is also somewhat misleading, I
think, in that "variable in (tag, ...)" conveys the idea that no
matter what the variable is, the items in parentheses will surely be
tags.  I suggest that we say something like "filter_variable in
(filter_value, ...)" and then document in the text that
filter_variable must currently always be TAG, and that the legal
values for filter_value are dependent on the choice of
filter_variable, and the legal choices for TAG are those listed in the
following table: .

The documentation contains the following claim with which I'm
extremely uncomfortable:

+ 
+  Triggers on ANY command support more commands than
+  just this list, and will only provide the command
+  tag argument as NOT NULL. Supporting more
+  commands is made so that you can actually block 
+  commands in one go.
+ 

A minor issue is that there's no notion of ANY any more; it's just a
consequence of leaving out the WHEN clause.  The bigger issue is that
I can't see any reason to do it that way.  Surely if we're firing the
trigger at all, we can arrange to have the command tag properly filled
in so that we can filter by it.

This might be a crazy idea, but... it seems like it would be awfully
sweet if we could find a way to avoid having to translate between node
tags (i.e. constants beginning with T_* that are used to identify the
type of statement that is executing) and event tags (i.e. constants
beginning with E_* that are used to identify the type of statement
that is executing).  Now obviously this is not quite possible because
in some cases the choice of E_* constant depends on not only the node
tag but also the type of object being operated on.  However, it seems
like this is a surmountable obstacle: since we no longer need to store
the E_* constants in a system catalog, they don't really need to be
integers.  For example, we could define something like this:

typedef struct
{
NodeTag nodetag;
ObjectType objecttype;
} NodeTagWithObjectType;

...and set the objecttype to a new OBJECT_DUMMY value or somesuch when
the 

Re: [HACKERS] pl/perl and utf-8 in sql_ascii databases

[Alex Hunsaker]

On Tue, Jul 3, 2012 at 2:59 AM, Kyotaro HORIGUCHI
 wrote:
> Hello, Here is regression test runs on pg's also built with
> cygwin-gcc and VC++.

Thank you!

> The patches attached following,
>
> - plperl_sql_ascii-4.patch : fix for pl/perl utf8 vs sql_ascii
> - plperl_sql_ascii_regress-1.patch : regression test for this patch.
>  I added some tests on encoding to this.
>
> I will mark this patch as 'ready for committer' after this.

Both patches look good to me..

> I've been stuck in mud trying to plperl work on windows
> environment. I saw many messages complaining that plperl wouldn't
> be built to work. For the convenience of those and myself, I
> describe the process of building postgresql with plperl on
> Windows with cygwin and VC++ I've done below.

Hrm, I don't develop on windows here, but out of curiosity, what were
the messages like?

>> - The remainder of the patch whic fixes the easy fixable leakes
>>   of palloc'ed memory won't be ported into 9.1. This is only for
>>   9.3dev.

> What should I do for this?

Just let the commiter decide? :-)

-- 
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] autocomplete - SELECT fx

[Josh Kupershmidt]

On Mon, Jul 2, 2012 at 1:13 AM, Pavel Stehule  wrote:

> I tested Peter's patch and it works well.

I liked it as well. But I'm not sure what should happen with the patch
now. It seems like it'd be commit-ready with just a tweak or two to
the query as I noted in my last mail, but Tom did seem opposed[1] to
the idea in the first thread, and no one else has spoken up recently
in favor of the idea, so maybe it should just be marked Rejected or
Returned with Feedback?

Josh

[1] http://archives.postgresql.org/message-id/13612.1328887227%40sss.pgh.pa.us

-- 
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: add conversion from pg_wchar to multibyte

[Tatsuo Ishii]

> Tatsuo Ishii  writes:
>>> So far as I can see, the only LCPRVn marker code that is actually in
>>> use right now is 0x9d --- there are no instances of 9a, 9b, or 9c
>>> that I can find.
>>> 
>>> I also read in the xemacs internals doc, at
>>> http://www.xemacs.org/Documentation/21.5/html/internals_26.html#SEC145
>>> that XEmacs thinks the marker code for private single-byte charsets
>>> is 0x9e (only) and that for private multi-byte charsets is 0x9f (only);
>>> moreover they think 0x9a-0x9d are potential future official multibyte
>>> charset codes.  I don't know how we got to the current state of using
>>> 0x9a-0x9d as private charset markers, but it seems pretty inconsistent
>>> with XEmacs.
> 
>> At the time when mule internal code was introduced to PostgreSQL,
>> xemacs did not have multi encoding capabilty and mule (a patch to
>> emacs) was the only implementation allowed to use multi encoding. So I
>> used the specification of mule documented in the URL I wrote.
> 
> I see.  Given that upstream has decided that a simpler definition is
> more appropriate, is there any reason not to follow their lead, to the
> extent that we can do so without breaking existing on-disk data?

Please let me spend week end to understand the their latest spec.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp

-- 
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: add conversion from pg_wchar to multibyte

[Tom Lane]

Robert Haas  writes:
> On Thu, Jul 5, 2012 at 7:11 PM, Tom Lane  wrote:
>> Hm, several of these routines seem to neglect to advance the "from"
>> pointer?

> Err... yeah.  That's not a bug I introduced, but I should have caught
> it... and it does make me wonder how well this code was tested.

> Does the attached look like an appropriate fix?

I'd be inclined to put the from++ and len-- at the bottom of each loop,
and in that order every time, just for consistency and obviousness.
But yeah, that's basically what's needed.

regards, tom lane

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

Re: [HACKERS] Patch: add conversion from pg_wchar to multibyte

[Robert Haas]

On Thu, Jul 5, 2012 at 7:11 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Sun, Jul 1, 2012 at 5:11 AM, Alexander Korotkov  
>> wrote:
>>> [ new patch ]
>
>> With the improved comments in pg_wchar.h, it seemed clear what needed
>> to be done here, so I fixed up the MULE conversion and committed this.
>>  I'd appreciate it if someone would check my work, but I think it's
>> right.
>
> Hm, several of these routines seem to neglect to advance the "from"
> pointer?

Err... yeah.  That's not a bug I introduced, but I should have caught
it... and it does make me wonder how well this code was tested.

Does the attached look like an appropriate fix?

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


wchar-advance-from.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] obsolete copyright notice

[Bruce Momjian]

On Thu, Jul 05, 2012 at 10:10:08PM +0200, Antonin Houska wrote:
> I found out by chance that \copyright command still contains 2011.
> Perhaps documentation of "new year's day changes" needs to be
> updated, if such exists.

What version of Postgres is this?  We don't update the copyright for
minor releases, so my bet is it is 9.1 because that was released in
2011.

-- 
  Bruce Momjian  http://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] Patch: add conversion from pg_wchar to multibyte

[Tom Lane]

Tatsuo Ishii  writes:
>> So far as I can see, the only LCPRVn marker code that is actually in
>> use right now is 0x9d --- there are no instances of 9a, 9b, or 9c
>> that I can find.
>> 
>> I also read in the xemacs internals doc, at
>> http://www.xemacs.org/Documentation/21.5/html/internals_26.html#SEC145
>> that XEmacs thinks the marker code for private single-byte charsets
>> is 0x9e (only) and that for private multi-byte charsets is 0x9f (only);
>> moreover they think 0x9a-0x9d are potential future official multibyte
>> charset codes.  I don't know how we got to the current state of using
>> 0x9a-0x9d as private charset markers, but it seems pretty inconsistent
>> with XEmacs.

> At the time when mule internal code was introduced to PostgreSQL,
> xemacs did not have multi encoding capabilty and mule (a patch to
> emacs) was the only implementation allowed to use multi encoding. So I
> used the specification of mule documented in the URL I wrote.

I see.  Given that upstream has decided that a simpler definition is
more appropriate, is there any reason not to follow their lead, to the
extent that we can do so without breaking existing on-disk data?

regards, tom lane

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

Re: [HACKERS] Patch: add conversion from pg_wchar to multibyte

[Tom Lane]

Robert Haas  writes:
> On Sun, Jul 1, 2012 at 5:11 AM, Alexander Korotkov  
> wrote:
>> [ new patch ]

> With the improved comments in pg_wchar.h, it seemed clear what needed
> to be done here, so I fixed up the MULE conversion and committed this.
>  I'd appreciate it if someone would check my work, but I think it's
> right.

Hm, several of these routines seem to neglect to advance the "from"
pointer?

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] Schema version management

[Tom Lane]

Christopher Browne  writes:
> On Thu, Jul 5, 2012 at 5:52 PM, Dimitri Fontaine  
> wrote:
>> Tom Lane  writes:
>>> FWIW, I'm attracted to the all-similarly-named-functions-together
>>> method, mainly because it dodges the problem of how to encode a
>>> function's argument list into a filename.  However, we're being
>>> short-sighted to only think of functions here.  What about operators?
>>> Or casts?  Those don't have simple names either.

>> I would argue like Álvaro that when dealing with operators and casts
>> you're probably writing an extension already, and we're providing
>> another way to deal with that.

> Indeed.  Is this something we ought to document as a recommendation?

This argument seems a bit irrelevant to me.  pg_dump doesn't get to pick
and choose what will be in the database it's told to dump.  If we're
going to do something like what Joel wants, we have to have file naming
conventions for operator and cast objects.  So we can't just leave them
out of the conversation (or if we do, we shouldn't be surprised when the
ensuing design sucks).

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] Schema version management

[Christopher Browne]

On Thu, Jul 5, 2012 at 5:52 PM, Dimitri Fontaine  wrote:
> Alvaro Herrera  writes:
>> However I am also against what seems to be the flow.  Normally, you
>> don't write overloaded plpgsql functions such as "equal".  Case in
>> point, the equality functions in core have funny names like "int4eq" and
>> so on.  Instead, at least in my experience, the overloaded functions
>> people seem to have in their databases are like do_stuff_to_foobars()
>> and you have one version for foos and another one for bars.
>
> +1
>
> I too want to have my overloaded functions all in the same file, as much
> as to have made that the only behavior in getddl.py:

That seems pretty appropriate to me.

The converse makes my head hurt...

If I have a bunch of overloaded functions, whose definitions *aren't*
really related, are we competing for the "obfuscated PostgreSQL"
contest?

In practice, that sounds like something I'd want to add to my list of
"fire people that do this!" Bad Practices.

>> If you're doing lots of equality functions, surely it would make more
>> sense to package them up as an extension anyway along with all the other
>> thingies you need for the type you're supposedly writing.  So it's a
>> completely different market than what we're aiming at here.
>
> +1
+1

> Tom Lane  writes:
>> FWIW, I'm attracted to the all-similarly-named-functions-together
>> method, mainly because it dodges the problem of how to encode a
>> function's argument list into a filename.  However, we're being
>> short-sighted to only think of functions here.  What about operators?
>> Or casts?  Those don't have simple names either.
>
> I would argue like Álvaro that when dealing with operators and casts
> you're probably writing an extension already, and we're providing
> another way to deal with that.

Indeed.  Is this something we ought to document as a recommendation?
It's not exactly reference material, but if it's a good practice,
perhaps it should be in the manuals somewhere...
-- 
When confronted by a difficult problem, solve it by reducing it to the
question, "How would the Lone Ranger handle this?"

-- 
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] Re: [COMMITTERS] pgsql: Fix mapping of PostgreSQL encodings to Python encodings.

[Jan Urbański]

On 05/07/12 23:30, Peter Eisentraut wrote:

On tor, 2012-07-05 at 22:53 +0200, Jan Urbański wrote:

The problem is that PLyUnicode_Bytes is (via an ifdef) used as
PyString_ToString on Python3, which means that there are numerous call
sites and new ones might appear in any moment. I'm not that keen on
invoking the traceback machinery on low-level encoding errors.


Why not?


Because it can lead to recursion errors, like the one this patch was 
supposed to fix. The traceback machinery calls into the encoding 
functions, because it converts Python strings (like function names) into 
C strings.


--
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] Schema version management

[Dimitri Fontaine]

Alvaro Herrera  writes:
> However I am also against what seems to be the flow.  Normally, you
> don't write overloaded plpgsql functions such as "equal".  Case in
> point, the equality functions in core have funny names like "int4eq" and
> so on.  Instead, at least in my experience, the overloaded functions
> people seem to have in their databases are like do_stuff_to_foobars()
> and you have one version for foos and another one for bars.

+1

I too want to have my overloaded functions all in the same file, as much
as to have made that the only behavior in getddl.py:

  https://github.com/dimitri/getddl

> If you're doing lots of equality functions, surely it would make more
> sense to package them up as an extension anyway along with all the other
> thingies you need for the type you're supposedly writing.  So it's a
> completely different market than what we're aiming at here.

+1

Tom Lane  writes:
> FWIW, I'm attracted to the all-similarly-named-functions-together
> method, mainly because it dodges the problem of how to encode a
> function's argument list into a filename.  However, we're being
> short-sighted to only think of functions here.  What about operators?
> Or casts?  Those don't have simple names either.

I would argue like Álvaro that when dealing with operators and casts
you're probably writing an extension already, and we're providing
another way to deal with that.

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] Re: [COMMITTERS] pgsql: Fix mapping of PostgreSQL encodings to Python encodings.

[Peter Eisentraut]

On tor, 2012-07-05 at 22:53 +0200, Jan Urbański wrote:
> The problem is that PLyUnicode_Bytes is (via an ifdef) used as 
> PyString_ToString on Python3, which means that there are numerous call
> sites and new ones might appear in any moment. I'm not that keen on 
> invoking the traceback machinery on low-level encoding errors.

Why not?


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

[HACKERS] Re: [COMMITTERS] pgsql: Fix mapping of PostgreSQL encodings to Python encodings.

[Jan Urbański]

On 05/07/12 22:37, Heikki Linnakangas wrote:

On 05.07.2012 23:31, Tom Lane wrote:

Heikki Linnakangas writes:

Fix mapping of PostgreSQL encodings to Python encodings.


The buildfarm doesn't like this --- did you check for side effects on
regression test results?


Hmm, I ran the regressions tests, but not with C encoding. With the
patch, you no longer get the errdetail you used to, when an encoding
conversion fails:


***
*** 41,47 

SELECT unicode_plan1();
ERROR: spiexceptions.InternalError: could not convert Python Unicode
object to PostgreSQL server encoding
- DETAIL: UnicodeEncodeError: 'ascii' codec can't encode character
u'\x80' in position 0: ordinal not in range(128)
CONTEXT: Traceback (most recent call last):
PL/Python function "unicode_plan1", line 3, in 
rv = plpy.execute(plan, [u"\x80"], 1)
--- 39,44 


We could just update the expected output, there's two expected outputs
for this test case and one of them is now wrong. But it'd actually be
quite a shame to lose that extra information, that's quite valuable.
Perhaps we should go back to using PLu_elog() here, and find some other
way to avoid the recursion.


Seems that the problem is that the LC_ALL=C makes Postgres use SQL_ASCII 
as the database encoding and as the comment states, translating PG's 
SQL_ASCII to Python's "ascii" is not ideal.


The problem is that PLyUnicode_Bytes is (via an ifdef) used as 
PyString_ToString on Python3, which means that there are numerous call 
sites and new ones might appear in any moment. I'm not that keen on 
invoking the traceback machinery on low-level encoding errors.


Hm, since PyUnicode_Bytes should get a unicode object and return bytes 
in the server encoding, we might just say that for SQL_ASCII we 
arbitrarily choose UTF-8 to encode the unicode codepoints, so we'd just 
set serverenc = "utf-8" in the first switch case.


That doesn't solve the problem of the missing error detail, though.

Jan

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

[HACKERS] Re: [COMMITTERS] pgsql: Fix mapping of PostgreSQL encodings to Python encodings.

[Heikki Linnakangas]

On 05.07.2012 23:31, Tom Lane wrote:

Heikki Linnakangas  writes:

Fix mapping of PostgreSQL encodings to Python encodings.


The buildfarm doesn't like this --- did you check for side effects on
regression test results?


Hmm, I ran the regressions tests, but not with C encoding. With the 
patch, you no longer get the errdetail you used to, when an encoding 
conversion fails:



***
*** 41,47 

  SELECT unicode_plan1();
  ERROR:  spiexceptions.InternalError: could not convert Python Unicode object 
to PostgreSQL server encoding
- DETAIL:  UnicodeEncodeError: 'ascii' codec can't encode character u'\x80' in 
position 0: ordinal not in range(128)
  CONTEXT:  Traceback (most recent call last):
PL/Python function "unicode_plan1", line 3, in 
  rv = plpy.execute(plan, [u"\x80"], 1)
--- 39,44 


We could just update the expected output, there's two expected outputs 
for this test case and one of them is now wrong. But it'd actually be 
quite a shame to lose that extra information, that's quite valuable. 
Perhaps we should go back to using PLu_elog() here, and find some other 
way to avoid the recursion.


- Heikki

--
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] pg_dump: Sort overloaded functions in deterministic order

[Joel Jacobson]

Roger that. I'm on it.

On Thursday, July 5, 2012, Tom Lane wrote:

> Joel Jacobson > writes:
> You may in fact need a new field --- I'm just saying it should be in the
> object-type-specific struct, eg FuncInfo, not DumpableObject.
>
> regards, tom lane
>

Re: [HACKERS] [PATCH] pg_dump: Sort overloaded functions in deterministic order

[Tom Lane]

Joel Jacobson  writes:
> I agree, good suggestion, I just didn't know how to implement it without a
> new field. I'll make a new attempt to get it right.

You may in fact need a new field --- I'm just saying it should be in the
object-type-specific struct, eg FuncInfo, not DumpableObject.

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] obsolete copyright notice

[Antonin Houska]

I found out by chance that \copyright command still contains 2011.
Perhaps documentation of "new year's day changes" needs to be updated, 
if such exists.


Tony H.

--
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] plpython issue with Win64 (PG 9.2)

[Jan Urbański]

On 05/07/12 21:37, Heikki Linnakangas wrote:

Committed. This bug was present in versions >= 9.0, so backpatched.


Thanks!


I used ereport() rather than elog() in the error message. Correct me if
that was wrong, but the point was to avoid PLy_elog(), because that
might cause recursion, and ereport() should be ok. I believe the message
should be translated, as it's quite possible to get that error, at least
if you use SQL_ASCII, so ereport() is more approriate than elog().


Yes, you're absolutely right.

Cheers,
Jan

--
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] plpython issue with Win64 (PG 9.2)

[Heikki Linnakangas]

On 04.07.2012 15:11, Jan Urbański wrote:

On 04/07/12 13:58, Asif Naeem wrote:

I have test the patch on Win64. postgres server is working fine now for
WIN1252. Thanks.


create function enctest() returns text as $$
return b'tr\xc3\xb3spido'.decode('**utf-8')
$$ language plpython3u;

select enctest(), encode(convert_to(enctest(), 'utf-8'), 'hex');



create function enctest() returns text as $$
return b'tr\xc3\xb3spido'.decode('utf-8')
$$ language plpython3u;
select enctest(), encode(convert_to(enctest(), 'utf-8'), 'hex');
enctest | encode
--+
tróspido | 7472c3b3737069646f
(1 row)

Please do let me know If you have any other query. Thanks.


Great, this looks correct.

Can we apply this to 9.2?


Committed. This bug was present in versions >= 9.0, so backpatched.

I used ereport() rather than elog() in the error message. Correct me if 
that was wrong, but the point was to avoid PLy_elog(), because that 
might cause recursion, and ereport() should be ok. I believe the message 
should be translated, as it's quite possible to get that error, at least 
if you use SQL_ASCII, so ereport() is more approriate than elog().


Thanks!

--
  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] [PATCH] pg_dump: Sort overloaded functions in deterministic order

[Joel Jacobson]

I agree, good suggestion, I just didn't know how to implement it without a
new field. I'll make a new attempt to get it right.

On Thursday, July 5, 2012, Tom Lane wrote:

> Joel Jacobson > writes:
> > New version, made a typo in last one.
>
> I'm not particularly happy with the idea of adding a sortkey field to
> DumpableObject as such, when most object types don't need it.  That just
> bloats the code and pg_dump's memory consumption.  It would be better to
> modify the already-existing object-type-specific special cases in
> DOTypeNameCompare to take additional information into account as needed.
>
> BTW, I see no reason to be adding extra calls of
> pg_get_function_identity_arguments.  What is wrong with the funcsig or
> aggsig strings that the code already computes?
>
> regards, tom lane
>

[HACKERS] Re: proposal and patch : support INSERT INTO...RETURNING with partitioned table using rule

[John Lumby]

First,  apologies for taking so long to reply to your post.

On Fri, 22 Jun 2012 09:55:13, Robert Haas wrote:

> On Wed, Jun 20, 2012 at 12:24 PM, John Lumby  
> wrote:
> > An INSERT which has a RETURNING clause and which is to be rewritten 
> >based on
> > a rule will be accepted if the rule is an "unconditional DO INSTEAD".
> > In general I believe "unconditional" means "no WHERE clause", but in 
> >practice
> > if the rule is of the form
> >    CREATE RULE insert_part_history as ON INSERT to history \
> >  DO INSTEAD SELECT history_insert_partitioned(NEW) returning NEW.id
> > this is treated as conditional and the query is rejected.
> 
> This isn't rejected because the query is treated as condition; it's
> rejected because it's not valid syntax.  A SELECT query can't have a
> RETURNING clause, because the target list (i.e. the part that
> immediately follows the SELECT) already serves that purpose. The fact
> that it's in a CREATE RULE statement is irrelevant.

Thanks for correcting me.   At the time,  it wasn't clear to me whether the 
RETURNING clause
in a CREATE RULE statement belonged to the CREATE RULE statement or the rule 
being created,
but now I see it's the latter.

> >   .  I propose to extend the rule system to recognize cases where the 
> >INSERT query specifies
> >  RETURNING and the rule promises to return a row,  and to then permit 
> >this query to run
> >  and return the expected row.   In effect,  to widen the definition of 
> >"unconditional"
> >  to handle cases such as my testcase.
> 
> That already (kind of) works:
> 
>  [...]
> 
> I do notice that the RETURNING clause of the INSERT can't reference
> NEW, which seems like a restriction that we probably ought to lift,
> but it doesn't seem to have much to do with your patch.
> 

The main use of my proposal is to be able to return the value of the sequence 
assigned
to the NEW.id column,  so yes   that is a serious restriction.

However,   even if that restriction is lifted,  it will not help with the case 
where
the rule is an invocation of a function,   which is the case I need.    For 
example,
in my testcase,  the function is building the SQL statement to be executed 
including the
 table name of the partitioned child table,   based on the timestamp in the NEW 
record.

Your comments have helped me realize that my original subject line did not 
accurately state
my requirement,  which should read
 "support INSERT INTO...RETURNING with partitioned table using rule which 
invokes a function"

And for this requirement as re-stated,  as far as I can tell,   current 
postgresql has no solution:
  .  only way to invoke a function in a rewrite rule is with SELECT function()
  .  SELECT does not permit RETURNING clause
  .  my proposal provides a way of relaxing this restriction for the case of 
SELECT in a rewrite rule.

I hope this describes the proposal a bit better.

John
  
-- 
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] pg_dump: Sort overloaded functions in deterministic order

[Tom Lane]

Joel Jacobson  writes:
> New version, made a typo in last one.

I'm not particularly happy with the idea of adding a sortkey field to
DumpableObject as such, when most object types don't need it.  That just
bloats the code and pg_dump's memory consumption.  It would be better to
modify the already-existing object-type-specific special cases in
DOTypeNameCompare to take additional information into account as needed.

BTW, I see no reason to be adding extra calls of
pg_get_function_identity_arguments.  What is wrong with the funcsig or
aggsig strings that the code already computes?

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] Schema version management

[Josh Berkus]

> I'd rather have the few overloaded-functions in one file (hopefully
> with deterministic ordering) and a sane, simple filename, than have
> every function in every database in a separate file with some strange
> mess in the filename that makes me cringe every time I see it.

Having tried it both ways, for an existing production project, I agree
with Adrian.  One file, multiple functions.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://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] waitpid in pg_basebackup

[Tom Lane]

Fujii Masao  writes:
> On Fri, Jul 6, 2012 at 2:01 AM, Tom Lane  wrote:
>> I will go ahead and remove that code.

> Thanks!

> BTW, I was just implementing the patch ;) Patch attached.

Oh, I'd already done it when I got your message :-(.  Looks like we
arrived at the same answers, though, except for slightly different
choices for how to do the Windows bit.

> Note that I've not tested the patch on Windows environment
> because I don't have that

Me either; I'll check the buildfarm results later.

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] waitpid in pg_basebackup

[Fujii Masao]

On Fri, Jul 6, 2012 at 2:01 AM, Tom Lane  wrote:
> I wrote:
>> I agree, let's drop the support for waitpid() not being present.
>
> BTW, some digging in the commit logs shows that postmaster.c's
> existing support for using wait3 in place of waitpid was added in
> commit a5494a2d92a2752c610b8b668a7d33478e90c160, "Various patches for
> nextstep by GregorHoffleit".  NextStep is presumably quite dead by
> now, and anyway we officially pulled support for it as of 9.2.
>
> I will go ahead and remove that code.

Thanks!

BTW, I was just implementing the patch ;) Patch attached.
Note that I've not tested the patch on Windows environment
because I don't have that

Regards,

-- 
Fujii Masao


drop_ifdef_waitpid_v1.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] Schema version management

[Aidan Van Dyk]

On Thu, Jul 5, 2012 at 12:57 PM, David E. Wheeler  wrote:
> On Jul 5, 2012, at 3:21 PM, Andrew Dunstan wrote:
>
>> No they are not necessarily one logical unit. You could have a bunch of
>> functions called, say, "equal" which have pretty much nothing to do with
>> each other, since they refer to different types.
>>
>> +1 from me for putting one function definition per file.
>
> +1 for an option (I prefer one file for my projects, but might need multiple 
> files for other projects).

-1

I'd rather have the few overloaded-functions in one file (hopefully
with deterministic ordering) and a sane, simple filename, than have
every function in every database in a separate file with some strange
mess in the filename that makes me cringe every time I see it.

a.

-- 
Aidan Van Dyk Create like a god,
ai...@highrise.ca   command like a king,
http://www.highrise.ca/   work like a slave.

-- 
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] User-Id Tracking when Portal was started

[Robert Haas]

On Wed, Jul 4, 2012 at 4:24 PM, Kohei KaiGai  wrote:
> 2012/7/4 Robert Haas :
>> On Tue, Jul 3, 2012 at 12:46 PM, Kohei KaiGai  wrote:
 My point is that it seems like a bug that the secContext gets restored
 in one case and not the other, depending on which user ID was specified
 in SET SESSION AUTHORIZATION.

>>> Sorry, the above description mention about a case when it does not use
>>> the marker to distinguish a case to switch user-id from a case not to 
>>> switch.
>>> (I though I was asked the behavior if this logic always switches /
>>> restores ids.)
>>>
>>> The patch itself works correctly, no regression test failed even though
>>> several tests switches user-id using SET SESSION AUTHORIZATION.
>>
>> I don't believe that proves anything.  There are lots of things that
>> aren't tested by the regression tests, and there's no guarantee that
>> any you've added cover all bases, either.  We always treat user-ID and
>> security context as a unit; you haven't given any reason why this case
>> should be handled differently, and I bet it shouldn't.
>>
> This patch always handles user-id and security context as a unit.
> In case when it was switched, then it shall be always restored.
> And, in case when it was not switched, then it shall never be restored.
>
> Was my explanation confusing?

It's not that your explanation is confusing; it's that it doesn't
match the code.

-- 
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] foreign key locks

[Alvaro Herrera]

Excerpts from Alvaro Herrera's message of vie jun 29 19:17:02 -0400 2012:

> I was only testing migrating from an old version into patched, not
> same-version upgrades.
> 
> I think I see what's happening here.

Okay, I have pushed the fix to github -- as I suspected, code-wise the
fix was simple.  I will post an updated consolidated patch later today.

make check of pg_upgrade now passes for me.

It would be nice that "make installcheck" would notify me that some
modules' tests are being skipped ...

-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 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] waitpid in pg_basebackup

[Tom Lane]

I wrote:
> I agree, let's drop the support for waitpid() not being present.

BTW, some digging in the commit logs shows that postmaster.c's
existing support for using wait3 in place of waitpid was added in
commit a5494a2d92a2752c610b8b668a7d33478e90c160, "Various patches for
nextstep by GregorHoffleit".  NextStep is presumably quite dead by
now, and anyway we officially pulled support for it as of 9.2.

I will go ahead and remove that code.

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] Schema version management

[David E. Wheeler]

On Jul 5, 2012, at 3:21 PM, Andrew Dunstan wrote:

> No they are not necessarily one logical unit. You could have a bunch of
> functions called, say, "equal" which have pretty much nothing to do with
> each other, since they refer to different types.
> 
> +1 from me for putting one function definition per file.

+1 for an option (I prefer one file for my projects, but might need multiple 
files for other projects).

David


-- 
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] enhanced error fields

[Pavel Stehule]

Hello

there is a updated patch:

* renamed auxiliary functions and moved it elog.c - header is new file
"relerror.h"
* new fields "constraint_table" and "trigger_table" - constraints and
triggers are related to relation in pg, not just to schema
* removed using implicit constraints without unique name
* better coverage of enhancing errors in source code
* removed "inline" keywords


>
> /* Class 23 - Integrity Constraint Violation */
>
> This would be a reasonably well-defined place to require that this new
> infrastructure be used going forward (emphasis on the well-defined).
> Note that the authors of third-party database drivers define exception
> classes whose structure reflects these errcodes.h codes. To be
> inconsistent here seems unacceptable, since some future client of,
> say, pqxx (the example that I am personally most familiar with) might
> reasonably hope to always see some relation name when they call the
> e.relation_name() of some pqxx::integrity_constraint_violation object.
> If we were to commit the patch as-is, that would not be possible,
> because the following such sites that have not been touched:
>


>
> src/backend/executor/execQual.c
> 3786:   
> (errcode(ERRCODE_NOT_NULL_VIOLATION),
>
> src/backend/utils/adt/domains.c
> 126:
> (errcode(ERRCODE_NOT_NULL_VIOLATION),

> src/backend/executor/execQual.c
> 3815:   
> (errcode(ERRCODE_CHECK_VIOLATION),
>
> src/backend/utils/adt/domains.c
> 162:
> (errcode(ERRCODE_CHECK_VIOLATION),
>

these exceptions are related to domains - we has not adequate fields
now - and these fields are not in standards

it needs some like DOMAIN_NAME and DOMAIN_SCHEMA ???

Regards

Pavel


eelog-2012-07-05.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] Schema version management

[Alvaro Herrera]

Excerpts from Christopher Browne's message of jue jul 05 12:10:09 -0400 2012:

> I wound up expanding the function arguments and using function + args
> as the name.  That leads to a risk of rather long names for functions,
> but there aren't many other ways possible.

Well, maybe not many, but you don't need many, only some.  You could
stringify the list of arguments and use a hash of the string.  That's
also unambiguous and the length is constrained, regardless of the number
of args.

-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 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] PG9.2 and FDW query planning.

[Tom Lane]

Ronan Dunklau  writes:
> Let's say I have an IMAP foreign data wrapper, and I write a query
> joining the table on itself using the In-Reply-To and Message-ID
> headers, is there anything I can do to avoid fetching all the mails
> from the remote server ?

> If I could somehow inform the planner that it can look up rows by
> message-id, thus avoiding the need to fetch everything from the remote
> server. Perhaps "persuading" the planner to use a nested-loop ?

OK, so what you're saying is that the imap server can effectively
provide an index on message_id.  What you'd do is create a parameterized
path that uses the tbl.message_id = other_tbl.in_reply_to join clause.
If that's enough cheaper than a full scan, the planner would select it.

FWIW, I'm not sure that it's sane to try to expose this stuff to python
yet.  It's complicated and still something of a moving target.  Once
we've got a few more C-coded FDWs that can do this type of optimization,
things might be stable enough that it'd be useful to try to provide a
high-level API.

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] Schema version management

[Joel Jacobson]

On Thu, Jul 5, 2012 at 6:09 PM, Tom Lane  wrote:

> Well, to be perfectly frank, I already doubt that this entire feature
> passes the complexity-versus-value test, because pg_dump is not a
> substitute for an SCM --- people who have got enough functions to need
> this sort of thing need to be keeping them somewhere else than in dump
> files.  Complicating things more by supporting multiple ways of doing it
> will make that worse.  I think you need to pick one design and stick
> with it, not try to paint the bikeshed every color suggested by anybody.
>

I agree it should be one option only, and again I think the one file
variant is best.

This is indeed not a substitute for an SCM, but a nice complement.

Personally, I use this feature already to commit the schema for all versions
of my databases (production, test, development) into a git repo every
minute.
It only commits if something has changed.

This makes it super easy to compare the schema of the actual production
database
between different points in time.

This would not be possible if only manually committing stuff to the normal
git repo,
where I also have all the functions, which I modify when developing and
testing.

pg_dump -> git means you can be 100% certain version X of the schema was
active in the production database at date/time T.

[HACKERS] PG9.2 and FDW query planning.

[Ronan Dunklau]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Hello.

I'm in the process of porting our multicorn extension to pg9.2, and
I'd like to take advantage of the GetForeignPaths hook.

The multicorn extension allows the creation of a FDW in python using a
simple API, and I'd like to be able to provide FDW implementors a way
to influence the planner.

Let's say I have an IMAP foreign data wrapper, and I write a query
joining the table on itself using the In-Reply-To and Message-ID
headers, is there anything I can do to avoid fetching all the mails
from the remote server ?

If I could somehow inform the planner that it can look up rows by
message-id, thus avoiding the need to fetch everything from the remote
server. Perhaps "persuading" the planner to use a nested-loop ?

If I understand correctly, I should build one (or more) list of
pathkeys, and add more foreign paths using those pathkeys.

There was a discussion about index on foreign tables back in march.
- From what I understand from this discussion, someone proposed to
locally store information about indexes on the foreign tables, but I
did not find anything on how to build a path "from scratch".

Thank you.

- -- 
Ronan Dunklau

-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.19 (GNU/Linux)

iQEcBAEBAgAGBQJP9bsIAAoJECTYLCgFy323KtsH/2/8AVfBRm75oWFMlU0l0oBC
ujxkt338PnpVi1V5gtE5GSRSwybPWytXAkgzIQ5/DEP/RmeW8pliV+0V7zvqlEWG
zgvfA7stRBWtIIIv6mdlTM0eBBgsFnoJLiJDTUDst5vAaj8vg8b+pX/ip7nSF5sw
dV2i3ir6JSsG4nJOcQ/kP6xg4Joan65pOwFDfwnx9pFnerT0YN9f87DRuohcj12e
fgWSqZkGU5nx9yCLWa294YzIFFY7lIjLowzEfg2eP2dVIM09GquKsSXyilJiMy4J
3QL64mUDF/pNZeH0LnpyGJfCfPlQsX4c554rZbO03tVeSEZMyVpCISLqutTnR9I=
=HwMc
-END PGP SIGNATURE-

-- 
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] Event Triggers reduced, v1

[Robert Haas]

On Wed, Jul 4, 2012 at 1:56 PM, Dimitri Fontaine  wrote:
> Dimitri Fontaine  writes:
>> Robert Haas  writes:
>>> No, I'm not asking you to add any more columns right now (in fact,
>>> please do not).  But the type of the existing column should change to
>>> text[].
>>
>> Ok, done in the attached. We now read text from either the user input in
>> the grammar or from the catalogs (in a 1-D array there), and convert to
>> our enum structure for internal code use.
>
> In the previous one I missed adapting pg_dump and psql, here's the
> updated set of patches. Sorry about that. This unexpected electrical
> shut down in the middle of the day was not cool. Nor was the second one.

Thanks.  There are some review comments from previous rounds that
don't seem to have been fully incorporated to this version:

- You only changed the definition of pg_event_triggers.evttags, not
the documentation.  I don't think we need pg_evttag_to_string aka
pg_event_trigger_command_to_string any more either.
- When you removed format_type_be_without_namespace, you didn't remove
either the function prototype or the related changes in format_type.c.
- I'm pretty sure that a previous review mentioned the compiler
warning in evtcache.c, which seems not to be fixed.  It doesn't look
harmless, either.
- The definitions of EVTG_FIRED_* are still present in
pg_event_trigger.h, even though they're longer used anywhere.
- The comments in parsenodes.h still refer to command triggers rather
than event triggers.  event_trigger.h contains a reference to
CommandTriggerData that should say EventTriggerData.  plperl.sgml has
vestiges of the old terminology as well.

In terms of the schema itself, I think we are almost there, but:

- I noticed while playing around with this that pg_dump emits a
completely empty owner field when dumping an event trigger.  At first
I thought that was just an oversight, but then I realized there's a
deeper reason for it: pg_event_trigger doesn't have an owner field.  I
think it should.  The only other objects in the system that don't have
owners are text search parsers and text search templates (and casts,
sort of).  It might seem redundant to have an owner even when
event-triggers are superuser-only, but we might want to try to relax
that restriction later.  Note that foreign data wrappers, which are
also superuser-create-only, do have an owner.  (Note that if we give
event triggers an owner, then we also need ALTER .. OWNER TO support
for them.)

- It seems pretty clear at this point that the cache you've
implemented in src/backend/utils/cache/evtcache.c is going to do all
the heavy lifting of converting the stored catalog representation to a
form that is suitable for quick access.  Given that, I wonder whether
we should go whole hog and change evtevent to a text field.  This
would simplify things for pg_dump and we could get rid of
pg_evtevent_to_string, at a cost of doing marginally more work when we
rebuild the event cache (but who really cares, given that you're
reading the entire table every time you rebuild the cache anyway?).

Thoughts?

Some other minor comments:

- In the \dy output, command_start is referred to as the "condition",
but the documentation calls it an "event" and the grammar calls it
"event_name".  "event" or "event_name" seems better, so I think this
is just a matter of changing psql to match.
- AlterEventTrigStmt defines tgenbled as char *; I think it should
just be char.  In gram.y, you can declare the enable_trigger
production as  (or ?) rather than .  At any rate
pstrdup(stmt->tgenabled)[0] doesn't look right; you don't need to copy
a string to fetch the first byte.
- Why did you create a separate file pg_event_trigger_fn.h instead of
just including that single prototype in pg_event_trigger.h?
- The EVENTTRIGGEROID syscache is used only in RemoveEventTriggerById.
 I don't think that's a sufficient justification for eating up more
memory for another system cache.  I think you should just remove that
cache and recode RemoveEventTriggerById to find the correct tuple via
an index scan.  See RemoveEventTriggerById for an example.

-- 
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] Schema version management

[Christopher Browne]

On Thu, Jul 5, 2012 at 11:59 AM, Tom Lane  wrote:
> Alvaro Herrera  writes:
>> Excerpts from Michael Glaesemann's message of jue jul 05 11:36:51 -0400 2012:
>>> If we're dumping objects (tables, views, functions, what-have-you) into 
>>> separate files,
>>> each of these functions is a separate object and should be in its own file.
>
>> Clearly there is no consensus here.
>
> FWIW, I'm attracted to the all-similarly-named-functions-together
> method, mainly because it dodges the problem of how to encode a
> function's argument list into a filename.  However, we're being
> short-sighted to only think of functions here.  What about operators?
> Or casts?  Those don't have simple names either.

If you stow them all together, that still leaves a question as to
whether or not they get stored in a deterministic order.

I was recently working on something of the same issue as part of a
schema differencing tool.  It was pointedly *not* sufficient to use
the internal name (e.g. - information_schema.routines.specific_name),
as I wanted to compare things between databases, and it's pretty
certain that oids will differ.

I wound up expanding the function arguments and using function + args
as the name.  That leads to a risk of rather long names for functions,
but there aren't many other ways possible.

Note that pg_autodoc  takes a similar
approach; it attaches function labels based on function + args.

Here's an expanded example in the Slony docs:



I wouldn't mind stowing functions together in one file, and I'd
actually not get too bent out of shape if the order was somewhat
nondeterministic.  But something like the autodoc naming seems like
the unambiguous answer.  Long, but unambiguous...
-- 
When confronted by a difficult problem, solve it by reducing it to the
question, "How would the Lone Ranger handle this?"

-- 
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] Schema version management

[Tom Lane]

Joel Jacobson  writes:
> On Thu, Jul 5, 2012 at 4:46 PM, Tom Lane  wrote:
>> pg_dump is already a bloated, nearly unmaintainable mess.  The very
>> last thing it needs is even more options.

> If you are referring to the code, I don't think that's a good argument
> against implementing new good features.
> The important ratio is the value of a feature compared to the increased
> complexity.

Well, to be perfectly frank, I already doubt that this entire feature
passes the complexity-versus-value test, because pg_dump is not a
substitute for an SCM --- people who have got enough functions to need
this sort of thing need to be keeping them somewhere else than in dump
files.  Complicating things more by supporting multiple ways of doing it
will make that worse.  I think you need to pick one design and stick
with it, not try to paint the bikeshed every color suggested by anybody.

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] Schema version management

[Joel Jacobson]

On Thu, Jul 5, 2012 at 5:59 PM, Tom Lane  wrote:

> FWIW, I'm attracted to the all-similarly-named-functions-together
> method, mainly because it dodges the problem of how to encode a
> function's argument list into a filename.  However, we're being
> short-sighted to only think of functions here.  What about operators?
> Or casts?  Those don't have simple names either.
>

Someone suggested to urlencode them. I think that's a quite good solution.

Personally, I don't have any user-defined operators or casts. Don't know
how common it is in general, but it must of course work for these as well.

Re: [HACKERS] Schema version management

[Tom Lane]

Alvaro Herrera  writes:
> Excerpts from Michael Glaesemann's message of jue jul 05 11:36:51 -0400 2012:
>> If we're dumping objects (tables, views, functions, what-have-you) into 
>> separate files,
>> each of these functions is a separate object and should be in its own file.

> Clearly there is no consensus here.

FWIW, I'm attracted to the all-similarly-named-functions-together
method, mainly because it dodges the problem of how to encode a
function's argument list into a filename.  However, we're being
short-sighted to only think of functions here.  What about operators?
Or casts?  Those don't have simple names either.

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] Schema version management

[Michael Glaesemann]

On Jul 5, 2012, at 11:52, Alvaro Herrera wrote:

> Isn't this a perfect example of stuff that, since it does much the same
> thing, should be in the same file so that you remember to fix them all
> together if you find a bug in one?

That's what tests are for.

Michael Glaesemann
grzm seespotcode net




-- 
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] Schema version management

[Alvaro Herrera]

Excerpts from Michael Glaesemann's message of jue jul 05 11:36:51 -0400 2012:
> 
> On Jul 5, 2012, at 11:17, Alvaro Herrera wrote:

> > However I am also against what seems to be the flow.  Normally, you
> > don't write overloaded plpgsql functions such as "equal".
> 
> I often write functions that perform fetches based on different criteria.
> For example, 
> 
> -- returns count of all orders for the given customer
> int function order_count(in_customer_name text)
> 
> -- returns count of all orders for the given customer since the given 
> timestamp
> int function order_count(in_customer_name text, in_since timestamp with time 
> zone)
> 
> -- returns count of orders for the given customer during a given interval
> int function order_count(in_customer_name text, in_from timestamp with time 
> zone, in_through timestamp with time zone) 

Isn't this a perfect example of stuff that, since it does much the same
thing, should be in the same file so that you remember to fix them all
together if you find a bug in one?

> Or, I'll write overloaded functions, one of which provides default values.
> 
> -- returns the set of members whose birthday is today. Calls 
> birthday_members(CURRENT_DATE)
> setof record function birthday_members()
> 
> -- returns the set of members whose birthday is on the given date, which 
> makes testing a lot easier
> setof record function birthday_members(in_date DATE)

Same.  This seems particularly the case if one implementation calls
another, more general one.

> Some may disagree that this is a "proper" usage of function overloading.
> Some may even argue that function names shouldn't be overloaded at all.
> However, I find this usage of function name overloading useful, especially
> for keeping function names relatively short.

I completely agree.

> If we're dumping objects (tables, views, functions, what-have-you) into 
> separate files,
> each of these functions is a separate object and should be in its own file.

Clearly there is no consensus here.

-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 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] Schema version management

[Michael Glaesemann]

On Jul 5, 2012, at 11:17, Alvaro Herrera wrote:

> 
> Excerpts from Tom Lane's message of jue jul 05 10:46:52 -0400 2012:
>> Joel Jacobson  writes:
>>> Maybe it could be made an option to pg_dump?
>> 
>> Ick.  Then we have to deal with all the downsides of *both* methods.
>> 
>> pg_dump is already a bloated, nearly unmaintainable mess.  The very
>> last thing it needs is even more options.
> 
> Agreed.
> 
> However I am also against what seems to be the flow.  Normally, you
> don't write overloaded plpgsql functions such as "equal".

I often write functions that perform fetches based on different criteria.
For example, 

-- returns count of all orders for the given customer
int function order_count(in_customer_name text)

-- returns count of all orders for the given customer since the given timestamp
int function order_count(in_customer_name text, in_since timestamp with time 
zone)

-- returns count of orders for the given customer during a given interval
int function order_count(in_customer_name text, in_from timestamp with time 
zone, in_through timestamp with time zone) 


Or, I'll write overloaded functions, one of which provides default values.

-- returns the set of members whose birthday is today. Calls 
birthday_members(CURRENT_DATE)
setof record function birthday_members()

-- returns the set of members whose birthday is on the given date, which makes 
testing a lot easier
setof record function birthday_members(in_date DATE)

Some may disagree that this is a "proper" usage of function overloading.
Some may even argue that function names shouldn't be overloaded at all.
However, I find this usage of function name overloading useful, especially
for keeping function names relatively short.

If we're dumping objects (tables, views, functions, what-have-you) into 
separate files,
each of these functions is a separate object and should be in its own file.

Michael Glaesemann
grzm seespotcode net




-- 
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] Schema version management

[Joel Jacobson]

On Thu, Jul 5, 2012 at 5:17 PM, Alvaro Herrera
wrote:

> Agreed.
>
> However I am also against what seems to be the flow.  Normally, you
> don't write overloaded plpgsql functions such as "equal".  Case in
> point, the equality functions in core have funny names like "int4eq" and
> so on.  Instead, at least in my experience, the overloaded functions
> people seem to have in their databases are like do_stuff_to_foobars()
> and you have one version for foos and another one for bars.
>
> If you're doing lots of equality functions, surely it would make more
> sense to package them up as an extension anyway along with all the other
> thingies you need for the type you're supposedly writing.  So it's a
> completely different market than what we're aiming at here.


True, very true, I didn't think about that, you are right, I fully agree.
My vote is therefore on the "put all overloaded functions in the same file"
variant.

Re: [HACKERS] Schema version management

[Alvaro Herrera]

Excerpts from Tom Lane's message of jue jul 05 10:46:52 -0400 2012:
> Joel Jacobson  writes:
> > Maybe it could be made an option to pg_dump?
> 
> Ick.  Then we have to deal with all the downsides of *both* methods.
> 
> pg_dump is already a bloated, nearly unmaintainable mess.  The very
> last thing it needs is even more options.

Agreed.

However I am also against what seems to be the flow.  Normally, you
don't write overloaded plpgsql functions such as "equal".  Case in
point, the equality functions in core have funny names like "int4eq" and
so on.  Instead, at least in my experience, the overloaded functions
people seem to have in their databases are like do_stuff_to_foobars()
and you have one version for foos and another one for bars.

If you're doing lots of equality functions, surely it would make more
sense to package them up as an extension anyway along with all the other
thingies you need for the type you're supposedly writing.  So it's a
completely different market than what we're aiming at here.

-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 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] Schema version management

[Joel Jacobson]

On Thu, Jul 5, 2012 at 4:46 PM, Tom Lane  wrote:

> Ick.  Then we have to deal with all the downsides of *both* methods.
>
> pg_dump is already a bloated, nearly unmaintainable mess.  The very
> last thing it needs is even more options.


When you say bloated, are you referring to the code or the command line
interface?

If you are referring to the code, I don't think that's a good argument
against implementing new good features.
The important ratio is the value of a feature compared to the increased
complexity.
In this case, it's very simple to implement both the --split option and the
fixing of dump order.
I'm not even a C coder and managed to implement it within less of an hour
effective coding.
We are talking ~100 lines of code, with comments and everything.

If you are referring to the command line interface and think it is bloated,
maybe the options should be hidden in the normal --help.
We could create a new --help-advanced text, where we could put these
options, and all other existing less common options.
I think this is a quite common and good way to handle the situation for
UNIX command line tools.

Re: [HACKERS] Schema version management

[Tom Lane]

Joel Jacobson  writes:
> Maybe it could be made an option to pg_dump?

Ick.  Then we have to deal with all the downsides of *both* methods.

pg_dump is already a bloated, nearly unmaintainable mess.  The very
last thing it needs is even more options.

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] Schema version management

[Joel Jacobson]

Maybe it could be made an option to pg_dump?

Some users and their systems might not even have overloaded functions,
and these users will of course prefer a nice looking filename, i.e. all
functions
having the same name kept in the same file. Which for them will mean only
one function per file anyway.

pg_dump --split --overloaded-functions-to-same-file

Other users and their systems might have a lot of overloaded functions,
like the equal() example mentioned, they will of course prefer to keep
all functions in separate files.

pg_dump --split --overloaded-functions-to-separate-files

Then, one can discuss which one should be the default option for --split,
I would prefer the same file variant, and think most other users would too,
except for users with a lot of overloaded functions.

Re: [HACKERS] Schema version management

[Vik Reykja]

On Thu, Jul 5, 2012 at 3:32 PM, Michael Glaesemann wrote:

>
> On Jul 5, 2012, at 9:21, Andrew Dunstan wrote:
>
> > No they are not necessarily one logical unit. You could have a bunch of
> > functions called, say, "equal" which have pretty much nothing to do with
> > each other, since they refer to different types.
> >
> > +1 from me for putting one function definition per file.
>
> +1. It might make sense to include some sort of argument type information.
> The function signature is
> really its identifier. The function name is only part of it.
>

I'll go against the flow here.  I would prefer to have all overloaded
functions in the same file.

Re: [HACKERS] Schema version management

[Michael Glaesemann]

On Jul 5, 2012, at 9:21, Andrew Dunstan wrote:

> No they are not necessarily one logical unit. You could have a bunch of
> functions called, say, "equal" which have pretty much nothing to do with
> each other, since they refer to different types.
> 
> +1 from me for putting one function definition per file.

+1. It might make sense to include some sort of argument type information. The 
function signature is
really its identifier. The function name is only part of it.

Michael Glaesemann
grzm seespotcode net




-- 
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] Schema version management

[Andrew Dunstan]

On Thu, Jul 5, 2012 at 4:04 AM, Gurjeet Singh wrote:

> On Thu, Jul 5, 2012 at 3:15 AM, Joel Jacobson  wrote:
>
>> On Thu, Jul 5, 2012 at 2:38 AM, Robert Haas wrote:
>>
>>> My vote is - when there's an overloaded function, put each version in
>>> its own file.  And name the files something like
>>> functionname_something.sql.  And just document that something may not
>>> be entirely stable.
>>
>>
>> I would agree that's better if the dump order isn't deterministic.
>>
>> However, it looks like an easy fix to make the dump order deterministic:
>> http://archives.postgresql.org/pgsql-hackers/2012-07/msg00232.php
>>
>> If the dump order is deterministic, I think its cleaner to put all
>> versions in the same file.
>>
>> Benefits:
>> + Pretty looking filename
>> + Same file structure for all object types, no special exception for
>> functions
>>
>
> I think there's a merit to keeping all overloaded variations of a function
> in a single file, apart from the simplicity and benefits noted above. A
> change in one variation of the function may also be applicable to other
> variations, say in bug-fixes or enhancements. So keeping all variations in
> one file would make sense, since it is logically one object.
>


No they are not necessarily one logical unit. You could have a bunch of
functions called, say, "equal" which have pretty much nothing to do with
each other, since they refer to different types.

+1 from me for putting one function definition per file.

cheers

andrew

Re: [HACKERS] [PATCH] pg_dump: Sort overloaded functions in deterministic order

[Joel Jacobson]

New version, made a typo in last one.


pg_dump_deterministic_order_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] [BUGS] Tab completion of function arguments not working in all cases

[Magnus Hagander]

On Mon, Jun 18, 2012 at 5:45 PM, Josh Kupershmidt  wrote:
> On Mon, Jun 18, 2012 at 3:56 AM, Dean Rasheed  
> wrote:
>> On 18 June 2012 04:21, Josh Kupershmidt  wrote:
>
>>> As a side note unrelated to this patch, I also dislike how function
>>> name tab-completions will not fill in the opening parenthesis, which
>>> makes for unnecessary work for the user, as one then has to type the
>>> parenthesis and hit tab again to get possible completions for the
>>> function arguments. The current behavior causes:
>>>  DROP FUNCTION my_f
>>>
>>> which completes to:
>>>  DROP FUNCTION my_function
>>>
>>> enter parenthesis, and hit tab:
>>>  DROP FUNCTION my_function(
>>>
>>> which, if there is only one match, could complete to:
>>>  DROP FUNCTION my_function(integer)
>>>
>>> when the last three steps could have been consolidated with better
>>> tab-completion. Perhaps this could be a TODO.
>>>
>>
>> Hmm, I find that it does automatically fill in the opening
>> parenthesis, but only once there is a space after the completed
>> function name. So
>> "DROP FUNCTION my_f" completes to "DROP FUNCTION my_function "
>> (note the space at the end). Then pressing  one more time gives
>> "DROP FUNCTION my_function ( ", and then pressing  again gives
>> the function arguments.
>>
>> Alternatively "DROP FUNCTION my_function" (no space after
>> function name) first completes to "DROP FUNCTION my_function " (adding
>> the space), and then completes with the opening parenthesis, and then
>> with the function arguments.
>>
>> It's a bit clunky, but I find that repeatedly pressing  is easier
>> than typing the opening bracket.
>
> Interesting, I see the behavior you describe on Linux, using psql +
> libreadline, and the behavior I showed (i.e. repeatedly pressing tab
> won't automatically fill in the function arguments after the function
> name is completed, seemingly because no space is deposited after the
> completed function name)  is with OS X 10.6, using psql + libedit.
>
> [snip]
>>> you get tab-completions for both "text)" and "bytea(", when you
>>> probably expected only the former. That's easy to fix though, please
>>> see attached v2 patch.
>>
>> Good catch.
>> I think that's a useful additional test, and is also consistent with
>> the existing code in Query_for_list_of_attributes.
>
> OK, I'll mark Ready for Committer in the CF.

Applied with minor revisions:
* the comment above the COMPLETE_WITH_xyz macros needed an update
* I renamed the macro to COMPLETE_WITH_FUNCTION_ARG - to make it even
more clear what it does

Thanks!

-- 
 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] pgsql_fdw in contrib

[Shigeru HANADA]

On Tue, Jun 26, 2012 at 10:50 PM, Kohei KaiGai  wrote:
>   * Regarding to deparseSimpleSql(), it pulls attributes being referenced
> from baserestrictinfo and reltargetlist using pull_var_clause().
> Is it unavailable to use local_conds instead of baserestrictinfo?
> We can optimize references to the variable being consumed at the
> remote side only. All we need to transfer is variables referenced
> in targetlist and local-clauses.

Indeed, fixed.

> In addition, is pull_var_clause() reasonable to list up all the attribute
> referenced at the both expression tree? It seems to be pull_varattnos()
> is more useful API in this situation.

Only for searching, yes.  However, sooner or later we need Var objects
to deparse remote SELECT clause, so pull_var_clause seems better here.
ISTM one of advantage to use bitmap is matching with another bitmap,
like index only scan code checks whether all used attributes is
contained by indexed attributes.

>   * Regarding to deparseRelation(), it scans simple_rte_array to fetch
> RangeTblEntry with relation-id of the target foreign table.
> It does not match correct entry if same foreign table is appeared in
> this list twice or more, like a case of self-join. I'd like to recommend
> to use simple_rte_array[baserel->relid] instead.

Reasonable idea.  After some thoughts, I think that deparseRelation
should receive RangeTblEntry instead of relation oid (and then
PlannerInfo is not necessary).

> In addition, it checks whether relkind is RELKIND_FOREIGN_TABLE,
> or not. It seems to me this check should be Assert(), if routines of
> pgsql_fdw is called towards regular relations.
>
>   * Regarding to deparseVar(), is it unnecessary to check relkind of
> the relation being referenced by Var node, isn't it?

IIRC, this is remain of past trial for general deparser...  Removed
unnecessary relkind checks.  Fixed.

>   * Regarding to deparseBoolExpr(), compiler raised a warning
> because op can be used without initialized.

Fixed.

>   * Even though it is harmless, sortConditions() is a misleading function
> name. How about categolizeConditions() or screeningConditions()?

How about classifyConditions?
# I hope native English speaker's help for wording issue... :-)

Regards,
-- 
Shigeru Hanada

-- 
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] Schema version management

[Gurjeet Singh]

On Thu, Jul 5, 2012 at 3:15 AM, Joel Jacobson  wrote:

> On Thu, Jul 5, 2012 at 2:38 AM, Robert Haas  wrote:
>
>> My vote is - when there's an overloaded function, put each version in
>> its own file.  And name the files something like
>> functionname_something.sql.  And just document that something may not
>> be entirely stable.
>
>
> I would agree that's better if the dump order isn't deterministic.
>
> However, it looks like an easy fix to make the dump order deterministic:
> http://archives.postgresql.org/pgsql-hackers/2012-07/msg00232.php
>
> If the dump order is deterministic, I think its cleaner to put all
> versions in the same file.
>
> Benefits:
> + Pretty looking filename
> + Same file structure for all object types, no special exception for
> functions
>

I think there's a merit to keeping all overloaded variations of a function
in a single file, apart from the simplicity and benefits noted above. A
change in one variation of the function may also be applicable to other
variations, say in bug-fixes or enhancements. So keeping all variations in
one file would make sense, since it is logically one object.

Best regards,
-- 
Gurjeet Singh
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

Re: [HACKERS] Schema version management

[Joel Jacobson]

On Thu, Jul 5, 2012 at 2:38 AM, Robert Haas  wrote:

> My vote is - when there's an overloaded function, put each version in
> its own file.  And name the files something like
> functionname_something.sql.  And just document that something may not
> be entirely stable.


I would agree that's better if the dump order isn't deterministic.

However, it looks like an easy fix to make the dump order deterministic:
http://archives.postgresql.org/pgsql-hackers/2012-07/msg00232.php

If the dump order is deterministic, I think its cleaner to put all versions
in the same file.

Benefits:
+ Pretty looking filename
+ Same file structure for all object types, no special exception for
functions