Re: [HACKERS] Split-up ECPG patches

[Albe Laurenz]

Tom Lane wrote:
>>> So I'd like to see an actual case made
>>> that there's a strong reason for not requiring FROM/IN in ecpg.
>> 
>> I guess there's only one, compatibility. 
> 
> Yeah.  Are there any other precompilers that actively reject FROM/IN
> here?  If we're just a bit more strict than they are, it's not as bad
> as if there is no common syntax subset.

Oracle:

http://download.oracle.com/docs/cd/B28359_01/appdev.111/b28427/pc_afemb.htm#i9340

Yours,
Laurenz Albe

-- 
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] 2PC state files on shared memory

[Tom Lane]

Michael Paquier  writes:
> After making a lot of tests, state file size is not more than 600B.
> In some cases, it reached a maximum of size of 712B and I used such
> transactions in my tests.

I can only say that that demonstrates you didn't test very many cases.
It is trivial to generate enormous state files --- try something with
a lot of subtransactions, for example, or a lot of files created or
deleted.  I remain of the opinion that asking users to estimate the
amount of shared memory needed for this patch will cripple its
usability.  We learned that lesson the hard way for FSM, I see no
reason we have to fail to learn from experience.

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 for 8.5, transformationHook

[Pavel Stehule]

2009/8/9 Jeff Davis :
> On Sun, 2009-07-26 at 15:29 +0200, Pavel Stehule wrote:
>> Hello
>>
>> new patch add new contrib "transformations" with three modules
>> anotation, decode and json.
>>
>> These modules are ported from my older work.
>>
>> Before applying this patch, please use named-fixed patch too. The hook
>> doesn't need it, but modules anotation and json depend on it.
>
> This is not a complete review of the patches, but I have read through
> the discussion and taken a brief look at the code from a use-case point
> of view (not a technical review).
>
> My general feeling for the use case of the patch is positive. Pavel
> showed a reasonable variety of valid use cases, and the possibility to
> make existing special cases (like XML) no longer special cases.
>
> However, there are causes for concern:
>
> 1. Robert Haas is concerned that the kind of transformations allowed
> might be too limited:
>
> http://archives.postgresql.org/pgsql-hackers/2009-07/msg01947.php

gram.y is hard limit of everything what we can do in parser. I thing
so there is possible mix two grams together (like enterprisedb do it -
integration plpgsql), but still first gram has to have some static
entry points - we can't do define new keyword and cannot define new
rules, because all is hardly static. It is bison limit. And without
changes parser's engine we cannot do some more.

I see some possibility in future - to add some like preprocessor for
main parser, or postprocessor (for badly processed statements). These
creatures allows to define new SQL statement pseudo integrated to
core. But this is different task absolutely independent to function
transformation hook.

But I don't thing so this is real limit. Really I don't would to
create new SQL statements now. With hook I am able to work with param
list and named param list. This allows lot of games over standard
function syntax.

>
> 2. Tom Lane is concerned about multiple hooks working together:
>
> http://archives.postgresql.org/pgsql-hackers/2009-04/msg01038.php
>

with well written hooks it isn't problem. You can check sample hooks
together. I agree, so bad hook can be wrong, but this is general
problem of all hooks in postgresql (all hooks in the world).

> 3. All throughout the thread, there is a general concern that this might
> not be exactly the right solution.
>
> I think we need to wait on this patch. Waiting will hopefully provide
> better answers to the following questions:
>
> * What other similar features exist in the SQL spec that require a
> similar special case now? If we added this hook, would those still
> require a special case?
>
> * Can anyone think of a better hook or API change that would answer
> these use cases?
>

If somebody find any general solution different than hook I for it.

> * Can anyone think of other features that almost fit this model, but
> that the hook won't quite work for?
>
> * If the hook can implement XML, should we refactor the XML support (and
> COALESCE, etc.) to use the hook for the sake of consistency? If the hook
> is not good enough for those features, that might indicate a problem.
>

Some XML functions (not all) and COALESCE should be refactorized. But
the range for hook is external modules. It's same like executor hooks
or some other hooks in PostgreSQL. It's more readable to use direct
access to code than hooks when it's possible.

> Considering that the next commitfest is only about a month away, I don't
> think that it is too much of a burden to wait.
>

ok I agree.

Pavel

> I didn't have time to do a complete review, so I can't provide much
> better direction than this right now.
>
> 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] machine-readable explain output v4

[Stefan Kaltenbrunner]

Tom Lane wrote:

Andrew Dunstan  writes:
I takle it back. It's still there at 
 
posted 3 days ago.


Hmm, I think the archive website must be mangling that somehow.
What I have in the code I'm reviewing is

if (es.format == EXPLAIN_FORMAT_XML)
appendStringInfoString(es.str,
"http://www.postgresql.org/2009/explain\";>\n");

I was planning to complain about the format of this URL --- shouldn't it
be more like http://www.postgresql.org/explain/v1 ? --- but there's
no semicolon.


that url seems too general anyway - can we do something like 
http://www.postgresql.org/schema/explain/v1 or 
http://www.postgresql.org/xml/2009/explain/?



Stefan

--
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 for 8.5, transformationHook

[Pavel Stehule]

2009/8/9 Alvaro Herrera :
> Jeff Davis escribió:
>> On Mon, 2009-04-20 at 18:53 +0200, Pavel Stehule wrote:
>> > b) it allows constructors for data types (ANSI SQL)
>> >
>> > datatype(typefield1[, typefiedl2[, typefiedl3[, ...]]]) returns type
>>
>> Can you describe this case in more detail? What section of SQL are you
>> referring to?
>
> Hmm, I see them in 4.7 "user-defined types".  However what's in SQL2003
> and the 2008 draft I have is:
>
> 3.1.6.6 constructor function: A niladic SQL-invoked function of which exactly
> one is implicitly specified for every structured type. An invocation of the
> constructor function for data type T returns a value V of the most specific
> type T such that V is not null and, for every observer function O defined for
> T, the invocation O(V) returns the default value of the attribute 
> corresponding
> to O.
>
> and later:
>
> 4.7.4 Constructors
> Associated with each structured type ST is one implicitly defined constructor
> function, if and only if ST is instantiable.
> Let TN be the name of a structured type T. The signature of the constructor
> function for T is TN() and its result data type is T. The invocation TN()
> returns a value V such that V is not null and, for every attribute A of T, 
> A(V)
> returns the default value of A. The most specific type of V is T.
> For every structured type ST that is instantiable, zero or more SQL-invoked
> constructor methods can be specified.  The names of those methods shall be
> equivalent to the name of the type for which they are specified.
>

yes - it is

Thank You

> So I'm not seeing those typefields anywhere.
>
> --
> Alvaro Herrera                                http://www.CommandPrompt.com/
> 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] machine-readable explain output v4

[Tom Lane]

Robert Haas  writes:
> Revised patch attached.  I'm not convinced this is as good as it can
> be, but I've been looking at this patch for so long that I'm starting
> to get cross-eyed, and I'd like to Tom at least have a look at this
> and assess it before we run out of CommitFest.

Committed after significant hacking to try to make the format
abstraction layer a tad more complete.

There are still some open issues:

* I still think we need a written spec for the non-text output formats.
One of the problems with machine reading of the text format is you have
to reverse-engineer what the possibilities are, and this patch hasn't
made that better.  A list of the possible fields, and the possible
values for those fields that have finite domains, would be a start.

* There are some decisions that seem a bit questionable to me, like
using "Parent Relationship" tags rather than having the child plans
as labeled attributes of the parent node.  But I can't really evaluate
this for lack of experience with designing XML/JSON structures.

* As already noted, the URL for the XML schema seems questionable.
I think that versioning should go more like v1, v2, ... instead of
being tied to a year.

* I complained earlier that I thought dumping expressions as text
was pretty bogus --- it'll leave anything that's trying to
do analysis in depth still having to parse complicated stuff.
I don't know exactly what I want instead, but at the very least it
seems like the variables used in an expression ought to be more
readily available.

Anyway, it's committed so that people can play with it.  We're a
lot more likely to get feedback if people actually try to use the
feature.

regards, tom lane

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

Re: [HACKERS] pg_stat_activity.application_name

[Jaime Casanova]

On Fri, Jul 17, 2009 at 3:19 AM, Peter Eisentraut wrote:
> On Thursday 16 July 2009 22:08:25 Kevin Grittner wrote:
>> On the admin list there was a request for an application name
>> column in pg_stat_activity.
>>
>> http://archives.postgresql.org/pgsql-admin/2009-07/msg00095.php
>>
>> This is available in a lot of other DBMS products, can be useful to
>> DBAs, and seems pretty cheap and easy.  Could we get that onto the
>> TODO list?
>
> A facility to show it in the logs (via log_line_prefix probably) would also be
> useful.
>

is there anyone working on this or have plans to work on this? if not,
i will give it a try as soon as this commitfest ends

-- 
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] WIP: to_char, support for EEEE format

[Brendan Jurd]

2009/8/9 Alvaro Herrera :
> Brendan Jurd escribió:
>
>> Here's version 6 of the  patch, now with an all-new implementation
>> of (normalised) scientific notation in numeric.c, via the functions
>> numeric_out_sci() and get_str_from_var_sci().  So  should now be
>> able to represent the full gamut of the numeric type.
>
> I noticed an ugly pattern in NUMDesc_prepare calling a cleanup function
> before every ereport(ERROR).  I think it's cleaner to replace that with
> a PG_TRY block; see attached.

Looks nice -- although doesn't have anything to do with the  patch
so perhaps deserves its own thread?

>
> I didn't go over the patch in much more detail.  (But the
> numeric_out_sci business got me thinking.)

Got you thinking about what?  I'd welcome any comments you have.

Cheers,
BJ

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

[Greg Stark]

On Mon, Aug 10, 2009 at 2:23 AM, Robert Haas wrote:
>
>> 2. It doesn't appear that any attention has been given to what happens
>> if CREATE OR REPLACE FUNCTION is used to change the parameter names of
>> an existing function.  Since the post-analysis representation of parameter
>> lists is still entirely positional, the actual effect on a function call
>> in a stored view or rule is nil --- it will still call the same function
>> it did before, passing the parameters that were originally identified to
>> be passed.  That might be considered good, but it's quite unlike what
>> will happen to function calls that are stored textually (eg, in plpgsql
>> functions).  Is that what we want?
>
> That sounds pretty dangerous.  What if someone renames a parameter so
> as to invert its sense, or something?  (automatically_delete_all_files
> becomes confirm_before_deleting, or somesuch)

There's also the existing users using positional notation to consider.
If all my callers are using positional notation then I might be kind
of annoyed if I can't fix the parameter names of my functions because
it would change the function signature. That would be a functionality
regression for me.

But on balance I don't see a better solution. If we allow people to
change the parameter names and they're used for named arguments then
it seems like the behaviour is not very clear and predictable no
matter what resolution we pick.



-- 
greg
http://mit.edu/~gsstark/resume.pdf

-- 
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] 2PC state files on shared memory

[Michael Paquier]

After making a lot of tests, state file size is not more than 600B.
In some cases, it reached a maximum of size of 712B and I used such
transactions in my tests.

> I think setting the size parameter for this would be a frightfully
> difficult problem; the fact that average installations wouldn't use it
> doesn't make that any better for those who would.  After our bad
> experiences with fixed-size FSM, I'm pretty wary of introducing new
> fixed-size structures that the user is expected to figure out how to
> size.
The patch has been designed such as if a state file has a size higher than
what has been decided by the user,
it will be written to disk instead of shared memory. So it will not
represent a danger for teh stability of the system.
The case of too many prepared transactions is also covered thanks to
max_prepared_transactions.

Regards,

-- 
Michael Paquier

NTT OSSC

Re: [HACKERS] change in timestamp output from 8.3 to 8.4

[Bruce Momjian]

Tom Lane wrote:
> Joe Conway  writes:
> > 1. Two functions were left in the 8.4 database
> > pg_toasttbl_drop(oid)
> > pg_toasttbl_recreate(oid, oid)
> 
> This is pg_migrator's fault --- it should probably clean those up
> when it's done.

Agreed.  The new pg_migrator 8.4.4 does clean those up when it finishes.

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

  + If your life is a hard drive, Christ can be your backup. +

-- 
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 - merged up to CVS HEAD

[Robert Haas]

On Sun, Aug 9, 2009 at 2:43 PM, Simon Riggs wrote:
> I've said very clearly that I am working on this and it's fairly
> laughable to suggest that anybody thought I wasn't. What more should I
> do to prove something is "active" if you won't accept my clearly spoken
> word? How did you decide I was idle exactly?

I think we looked at the fact that you haven't posted an updated
version of this patch in almost 6 months.

...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] Review: Patch for contains/overlap of polygons

[Robert Haas]

On Sun, Aug 9, 2009 at 10:01 PM, Josh Williams wrote:
>  How's that for over-engineering? ;)

Top notch.

...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] machine-readable explain output v4

[Andres Freund]

On Monday 10 August 2009 03:43:22 Andres Freund wrote:
> On Monday 10 August 2009 03:34:36 Robert Haas wrote:
> > On Sun, Aug 9, 2009 at 9:32 PM, Tom Lane wrote:
> > > Andres Freund  writes:
> > >> Adding the notion of opening a 'empty' Group together with X_OPENCLOSE
> > >> or handling of X_OPENING|X_CLOSING would allow to handle empty tags
> > >> like in ExplainOneUtility ().
> > >
> > > Yeah, I was just wondering what to do with the  code.  I'm
> > > not sure if it's worth trying to fold it into the OpenGroup/CloseGroup
> > > code --- it might be easier to have an ExplainEmptyGroup or something
> > > like that.
> >
> > Well there's no rule that says you can't emit it as
> > 
> > 
> That wont work nicely with json output.
I have not the slightest clue what I was thinking while typing that...

Andres

-- 
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] Review: Patch for contains/overlap of polygons

[Josh Williams]

