Re: [HACKERS] Split-up ECPG patches
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. 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] WIP: to_char, support for EEEE format
2009/8/9 Tom Lane t...@sss.pgh.pa.us: Brendan Jurd dire...@gmail.com 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
Re: [HACKERS] WIP: to_char, support for EEEE format
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(NUMDesc *num, FormatNode
*** 1207,1227
case NUM_V:
if (IS_DECIMAL(num))
- {
Re: [HACKERS] hot standby - merged up to CVS HEAD
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] slow commits with heavy temp table usage in 8.4.0
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] revised hstore patch
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] Split-up ECPG patches
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
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] a short trip in the wayback machine
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] Alpha releases: How to tag
On Saturday 08 August 2009 01:28:34 Tom Lane wrote: David Fetter da...@fetter.org 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 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 - http://people.planetpostgresql.org/andrew/index.php?/archives/31-A-tiny-psql-tip.html#comments ;-) 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] a short trip in the wayback machine
Andrew Dunstan and...@dunslane.net 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] Split-up ECPG patches
Tom Lane írta: Boszormenyi Zoltan z...@cybertec.at 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] join removal
Robert Haas robertmh...@gmail.com 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] revised hstore patch
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.56content-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 br...@momjian.ushttp://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
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 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.56content-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.57content-type=text/x-cvsweb-markup -- Bruce Momjian br...@momjian.ushttp://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
On Sun, Aug 9, 2009 at 5:19 PM, Tom Lanet...@sss.pgh.pa.us 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] mixed, named notation support
--On 9. August 2009 12:27:53 -0400 Tom Lane t...@sss.pgh.pa.us 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] revised hstore patch
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 br...@momjian.ushttp://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 maili...@oopsware.de writes: --On 9. August 2009 12:27:53 -0400 Tom Lane t...@sss.pgh.pa.us 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] Split-up ECPG patches
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] Split-up ECPG patches
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 mes...@postgresql.org 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] Alpha releases: How to tag
Tom Lane wrote: Andrew Dunstan and...@dunslane.net 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 br...@momjian.ushttp://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
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] mixed, named notation support
--On 9. August 2009 13:00:07 -0400 Tom Lane t...@sss.pgh.pa.us 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] mixed, named notation support
On Sun, Aug 9, 2009 at 12:27 PM, Tom Lanet...@sss.pgh.pa.us 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] Split-up ECPG patches
Boszormenyi Zoltan írta:
Tom Lane írta:
Boszormenyi Zoltan z...@cybertec.at 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 str relation_name copy_file_name
database_name access_method_clause access_method attr_name
! index_name name file_name cluster_index_specification
%type list func_name handler_name qual_Op qual_all_Op subquery_Op
opt_class opt_validator validator_clause
--- 253,259
%type str 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 list func_name handler_name qual_Op qual_all_Op subquery_Op
opt_class opt_validator validator_clause
*** static TypeName *TableFuncTypeName(List
*** 331,337
%type ival opt_column event cursor_options opt_hold opt_set_data
%type objtype reindex_type drop_type comment_type
! %type node fetch_direction select_limit_value select_offset_value
select_offset_value2 opt_select_fetch_first_value
%type ival row_or_rows first_or_next
--- 331,337
%type ival opt_column event cursor_options opt_hold opt_set_data
%type objtype reindex_type drop_type comment_type
! %type node fetch_args select_limit_value select_offset_value
select_offset_value2 opt_select_fetch_first_value
%type ival 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;
- n-portalname = $4;
- n-ismove = FALSE;
-
Re: [HACKERS] a short trip in the wayback machine
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 - http://people.planetpostgresql.org/andrew/index.php?/archives/31-A-tiny-ps 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] mixed, named notation support
Robert Haas robertmh...@gmail.com writes: On Sun, Aug 9, 2009 at 12:27 PM, Tom Lanet...@sss.pgh.pa.us 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] Review: Patch for contains/overlap of polygons
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 is 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 br...@momjian.ushttp://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
On Sun, Aug 9, 2009 at 2:17 PM, Tom Lanet...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Sun, Aug 9, 2009 at 12:27 PM, Tom Lanet...@sss.pgh.pa.us 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
On Sun, Aug 9, 2009 at 2:20 PM, Bruce Momjianbr...@momjian.us 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
[HACKERS] Issues for named/mixed function notation patch
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] Issues for named/mixed function notation patch
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
Re: [HACKERS] hot standby - merged up to CVS HEAD
On Sun, Aug 9, 2009 at 6:11 AM, Simon Riggssi...@2ndquadrant.com 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] mixed, named notation support
Robert Haas robertmh...@gmail.com writes: On Sun, Aug 9, 2009 at 2:17 PM, Tom Lanet...@sss.pgh.pa.us 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] Review: Patch for contains/overlap of polygons
On Sun, Aug 09, 2009 at 02:29:44PM -0400, Robert Haas wrote: On Sun, Aug 9, 2009 at 2:20 PM, Bruce Momjianbr...@momjian.us 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] machine-readable explain output v4
Robert Haas robertmh...@gmail.com 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] hot standby - merged up to CVS HEAD
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] hot standby - merged up to CVS HEAD
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 br...@momjian.ushttp://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] machine-readable explain output v4
On Sun, Aug 9, 2009 at 3:57 PM, Tom Lanet...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com 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] Issues for named/mixed function notation patch
On Sun, Aug 9, 2009 at 7:30 PM, Tom Lanet...@sss.pgh.pa.us 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] GRANT ON ALL IN schema
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 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] GRANT ON ALL IN schema
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 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] machine-readable explain output v4
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 p and body 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: pg:explain 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 http://archives.postgresql.org/pgsql-hackers/2009-07/msg01944.php and I noticed this which makes me suspicious: + if (es.format == EXPLAIN_FORMAT_XML) + appendStringInfoString(es.str, + explain xmlns=\http://www.postgresql.org/2009/explain\; http://www.postgresql.org/2009/explain%5C%22;\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] machine-readable explain output v4
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: pg:explain 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 http://archives.postgresql.org/pgsql-hackers/2009-07/msg01944.php and I noticed this which makes me suspicious: + if (es.format == EXPLAIN_FORMAT_XML) + appendStringInfoString(es.str, + explain xmlns=\http://www.postgresql.org/2009/explain\; http://www.postgresql.org/2009/explain%5C%22;\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
Andres Freund wrote: BTW, has anyone tried validating the XML at all? I just looked very briefly at the patch at http://archives.postgresql.org/pgsql-hackers/2009-07/msg01944.php and I noticed this which makes me suspicious: + if (es.format == EXPLAIN_FORMAT_XML) + appendStringInfoString(es.str, + explain xmlns=\http://www.postgresql.org/2009/explain\; http://www.postgresql.org/2009/explain%5C%22;\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
Andrew Dunstan wrote: Andres Freund wrote: BTW, has anyone tried validating the XML at all? I just looked very briefly at the patch at http://archives.postgresql.org/pgsql-hackers/2009-07/msg01944.php and I noticed this which makes me suspicious: + if (es.format == EXPLAIN_FORMAT_XML) + appendStringInfoString(es.str, + explain xmlns=\http://www.postgresql.org/2009/explain\; http://www.postgresql.org/2009/explain%5C%22;\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 http://archives.postgresql.org/pgsql-hackers/2009-08/msg00485.php 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
Andres Freund and...@anarazel.de 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, Item);
str = escape_xml((const char *) lfirst(lc));
appendStringInfoString(es-str, str);
pfree(str);
appendStringInfoString(es-str, /Item\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);
ExplainXMLTag(qlabel, X_CLOSING |
Re: [HACKERS] machine-readable explain output v4
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 http://archives.postgresql.org/pgsql-hackers/2009-07/msg01944.php and I noticed this which makes me suspicious: + if (es.format == EXPLAIN_FORMAT_XML) + appendStringInfoString(es.str, + explain xmlns=\http://www.postgresql.org/2009/explain\; http://www.postgresql.org/2009/explain%5C%22;\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 http://archives.postgresql.org/pgsql-hackers/2009-08/msg00485.php 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
On Sun, Aug 9, 2009 at 8:48 PM, Andrew Dunstanand...@dunslane.net 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 http://archives.postgresql.org/pgsql-hackers/2009-07/msg01944.php and I noticed this which makes me suspicious: + if (es.format == EXPLAIN_FORMAT_XML) + appendStringInfoString(es.str, + explain xmlns=\http://www.postgresql.org/2009/explain\; http://www.postgresql.org/2009/explain%5C%22;\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 http://archives.postgresql.org/pgsql-hackers/2009-08/msg00485.php 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
Andrew Dunstan and...@dunslane.net writes: I takle it back. It's still there at http://archives.postgresql.org/pgsql-hackers/2009-08/msg00485.php 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, explain xmlns=\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
On Sun, Aug 9, 2009 at 9:03 PM, Tom Lanet...@sss.pgh.pa.us wrote: Andrew Dunstan and...@dunslane.net writes: I takle it back. It's still there at http://archives.postgresql.org/pgsql-hackers/2009-08/msg00485.php 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, explain xmlns=\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
On Sun, Aug 9, 2009 at 8:53 PM, Tom Lanet...@sss.pgh.pa.us wrote: Andres Freund and...@anarazel.de 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 Plan tag could have children like Startup-Cost (one each) and could also have its inner, outer, and sub-plans in there as Plan tags right under the parent Plan. 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 Plan has a child called Plans which in turn contains each Plan 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
On Monday 10 August 2009 02:53:16 Tom Lane wrote: Andres Freund and...@anarazel.de 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 (Notify /). 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] Issues for named/mixed function notation patch
On Sun, Aug 9, 2009 at 2:30 PM, Tom Lanet...@sss.pgh.pa.us 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 committable until these issues are resolved.
Is it realistic to think that this will
Re: [HACKERS] machine-readable explain output v4
Andres Freund and...@anarazel.de 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 (Notify /). Yeah, I was just wondering what to do with the Notify / 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] machine-readable explain output v4
On Sun, Aug 9, 2009 at 9:32 PM, Tom Lanet...@sss.pgh.pa.us wrote: Andres Freund and...@anarazel.de 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 (Notify /). Yeah, I was just wondering what to do with the Notify / 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 Notify /Notify ...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
Robert Haas robertmh...@gmail.com writes: On Sun, Aug 9, 2009 at 2:30 PM, Tom Lanet...@sss.pgh.pa.us 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
On Monday 10 August 2009 03:34:36 Robert Haas wrote: On Sun, Aug 9, 2009 at 9:32 PM, Tom Lanet...@sss.pgh.pa.us wrote: Andres Freund and...@anarazel.de 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 (Notify /). Yeah, I was just wondering what to do with the Notify / 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 Notify /Notify 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] machine-readable explain output v4
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 Plan tag could have children like Startup-Cost (one each) and could also have its inner, outer, and sub-plans in there as Plan tags right under the parent Plan. 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] Issues for named/mixed function notation patch
On Sun, Aug 9, 2009 at 9:36 PM, Tom Lanet...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Sun, Aug 9, 2009 at 2:30 PM, Tom Lanet...@sss.pgh.pa.us 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] Review: Patch for contains/overlap of polygons
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 Momjianbr...@momjian.us 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] machine-readable explain output v4
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 Lanet...@sss.pgh.pa.us wrote: Andres Freund and...@anarazel.de 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 (Notify /). Yeah, I was just wondering what to do with the Notify / 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 Notify /Notify 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
On Sun, Aug 9, 2009 at 10:01 PM, Josh Williamsjoshwilli...@ij.net 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] hot standby - merged up to CVS HEAD
On Sun, Aug 9, 2009 at 2:43 PM, Simon Riggssi...@2ndquadrant.com 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] change in timestamp output from 8.3 to 8.4
Tom Lane wrote: Joe Conway m...@joeconway.com 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 br...@momjian.ushttp://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] [PATCH] 2PC state files on shared memory
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] Issues for named/mixed function notation patch
On Mon, Aug 10, 2009 at 2:23 AM, Robert Haasrobertmh...@gmail.com 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] WIP: to_char, support for EEEE format
2009/8/9 Alvaro Herrera alvhe...@commandprompt.com: 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] pg_stat_activity.application_name
On Fri, Jul 17, 2009 at 3:19 AM, Peter Eisentrautpete...@gmx.net 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] machine-readable explain output v4
Robert Haas robertmh...@gmail.com 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