On Sun, 2009-08-09 at 13:27 -0600, Joshua Tolley wrote:
> On Sun, Aug 09, 2009 at 02:29:44PM -0400, Robert Haas wrote:
> > On Sun, Aug 9, 2009 at 2:20 PM, Bruce Momjian wrote:
> > > This is a nice section layout for a patch review report --- should we
> > > provide an email template like this one for reviewers to use?
> > 
> > We could, but it might be over-engineering.  Those particular section
> > headers might not be applicable to someone else's review.
> 
> I've just added a link to this email to the "Reviewing a Patch" wiki page
> (http://wiki.postgresql.org/wiki/Reviewing_a_Patch). Do with it as you see fit
> :)

Sweet. :)

Actually that was mainly for keeping organized and sane when conducting
my first review, and it seemed to translate well into the email when it
came time to write it up.

The appropriate sections* most certainly would change patch-to-patch --
reviewer-to-reviewer, even -- so a set template wouldn't be appropriate.
But as a style recommendation it could make sense.  I'd made a mental
note to try and refine the formatting next time around, but I haven't
been back to request another yet.

On that note, and now that I'm back online and clean of Pennsic dust,
anything else in this CommitFest in need of a last minute Windows
run-through?

- Josh Williams

* I could envision having the ability to write reviews directly into the
commitfest web app, where one could define and tag sections.  Then
anyone curious about a patch's performance implications, for example,
could pull down and read just the performance results of potentially
multiple reviewers.  How's that for over-engineering? ;)



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

[Robert Haas]

On Sun, Aug 9, 2009 at 9:36 PM, Tom Lane wrote:
> Robert Haas  writes:
>> On Sun, Aug 9, 2009 at 2:30 PM, Tom Lane wrote:
>>> I'm going to mark the patch Waiting on Author, since it's not close
>>> to being committable until these issues are resolved.
>
>> Is it realistic to think that this will be finished and committed this week?
>
> I didn't want to prejudge that question.  We still have the rest of the
> week, and there's not that much else left to do, at least from my
> standpoint (some of the other committers still have stuff on their
> plates).

Fair point, my impatience is showing.  Sorry.

...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] machine-readable explain output v4

[Andrew Dunstan]

Robert Haas wrote:

One subtle point that isn't documented and probably should be is that
JSON can't support a container that behaves partly like a list and
partly like a hash, as XML can.  So for example in XML a  tag
could have children like  (one each) and could also have
its inner, outer, and sub-plans in there as  tags right under
the parent .  I'm not sure this would be good design anyway, but
it COULD be done.  In JSON, this will crash and burn, because the
container is either an array (which precludes labelling the elements)
or a hash (which precludes duplicates).


  


Right, this is fairly well known, I think. There are methods to map XML 
to JSON, and it can even be done in such a way that you can make a 
complete round trip, but in the schemes I've seen the JSON doesn't 
really look like what you would use if you designed the JSON document 
from scratch, or if it does then you're losing something.


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] machine-readable explain output v4

[Andres Freund]

On Monday 10 August 2009 03:34:36 Robert Haas wrote:
> On Sun, Aug 9, 2009 at 9:32 PM, Tom Lane wrote:
> > Andres Freund  writes:
> >> Adding the notion of opening a 'empty' Group together with X_OPENCLOSE
> >> or handling of X_OPENING|X_CLOSING would allow to handle empty tags like
> >> in ExplainOneUtility ().
> >
> > Yeah, I was just wondering what to do with the  code.  I'm
> > not sure if it's worth trying to fold it into the OpenGroup/CloseGroup
> > code --- it might be easier to have an ExplainEmptyGroup or something
> > like that.
>
> Well there's no rule that says you can't emit it as
>
> 
> 
That wont work nicely with json output.

Andres

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

[Tom Lane]

Robert Haas  writes:
> On Sun, Aug 9, 2009 at 2:30 PM, Tom Lane wrote:
>> I'm going to mark the patch Waiting on Author, since it's not close
>> to being committable until these issues are resolved.

> Is it realistic to think that this will be finished and committed this week?

I didn't want to prejudge that question.  We still have the rest of the
week, and there's not that much else left to do, at least from my
standpoint (some of the other committers still have stuff on their
plates).

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] machine-readable explain output v4

[Robert Haas]

On Sun, Aug 9, 2009 at 9:32 PM, Tom Lane wrote:
> Andres Freund  writes:
>> Adding the notion of opening a 'empty' Group together with X_OPENCLOSE or
>> handling of X_OPENING|X_CLOSING would allow to handle empty tags like in
>> ExplainOneUtility ().
>
> Yeah, I was just wondering what to do with the  code.  I'm
> not sure if it's worth trying to fold it into the OpenGroup/CloseGroup
> code --- it might be easier to have an ExplainEmptyGroup or something
> like that.

Well there's no rule that says you can't emit it as




...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] machine-readable explain output v4

[Tom Lane]

Andres Freund  writes:
> Adding the notion of opening a 'empty' Group together with X_OPENCLOSE or 
> handling of X_OPENING|X_CLOSING would allow to handle empty tags like in 
> ExplainOneUtility ().

Yeah, I was just wondering what to do with the  code.  I'm
not sure if it's worth trying to fold it into the OpenGroup/CloseGroup
code --- it might be easier to have an ExplainEmptyGroup or something
like that.

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

[Robert Haas]

On Sun, Aug 9, 2009 at 2:30 PM, Tom Lane wrote:
> I've now read most of this patch, and I think there are some things that
> need rework, and perhaps debate about what we want.
>
> 1. As I already mentioned, I think the mixed-notation business is a bad
> idea.  It's unintuitive, it's not especially useful, and it substantially
> increases our risk of being semantically incompatible with whatever the
> SQL committee might someday do in this area.  I think we should disallow
> it until we see what they do.  I gather that this might be an unpopular
> position though.

LOL.  I already did my yelling and screaming on this point... though
it's all good-natured, in case that doesn't come through in the email.

> 2. It doesn't appear that any attention has been given to what happens
> if CREATE OR REPLACE FUNCTION is used to change the parameter names of
> an existing function.  Since the post-analysis representation of parameter
> lists is still entirely positional, the actual effect on a function call
> in a stored view or rule is nil --- it will still call the same function
> it did before, passing the parameters that were originally identified to
> be passed.  That might be considered good, but it's quite unlike what
> will happen to function calls that are stored textually (eg, in plpgsql
> functions).  Is that what we want?

That sounds pretty dangerous.  What if someone renames a parameter so
as to invert its sense, or something?  (automatically_delete_all_files
becomes confirm_before_deleting, or somesuch)

> Or should we consider that parameter
> names are now part of the API of a function, and forbid CREATE OR REPLACE
> FUNCTION from changing them?  Or perhaps we should recheck parameter name
> matching someplace after analysis, perhaps at default-expansion time?

I'm not sure what the right way to go with this is, but we have to
think about how it plays with function overloading - can I define two
identically-named functions with different sets of positional
parameters, and then resolve the function call based on which
parameters are specified?

> 3. In the same vein, CREATE FUNCTION doesn't disallow duplicate parameter
> names, nor functions that have names for some but not all parameters.
> The patch appears to cope with the latter case but not the former.
> Should we disallow these things in CREATE FUNCTION, or make the patch
> never match such functions, or what?

I think duplicate parameter names shouldn't be allowed.

> 4. No attention has been given to making ruleutils.c list out named or
> mixed function calls correctly.
>
> 5. I don't like anything about the "leaky list" representation of
> analyzed function arguments.  Having null subexpressions in unexpected
> places isn't a good idea --- it's likely to cause crashes in code that
> isn't being really cautious.  Moreover, if we did ultimately support
> mixed notation, there's no way to list it out correctly on the basis
> of this representation, because you can't tell which arguments were
> named and which weren't.  I think it would be better to keep the
> ArgExprs in the transformed parameter list and have the planner
> remove them (and reorder the arguments if needed) when it does
> default-argument expansion.  Depending on what you think about point
> #2, the transformed ArgExprs might need to carry the argument number
> instead of the argument name, but in any case they'd just be there
> for named arguments.  This approach probably will also reduce the
> amount of change in the parser, since you won't have to separate
> the names from the argument list and pass those all over the place
> separately.
>
> Some minor style issues:
>
> * ArgExpr is confusingly named and incorrectly documented, since it's
> only used for named arguments.  Perhaps NamedArgExpr would be better.
> Also, it'd probably be a good idea to include a location field in it
> (pointing at the parameter name not the argument expression).
>
> * Most of the PG source code just writes "short" or "long",
> not "short int".  Actually I wonder whether "int2" wouldn't
> be preferred anyway, since that's how the relevant pg_proc
> columns are declared.
>
> * The error messages aren't even approximately conformant to style guide.
>
> * Please avoid useless whitespace changes, especially ones as
> ill-considered as this:
>
> ***
> *** 10028,10034 
>                ;
>
>
> ! name:         ColId                                                         
>           { $$ = $1; };
>
>  database_name:
>                        ColId                                                  
>                  { $$ = $1; };
> --- 10056,10062 
>                ;
>
>
> ! name:         ColId                                                         
>   { $$ = $1; };
>
>  database_name:
>                        ColId                                                  
>                  { $$ = $1; };
>
> I'm going to mark the patch Waiting on Author, since it's not close
> to being commit

Re: [HACKERS] machine-readable explain output v4

[Andres Freund]

On Monday 10 August 2009 02:53:16 Tom Lane wrote:
> Andres Freund  writes:
> > On Monday 10 August 2009 01:21:35 Andrew Dunstan wrote:
> >> That ";" after the attribute is almost certainly wrong. This is a
> >> classic case of what I was talking about a month or two ago. Building up
> >> XML (or any structured doc, really, XML is not special in this regard)
> >> by ad hoc methods is horribly error prone. if you don't want to rely on
> >> libxml, then I think you need to develop a lightweight abstraction
> >> rather than just appending to a StringInfo.
> >
> > While it would be possible to add another step inbetween and generate a
> > format neutral tree and generate the different formats out of it I am not
> > sure that this is worthwile.
> > The current text format will need to stay special cased anyway because
> > its far to inconsistent to generate it from anything abstract and I don't
> > see any completely new formats coming (i.e. not just optional parts)?
>
> The patch as-submitted does have a lightweight abstraction layer of
> sorts, but the main code line feels free to ignore that and hack around
> it.  I agree that we can't avoid special-casing the text output, but
> I'm trying to improve the encapsulation otherwise.  What I've got at the
> moment is attached.  I'd be interested in comments on the grouping
> notion in particular --- I reverse-engineered that out of what the code
> was doing, and I'm sure it could use improvement.
Adding the notion of opening a 'empty' Group together with X_OPENCLOSE or 
handling of X_OPENING|X_CLOSING would allow to handle empty tags like in 
ExplainOneUtility ().

Otherwise it seems to look nice.

Andres

-- 
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] machine-readable explain output v4

[Robert Haas]

On Sun, Aug 9, 2009 at 8:53 PM, Tom Lane wrote:
> Andres Freund  writes:
>> On Monday 10 August 2009 01:21:35 Andrew Dunstan wrote:
>>> That ";" after the attribute is almost certainly wrong. This is a classic
>>> case of what I was talking about a month or two ago. Building up XML (or
>>> any structured doc, really, XML is not special in this regard) by ad hoc
>>> methods is horribly error prone. if you don't want to rely on libxml, then
>>> I think you need to develop a lightweight abstraction rather than just
>>> appending to a StringInfo.
>
>> While it would be possible to add another step inbetween and generate a 
>> format
>> neutral tree and generate the different formats out of it I am not sure that
>> this is worthwile.
>> The current text format will need to stay special cased anyway because its 
>> far
>> to inconsistent to generate it from anything abstract and I don't see any
>> completely new formats coming (i.e. not just optional parts)?
>
> The patch as-submitted does have a lightweight abstraction layer of
> sorts, but the main code line feels free to ignore that and hack around
> it.  I agree that we can't avoid special-casing the text output, but
> I'm trying to improve the encapsulation otherwise.  What I've got at the
> moment is attached.  I'd be interested in comments on the grouping
> notion in particular --- I reverse-engineered that out of what the code
> was doing, and I'm sure it could use improvement.

I haven't tested it but it looks pretty good to me.  I probably should
have done something like this to begin with, but I fell into the trap
of being too timid about introducing new concepts.

One subtle point that isn't documented and probably should be is that
JSON can't support a container that behaves partly like a list and
partly like a hash, as XML can.  So for example in XML a  tag
could have children like  (one each) and could also have
its inner, outer, and sub-plans in there as  tags right under
the parent .  I'm not sure this would be good design anyway, but
it COULD be done.  In JSON, this will crash and burn, because the
container is either an array (which precludes labelling the elements)
or a hash (which precludes duplicates).

I've avoided this problem by introducing an additional layer of
grouping whenever this situation comes up; for symmetry it exists in
both output formats.   So for example in the above-mentioned case the
 has a child called  which in turn contains each 
that belongs under the parent; in JSON, this maps nicely to a property
called "Plans" which points to an array of hashes, each of which
represents a plan.

We should probably have a long comment somewhere in explain.c
explaining all of this.  It's the sort of thing that you figure out
you need to have when you're writing the code, but it might not be too
obvious reading the code after the fact.  It's also a reason why I'm
really glad I decided to write two alternative output formats;
otherwise, I fear that we would have blundered into a bunch of
XML-isms that wouldn't have been translatable into anything else.

...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] machine-readable explain output v4

[Robert Haas]

On Sun, Aug 9, 2009 at 9:03 PM, Tom Lane wrote:
> Andrew Dunstan  writes:
>> I takle it back. It's still there at
>> 
>> posted 3 days ago.
>
> Hmm, I think the archive website must be mangling that somehow.
> What I have in the code I'm reviewing is
>
>    if (es.format == EXPLAIN_FORMAT_XML)
>        appendStringInfoString(es.str,
>            "http://www.postgresql.org/2009/explain\";>\n");
>
> I was planning to complain about the format of this URL --- shouldn't it
> be more like http://www.postgresql.org/explain/v1 ? --- but there's
> no semicolon.

Peter Eisentraut suggested it.  Take it up with him because I don't care.  :-)

...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] machine-readable explain output v4

[Tom Lane]

Andrew Dunstan  writes:
> I takle it back. It's still there at 
>  
> posted 3 days ago.

Hmm, I think the archive website must be mangling that somehow.
What I have in the code I'm reviewing is

if (es.format == EXPLAIN_FORMAT_XML)
appendStringInfoString(es.str,
"http://www.postgresql.org/2009/explain\";>\n");

I was planning to complain about the format of this URL --- shouldn't it
be more like http://www.postgresql.org/explain/v1 ? --- but there's
no semicolon.

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] machine-readable explain output v4

[Robert Haas]

On Sun, Aug 9, 2009 at 8:48 PM, Andrew Dunstan wrote:
>
>
> Andrew Dunstan wrote:
>>
>>
>> Andres Freund wrote:

 BTW, has anyone tried validating the XML at all? I just looked very
 briefly at the patch at
  and
 I noticed this which makes me suspicious:

 +     if (es.format == EXPLAIN_FORMAT_XML)
 +         appendStringInfoString(es.str,
 +             "http://www.postgresql.org/2009/explain\";
 ;>\n");

>>>
>>> That bug is fixed - as referenced above I wrote a schema and validated
>>> it. So, yes, the generated XML was valid at least before the last round of
>>> refactoring. And I looked through the output quite a bit so I would
>>> surprised if there is such a breakage.
>>> then someone needs to update the commitfest page with the lastest patch.
>>> That's the link I followed.
>>>
>>>
>>>
>>
>> Hmm. I wonder how i got the link to an old version of the patch.
>>
>> Anyway, I'm glad it's fixed.
>
>
> I takle it back. It's still there at
>  posted 3
> days ago.

What the hell?  I have every version of that patch I've ever submitted
in ~/patch/explain-as-submitted, and that extra semicolon is not there
in any of them.  Furthermore, when I open up the attachment from my
sent mail, the semicolon isn't there either.  Yet I see it at the link
you provided just as clearly as you do.  Is there a bug in the
archives code???

...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] machine-readable explain output v4

[Andres Freund]

On Monday 10 August 2009 02:48:29 Andrew Dunstan wrote:
> Andrew Dunstan wrote:
> > Andres Freund wrote:
> >>> BTW, has anyone tried validating the XML at all? I just looked very
> >>> briefly at the patch at
> >>>  and
> >>> I noticed this which makes me suspicious:
> >>>
> >>> + if (es.format == EXPLAIN_FORMAT_XML)
> >>> + appendStringInfoString(es.str,
> >>> + " >>> xmlns=\"http://www.postgresql.org/2009/explain\";
> >>> ;>\n");
> >>
> >> That bug is fixed - as referenced above I wrote a schema and
> >> validated it. So, yes, the generated XML was valid at least before
> >> the last round of refactoring. And I looked through the output quite
> >> a bit so I would surprised if there is such a breakage.
> >> then someone needs to update the commitfest page with the lastest
> >> patch. That's the link I followed.
> >
> > Hmm. I wonder how i got the link to an old version of the patch.
> >
> > Anyway, I'm glad it's fixed.
>
> I takle it back. It's still there at
> 
> posted 3 days ago.
That seems to be a failure of the mail interface? I neither see it when 
opening the attachement manually nor in the git repository?

Andres

-- 
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] machine-readable explain output v4

[Tom Lane]

Andres Freund  writes:
> On Monday 10 August 2009 01:21:35 Andrew Dunstan wrote:
>> That ";" after the attribute is almost certainly wrong. This is a classic
>> case of what I was talking about a month or two ago. Building up XML (or
>> any structured doc, really, XML is not special in this regard) by ad hoc
>> methods is horribly error prone. if you don't want to rely on libxml, then
>> I think you need to develop a lightweight abstraction rather than just
>> appending to a StringInfo.

> While it would be possible to add another step inbetween and generate a 
> format 
> neutral tree and generate the different formats out of it I am not sure that 
> this is worthwile.
> The current text format will need to stay special cased anyway because its 
> far 
> to inconsistent to generate it from anything abstract and I don't see any 
> completely new formats coming (i.e. not just optional parts)?

The patch as-submitted does have a lightweight abstraction layer of
sorts, but the main code line feels free to ignore that and hack around
it.  I agree that we can't avoid special-casing the text output, but
I'm trying to improve the encapsulation otherwise.  What I've got at the
moment is attached.  I'd be interested in comments on the grouping
notion in particular --- I reverse-engineered that out of what the code
was doing, and I'm sure it could use improvement.

regards, tom lane


/*
 * Explain a property, such as sort keys or targets, that takes the form of
 * a list of unlabeled items.  "data" is a list of C strings.
 */
static void
ExplainPropertyList(const char *qlabel, List *data, ExplainState *es)
{
ListCell   *lc;
boolfirst = true;

switch (es->format)
{
case EXPLAIN_FORMAT_TEXT:
appendStringInfoSpaces(es->str, es->indent * 2);
appendStringInfo(es->str, "  %s: ", qlabel);
foreach(lc, data)
{
if (!first)
appendStringInfoString(es->str, ", ");
appendStringInfoString(es->str, (const char *) 
lfirst(lc));
first = false;
}
appendStringInfoChar(es->str, '\n');
break;

case EXPLAIN_FORMAT_XML:
ExplainXMLTag(qlabel, X_OPENING, es);
foreach(lc, data)
{
char   *str;

appendStringInfoSpaces(es->str, es->indent * 2 
+ 2);
appendStringInfoString(es->str, "");
str = escape_xml((const char *) lfirst(lc));
appendStringInfoString(es->str, str);
pfree(str);
appendStringInfoString(es->str, "\n");
}
ExplainXMLTag(qlabel, X_CLOSING, es);
break;

case EXPLAIN_FORMAT_JSON:
ExplainJSONLineEnding(es);
appendStringInfoSpaces(es->str, es->indent * 2);
escape_json(es->str, qlabel);
appendStringInfoString(es->str, ": [");
foreach(lc, data)
{
if (!first)
appendStringInfoString(es->str, ", ");
escape_json(es->str, (const char *) lfirst(lc));
first = false;
}
appendStringInfoChar(es->str, ']');
break;
}
}

#define ExplainPropertyText(qlabel, value, es)  \
ExplainProperty(qlabel, value, false, es)

/*
 * Explain a simple property.
 *
 * If "numeric" is true, the value is a number (or other value that
 * doesn't need quoting in JSON).
 */
static void
ExplainProperty(const char *qlabel, const char *value, bool numeric,
ExplainState *es)
{
switch (es->format)
{
case EXPLAIN_FORMAT_TEXT:
appendStringInfoSpaces(es->str, es->indent * 2);
appendStringInfo(es->str, "  %s: %s\n", qlabel, value);
break;

case EXPLAIN_FORMAT_XML:
{
char   *str;

appendStringInfoSpaces(es->str, es->indent * 2);
ExplainXMLTag(qlabel, X_OPENING | 
X_NOWHITESPACE, es);
str = escape_xml(value);
appendStringInfoString(es->str, str);
pfree(str);

Re: [HACKERS] machine-readable explain output v4

[Andrew Dunstan]

Andrew Dunstan wrote:



Andres Freund wrote:

BTW, has anyone tried validating the XML at all? I just looked very
briefly at the patch at
 and
I noticed this which makes me suspicious:

+ if (es.format == EXPLAIN_FORMAT_XML)
+ appendStringInfoString(es.str,
+ "xmlns=\"http://www.postgresql.org/2009/explain\";

;>\n");

That bug is fixed - as referenced above I wrote a schema and 
validated it. So, yes, the generated XML was valid at least before 
the last round of refactoring. And I looked through the output quite 
a bit so I would surprised if there is such a breakage.
then someone needs to update the commitfest page with the lastest 
patch. That's the link I followed.






Hmm. I wonder how i got the link to an old version of the patch.

Anyway, I'm glad it's fixed.



I takle it back. It's still there at 
 
posted 3 days ago.



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] machine-readable explain output v4

[Andrew Dunstan]

Andres Freund wrote:

BTW, has anyone tried validating the XML at all? I just looked very
briefly at the patch at
 and
I noticed this which makes me suspicious:

+   if (es.format == EXPLAIN_FORMAT_XML)
+   appendStringInfoString(es.str,
+   "http://www.postgresql.org/2009/explain\";
;>\n");

That bug is fixed - as referenced above I wrote a schema and validated it. So, 
yes, the generated XML was valid at least before the last round of 
refactoring. And I looked through the output quite a bit so I would surprised 
if there is such a breakage.

then someone needs to update the commitfest page with the lastest patch. That's 
the link I followed.


  
  


Hmm. I wonder how i got the link to an old version of the patch.

Anyway, I'm glad it's fixed.


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] machine-readable explain output v4

[Andres Freund]

On Monday 10 August 2009 01:21:35 Andrew Dunstan wrote:
> Robert Haas wrote:
> > The one significant representational choice that I'm aware of having
> > made is to use nested tags rather than attributes in the XML format.
> > This seems to me to offer several advantages.  First, it's clearly
> > impossible to standardize on attributes, because attributes can only
> > be text, and it seems to me that if we're going to try to output
> > structured data, we want to take that as far as we can, and we have
> > attributes (like sort keys) that are lists rather than scalars.  Using
> > tags means that they can have substructure when needed.  Second, it
> > seems likely to me that people will want to extend explain further in
> > the future: indeed, that was the whole point of the explain-options
> > patch which was already committed.  That's pretty simple in the
> > current design - just add a few more calls to ExplainPropertyText or
> > ExplainPropertyList in the appropriate place, and you're done.  I'm
> > pretty sure that splitting things up between attributes and nested
> > tags would complicate such modifications.
> The XML Schema standard is a language for specifying the structure of a
> given XML document type, and while it is undoubtedly complex, it is also
> much more powerful than the older DTD mechanism. I think we should be
> creating (and publishing) an XML Schema specification for any XML
> documents we are producing. There are a number of members of the
> community who are equipped to help produce these.
I produced/mailed a relaxng version for a a bit older version and I plan to 
refresh and document it once the format seems suitably stable. I am not sure 
it is yet. If yes, this should not take that long...
(Relaxng because you easily can convert it into most other XML schema 
description languages)

> There is probably a good case for using an explicit namespace with such
> docs. So we might have something like:
>
>  xmlns:pg="http://www.postgresql.org/xmlspecs/explain/v1.xsd";> 
>
> BTW, has anyone tried validating the XML at all? I just looked very
> briefly at the patch at
>  and
> I noticed this which makes me suspicious:
>
> + if (es.format == EXPLAIN_FORMAT_XML)
> + appendStringInfoString(es.str,
> + " xmlns=\"http://www.postgresql.org/2009/explain\";
> ;>\n");
That bug is fixed - as referenced above I wrote a schema and validated it. So, 
yes, the generated XML was valid at least before the last round of 
refactoring. And I looked through the output quite a bit so I would surprised 
if there is such a breakage.

> That ";" after the attribute is almost certainly wrong. This is a classic
> case of what I was talking about a month or two ago. Building up XML (or
> any structured doc, really, XML is not special in this regard) by ad hoc
> methods is horribly error prone. if you don't want to rely on libxml, then
> I think you need to develop a lightweight abstraction rather than just
> appending to a StringInfo.
Actually by now a non-insignificant portion already "outsources" this - only 
some special cases (empty attributes, no newlines wanted, initial element with 
namespace) do not do this.

While it would be possible to add another step inbetween and generate a format 
neutral tree and generate the different formats out of it I am not sure that 
this is worthwile.
The current text format will need to stay special cased anyway because its far 
to inconsistent to generate it from anything abstract and I don't see any 
completely new formats coming (i.e. not just optional parts)?

Andres

-- 
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] machine-readable explain output v4

[Andrew Dunstan]

Robert Haas wrote:

The one significant representational choice that I'm aware of having
made is to use nested tags rather than attributes in the XML format.
This seems to me to offer several advantages.  First, it's clearly
impossible to standardize on attributes, because attributes can only
be text, and it seems to me that if we're going to try to output
structured data, we want to take that as far as we can, and we have
attributes (like sort keys) that are lists rather than scalars.  Using
tags means that they can have substructure when needed.  Second, it
seems likely to me that people will want to extend explain further in
the future: indeed, that was the whole point of the explain-options
patch which was already committed.  That's pretty simple in the
current design - just add a few more calls to ExplainPropertyText or
ExplainPropertyList in the appropriate place, and you're done.  I'm
pretty sure that splitting things up between attributes and nested
tags would complicate such modifications.

  
  


In general, in XML one uses an attribute for a named property of an 
object that can only have one value at a time. A classic example is the 
dimensions of an object - it can only have one width and height. 
Children (nested tags, particularly) are used for things it can have an 
arbitrary number of, or things which in turn can have children.  the 
HTML  and  elements are (respectively) examples of these. 
Generally, attribute values especially should be short - I recently saw 
an example that had an entire image hex encoded in an XML attribute, 
which struck me as just horrible. Enumerations, date and time values, 
booleans, measurements - these are common types of attribute values. 
Extracting a value from an attribute is no more or less difficult than 
from a nested tag, using the XPath query language.


The XML Schema standard is a language for specifying the structure of a 
given XML document type, and while it is undoubtedly complex, it is also 
much more powerful than the older DTD mechanism. I think we should be 
creating (and publishing) an XML Schema specification for any XML 
documents we are producing. There are a number of members of the 
community who are equipped to help produce these.


There is probably a good case for using an explicit namespace with such 
docs. So we might have something like:


   http://www.postgresql.org/xmlspecs/explain/v1.xsd";> 

BTW, has anyone tried validating the XML at all? I just looked very 
briefly at the patch at 
 and 
I noticed this which makes me suspicious:


+   if (es.format == EXPLAIN_FORMAT_XML)
+   appendStringInfoString(es.str,
+   "http://www.postgresql.org/2009/explain\"; 
;>\n");


That ";" after the attribute is almost certainly wrong. This is a classic case 
of what I was talking about a month or two ago. Building up XML (or any structured doc, 
really, XML is not special in this regard) by ad hoc methods is horribly error prone. if 
you don't want to rely on libxml, then I think you need to develop a lightweight 
abstraction rather than just appending to a StringInfo.





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

[Petr Jelinek]

Petr Jelinek wrote:

I attached revised version of the patch. Changes, thoughts:
- SCHEMA is mandatory now
- removed VIEWS and GRANT ON VIEW since it looks like only me and 
Stephen want it there
- the patch is now made so that adding new filters in the future won't 
mean tearing of half of the parser code and replacing it
- I decided to go with GRANT ON ALL IN SCHEMA syntax, because I am 
thinking there is no difference in adding extended syntax to the 
standard command in GRANT and in SELECT, ALTER TABLE and other 
commands we extended. And I don't see any way standard could add 
exactly same syntax for doing something completely different (which is 
the only way they could break this).
Argh, why does this always happen to me ? Immediately after sending the 
patch I realized there needs to be one more little change done (merging 
tables and views in the getNamespacesObjectsOids function).


--
Regards
Petr Jelinek (PJMODOS)



grantonall-20090810v2.diff.gz
Description: Unix tar archive

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

[Petr Jelinek]

Hi,

I attached revised version of the patch. Changes, thoughts:
- SCHEMA is mandatory now
- removed VIEWS and GRANT ON VIEW since it looks like only me and 
Stephen want it there
- the patch is now made so that adding new filters in the future won't 
mean tearing of half of the parser code and replacing it
- I decided to go with GRANT ON ALL IN SCHEMA syntax, because I am 
thinking there is no difference in adding extended syntax to the 
standard command in GRANT and in SELECT, ALTER TABLE and other commands 
we extended. And I don't see any way standard could add exactly same 
syntax for doing something completely different (which is the only way 
they could break this).


--
Regards
Petr Jelinek (PJMODOS)



grantonall-20090810.diff.gz
Description: Unix tar archive

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

[Petr Jelinek]

Josh Berkus wrote:

I disagree here.  While it's nice to be MySQL-compatible, a glob "*" is
not at all consistent with other SQL syntax, whereas "ALL" and "GRANT ON
ALL IN SCHEMA " are.
  
The * was reaction to Toms fears of standard adding GRANT ON ALL with 
conflicting meaning, but I don't really see that as relevant point 
anymore (see my submission of the revised patch).



The answer as far as the standard is concerned is, why not make an
effort to get this into the standard?
  

We can try :) do we have somebody in the committee ?


And how do we want to filter default acls ?
  

My opinion is that the best way to do this would be ALTER DEFAULT
PRIVILEGES GRANT ..., without any additional filters, it would just
affect the role which runs this command. I think this is best solution
because ALTER SCHEMA forces creation of many schemas that might not have
anything to do with structure of the database (if you want different
default privileges for different things). Also having default privileges
per role with filters on various things will IMHO create more confusion
than good. And finally if somebody wants to have different default
privileges for different things than he can just create child roles with
different default privileges and use SET SESSION AUTHORIZATION to switch
between them.



I'm not sure if I'm agreeing or disagreeing with you here, but I'll say
that it doesn't help a user have a consistent setup for assigning
privileges.  GRANT ON ALL working per *schema* while ALTER DEFAULT
working per *role* will just create confusion and not improve the
managability of privileges in PostgreSQL.  We need a DEFAULT and a GRANT
ALL statement which can be executed on the same scope so that users can
easily set up a coherent access control scheme.

For my part, I *do* use schema to control my security context for
database objects; I find that it's a convenience to be able to take
objects which a role has no permissions on out of its visibility
(through search_path) as well.  And schema-based security mentally maps
to directory-based permissions, which unix sysadmins instinctively
understand.  So I think that a form of GRANT ALL/DEFAULT which supported
schema-scoping would be useful to a *lot* more people than one which didn't.

I do understand that other scopes (such as scoping by object owner) are
equally valid and maybe more consistent with the SQL permissions model.
 However, I think that role-scoping is not as intuitively understandible
to most users and would be, for that reason, less used and less useful.
  
I was discussing this with Stephen and I agree now that schema based 
filtering is the best way. The role based filtering I proposed would 
mean user would have to have create role privilege to really take 
advantage of default acls, also it wouldn't really solve the real world 
problems which default acls aims to solve. I also agree on the point 
that GRANT ON ALL and DEFAULT PRIVILEGES should have same or similar filter.


So currently I see the next step being rewriting the patch for the ALTER 
DEFAULT PRIVILEGES IN SCHEMA schemaname GRANT ... and leaving the 
functionality itself unchanged (with the exception of having VIEW as 
separate object which I will remove).


--
Regards
Petr Jelinek (PJMODOS)

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

[Greg Stark]

On Sun, Aug 9, 2009 at 7:30 PM, Tom Lane wrote:
> 1. As I already mentioned, I think the mixed-notation business is a bad
> idea.  It's unintuitive, it's not especially useful, and it substantially
> increases our risk of being semantically incompatible with whatever the
> SQL committee might someday do in this area.  I think we should disallow
> it until we see what they do.  I gather that this might be an unpopular
> position though.

It seems like we could safely allow the cases which are unambiguous.
Namely where the call has a sequence of unnamed parameters followed by
some named parameters all of which refer to parameters which come
later.

So foo(1,2,3 as x,4 as y) would be legal as long as x and y were not
one of the first three arguments.

That's a pretty common idiom when you have a function which does
something normal to the first few arguments and then has a bunch of
non-standard modes which can be optionally invoked. Take for example
the perl DBI's connect method which takes a data source, username,
authentication token, then a list of parameters which can be any of
dozens of parameters (actually it takes a fifth argument which is a
hashref -- but the point here is just that it's a common style, not
that we should be copying perl's solution).

The reason I'm saying this is safe is because there's just no other
possible interpretation. As long as it only covers the unambiguous
cases then there's no other meaning other implementations can define
which this would conflict with.


-- 
greg
http://mit.edu/~gsstark/resume.pdf

-- 
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] machine-readable explain output v4

[Robert Haas]

On Sun, Aug 9, 2009 at 3:57 PM, Tom Lane wrote:
> Robert Haas  writes:
>> Revised patch attached.  I'm not convinced this is as good as it can
>> be, but I've been looking at this patch for so long that I'm starting
>> to get cross-eyed, and I'd like to Tom at least have a look at this
>> and assess it before we run out of CommitFest.
>
> I'm starting to look at this now.  I feel unqualified to opine on the
> quality of the XML/JSON schema design, and given the utter lack of
> documentation of what that design is, I'm not sure that anyone who
> could comment on it has done so.  Could we have a spec please?

*scratches head*

You're not the first person to make that request, and I'd like to
respond to it to well, but I don't really know what to write.  Most of
the discussion about the XML/JSON output format thus far has been
around things like whether we should downcase everything, and even the
people offering these comments have mostly labelled them with words to
the effect of "I know this is trival but...".  I think that the reason
for this is that fundamentally explain output is fundamentally a tree,
and XML and JSON both have ways of representing a tree with properties
hanging off the nodes, and this patch uses those ways.  I can't figure
out what else there is, so I don't know what I'm explaining why I
didn't do.

The one significant representational choice that I'm aware of having
made is to use nested tags rather than attributes in the XML format.
This seems to me to offer several advantages.  First, it's clearly
impossible to standardize on attributes, because attributes can only
be text, and it seems to me that if we're going to try to output
structured data, we want to take that as far as we can, and we have
attributes (like sort keys) that are lists rather than scalars.  Using
tags means that they can have substructure when needed.  Second, it
seems likely to me that people will want to extend explain further in
the future: indeed, that was the whole point of the explain-options
patch which was already committed.  That's pretty simple in the
current design - just add a few more calls to ExplainPropertyText or
ExplainPropertyList in the appropriate place, and you're done.  I'm
pretty sure that splitting things up between attributes and nested
tags would complicate such modifications.

Peter Eisentraut, in an earlier review of this patch, complained about
the format as well, saying something along the lines of "this is
trying to be all things to all people".  I don't want to dismiss that
criticism, but neither can I put my finger on the problem.  In an
ideal world, we'd like to be all things to all people, but it's
usually not possible to achieve that in practice.  Still, it's not
clear to me what need this wouldn't serve.  It's possible to generate
the text format from the XML or JSON format, so it should be
well-suited to graphical presentation of explain output.  It's also
possible to grope through the output and, say, find the average cost
of all your seqscan nodes, or verify the absence of merge joins, or
anything of that sort that someone might think that they want to do.

In a nutshell, the design is "take all the fields we have now and put
XML/JSON markup around them so they're easier to get to".  Maybe
that's not enough of a design, but I don't have any other ideas.

> Also, the JSON code seems a bit messy/poorly factorized.  Is there
> a reason for that, or just it's not as mature as the XML code?

I wrote them together, so it's not a question of code maturity, but I
wouldn't rule out me being dumb.  I'm open to suggestions...  AFAICS,
the need to comma-separate list and hash elements is most of the
problem.  I had thought about replacing es->needs_separator with a
list so that we could push/pop elements, but I wasn't totally sure
whether that was a good idea.

...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 - merged up to CVS HEAD

[Bruce Momjian]

Simon Riggs wrote:
> On Sat, 2009-08-08 at 13:12 -0400, Bruce Momjian wrote:
> > Simon Riggs wrote:
> > > 
> > > I'm not sure why you're stirring this up again.
> > > 
> 
> > You stated:
> > 
> > - It's going to be very confusing if people submit their own versions of
> > - it. So now we have mine, Heikki's and Robert's. I'd like this to stop
> > - please, have a little faith and a little patience. Presumably Robert's
> > - rebasing patch is best place to start from now for later work.
> > 
> > I assume your last sentence is saying exactly that Robert's version
> > should be used as the most current reprsentation of this feature patch.
> 
> That isn't what I meant then and isn't what I think now: that patch is
> not verified.

I am not sure how to respond to you when I can't even interpret what you
say in emails, e.g. "Presumably Robert's rebasing patch is best place to
start from now for later work."

> As you point out, people can do anything they want with submitted code,
> so they may make any judgement they wish about that patch. If anybody
> thinks any good will come from ignoring the opinion of the original
> author, go right ahead.

Right.  At some point more people are going to get involved and complete
the patch --- historically this is the way complex patches have evolved,
and I think many of your patches are in that group.

> > The bottom line is that you think you have ownership of the patch and
> > the feature --- you do not.
> > 
> > You are right you don't have to justify anything, but neither can you
> > claim ownership of the patch/feature and complain that others are
> > working on it too.  This is a community project --- if you want your
> > patches to remain your property, I suggest you no longer post them to
> > our community lists.  If you are actively working on patches, I assume
> > others will not duplicate your work, but if you are idle, others are
> > encouraged to keep improving the patch.  Again, if you don't like
> > that,
> > then perhaps the community-development process isn't for you.
> 
> I've *never* spoken of code or feature ownership. But this is a
> community project and I can request teamwork from other contributors,
> which is what I did.
> 
> I've said very clearly that I am working on this and it's fairly
> laughable to suggest that anybody thought I wasn't. What more should I
> do to prove something is "active" if you won't accept my clearly spoken
> word? How did you decide I was idle exactly? 

Your statement of 15 Jul 2009 stated:

- I've said very clearly that I would work on this for 8.5 [at the
- developer meeting] and also that it wouldn't be ready for the first
- commit fest, when asked. I was told recently that someone heard the
- patch was dead; I've never said that, but I would like a summer holiday.

I assume that means you were not actively working on it, hence my
conclusion, which is probably wrong because I can't manage to interpret
your emails.  :-(

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

  + If your life is a hard drive, Christ can be your backup. +

-- 
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 - merged up to CVS HEAD

[Simon Riggs]

On Sat, 2009-08-08 at 13:12 -0400, Bruce Momjian wrote:
> Simon Riggs wrote:
> > 
> > I'm not sure why you're stirring this up again.
> > 

> You stated:
> 
> - It's going to be very confusing if people submit their own versions of
> - it. So now we have mine, Heikki's and Robert's. I'd like this to stop
> - please, have a little faith and a little patience. Presumably Robert's
> - rebasing patch is best place to start from now for later work.
> 
> I assume your last sentence is saying exactly that Robert's version
> should be used as the most current reprsentation of this feature patch.

That isn't what I meant then and isn't what I think now: that patch is
not verified.

The reason for my objection was that accepting patches had already
caused significant setbacks on this complex patch. I won't be ignoring
Robert's work, which would be petty, but I won't be picking it up
wholesale either, nor will I be providing a review of it. Nor Heikki's,
nor anyone elses.

I am moving forward the parts of the patch that I consider worth
submitting. I need to be happy with every single line of code before I
submit it; it's too easy to make a mistake otherwise. I'm not going to
submit something that I can't verify, any more than I would expect any
committer to commit code they can't verify either. The current dev team
(Simon, Gianni, Gabriele) only has time to spend on testing one patch,
not various ones. I do hope to receive comments from reviewers and will
include consensus changes into the code. And as I mentioned elsewhere,
there are still changes/features to add to the code itself.

As you point out, people can do anything they want with submitted code,
so they may make any judgement they wish about that patch. If anybody
thinks any good will come from ignoring the opinion of the original
author, go right ahead.

> The bottom line is that you think you have ownership of the patch and
> the feature --- you do not.
> 
> You are right you don't have to justify anything, but neither can you
> claim ownership of the patch/feature and complain that others are
> working on it too.  This is a community project --- if you want your
> patches to remain your property, I suggest you no longer post them to
> our community lists.  If you are actively working on patches, I assume
> others will not duplicate your work, but if you are idle, others are
> encouraged to keep improving the patch.  Again, if you don't like
> that,
> then perhaps the community-development process isn't for you.

I've *never* spoken of code or feature ownership. But this is a
community project and I can request teamwork from other contributors,
which is what I did.

I've said very clearly that I am working on this and it's fairly
laughable to suggest that anybody thought I wasn't. What more should I
do to prove something is "active" if you won't accept my clearly spoken
word? How did you decide I was idle exactly? 

I'll make sure to do regular blogs on what I'm working on.

I have no problem with Robert. I have no problem with Robert completing
my inactive patches - he is doing exactly that with join removal and I
haven't uttered a word. If I felt as you think I do, then surely I would
have objected to both. Yet I have only objected on the one patch that
I've said clearly I'm working on, with specific reasons. If Robert
hadn't been present when I said it, I might have reacted differently.

To everybody and anybody: please don't submit alternative versions of a
patch that other hackers have said they are working on, and don't have
conversations about those projects on diverse threads. That's not a
claim of code or feature ownership, it's just common sense teamwork on
an important development project. 

-- 
 Simon Riggs   www.2ndQuadrant.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] machine-readable explain output v4

[Tom Lane]

Robert Haas  writes:
> Revised patch attached.  I'm not convinced this is as good as it can
> be, but I've been looking at this patch for so long that I'm starting
> to get cross-eyed, and I'd like to Tom at least have a look at this
> and assess it before we run out of CommitFest.

I'm starting to look at this now.  I feel unqualified to opine on the
quality of the XML/JSON schema design, and given the utter lack of
documentation of what that design is, I'm not sure that anyone who
could comment on it has done so.  Could we have a spec please?

Also, the JSON code seems a bit messy/poorly factorized.  Is there
a reason for that, or just it's not as mature as the XML 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] Review: Patch for contains/overlap of polygons

[Joshua Tolley]

On Sun, Aug 09, 2009 at 02:29:44PM -0400, Robert Haas wrote:
> On Sun, Aug 9, 2009 at 2:20 PM, Bruce Momjian wrote:
> > This is a nice section layout for a patch review report --- should we
> > provide an email template like this one for reviewers to use?
> 
> We could, but it might be over-engineering.  Those particular section
> headers might not be applicable to someone else's review.

I've just added a link to this email to the "Reviewing a Patch" wiki page
(http://wiki.postgresql.org/wiki/Reviewing_a_Patch). Do with it as you see fit
:)

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com


signature.asc
Description: Digital signature

Re: [HACKERS] mixed, named notation support

[Tom Lane]

Robert Haas  writes:
> On Sun, Aug 9, 2009 at 2:17 PM, Tom Lane wrote:
>> I think this patch is an exercise in
>> guessing at what the SQL committee will eventually do, and as such, we
>> should avoid like the plague making any guesses that carry significant
>> risk of being semantically incompatible with what they eventually do.
>> The risk/reward ratio isn't good enough.

> I completely agree; I would have chosen to boot the entire patch for
> exactly that reason.

Well, that's certainly still an available option.  But people have been
asking for this type of feature for a long time.  I think we should be
willing to include something along this line.  What I don't want to do
is include something that might be semantically incompatible with the
eventual standard --- by which I mean accepting a call that the standard
also accepts, but specifies doing something different than what we do.
I'd prefer to omit inessential functionality rather than risk that.

It might be that the patch does, or can be made to, throw error in any
case that's even slightly questionable, while still allowing mixed cases
that seem certain to have only one possible interpretation.  But I'm not
convinced that's where it's at today.

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] hot standby - merged up to CVS HEAD

[Robert Haas]

On Sun, Aug 9, 2009 at 6:11 AM, Simon Riggs wrote:
> I'm working on HS; I've said so clearly and say it again now. To my
> knowledge, no other Postgres project has committed to a timetable for
> delivery, so I'm not clear why you think one should have been given
> here, or why the absence of such a timetable implies anything. Dev tree

Well, basically because otherwise nobody except you can do anything.
In your last email you wrote:

> assistance from any and every other hacker is welcome in producing that.

What I need is some help figuring out when and how I can provide that
assistance and what I can do.

At the moment, the version of the patch that I last posted does not
apply due to a conflict in sinval.h, I believe due to conflicts
introduced by Fujii Masao's signal multiplexing patch.  I haven't had
time to look at that in any detail yet, but I'd like to do do so soon.
 There are some other things that look like easy cleanups that I think
I could knock out as well; see my original email on this thread.  Of
course, your input on those items would be invaluable.  Of course, if
you've already done some of this work, that would be great, but then
it seems like you ought to have let us know that you were doing it
before you did it, and posted the updated patch to -hackers
afterwards, just as you asked me to do.

Working disconnected from everyone else until September 15th (or
November 15th) and then posting the patch will make it very, very
difficult for anyone else to do anything useful.   When I was working
the explain patches, I posted them regularly to -hackers, and
published a git repository on git.postgresql.org, which meant that
anyone could follow along at home if they wished.  Since Hot Standby
is more interesting before breakfast than machine-readable explain
output is all day, the same approach seems desirable here.  At the
very least, I think you should post your progress weekly so that we
can read, review, comment, propose changes...

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

[Tom Lane]

Oh, another thing: the present restriction that all function parameters
after the first one with a default must also have defaults is based on
limitations of positional call notation.  Does it make sense to relax
that restriction once we allow named call notation, and if so what
should we do exactly?  (This could be addressed in a followup patch,
it doesn't necessarily have to be dealt with immediately.)

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

[Tom Lane]

I've now read most of this patch, and I think there are some things that
need rework, and perhaps debate about what we want.

1. As I already mentioned, I think the mixed-notation business is a bad
idea.  It's unintuitive, it's not especially useful, and it substantially
increases our risk of being semantically incompatible with whatever the
SQL committee might someday do in this area.  I think we should disallow
it until we see what they do.  I gather that this might be an unpopular
position though.

2. It doesn't appear that any attention has been given to what happens
if CREATE OR REPLACE FUNCTION is used to change the parameter names of
an existing function.  Since the post-analysis representation of parameter
lists is still entirely positional, the actual effect on a function call
in a stored view or rule is nil --- it will still call the same function
it did before, passing the parameters that were originally identified to
be passed.  That might be considered good, but it's quite unlike what
will happen to function calls that are stored textually (eg, in plpgsql
functions).  Is that what we want?  Or should we consider that parameter
names are now part of the API of a function, and forbid CREATE OR REPLACE
FUNCTION from changing them?  Or perhaps we should recheck parameter name
matching someplace after analysis, perhaps at default-expansion time?

3. In the same vein, CREATE FUNCTION doesn't disallow duplicate parameter
names, nor functions that have names for some but not all parameters.
The patch appears to cope with the latter case but not the former.
Should we disallow these things in CREATE FUNCTION, or make the patch
never match such functions, or what?

4. No attention has been given to making ruleutils.c list out named or
mixed function calls correctly.

5. I don't like anything about the "leaky list" representation of
analyzed function arguments.  Having null subexpressions in unexpected
places isn't a good idea --- it's likely to cause crashes in code that
isn't being really cautious.  Moreover, if we did ultimately support
mixed notation, there's no way to list it out correctly on the basis
of this representation, because you can't tell which arguments were
named and which weren't.  I think it would be better to keep the
ArgExprs in the transformed parameter list and have the planner
remove them (and reorder the arguments if needed) when it does
default-argument expansion.  Depending on what you think about point
#2, the transformed ArgExprs might need to carry the argument number
instead of the argument name, but in any case they'd just be there
for named arguments.  This approach probably will also reduce the
amount of change in the parser, since you won't have to separate
the names from the argument list and pass those all over the place
separately.

Some minor style issues:

* ArgExpr is confusingly named and incorrectly documented, since it's
only used for named arguments.  Perhaps NamedArgExpr would be better.
Also, it'd probably be a good idea to include a location field in it
(pointing at the parameter name not the argument expression).

* Most of the PG source code just writes "short" or "long",
not "short int".  Actually I wonder whether "int2" wouldn't
be preferred anyway, since that's how the relevant pg_proc
columns are declared.

* The error messages aren't even approximately conformant to style guide.

* Please avoid useless whitespace changes, especially ones as
ill-considered as this:

***
*** 10028,10034 
;
  
  
! name: ColId   
{ $$ = $1; };
  
  database_name:
ColId   
{ $$ = $1; };
--- 10056,10062 
;
  
  
! name: ColId   
{ $$ = $1; };
  
  database_name:
ColId   
{ $$ = $1; };

I'm going to mark the patch Waiting on Author, since it's not close
to being committable until these issues are resolved.

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] Review: Patch for contains/overlap of polygons

[Robert Haas]

On Sun, Aug 9, 2009 at 2:20 PM, Bruce Momjian wrote:
> This is a nice section layout for a patch review report --- should we
> provide an email template like this one for reviewers to use?

We could, but it might be over-engineering.  Those particular section
headers might not be applicable to someone else's review.

...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] mixed, named notation support

[Robert Haas]

On Sun, Aug 9, 2009 at 2:17 PM, Tom Lane wrote:
> Robert Haas  writes:
>> On Sun, Aug 9, 2009 at 12:27 PM, Tom Lane wrote:
>>> Now that I've started to read this patch ... exactly what is the
>>> argument for allowing a "mixed" notation (some of the parameters named
>>> and some not)?  ISTM that just serves to complicate both the patch
>>> and the user's-eye view, for no real benefit.
>
>> Wow, I can't imagine not supporting that.  Doesn't every language that
>> supports anything like named parameters also support a mix?
>
> I don't know if that's true, [...]

I'm fairly sure it is.

> and I definitely don't know if they all
> handle corner cases identically.

I'm 100% positive that they don't.

> I think this patch is an exercise in
> guessing at what the SQL committee will eventually do, and as such, we
> should avoid like the plague making any guesses that carry significant
> risk of being semantically incompatible with what they eventually do.
> The risk/reward ratio isn't good enough.

I completely agree; I would have chosen to boot the entire patch for
exactly that reason.  However, given that you don't seem to want to do
that, I was just offering my thoughts on the various possible methods
of eliminating that risk, since the one you're suggesting doesn't seem
ideal to me.  I thought that perhaps providing some examples of how
other programming languages handle this problem might be helpful - in
particular, I think the Lisp model, where each parameter must be
EITHER named or positional, has a lot to recommend it.

There is little doubt here that what gets committed here will be what
you choose to commit.

...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] Review: Patch for contains/overlap of polygons

[Bruce Momjian]

This is a nice section layout for a patch review report --- should we
provide an email template like this one for reviewers to use?

---

Josh Williams wrote:
> Teodor, et al,
> 
> This is a review of the Polygons patch:
> http://archives.postgresql.org/message-id/4a5761a2.8070...@sigaev.ru
> 
> There hasn't been any discussion, at least that I've been able to find.
> 
> Contents & Purpose
> ==
> This small patch primarily fixes a couple polygon functions,
> poly_overlap (the && operator) and poly_contain (@>).  Previously the
> functions only performed simple bounding box calculations or checks
> based on sets of points.  That works only for the simplest of cases;
> this patch accounts for more complex shapes.
> 
> Included in the patch are new regression test cases, but no changes to
> documentation.  The patch only corrects the behavior of existing
> functions, though, so perhaps no changes to the documentation are
> expected.
> 
> Initial Run
> ===
> The patch applies cleanly to HEAD. The regression tests all pass
> successfully against the new patch, but fail against pre-patched HEAD,
> so the test cases are sane and do cover the new behavior.  As far as I
> can see the math behind the new calculations seems sound.
> 
> Performance
> ===
> Despite the functions in question containing an order of magnitude more
> code the operators performed faster than the previous versions in my
> test run.  Though I have a feeling that may have more to do with this
> laptop's processor speed and/or the rather trivial test cases being
> thrown at it.  In any case having the operators work correctly should
> far outweigh the negligible performance impact.
> 
> Nitpicking & Conclusion
> ===
> The patch splits out and adds a couple helper functions next to the
> existing ones in geo_ops.c, but would those be better defined down in
> the Private routines section?
> 
> There's a #define in the middle of the touched_lseg_inside_poly()
> function.  The macro is only used in that function and a couple of times
> at that, but it still feels somewhat out of place.  Perhaps that'd be
> better placed with other #define's near the top?
> 
> I could certainly be wrong in both cases. :)  There's also two "int i"s
> declared in poly_contain().
> 
> Otherwise it seems to do exactly what it promises.  I could see the
> correct behavior of these operators being important for GIS
> applications.   +1 for committer review.
> 
> - Josh Williams
> 
> 
> 
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

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

  + If your life is a hard drive, Christ can be your backup. +

-- 
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] mixed, named notation support

[Tom Lane]

Robert Haas  writes:
> On Sun, Aug 9, 2009 at 12:27 PM, Tom Lane wrote:
>> Now that I've started to read this patch ... exactly what is the
>> argument for allowing a "mixed" notation (some of the parameters named
>> and some not)?  ISTM that just serves to complicate both the patch
>> and the user's-eye view, for no real benefit.

> Wow, I can't imagine not supporting that.  Doesn't every language that
> supports anything like named parameters also support a mix?

I don't know if that's true, and I definitely don't know if they all
handle corner cases identically.  I think this patch is an exercise in
guessing at what the SQL committee will eventually do, and as such, we
should avoid like the plague making any guesses that carry significant
risk of being semantically incompatible with what they eventually do.
The risk/reward ratio isn't good enough.

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] a short trip in the wayback machine

[Peter Eisentraut]

On Sunday 09 August 2009 17:57:23 Andrew Dunstan wrote:
> Peter Eisentraut wrote:
> > On Sunday 09 August 2009 03:53:55 Andrew Dunstan wrote:
> >> the documentation of psql's --no-readline option was removed
> >> (psql-ref.sgml v 1.23). I think this was a mistake and it should be
> >> restored :-)
> >
> > Does that option have a point?  Should the option be removed, perhaps?
>
> It has at least one prominent user -
> ql-tip.html#comments> ;-)

OK, if you re-document it, it may be useful to mention that as a use case.

-- 
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] Split-up ECPG patches

[Boszormenyi Zoltan]

Boszormenyi Zoltan írta:
> Tom Lane írta:
>   
>> Boszormenyi Zoltan  writes:
>>   
>> 
>>> Tom Lane írta:
>>> 
>>>   
 I'd look at requiring from_in as being the least-bad alternative.
   
 
>>   
>> 
>>> Hm. "FETCH FORWARD variable" can only be a rowcount var
>>> only if there's something afterwards, no? With the proposed
>>> change in fetch_direction (moving FORWARD and BACKWARD
>>> without the rowcount upper to the parent rules) now the parser is
>>> able to look behind "FORWARD variable"...
>>> 
>>>   
>> The fundamental reason that there's a problem here is that ecpg has
>> decided to accept a syntax that the backend doesn't (ie, FETCH with a
>> fetch direction but no FROM/IN).  I think that that's basically a bad
>> idea: it's not helpful to users to be inconsistent, and it requires ugly
>> hacks in ecpg, and now ugly hacks in the core grammar as well.  We
>> should resolve it either by taking out that syntax from ecpg, or by
>> making the backend accept it too.  Not by uglifying the grammars some
>> more in order to keep them inconsistent.
>>
>> If we were going to allow it in the core, I think moving the cursor
>> name into the fetch_direction production might work, ie, change
>> fetch_direction to fetch_args and make it cover everything that
>> FETCH and MOVE share.  Probably from_in could become opt_from_in,
>> since the alternatives for it are fully reserved words already, and we
>> wouldn't need to double up any of the fetch_direction productions.
>>
>>  regards, tom lane
>>   
>> 
>
> Your guess about making from_in into opt_from_in
> seems good, mostly. I tried doing exactly that and simply
> adding an empty match into from_in, I got shift/reduce conflicts
> only in "opt_from_in cursor_name". So, this rule has to be
> unrolled into 3 rules, or keeping a separate "from_in" just for
> having a separate "cursor_name" and "from_in cursor_name".
> I decided that I use the second method, it's shorter.
>   

OK, here's the WIP patch for the unified core/ecpg grammar,
with opt_from_in. But I am still getting the 2 shift/reduce
conflicts exactly for the FORWARD and BACKWARD rules
that I was getting originally. Can you look at this patch and
point me to the right direction in solving it? Thanks in advance.

> Best regards,
> Zoltán Böszörményi
>
>   


-- 
Bible has answers for everything. Proof:
"But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil." (Matthew 5:37) - basics of digital technology.
"May your kingdom come" - superficial description of plate tectonics

--
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
http://www.postgresql.at/

diff -dcrpN pgsql.orig/src/backend/parser/gram.y pgsql/src/backend/parser/gram.y
*** pgsql.orig/src/backend/parser/gram.y	2009-08-03 10:38:28.0 +0200
--- pgsql/src/backend/parser/gram.y	2009-08-09 17:48:36.0 +0200
*** static TypeName *TableFuncTypeName(List 
*** 253,259 
  
  %type 		relation_name copy_file_name
  database_name access_method_clause access_method attr_name
! index_name name file_name cluster_index_specification
  
  %type 	func_name handler_name qual_Op qual_all_Op subquery_Op
  opt_class opt_validator validator_clause
--- 253,259 
  
  %type 		relation_name copy_file_name
  database_name access_method_clause access_method attr_name
! index_name name cursor_name file_name cluster_index_specification
  
  %type 	func_name handler_name qual_Op qual_all_Op subquery_Op
  opt_class opt_validator validator_clause
*** static TypeName *TableFuncTypeName(List 
*** 331,337 
  %type 	opt_column event cursor_options opt_hold opt_set_data
  %type 	reindex_type drop_type comment_type
  
! %type 	fetch_direction select_limit_value select_offset_value
  select_offset_value2 opt_select_fetch_first_value
  %type 	row_or_rows first_or_next
  
--- 331,337 
  %type 	opt_column event cursor_options opt_hold opt_set_data
  %type 	reindex_type drop_type comment_type
  
! %type 	fetch_args select_limit_value select_offset_value
  select_offset_value2 opt_select_fetch_first_value
  %type 	row_or_rows first_or_next
  
*** reloption_elem:	
*** 1915,1921 
   */
  
  ClosePortalStmt:
! 			CLOSE name
  {
  	ClosePortalStmt *n = makeNode(ClosePortalStmt);
  	n->portalname = $2;
--- 1915,1921 
   */
  
  ClosePortalStmt:
! 			CLOSE cursor_name
  {
  	ClosePortalStmt *n = makeNode(ClosePortalStmt);
  	n->portalname = $2;
*** comment_text:
*** 4082,4223 
   *
   */
  
! FetchStmt:	FETCH fetch_direction from_in name
  {
  	FetchStmt *n = (FetchStmt *) $2;

Re: [HACKERS] mixed, named notation support

[Robert Haas]

On Sun, Aug 9, 2009 at 12:27 PM, Tom Lane wrote:
> Now that I've started to read this patch ... exactly what is the
> argument for allowing a "mixed" notation (some of the parameters named
> and some not)?  ISTM that just serves to complicate both the patch
> and the user's-eye view, for no real benefit.

Wow, I can't imagine not supporting that.  Doesn't every language that
supports anything like named parameters also support a mix?  LISP is
the obvious example, but I think most major scripting languages (Perl,
Ruby, etc.) support something akin to this through a trailing hash
argument.  C# apparently supports both for something called attribute
classes (don't ask me what they are).  Ada also supports both styles,
and you can mix them.

http://msdn.microsoft.com/en-us/library/aa664614(VS.71).aspx
http://goanna.cs.rmit.edu.au/~dale/ada/aln/8_subprograms.html#RTFToC11

The case where this is really useful is when you have lots of
parameters most of which don't need to be set to a non-default value
most of the time, but a few of which always need to be specified.  In
C you might do something like this:

int frobozz(int common1, int common2, struct frobozz_options *fopt);
/* fopt == NULL for defaults */
void init_frobozz_options(struct frobozz_options *);

...but the named/mixed syntax is more compact, and certainly nice to
have, even if I'm emphatically not in love with the syntax we're stuck
with.

ISTM that the Ada rule that all positional must proceed all named is a
good one, and to resolve the problem you're talking about here it
seems we should probably also make a rule that specifying a value for
the same parameter more than once (either both position and named, or
named twice) is an error.

It would also be quite reasonable to impose a requirement that
parameters can only be specified using named notation if it has been
explicitly enabled for that particular parameter.  In LISP, there are
four kinds of parameters (required, optional, rest, keyword) and any
particular parameter gets to be exactly one of those four things.  The
rule for resolving ambiguities may not be what you want, but it's
well-defined and pretty reasonable.

http://gigamonkeys.com/book/functions.html#mixing-different-parameter-types

...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] mixed, named notation support

[Bernd Helmle]

--On 9. August 2009 13:00:07 -0400 Tom Lane  wrote:


Mph.  Does Oracle adopt the same semantics for what a mixed call means?


I had a look at the Oracle documentation while reviewing this patch, and i 
thought we are pretty close to what they do. Maybe Pavel can comment more 
on it.



Because my next complaint was going to be that this definition was
poorly chosen anyway --- it seems confusing, unintuitive, and
restrictive.  If the function is defined as having parameters (a,b,c)
then what does this do:

select foo(1, 2, 3 as b);

and what's the argument for having it do that rather than something
else?


Since b is ambiguous we error out (I don't know what Oracle does, but i 
would be surprised if they do anything different).


--
Thanks

Bernd

--
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] Split-up ECPG patches

[Boszormenyi Zoltan]

Michael Meskes írta:
> On Sat, Aug 08, 2009 at 04:57:57PM -0400, Tom Lane wrote:
>   
>> The fundamental reason that there's a problem here is that ecpg has
>> decided to accept a syntax that the backend doesn't (ie, FETCH with a
>> fetch direction but no FROM/IN).  I think that that's basically a bad
>> 
>
> Which was added because most if not all other precompilers allow this syntax
> and of course it didn't do any harm until now.
>   

:-( Why me? ;-)

>> idea: it's not helpful to users to be inconsistent, and it requires ugly
>> hacks in ecpg, and now ugly hacks in the core grammar as well.  We
>> should resolve it either by taking out that syntax from ecpg, or by
>> making the backend accept it too.  Not by uglifying the grammars some
>> more in order to keep them inconsistent.
>> 
>
> Couldn't agree more.
>
> I'd like to figure out exactly what syntax other DBMSes accept. It appears
> Informix allows the cursor name as a variable but has neither FORWARD/BACKWARD
> nor FROM/IN. Zoltan, could you please check whether my docs are right? 
>   

Yes, your docs seems to be right. From my docs, Informix allows these:

FETCH
{ [NEXT] | PRIOR | PREVIOUS | FIRST | LAST | CURRENT |
ABSOLUTE pos_var_or_const |
RELATIVE { [+]pos_var_or_const | -pos_const }
}
{ cursor_id | cursor_var }
{ USING [SQL] DESCRIPTOR ... | INTO host_var_list... }

There's no FROM or IN anywhere in the syntax snake maze graph.

> A quick google search seems to suggest that the same holds for Oracle that
> apparently allows less options.
>
> Michael
>   

Best regards,
Zoltán Böszörményi

-- 
Bible has answers for everything. Proof:
"But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil." (Matthew 5:37) - basics of digital technology.
"May your kingdom come" - superficial description of plate tectonics

--
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
http://www.postgresql.at/


-- 
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] Alpha releases: How to tag

[Bruce Momjian]

Tom Lane wrote:
> Andrew Dunstan  writes:
> > I think it's a lot more nebulous than that. At the same time I think the 
> > days when we can blithely change the on-disk format with hardly a 
> > thought for migration are over. IOW, there's agreement things have to 
> > change, but the exact shape of the change is not yet clear (at least to 
> > me) ;-)
> 
> Yeah.  I think we're going to start paying more than zero attention to
> this, but we don't yet have a handle on what the real parameters are.
> In particular, it's hard to argue that pg_migrator has yet achieved
> more than experimental status, so accepting or rejecting patches on
> the grounds of whether they would or would not break pg_migrator might
> be a bit premature.  And at the other end of the spectrum, nobody except
> Zdenek wants to deal with changes as invasive as the ones he's proposed.
> So we're still feeling our way here.  We do *not* have a framework in
> which someone could submit a patch that includes an on-disk migration
> aspect, so David's position that we should immediately institute a
> hard requirement for such seems a bit ivory-tower.  We might as well
> just say that the on-disk format is frozen, because that's what the
> effect would be.
> 
> But having said all that, I'm okay with a go-slow position on on-disk
> changes for the next little while.  That would give us some breathing
> room to see if something like pg_migrator is really workable at all.
> We need to find out just how far that approach goes before we can make
> many decisions in this area.

I would be happy if we can just allow pg_migrator to _detect_
incompatible changes, and that nothing _major_ changes between releases
that pg_migrator can't fix (we fixed the sequence changes in 8.4). 

/contrib changes are part of the first group, so I am happy to know
about them and detect them;  changing the tuple format, on the other
hand, could make pg_migrator useless.

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

  + If your life is a hard drive, Christ can be your backup. +

-- 
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] Split-up ECPG patches

[Tom Lane]

Michael Meskes  writes:
> On Sat, Aug 08, 2009 at 05:29:06PM -0400, Tom Lane wrote:
>> around nontrivial expressions.  So I'd like to see an actual case made
>> that there's a strong reason for not requiring FROM/IN in ecpg.

> I guess there's only one, compatibility. 

Yeah.  Are there any other precompilers that actively reject FROM/IN
here?  If we're just a bit more strict than they are, it's not as bad
as if there is no common syntax subset.

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] Split-up ECPG patches

[Michael Meskes]

On Sat, Aug 08, 2009 at 05:29:06PM -0400, Tom Lane wrote:
> around nontrivial expressions.  So I'd like to see an actual case made
> that there's a strong reason for not requiring FROM/IN in ecpg.

I guess there's only one, compatibility. 

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
ICQ: 179140304, AIM/Yahoo/Skype: michaelmeskes, Jabber: mes...@jabber.org
Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL!

-- 
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] Split-up ECPG patches

[Michael Meskes]

On Sat, Aug 08, 2009 at 04:57:57PM -0400, Tom Lane wrote:
> The fundamental reason that there's a problem here is that ecpg has
> decided to accept a syntax that the backend doesn't (ie, FETCH with a
> fetch direction but no FROM/IN).  I think that that's basically a bad

Which was added because most if not all other precompilers allow this syntax
and of course it didn't do any harm until now.

> idea: it's not helpful to users to be inconsistent, and it requires ugly
> hacks in ecpg, and now ugly hacks in the core grammar as well.  We
> should resolve it either by taking out that syntax from ecpg, or by
> making the backend accept it too.  Not by uglifying the grammars some
> more in order to keep them inconsistent.

Couldn't agree more.

I'd like to figure out exactly what syntax other DBMSes accept. It appears
Informix allows the cursor name as a variable but has neither FORWARD/BACKWARD
nor FROM/IN. Zoltan, could you please check whether my docs are right? 

A quick google search seems to suggest that the same holds for Oracle that
apparently allows less options.

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
ICQ: 179140304, AIM/Yahoo/Skype: michaelmeskes, Jabber: mes...@jabber.org
Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL!

-- 
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] mixed, named notation support

[Tom Lane]

Bernd Helmle  writes:
> --On 9. August 2009 12:27:53 -0400 Tom Lane  wrote:
>> Now that I've started to read this patch ... exactly what is the
>> argument for allowing a "mixed" notation (some of the parameters named
>> and some not)?  ISTM that just serves to complicate both the patch
>> and the user's-eye view, for no real benefit.

> Hmm, Oracle has started supporting it in recent versions, too. So one 
> advantage would be at least some sort of compatibility for another favorite 
> database.

Mph.  Does Oracle adopt the same semantics for what a mixed call means?
Because my next complaint was going to be that this definition was
poorly chosen anyway --- it seems confusing, unintuitive, and
restrictive.  If the function is defined as having parameters (a,b,c)
then what does this do:

select foo(1, 2, 3 as b);

and what's the argument for having it do that rather than something
else?

If the behavior isn't *exactly* like Oracle in corner cases like this,
I think partial compatibility is worse than none.  And in any case the
point stands that the more behavior you invent here, the more likely
you are to get blindsided by the eventual SQL standard.

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] revised hstore patch

[Bruce Momjian]

Andrew Dunstan wrote:
> 
> 
> Bruce Momjian wrote:
> > Tom Lane wrote:
> >   
> >> Perhaps an appropriate thing to do is separate out the representation
> >> change from the other new features, and apply just the latter for now.
> >> Or maybe we should think about having two versions of hstore.  This
> >> is all tied up in the problem of having a decent module infrastructure
> >> (which I hope somebody is working on for 8.5).  I don't know where
> >> we're going to end up for 8.5, but I'm disinclined to let a fairly
> >> minor contrib feature improvement break upgrade-compatibility before
> >> we've even really started the cycle.
> >> 
> >
> > I can just have pg_migrator detect hstore and require it be removed
> > before upgrading;  we did that already for 8.3 to 8.4 and I am assuming
> > we will continue to have cases there pg_migrator just will not work.
> >
> >   
> 
> The more things you exclude the less useful the tool will be. I'm 
> already fairly sure it will be unusable for all or almost all my clients 
> who use 8.3.

Sorry to hear that.  You have studied the existing limitations in the
README, right?

I think it is important to report cases where pg_migrator doesn't work,
but I don't think we will ever avoid such cases.  We can't stop Postgres
from moving forward, so my bet is we are always going to have such cases
where pg_migrator doesn't work.

I can't imagine losing a huge percentage of pg_migrator users via hstore
changes, especially since you can migrate hstore manually and still use
pg_migrator.

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

  + If your life is a hard drive, Christ can be your backup. +

-- 
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] mixed, named notation support

[Bernd Helmle]

--On 9. August 2009 12:27:53 -0400 Tom Lane  wrote:


Now that I've started to read this patch ... exactly what is the
argument for allowing a "mixed" notation (some of the parameters named
and some not)?  ISTM that just serves to complicate both the patch
and the user's-eye view, for no real benefit.


Hmm, Oracle has started supporting it in recent versions, too. So one 
advantage would be at least some sort of compatibility for another favorite 
database.


From a user's point of view, i see one use case in calling functions with 
multiple default argument values, where only one of those value needs to be 
overwritten, e.g.


SELECT foo(1, 100, 'this' AS one);
SELECT foo(1, 102, 'other' AS two);
SELECT foo(1, 100, 'another' AS three);

where one, two, three are arguments with specific default values.

--
Thanks

Bernd

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

[Greg Stark]

On Sun, Aug 9, 2009 at 5:19 PM, Tom Lane wrote:
>
>> I am having a hard time wrapping my brain around what it means to have
>> multiple, incompatible notions of equality... any help appreciated!
>
> Well, for instance a complex-number datatype could have one btree
> opclass that sorts on absolute value (distance from 0,0) and another
> opclass that sorts on real part.  In the first case "equal" values would
> be members of the same circle around the origin, in the second case they
> are members of the same vertical line.

The example that came to mind for me was a case-insensitive btree
class for text.

-- 
greg
http://mit.edu/~gsstark/resume.pdf

-- 
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] revised hstore patch

[Bruce Momjian]

Bruce Momjian wrote:
> Andrew Dunstan wrote:
> > > I can't imagine losing a huge percentage of pg_migrator users via hstore
> > > changes, especially since you can migrate hstore manually and still use
> > > pg_migrator.
> > >
> > >   
> > 
> > We could finesse the hstore issues, but we are already blown out of the 
> > water right now by the enum issue. My biggest end client has been 
> > replacing small lookup tables with enums ever since they migrated to 8.3 
> > many months ago. Another end client is currently moving to implement FTS 
> 
> Ah, yea, enum is an issue.
> 
> > on 8.4, and they will be hit by the tsvector/GIN restrictions in the 
> > future unless we fix that. All I was saying is that the more such 
> 
> FTS will be fine for migration from 8.4 to 8.5;  it was only the
> internal format change that caused FTS migration not to work from 8.3 to
> 8.4 (index rebuild required).  There is nothing about FTS that prevents
> migration.
> 
> Here is the pg_migrator README with the restrictions:
> 
>   
> http://cvs.pgfoundry.org/cgi-bin/cvsweb.cgi/pg-migrator/pg_migrator/README?rev=1.56&content-type=text/x-cvsweb-markup
> 
> I will need to document that many of these are 8.3-8.4-only migration
> issues.

Done:


http://cvs.pgfoundry.org/cgi-bin/cvsweb.cgi/pg-migrator/pg_migrator/README?rev=1.57&content-type=text/x-cvsweb-markup

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

  + If your life is a hard drive, Christ can be your backup. +

-- 
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] mixed, named notation support

[Tom Lane]

Now that I've started to read this patch ... exactly what is the
argument for allowing a "mixed" notation (some of the parameters named
and some not)?  ISTM that just serves to complicate both the patch
and the user's-eye view, for no real benefit.

Considering that we are worried about someday having to adjust to a
SQL standard in this area, I think we ought to be as conservative as
possible about what we introduce as user-visible features here.
As an example, if they do go with "=>" as the parameter marker,
mixed notation would become a seriously bad idea because it would be
impossible to distinguish incidental use of => as an operator from
mixed notation.

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] revised hstore patch

[Bruce Momjian]

Andrew Dunstan wrote:
> > I can't imagine losing a huge percentage of pg_migrator users via hstore
> > changes, especially since you can migrate hstore manually and still use
> > pg_migrator.
> >
> >   
> 
> We could finesse the hstore issues, but we are already blown out of the 
> water right now by the enum issue. My biggest end client has been 
> replacing small lookup tables with enums ever since they migrated to 8.3 
> many months ago. Another end client is currently moving to implement FTS 

Ah, yea, enum is an issue.

> on 8.4, and they will be hit by the tsvector/GIN restrictions in the 
> future unless we fix that. All I was saying is that the more such 

FTS will be fine for migration from 8.4 to 8.5;  it was only the
internal format change that caused FTS migration not to work from 8.3 to
8.4 (index rebuild required).  There is nothing about FTS that prevents
migration.

Here is the pg_migrator README with the restrictions:

  
http://cvs.pgfoundry.org/cgi-bin/cvsweb.cgi/pg-migrator/pg_migrator/README?rev=1.56&content-type=text/x-cvsweb-markup

I will need to document that many of these are 8.3-8.4-only migration
issues.

> restrictions there are the less people will be able to use the tool. 
> Surely that is undeniable. I think it's great we (i.e. you) have made a 
> start on this, but there is a long way to go.

Yes, I just don't want pg_migrator to be seen as a "complainer" and
something that is always a drag on progress.  Even if we had no data
format change, using pg_migrator is still a 14-step process, so my guess
is that only people with large databases where dump/reload is
unreasonably long will use it, and in such cases, having to manually
migrate some items is probably acceptable.

What is important for me is that when pg_migrator succeeds, it is
reliable, and when it can't migrate something, it is clear.

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

  + If your life is a hard drive, Christ can be your backup. +

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

[Tom Lane]

Robert Haas  writes:
> distinct_col_search() is going to return the relevant equality
> operator from the argument list, which is ultimately going to come
> from the RestrictInfo for the join clause.  So I need to see whether
> that's compatible with the index, but equality_ops_are_compatible()
> wants two equality operators, and what I have is one equality operator
> and one operator class.

For that you just check if the operator is a member of the class.
(You might need to verify that it's an equality operator in the class
too; not clear if the context is enough to be sure that it's not '<'
for example.)

Note that you really want to think about opfamilies not opclasses.
So if you have an index opclass you really get its containing family
and look for membership in that.

> I am having a hard time wrapping my brain around what it means to have
> multiple, incompatible notions of equality... any help appreciated!

Well, for instance a complex-number datatype could have one btree
opclass that sorts on absolute value (distance from 0,0) and another
opclass that sorts on real part.  In the first case "equal" values would
be members of the same circle around the origin, in the second case they
are members of the same vertical line.

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] Split-up ECPG patches

[Boszormenyi Zoltan]

Tom Lane írta:
> Boszormenyi Zoltan  writes:
>   
>> Tom Lane írta:
>> 
>>> I'd look at requiring from_in as being the least-bad alternative.
>>>   
>
>   
>> Hm. "FETCH FORWARD variable" can only be a rowcount var
>> only if there's something afterwards, no? With the proposed
>> change in fetch_direction (moving FORWARD and BACKWARD
>> without the rowcount upper to the parent rules) now the parser is
>> able to look behind "FORWARD variable"...
>> 
>
> The fundamental reason that there's a problem here is that ecpg has
> decided to accept a syntax that the backend doesn't (ie, FETCH with a
> fetch direction but no FROM/IN).  I think that that's basically a bad
> idea: it's not helpful to users to be inconsistent, and it requires ugly
> hacks in ecpg, and now ugly hacks in the core grammar as well.  We
> should resolve it either by taking out that syntax from ecpg, or by
> making the backend accept it too.  Not by uglifying the grammars some
> more in order to keep them inconsistent.
>
> If we were going to allow it in the core, I think moving the cursor
> name into the fetch_direction production might work, ie, change
> fetch_direction to fetch_args and make it cover everything that
> FETCH and MOVE share.  Probably from_in could become opt_from_in,
> since the alternatives for it are fully reserved words already, and we
> wouldn't need to double up any of the fetch_direction productions.
>
>   regards, tom lane
>   

Your guess about making from_in into opt_from_in
seems good, mostly. I tried doing exactly that and simply
adding an empty match into from_in, I got shift/reduce conflicts
only in "opt_from_in cursor_name". So, this rule has to be
unrolled into 3 rules, or keeping a separate "from_in" just for
having a separate "cursor_name" and "from_in cursor_name".
I decided that I use the second method, it's shorter.

Best regards,
Zoltán Böszörményi

-- 
Bible has answers for everything. Proof:
"But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil." (Matthew 5:37) - basics of digital technology.
"May your kingdom come" - superficial description of plate tectonics

--
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
http://www.postgresql.at/


-- 
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] a short trip in the wayback machine

[Tom Lane]

Andrew Dunstan  writes:
> the documentation of psql's --no-readline option was removed 
> (psql-ref.sgml v 1.23). I think this was a mistake and it should be 
> restored :-) I'm quite never sure how far back to take pure docs 
> patches, though. Should I just fix HEAD, or HEAD plus 8.4, or all the 
> way back to 7.4?

Clearly a mistake.  If you have the energy to patch it all the way
back, please do.

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] a short trip in the wayback machine

[Andrew Dunstan]

Peter Eisentraut wrote:

On Sunday 09 August 2009 03:53:55 Andrew Dunstan wrote:
  

the documentation of psql's --no-readline option was removed
(psql-ref.sgml v 1.23). I think this was a mistake and it should be
restored :-)



Does that option have a point?  Should the option be removed, perhaps?

  


It has at least one prominent user - 
 
;-)


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] Alpha releases: How to tag

[Peter Eisentraut]

On Saturday 08 August 2009 01:28:34 Tom Lane wrote:
> David Fetter  writes:
> > I am not suggesting that this change be immediate, and it's not ivory
> > tower.  It's just how everybody else does it.
>
> You keep saying that, and it's completely meaningless.  What do you know
> about the development practices of Oracle, or DB2, or even Mysql?

Whenever MySQL requires a disk format change, they write a new storage engine.

-- 
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] a short trip in the wayback machine

[Peter Eisentraut]

On Sunday 09 August 2009 03:53:55 Andrew Dunstan wrote:
> the documentation of psql's --no-readline option was removed
> (psql-ref.sgml v 1.23). I think this was a mistake and it should be
> restored :-)

Does that option have a point?  Should the option be removed, perhaps?

-- 
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] revised hstore patch

[Andrew Dunstan]

Bruce Momjian wrote:


I can just have pg_migrator detect hstore and require it be removed
before upgrading;  we did that already for 8.3 to 8.4 and I am assuming
we will continue to have cases there pg_migrator just will not work.

  
  
The more things you exclude the less useful the tool will be. I'm 
already fairly sure it will be unusable for all or almost all my clients 
who use 8.3.



Sorry to hear that.  You have studied the existing limitations in the
README, right?

I think it is important to report cases where pg_migrator doesn't work,
but I don't think we will ever avoid such cases.  We can't stop Postgres
from moving forward, so my bet is we are always going to have such cases
where pg_migrator doesn't work.

I can't imagine losing a huge percentage of pg_migrator users via hstore
changes, especially since you can migrate hstore manually and still use
pg_migrator.

  


We could finesse the hstore issues, but we are already blown out of the 
water right now by the enum issue. My biggest end client has been 
replacing small lookup tables with enums ever since they migrated to 8.3 
many months ago. Another end client is currently moving to implement FTS 
on 8.4, and they will be hit by the tsvector/GIN restrictions in the 
future unless we fix that. All I was saying is that the more such 
restrictions there are the less people will be able to use the tool. 
Surely that is undeniable. I think it's great we (i.e. you) have made a 
start on this, but there is a long way to go.


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] Split-up ECPG patches

[Boszormenyi Zoltan]

Boszormenyi Zoltan írta:
> Tom Lane írta:
>   
>> I wrote:
>>   
>> 
>>> The fundamental reason that there's a problem here is that ecpg has
>>> decided to accept a syntax that the backend doesn't (ie, FETCH with a
>>> fetch direction but no FROM/IN).  I think that that's basically a bad
>>> idea: it's not helpful to users to be inconsistent, and it requires ugly
>>> hacks in ecpg, and now ugly hacks in the core grammar as well.  We
>>> should resolve it either by taking out that syntax from ecpg, or by
>>> making the backend accept it too.  Not by uglifying the grammars some
>>> more in order to keep them inconsistent.
>>> 
>>>   
>> On looking a bit closer at this: I think the reason the core grammar
>> requires FROM/IN after fetch_direction is to leave the door open for
>> someday generalizing the fetch count to be an expression, not just an
>> integer constant.  If we made FROM/IN optional, then doing that would
>> require some ugly syntax hack or other, such as requiring parentheses
>> around nontrivial expressions.  So I'd like to see an actual case made
>> that there's a strong reason for not requiring FROM/IN in ecpg.
>>
>>  regards, tom lane
>>   
>> 
>
> The only reason is I think was the Informix-compatible mode.
> I don't know if it's strong enough, though.
>   

I am looking at the original gram.y and there's a special cased
"optional FROM/IN" case already for FETCH and MOVE:
"FETCH name" and "MOVE name". Which can be seen
as optional FORWARD (already in fetch_direction, as its
first rule) and optional FROM/IN.

> Best regards,
> Zoltán Böszörményi
>
>   


-- 
Bible has answers for everything. Proof:
"But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil." (Matthew 5:37) - basics of digital technology.
"May your kingdom come" - superficial description of plate tectonics

--
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
http://www.postgresql.at/


-- 
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] revised hstore patch

[Andrew Dunstan]

Bruce Momjian wrote:

Tom Lane wrote:
  

Perhaps an appropriate thing to do is separate out the representation
change from the other new features, and apply just the latter for now.
Or maybe we should think about having two versions of hstore.  This
is all tied up in the problem of having a decent module infrastructure
(which I hope somebody is working on for 8.5).  I don't know where
we're going to end up for 8.5, but I'm disinclined to let a fairly
minor contrib feature improvement break upgrade-compatibility before
we've even really started the cycle.



I can just have pg_migrator detect hstore and require it be removed
before upgrading;  we did that already for 8.3 to 8.4 and I am assuming
we will continue to have cases there pg_migrator just will not work.

  


The more things you exclude the less useful the tool will be. I'm 
already fairly sure it will be unusable for all or almost all my clients 
who use 8.3.


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] slow commits with heavy temp table usage in 8.4.0

[James Mansion]

Tom Lane wrote:

the function time and the commit time a lot better.  But I'm not sure
if the use-case is popular enough to deserve such a hack.

  

For some OLTP workloads, it makes a lot of sense to spool tuples of
primary key plus new fields into a temp table and then doing a single update
or delete operation referencing the temp table. Perhaps not so much
for code designed for postgres where there is some extra flexibility
with array params and the like, but for code that targets other systems
as well.

Having temp tables be as fast as possible is quite a big win in this case.


--
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 - merged up to CVS HEAD

[Simon Riggs]

On Sat, 2009-08-08 at 22:02 -0400, Robert Haas wrote:

> I think it would also be fair to point out that you keep saying that
> you're going to deliver this patch for 8.5, but you haven't provided
> any real timetable as to when you're going to start working on it or
> when it'll be completed.  Because this patch IS so important to the
> community, people want to know the answers to those questions.  That
> is exactly why you were asked about your schedule at PGcon; and you
> demurred.  

I'm not sure I have ever demurred on anything, a failing I'm sure.

I mentioned that HS would make sense to go in *after* Synch Rep and I
stand by that, since we only have so many people that understand that
area and we cannot do everything at once.

HS is important. That is why I have put so much time and money into
being in a position where the end of the tunnel is in sight. Had I not
done so, we wouldn't even be discussing it.

> I think it's unfair to ask
> other people to wait for you to work on something when you haven't
> committed to a timetable for working on it

I've not asked anybody to wait. I tried very, very hard to get HS into
8.4 and many people were opposed to that. The next release of Postgres
isn't released until next year. If it matters as to which month it goes
into Postgres, I've not heard anybody explain why the exact month is
important. I don't see anything there myself of concern to the
community. 

I'm working on HS; I've said so clearly and say it again now. To my
knowledge, no other Postgres project has committed to a timetable for
delivery, so I'm not clear why you think one should have been given
here, or why the absence of such a timetable implies anything. Dev tree
only opened again about a month ago, the dates of which were not
published in advance, so no detailed planning was possible for people
contributing to the beta and release of 8.4.

Want an HS Timetable? Well, I will try to complete it for next
commit-fest, but there may be issues that mean it comes in the next one
after that. So Sept 15, or maybe Nov 15. My understanding is that
community wants quality and so that is my #1 priority. I'll make code
available on Sept 15, so that we either have a WIP patch or a
request-for-commit patch, not sure which it will be.

> I understand that your #1 priority needs to be the work for
> which you get paid the most money, but I think it's unfair to ask
> other people to wait for you to work on something when you haven't
> committed to a timetable for working on it.  It might be unfair to ask
> it even if you had committed to a timetable and that timetable was
> well out in the future, but it's certainly unfair when there is no
> timetable at all.

I'm not embarrassed by discussing money but that doesn't make it my
personal priority. I'm sure you didn't mean to imply I was mercenary.

I've contributed to the community for years, mostly unpaid. Which means
I do at some point have to take work that pays. If I have ever got paid
for working on Postgres, it has always been at a much lower rate than I
would have otherwise received, so if anything I've lost money by working
on Postgres. My choice. I parted with EDB specifically to allow me to
spend more time working on software for Postgres, which would otherwise
have certainly been denied me. My choice, nobody else's and one that has
worked well for me. I've chosen contribution to this community over
money many, many times.

The current team will continue working on HS; assistance from any and
every other hacker is welcome in producing that. Not all effort is
productive teamwork, however, and I encourage anyone that wishes to help
on a project prior to patch submission to contact the patch author to
discuss that first, to coordinate and avoid wasted effort. People
interested in review and test need not make contact, since they'll have
access to the code in the normal way.

-- 
 Simon Riggs   www.2ndQuadrant.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] WIP: to_char, support for EEEE format

[Alvaro Herrera]

Brendan Jurd escribió:

> Here's version 6 of the  patch, now with an all-new implementation
> of (normalised) scientific notation in numeric.c, via the functions
> numeric_out_sci() and get_str_from_var_sci().  So  should now be
> able to represent the full gamut of the numeric type.

I noticed an ugly pattern in NUMDesc_prepare calling a cleanup function
before every ereport(ERROR).  I think it's cleaner to replace that with
a PG_TRY block; see attached.

I didn't go over the patch in much more detail.  (But the
numeric_out_sci business got me thinking.)

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Index: src/backend/utils/adt/formatting.c
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/utils/adt/formatting.c,v
retrieving revision 1.159
diff -c -p -r1.159 formatting.c
*** src/backend/utils/adt/formatting.c	6 Jul 2009 19:11:39 -	1.159
--- src/backend/utils/adt/formatting.c	9 Aug 2009 07:23:34 -
*** NUMDesc_prepare(NUMDesc *num, FormatNode
*** 1044,1059 
  	if (n->type != NODE_TYPE_ACTION)
  		return;
  
  	switch (n->key->id)
  	{
  		case NUM_9:
  			if (IS_BRACKET(num))
- 			{
- NUM_cache_remove(last_NUMCacheEntry);
  ereport(ERROR,
  		(errcode(ERRCODE_SYNTAX_ERROR),
  		 errmsg("\"9\" must be ahead of \"PR\"")));
- 			}
  			if (IS_MULTI(num))
  			{
  ++num->multi;
--- 1044,1058 
  	if (n->type != NODE_TYPE_ACTION)
  		return;
  
+ 	PG_TRY();
+ 	{
  	switch (n->key->id)
  	{
  		case NUM_9:
  			if (IS_BRACKET(num))
  ereport(ERROR,
  		(errcode(ERRCODE_SYNTAX_ERROR),
  		 errmsg("\"9\" must be ahead of \"PR\"")));
  			if (IS_MULTI(num))
  			{
  ++num->multi;
*** NUMDesc_prepare(NUMDesc *num, FormatNode
*** 1067,1078 
  
  		case NUM_0:
  			if (IS_BRACKET(num))
- 			{
- NUM_cache_remove(last_NUMCacheEntry);
  ereport(ERROR,
  		(errcode(ERRCODE_SYNTAX_ERROR),
  		 errmsg("\"0\" must be ahead of \"PR\"")));
- 			}
  			if (!IS_ZERO(num) && !IS_DECIMAL(num))
  			{
  num->flag |= NUM_F_ZERO;
--- 1066,1074 
*** NUMDesc_prepare(NUMDesc *num, FormatNode
*** 1096,1114 
  			num->need_locale = TRUE;
  		case NUM_DEC:
  			if (IS_DECIMAL(num))
- 			{
- NUM_cache_remove(last_NUMCacheEntry);
  ereport(ERROR,
  		(errcode(ERRCODE_SYNTAX_ERROR),
  		 errmsg("multiple decimal points")));
- 			}
  			if (IS_MULTI(num))
- 			{
- NUM_cache_remove(last_NUMCacheEntry);
  ereport(ERROR,
  		(errcode(ERRCODE_SYNTAX_ERROR),
  	 errmsg("cannot use \"V\" and decimal point together")));
- 			}
  			num->flag |= NUM_F_DECIMAL;
  			break;
  
--- 1092,1104 
*** NUMDesc_prepare(NUMDesc *num, FormatNode
*** 1118,1136 
  
  		case NUM_S:
  			if (IS_LSIGN(num))
- 			{
- NUM_cache_remove(last_NUMCacheEntry);
  ereport(ERROR,
  		(errcode(ERRCODE_SYNTAX_ERROR),
  		 errmsg("cannot use \"S\" twice")));
- 			}
  			if (IS_PLUS(num) || IS_MINUS(num) || IS_BRACKET(num))
- 			{
- NUM_cache_remove(last_NUMCacheEntry);
  ereport(ERROR,
  		(errcode(ERRCODE_SYNTAX_ERROR),
  		 errmsg("cannot use \"S\" and \"PL\"/\"MI\"/\"SG\"/\"PR\" together")));
- 			}
  			if (!IS_DECIMAL(num))
  			{
  num->lsign = NUM_LSIGN_PRE;
--- 1108,1120 
*** NUMDesc_prepare(NUMDesc *num, FormatNode
*** 1148,1159 
  
  		case NUM_MI:
  			if (IS_LSIGN(num))
- 			{
- NUM_cache_remove(last_NUMCacheEntry);
  ereport(ERROR,
  		(errcode(ERRCODE_SYNTAX_ERROR),
  		 errmsg("cannot use \"S\" and \"MI\" together")));
- 			}
  			num->flag |= NUM_F_MINUS;
  			if (IS_DECIMAL(num))
  num->flag |= NUM_F_MINUS_POST;
--- 1132,1140 
*** NUMDesc_prepare(NUMDesc *num, FormatNode
*** 1161,1172 
  
  		case NUM_PL:
  			if (IS_LSIGN(num))
- 			{
- NUM_cache_remove(last_NUMCacheEntry);
  ereport(ERROR,
  		(errcode(ERRCODE_SYNTAX_ERROR),
  		 errmsg("cannot use \"S\" and \"PL\" together")));
- 			}
  			num->flag |= NUM_F_PLUS;
  			if (IS_DECIMAL(num))
  num->flag |= NUM_F_PLUS_POST;
--- 1142,1150 
*** NUMDesc_prepare(NUMDesc *num, FormatNode
*** 1174,1197 
  
  		case NUM_SG:
  			if (IS_LSIGN(num))
- 			{
- NUM_cache_remove(last_NUMCacheEntry);
  ereport(ERROR,
  		(errcode(ERRCODE_SYNTAX_ERROR),
  		 errmsg("cannot use \"S\" and \"SG\" together")));
- 			}
  			num->flag |= NUM_F_MINUS;
  			num->flag |= NUM_F_PLUS;
  			break;
  
  		case NUM_PR:
  			if (IS_LSIGN(num) || IS_PLUS(num) || IS_MINUS(num))
- 			{
- NUM_cache_remove(last_NUMCacheEntry);
  ereport(ERROR,
  		(errcode(ERRCODE_SYNTAX_ERROR),
  		 errmsg("cannot use \"PR\" and \"S\"/\"PL\"/\"MI\"/\"SG\" together")));
- 			}
  			num->flag |= NUM_F_BRACKET;
  			break;
  
--- 1152,1169 
*** NUMDesc_prepare(NU

Re: [HACKERS] WIP: to_char, support for EEEE format

[Brendan Jurd]

2009/8/9 Tom Lane :
> Brendan Jurd  writes:
>> That would allow for a maximum of 10 exponent digits.  As an aside, I
>> note that int4out() hardcodes the maximum number of digits rather than
>> exposing a constant (c.f. MAXINT8LEN in int8.c).  I'm considering
>> adding MAXINT2LEN and MAXINT4LEN to int.c in passing.  Excessive
>> tinkering, or worthy improvement?
>
> Don't really care.  short and int are the same sizes on all platforms of
> interest, and are likely to remain so --- if they don't, we'll have way
> more places to fix than this one.  INT8 has historically been more
> platform-dependent.
>

"Excessive tinkering" it is, then.  I went ahead with hardcoding the
expected 10 digits.

Here's version 6 of the  patch, now with an all-new implementation
of (normalised) scientific notation in numeric.c, via the functions
numeric_out_sci() and get_str_from_var_sci().  So  should now be
able to represent the full gamut of the numeric type.

regression=# select to_char(-1e-1000, '9.99');
   to_char
-
 -1.00e-1000
(1 row)

I also noticed that  wasn't outputting a leading space for
positive numbers like the rest of to_char does.  This meant that
positive and negative numbers weren't aligned.  The logic to do this
for ordinary decimal representations is tied up in NUM_processor() and
NUM_numpart_to_char(), so unfortunately I wasn't able to reuse it.
Instead I just frobbed the code in the *_to_char() functions to make
this work.

Noting that Robert has dropped the patch from the July CF (and having
no objection to that) I'm going to submit this for September.

Cheers,
BJ


_6.diff.bz2
Description: BZip2 compressed data


_5-to-6.diff.bz2
Description: BZip2 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