Re: [HACKERS] One-Shot Plans
On Tue, Jun 14, 2011 at 1:25 PM, Simon Riggs si...@2ndquadrant.com wrote:
We can work out the various paths through the traffic cop to see when
a plan will be a one-shot - planned and then executed immediately,
then discarded.
In those cases we can take advantage of better optimisations. Most
interestingly, we can evaluate stable functions at plan time, to allow
us to handle partitioning and partial indexes better.
Patch attached. Works...
this breaks test guc.c for me... specifically the test at
src/test/regress/sql/guc.sql circa line 226:
set work_mem = '3MB';
-- but SET isn't
create or replace function myfunc(int) returns text as $$
begin
set work_mem = '2MB';
return current_setting('work_mem');
end $$
language plpgsql
set work_mem = '1MB';
select myfunc(0), current_setting('work_mem');
regressions.diff
*** 656,662
select myfunc(0), current_setting('work_mem');
myfunc | current_setting
+-
! 2MB| 2MB
(1 row)
set work_mem = '3MB';
--- 656,662
select myfunc(0), current_setting('work_mem');
myfunc | current_setting
+-
! 2MB| 3MB
(1 row)
set work_mem = '3MB';
it seems that the effect of SET is being discarded
--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] hstore - Implementation and performance issues around its operators
Hi,
We did a benchmark comparing a Key-Value-Pairs stored as EAV db schema
versus hstore. The results are promising in favor of hstore but there are some
question which remain.
1. Obviously the '@' has to be used in order to let use the GiST index.
Why is the '-' operator not supported by GiST ('-' is actually
mentioned in all examples of the doc.)?
2. Currently the hstore elements are stored in order as they are
coming from the insert statement / constructor.
Why are the elements not ordered i.e. why is the hstore not cached in
all hstore functions (like hstore_fetchval etc.)?
3. In the source code 'hstore_io.c' one finds the following enigmatic
note: ... very large hstore values can't be output. this could be
fixed, but many other data types probably have the same issue.
What is the max. length of a hstore (i.e. the max. length of the sum
of all elements in text representation)?
4. Last, I don't fully understand the following note in the hstore
doc. (http://www.postgresql.org/docs/current/interactive/hstore.html
):
Notice that the old names are reversed from the convention
formerly followed by the core geometric data types!
Why names? Why not rather 'operators' or 'functions'?
What does this reversed from the convention mean concretely?
Yours, Stefan
P.S. I already tried to ask these questions to postgres-performance
and to the hstore authors without success...
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [COMMITTERS] pgsql: Fixed string in German translation that causes segfault.
On Mon, Jun 20, 2011 at 04:44:20PM -0400, Tom Lane wrote: My recollection is that the current setup was created mainly so that translators wouldn't need to be given commit privileges on the main repo. Giving them a separate repo to work in might be all right, but of course whoever does the merges would have to be careful to only accept changes made to the .po files and not anything else. IIRC this is exactly what git submodules are for. We could do a git archive only for translations as a submodule for our main git. That way translators would only clone the translation git, while we still have the translations in the source tree in the main git. At least this is how I think it works. 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 Jabber: michael.meskes at googlemail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, 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] Range Types and extensions
On Mon, 2011-06-20 at 12:54 -0700, Darren Duncan wrote: That DOMAIN-based solution ostensibly sounds like a good one then, under the circumstances. It's not bad from a theoretical standpoint, but it does require some extra type annotation, which is not really the SQL way. What I *don't* want to see is for things like ranges to have their own collations and the like. I'm not 100% sure what you mean here. If you mean that you don't want range types to pay attention to COLLATE clauses, etc., then I agree. I would also agree if you mean that range values should not carry the collation with them. However, it looks like we might try to make the opclass/collation pair a property of the range type definition. That seems nice, because it allows us to keep the nice properties of ranges as well as the type inference and polymorphism for everything except the constructors. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Range Types and extensions
On Mon, 2011-06-20 at 13:43 -0400, Tom Lane wrote: The other viable alternative seems to be to require those two properties (btree opclass and collation) to be part of a specific range type definition. The complaint about that seemed to be that we couldn't infer an ANYRANGE type given only ANYELEMENT, but could we alleviate that by identifying one range type as the default for the base type, and then using that one in cases where we have no ANYRANGE input? Yes, that sounds similar to Florian's suggestion, and I think there may be a solution down this path. However, if we're going to have range types with non-default orderings, then we need a way to construct them. I suggested that, if constructors are the primary problem case, then just generate non-polymorphic constructors at range type definition time, named after the range type name. I'll look into that approach. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Latch implementation that wakes on postmaster death on both win32 and Unix
Thanks for giving this your attention Fujii. Attached patch addresses
your concerns.
On 20 June 2011 05:53, Fujii Masao masao.fu...@gmail.com wrote:
'hifd' should be initialized to 'selfpipe_readfd' before the above
'if' block. Otherwise,
'hifd = postmaster_alive_fds[POSTMASTER_FD_WATCH]' might have no effect.
That's an oversight that I should have caught. Fixed.
Why does the archive still need to wake up periodically?
That is consistent with its earlier behaviour...she wakes up
occasionally to allow herself to be proactive. This comment does not
refer to the frequent updates that currently occur within the tight
polling loop. I think any concern about that would apply equally to
the original, unpatched code.
Is the variable 'flag' really required? It's not used by fcntl() to
set the fd nonblocking.
Yes, it's superfluous. Removed.
Is FNONBLOCK equal to O_NONBLOCK? If yes, we should use O_NONBLOCK
for the sake of consistency? In other code (e.g., noblock.c), O_NONBLOCK is
used
rather than FNONBLOCK.
FNONBLOCK is just an alias for O_NONBLOCK, so it seems reasonable to
be consistent in which variant we use. I have found suggestions that
it might break the build on OSX, so if that's true there's an
excellent reason to prefer the latter.
I think that it's worth that walsender checks the postmaster death event. No?
It does check it, but only in the same way that it always has (a tight
polling loop). I would like to make walsender use the new
functionality. That is another patch though, that I thought best to
have independently reviewed, only when this patch is committed. I've
only made the walsender use the new interface, changing as little as
possible and not affecting walsender's behaviour, as a stopgap towards
that patch.
--
Peter Geoghegan http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index aa0b029..691ac42 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -10161,7 +10161,7 @@ retry:
/*
* Wait for more WAL to arrive, or timeout to be reached
*/
- WaitLatch(XLogCtl-recoveryWakeupLatch, 500L);
+ WaitLatch(XLogCtl-recoveryWakeupLatch, WL_LATCH_SET | WL_TIMEOUT, 500L);
ResetLatch(XLogCtl-recoveryWakeupLatch);
}
else
diff --git a/src/backend/port/unix_latch.c b/src/backend/port/unix_latch.c
index 6dae7c9..f97d838 100644
--- a/src/backend/port/unix_latch.c
+++ b/src/backend/port/unix_latch.c
@@ -93,7 +93,9 @@
#endif
#include miscadmin.h
+#include postmaster/postmaster.h
#include storage/latch.h
+#include storage/pmsignal.h
#include storage/shmem.h
/* Are we currently in WaitLatch? The signal handler would like to know. */
@@ -188,22 +190,25 @@ DisownLatch(volatile Latch *latch)
* backend-local latch initialized with InitLatch, or a shared latch
* associated with the current process by calling OwnLatch.
*
- * Returns 'true' if the latch was set, or 'false' if timeout was reached.
+ * Returns bit field indicating which condition(s) caused the wake-up.
+ *
+ * Note that there is no guarantee that callers will have all wake-up conditions
+ * returned, but we will report at least one.
*/
-bool
-WaitLatch(volatile Latch *latch, long timeout)
+int
+WaitLatch(volatile Latch *latch, int wakeEvents, long timeout)
{
- return WaitLatchOrSocket(latch, PGINVALID_SOCKET, false, false, timeout) 0;
+ return WaitLatchOrSocket(latch, wakeEvents, PGINVALID_SOCKET, timeout);
}
/*
* Like WaitLatch, but will also return when there's data available in
- * 'sock' for reading or writing. Returns 0 if timeout was reached,
- * 1 if the latch was set, 2 if the socket became readable or writable.
+ * 'sock' for reading or writing.
+ *
+ * Returns same bit mask and makes same guarantees as WaitLatch.
*/
int
-WaitLatchOrSocket(volatile Latch *latch, pgsocket sock, bool forRead,
- bool forWrite, long timeout)
+WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock, long timeout)
{
struct timeval tv,
*tvp = NULL;
@@ -211,12 +216,13 @@ WaitLatchOrSocket(volatile Latch *latch, pgsocket sock, bool forRead,
fd_set output_mask;
int rc;
int result = 0;
+ bool found = false;
if (latch-owner_pid != MyProcPid)
elog(ERROR, cannot wait on a latch owned by another process);
/* Initialize timeout */
- if (timeout = 0)
+ if (timeout = 0 (wakeEvents WL_TIMEOUT))
{
tv.tv_sec = timeout / 100L;
tv.tv_usec = timeout % 100L;
@@ -224,7 +230,7 @@ WaitLatchOrSocket(volatile Latch *latch, pgsocket sock, bool forRead,
}
waiting = true;
- for (;;)
+ do
{
int hifd;
@@ -235,16 +241,31 @@ WaitLatchOrSocket(volatile Latch *latch, pgsocket sock, bool forRead,
* do that), and the select() will return immediately.
*/
drainSelfPipe();
- if (latch-is_set)
+ if (latch-is_set (wakeEvents WL_LATCH_SET))
{
-
Re: [HACKERS] WIP: Fast GiST index build
Hi! I've created section about testing in project wiki page: http://wiki.postgresql.org/wiki/Fast_GiST_index_build_GSoC_2011#Testing_results Do you have any notes about table structure? As you can see I found that CPU usage might be much higher with gist_trgm_ops. I believe it's due to relatively expensive penalty method in that opclass. But, probably index build can be still faster when index doesn't fit cache even for gist_trgm_ops. Also with that opclass index quality is slightly worse but the difference is not dramatic. -- With best regards, Alexander Korotkov.
Re: [HACKERS] hstore - Implementation and performance issues around its operators
Hi!
On Tue, Jun 21, 2011 at 12:04 PM, Stefan Keller sfkel...@gmail.com wrote:
1. Obviously the '@' has to be used in order to let use the GiST index.
Why is the '-' operator not supported by GiST ('-' is actually
mentioned in all examples of the doc.)?
I believe it's an architecture problem. You actually need not '-' operator
to be supported by GiST but column - 'field_name' = value expression.
Probably, I'm missing something, but I think supporting of this require
significant catalog changes.
--
With best regards,
Alexander Korotkov.
Re: [HACKERS] Re: [COMMITTERS] pgsql: Fixed string in German translation that causes segfault.
On Tue, Jun 21, 2011 at 10:20, Michael Meskes mes...@postgresql.org wrote: On Mon, Jun 20, 2011 at 04:44:20PM -0400, Tom Lane wrote: My recollection is that the current setup was created mainly so that translators wouldn't need to be given commit privileges on the main repo. Giving them a separate repo to work in might be all right, but of course whoever does the merges would have to be careful to only accept changes made to the .po files and not anything else. IIRC this is exactly what git submodules are for. We could do a git archive only for translations as a submodule for our main git. That way translators would only clone the translation git, while we still have the translations in the source tree in the main git. At least this is how I think it works. AFAIK (but I could be wrong), git submodules requires the files to be in *one* subdirectory. Our .po files are distributed all across the backend. So we'd have to make (and backpatch) som rather large changes in how these things are built in order to use that. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: a validator for configuration files
On Jun 20, 2011, at 6:22 PM, Florian Pflug wrote: On Jun20, 2011, at 17:02 , Alexey Klyukin wrote: I don't think it has changed at all. Previously, we did goto cleanup_list (or cleanup_exit in ParseConfigFp) right after the first error, no matter whether that was a postmaster or its child. What I did in my patch is removing the goto for the postmaster's case. It was my intention to exit after the initial error for the postmaster's child, to avoid complaining about all errors both in the postmaster and in the normal backend (imagine seeing 100 errors from the postmaster and the same 100 from each of the backends if your log level is DEBUG2). I think the postmaster's child case won't cause any problems, since we do exactly what we used to do before. Hm, I think you miss-understood what I was trying to say, probably because I explained it badly. Let me try again. I fully agree that there *shouldn't* be any difference in behaviour, because it *shouldn't* matter whether we abort early or not - we won't have applied any of the settings anway. But. The code the actually implements the check settings first, apply later logic isn't easy to read. Now, assume that this code has a bug. Then, with your patch applied, we might end up with the postmaster applying a setting (because it didn't abort early) but the backend ignoring it (because they did abort early). This is obviously bad. Depending on the setting, the consequences may range from slightly confusing behaviour to outright crashes I guess... Now, the risk of that happening might be very small. But on the other hand, the benefit is pretty small also - you get a little less output for log level DEBUG2, that's it. A level that people probably don't use for the production databases anyway. This convinced me that the risk/benefit ratio isn't high enough to warrant the early abort. Another benefit of removing the check is that it reduces code complexity. Maybe not when measured in line counts, but it's one less outside factor that changes ProcessConfigFiles()'s behaviour and thus one thing less you need to think when you modify that part again in the future. Again, it's a small benefit, but IMHO it still outweights the benefit. While I agree that making the code potentially less bug prone is a good idea, I don't agree with the statement that since DEBUG2 output gets rarely turned on we can make it less usable, hoping that nobody would use in production. Having said that, this is my personal opinion and whoever will eventually commit this may very will assess the cost/benefit ratio differently. So, if after this more detailed explanations of my reasoning, you still feel that it makes sense to keep the early abort, then feel free to mark the patch Ready for Committer nevertheless. I'd say that this issue is a judgement call. Depending on a point of view, both arguments are valid. I've marked this patch as 'Ready for Committer' w/o removing the early abort stuff, but if there will be more people willing to remove them - I'll do that. Thank you, Alexey. -- Command Prompt, Inc. http://www.CommandPrompt.com PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pika buildfarm member failure on isolationCheck tests
On 21.06.2011 05:18, Dan Ports wrote: The first patch addresses this bug by re-adding SXACT_FLAG_ROLLED_BACK, in a more limited form than its previous incarnation. We need to be able to distinguish transactions that have already called ReleasePredicateLocks and are thus eligible for cleanup from those that have been merely marked for abort by other backends. Transactions that are ROLLED_BACK are excluded from SxactGlobalXmin calculations, but those that are merely DOOMED need to be included. Also update a couple of assertions to ensure we only try to clean up ROLLED_BACK transactions. Thanks, committed. The second patch fixes a bug in PreCommit_CheckForSerializationFailure. This function checks whether there's a dangerous structure of the form far --- near --- me where neither the far or near transactions have committed. If so, it aborts the near transaction by marking it as DOOMED. However, that transaction might already be PREPARED. We need to check whether that's the case and, if so, abort the transaction that's trying to commit instead. Yep, committed. All the other places where we set the DOOMED flag seem to handle that already. In the long term, I'm not sure this is the best way to handle this. It feels a bit silly to set the flag, release the SerializableXactHashLock, and reacquire it later to remove the transaction from the hash table. Surely that could be done in some more straightforward way. But I don't feel like fiddling with it this late in the release cycle. One of the prepared_xacts regression tests actually hits this bug. I removed the anomaly from the duplicate-gids test so that it fails in the intended way, and added a new test to check serialization failures with a prepared transaction. Hmm, I have ran make installcheck with default_transaction_isolation='serializable' earlier. I wonder why I didn't see that. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [COMMITTERS] pgsql: Fixed string in German translation that causes segfault.
On Tue, Jun 21, 2011 at 01:36:05PM +0200, Magnus Hagander wrote: AFAIK (but I could be wrong), git submodules requires the files to be in *one* subdirectory. Our .po files are distributed all across the backend. So we'd have to make (and backpatch) som rather large changes in how these things are built in order to use that. Ah, good point. 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 Jabber: michael.meskes at googlemail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, 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] Re: [COMMITTERS] pgsql: Fixed string in German translation that causes segfault.
Excerpts from Magnus Hagander's message of mar jun 21 07:36:05 -0400 2011: On Tue, Jun 21, 2011 at 10:20, Michael Meskes mes...@postgresql.org wrote: On Mon, Jun 20, 2011 at 04:44:20PM -0400, Tom Lane wrote: My recollection is that the current setup was created mainly so that translators wouldn't need to be given commit privileges on the main repo. Giving them a separate repo to work in might be all right, but of course whoever does the merges would have to be careful to only accept changes made to the .po files and not anything else. IIRC this is exactly what git submodules are for. We could do a git archive only for translations as a submodule for our main git. That way translators would only clone the translation git, while we still have the translations in the source tree in the main git. At least this is how I think it works. AFAIK (but I could be wrong), git submodules requires the files to be in *one* subdirectory. Our .po files are distributed all across the backend. So we'd have to make (and backpatch) som rather large changes in how these things are built in order to use that. If git submodules are so cool that we still want to use them, maybe we still can -- can a submodule be submodule of more than one module? If so, we could create one submodule for each subdir that the translations are stored in (about 20 currently), and then have a pgtranslation meta-project that binds them all together as submodules. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TABLE lock strength reduction patch is unsafe
On Mon, Jun 20, 2011 at 5:56 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Sun, Jun 19, 2011 at 5:13 PM, Simon Riggs si...@2ndquadrant.com wrote: We scan pg_class in two ways: to rebuild a relcache entry based on a relation's oid (easy fix). We also scan pg_class to resolve the name to oid mapping. The name to oid mapping is performed *without* a lock on the relation, since we don't know which relation to lock. So the name lookup can fail if we are in the middle of a pg_class update. This is an existing potential bug in Postgres unrelated to my patch. If this is a pre-existing bug, then it's not clear to me why we need to do anything about it at all right now. Yeah. This behavior has been there since day zero, and there have been very few complaints about it. But note that there's only a risk for pg_class updates, not any other catalog, and there is exactly one kind of failure with very predictable consequences. The ALTER TABLE patch has greatly expanded the scope of the issue, and that *is* a regression compared to prior releases. It's not entirely clear to me how many additional failure cases we've bought ourselves with this patch. The particular one you've demonstrated seems pretty similar to the on we already had, although possibly the window for it is wider. Did you run the same test you used on 9.1 on 9.0 for comparison? BTW, it seems to me that this issue is closely related to what Noah is trying to fix here: http://archives.postgresql.org/message-id/20110612191843.gf21...@tornado.leadboat.com I think there are two general problems here. One, we use SnapshotNow to scan system catalogs, and SnapshotNow semantics are a mess. Two, even if we used an MVCC snapshot to scan the system catalogs, it's not necessarily safe or correct to latch onto an old row version, because the row update is combined with other actions that are not MVCC-safe, like removing files on disk. Breaking it down a bit more: A. The problem with using a DDL lock level AccessExclusiveLock, AFAICS, is entirely due to SnapshotNow semantics. Any row version we can possibly see is OK, but we had better see exactly one. Or at least not less than one. B. The problem with name lookups failing in the middle of a pg_class update is also entirely due to SnapshotNow semantics. C. The problem Noah is complaining about is NOT due to SnapshotNow semantics. If someone does BEGIN; DROP TABLE foo; CREATE TABLE foo(); COMMIT, it is no longer OK for anyone to be looking at the old foo. An MVCC snapshot is enough to guarantee that we'll see some row, but examining the old one won't cut it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TABLE lock strength reduction patch is unsafe
On Mon, Jun 20, 2011 at 6:55 PM, Simon Riggs si...@2ndquadrant.com wrote: I agree the scope for RELOID errors increased with my 9.1 patch. I'm now happy with the locking patch (attached), which significantly reduces the scope - back to the original error scope, in my testing. I tried to solve both, but I think that's a step too far given the timing. It seems likely that there will be objections to this patch. All I would say is that issuing a stream of ALTER TABLEs against the same table is not a common situation; if it were we would have seen more of the pre-existing bug. ALTER TABLE command encompasses many subcommands and we should evaluate each subcommand differently when we decide what to do. Well, my principal objection is that I think heavyweight locking is an excessively expensive solution to this problem. I think the patch is simple enough that I wouldn't object to applying it on those grounds even at this late date, but I bet if we do some benchmarking on the right workload we'll find a significant performance regression. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
Excerpts from Pavel Stehule's message of mar jun 21 00:59:44 -0400 2011:
yes - it has a sense. Quoting changes sense from keyword to literal.
But then I see a significant inconsistency - every know keywords
should be only tokens.
else if (strcmp(token, pamservice) == 0)
- {
- REQUIRE_AUTH_OPTION(uaPAM, pamservice, pam);
- parsedline-pamservice = pstrdup(c);
- }
because pamservice - is known keyword, but 'pamservice' is some
literal without any mean. You should to use a makro token_is_keyword
more often.
Yeah, I wondered about this too (same with auth types, i.e. do we accept
quoted hostssl and so on or should that by rejected?). I opted for
leaving it alone, but maybe this needs to be fixed. (Now that I think
about it, what we should do first is verify whether it works with quotes
in the unpatched code).
--
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
2011/6/21 Alvaro Herrera alvhe...@commandprompt.com:
Excerpts from Pavel Stehule's message of mar jun 21 00:59:44 -0400 2011:
yes - it has a sense. Quoting changes sense from keyword to literal.
But then I see a significant inconsistency - every know keywords
should be only tokens.
else if (strcmp(token, pamservice) == 0)
- {
- REQUIRE_AUTH_OPTION(uaPAM, pamservice, pam);
- parsedline-pamservice = pstrdup(c);
- }
because pamservice - is known keyword, but 'pamservice' is some
literal without any mean. You should to use a makro token_is_keyword
more often.
Yeah, I wondered about this too (same with auth types, i.e. do we accept
quoted hostssl and so on or should that by rejected?). I opted for
leaving it alone, but maybe this needs to be fixed. (Now that I think
about it, what we should do first is verify whether it works with quotes
in the unpatched code).
It's question about compatibility - sure. But a description inside
pg_hba.conf speaks cleanly - quoting means a lost of original
semantic.
And if we allow a quoting somewhere, then I can't imagine a
description - somewhere quoting means a string literal, somewhere
have not impact?
Regards
Pavel Stehule
--
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
Excerpts from Pavel Stehule's message of mar jun 21 10:04:26 -0400 2011:
2011/6/21 Alvaro Herrera alvhe...@commandprompt.com:
Excerpts from Pavel Stehule's message of mar jun 21 00:59:44 -0400 2011:
yes - it has a sense. Quoting changes sense from keyword to literal.
But then I see a significant inconsistency - every know keywords
should be only tokens.
else if (strcmp(token, pamservice) == 0)
- {
- REQUIRE_AUTH_OPTION(uaPAM, pamservice, pam);
- parsedline-pamservice = pstrdup(c);
- }
because pamservice - is known keyword, but 'pamservice' is some
literal without any mean. You should to use a makro token_is_keyword
more often.
Yeah, I wondered about this too (same with auth types, i.e. do we accept
quoted hostssl and so on or should that by rejected?). I opted for
leaving it alone, but maybe this needs to be fixed. (Now that I think
about it, what we should do first is verify whether it works with quotes
in the unpatched code).
I tested it and it works: This line
local @dbs +b trust
is accepted and it works in the unpatched code. I don't think we want
to break people's existing pg_hba.conf files for no reason. I doubt
that many people are using pg_hba.conf tokens with quotes, mind you, but
there might be some ...
In any case, if people here thinks we should tighten this, it's easy to
do on top of this patch by changing the strcmp() calls to
token_is_keyword, as you say. Let's not burden this patch with the
responsibility of doing so, because that's likely to get it punted.
--
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
2011/6/21 Alvaro Herrera alvhe...@commandprompt.com:
Excerpts from Pavel Stehule's message of mar jun 21 10:04:26 -0400 2011:
2011/6/21 Alvaro Herrera alvhe...@commandprompt.com:
Excerpts from Pavel Stehule's message of mar jun 21 00:59:44 -0400 2011:
yes - it has a sense. Quoting changes sense from keyword to literal.
But then I see a significant inconsistency - every know keywords
should be only tokens.
else if (strcmp(token, pamservice) == 0)
- {
- REQUIRE_AUTH_OPTION(uaPAM, pamservice, pam);
- parsedline-pamservice = pstrdup(c);
- }
because pamservice - is known keyword, but 'pamservice' is some
literal without any mean. You should to use a makro token_is_keyword
more often.
Yeah, I wondered about this too (same with auth types, i.e. do we accept
quoted hostssl and so on or should that by rejected?). I opted for
leaving it alone, but maybe this needs to be fixed. (Now that I think
about it, what we should do first is verify whether it works with quotes
in the unpatched code).
I tested it and it works: This line
local @dbs +b trust
is accepted and it works in the unpatched code. I don't think we want
to break people's existing pg_hba.conf files for no reason. I doubt
that many people are using pg_hba.conf tokens with quotes, mind you, but
there might be some ...
In any case, if people here thinks we should tighten this, it's easy to
do on top of this patch by changing the strcmp() calls to
token_is_keyword, as you say. Let's not burden this patch with the
responsibility of doing so, because that's likely to get it punted.
It is time to discuss about it.
I thinking so current behave is strange and should be fixed - it
doesn't respect a description stored in pg_hba.conf. I agree, so this
will have impact on compatibility, but pg_hba is config file so this
impact is not too hard. The cleaning now can carry a benefit in
future, when pg_hba can be more complex.
Regards
Pavel
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
On Tue, Jun 21, 2011 at 10:15:50AM -0400, Alvaro Herrera wrote:
Excerpts from Pavel Stehule's message of mar jun 21 10:04:26 -0400 2011:
2011/6/21 Alvaro Herrera alvhe...@commandprompt.com:
Excerpts from Pavel Stehule's message of mar jun 21 00:59:44 -0400 2011:
yes - it has a sense. Quoting changes sense from keyword to literal.
But then I see a significant inconsistency - every know keywords
should be only tokens.
else if (strcmp(token, pamservice) == 0)
- {
- REQUIRE_AUTH_OPTION(uaPAM, pamservice, pam);
- parsedline-pamservice = pstrdup(c);
- }
because pamservice - is known keyword, but 'pamservice' is some
literal without any mean. You should to use a makro token_is_keyword
more often.
Yeah, I wondered about this too (same with auth types, i.e. do we accept
quoted hostssl and so on or should that by rejected?). I opted for
leaving it alone, but maybe this needs to be fixed. (Now that I think
about it, what we should do first is verify whether it works with quotes
in the unpatched code).
I tested it and it works: This line
local @dbs +b trust
is accepted and it works in the unpatched code. I don't think we want
to break people's existing pg_hba.conf files for no reason. I doubt
that many people are using pg_hba.conf tokens with quotes, mind you, but
there might be some ...
In any case, if people here thinks we should tighten this, it's easy to
do on top of this patch by changing the strcmp() calls to
token_is_keyword, as you say. Let's not burden this patch with the
responsibility of doing so, because that's likely to get it punted.
Hmm, would it be possible to add some deprecation warnings for this case
without making the code too messy? Perhaps with a macro
token_should_be_keyword. That's the usual path to tightening syntax.
Ross
--
Ross Reedstrom, Ph.D. reeds...@rice.edu
Systems Engineer Admin, Research Scientistphone: 713-348-6166
Connexions http://cnx.orgfax: 713-348-3665
Rice University MS-375, Houston, TX 77005
GPG Key fingerprint = F023 82C8 9B0E 2CC6 0D8E F888 D3AE 810E 88F0 BEDE
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
Alvaro Herrera alvhe...@commandprompt.com writes: Excerpts from Pavel Stehule's message of mar jun 21 00:59:44 -0400 2011: because pamservice - is known keyword, but 'pamservice' is some literal without any mean. You should to use a makro token_is_keyword more often. Yeah, I wondered about this too (same with auth types, i.e. do we accept quoted hostssl and so on or should that by rejected?). I opted for leaving it alone, but maybe this needs to be fixed. (Now that I think about it, what we should do first is verify whether it works with quotes in the unpatched code). AFAICS, this is only important in places where the syntax allows either a keyword or an identifier. If only a keyword is possible, there is no value in rejecting it because it's quoted. And, when you do the test, I think you'll find that it would be breaking hba files that used to work (though admittedly, it's doubtful that there are any such in the field). 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] Re: [COMMITTERS] pgsql: Fixed string in German translation that causes segfault.
On Tue, Jun 21, 2011 at 15:36, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Magnus Hagander's message of mar jun 21 07:36:05 -0400 2011: On Tue, Jun 21, 2011 at 10:20, Michael Meskes mes...@postgresql.org wrote: On Mon, Jun 20, 2011 at 04:44:20PM -0400, Tom Lane wrote: My recollection is that the current setup was created mainly so that translators wouldn't need to be given commit privileges on the main repo. Giving them a separate repo to work in might be all right, but of course whoever does the merges would have to be careful to only accept changes made to the .po files and not anything else. IIRC this is exactly what git submodules are for. We could do a git archive only for translations as a submodule for our main git. That way translators would only clone the translation git, while we still have the translations in the source tree in the main git. At least this is how I think it works. AFAIK (but I could be wrong), git submodules requires the files to be in *one* subdirectory. Our .po files are distributed all across the backend. So we'd have to make (and backpatch) som rather large changes in how these things are built in order to use that. If git submodules are so cool that we still want to use them, maybe we still can -- can a submodule be submodule of more than one module? If so, we could create one submodule for each subdir that the translations are stored in (about 20 currently), and then have a pgtranslation meta-project that binds them all together as submodules. Can you? Yes. But I doubt that would be very convenient to work with that many submodules in general. If that's what we're looking at, what we have now seems a lot more convenient. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
2011/6/21 Tom Lane t...@sss.pgh.pa.us: Alvaro Herrera alvhe...@commandprompt.com writes: Excerpts from Pavel Stehule's message of mar jun 21 00:59:44 -0400 2011: because pamservice - is known keyword, but 'pamservice' is some literal without any mean. You should to use a makro token_is_keyword more often. Yeah, I wondered about this too (same with auth types, i.e. do we accept quoted hostssl and so on or should that by rejected?). I opted for leaving it alone, but maybe this needs to be fixed. (Now that I think about it, what we should do first is verify whether it works with quotes in the unpatched code). AFAICS, this is only important in places where the syntax allows either a keyword or an identifier. If only a keyword is possible, there is no value in rejecting it because it's quoted. And, when you do the test, I think you'll find that it would be breaking hba files that used to work (though admittedly, it's doubtful that there are any such in the field). It should be better documented. I don't think so this is good solution, but this is not too important. regards Pavel 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] ALTER TABLE lock strength reduction patch is unsafe
Robert Haas robertmh...@gmail.com writes: On Mon, Jun 20, 2011 at 5:56 PM, Tom Lane t...@sss.pgh.pa.us wrote: Yeah. This behavior has been there since day zero, and there have been very few complaints about it. But note that there's only a risk for pg_class updates, not any other catalog, and there is exactly one kind of failure with very predictable consequences. The ALTER TABLE patch has greatly expanded the scope of the issue, and that *is* a regression compared to prior releases. It's not entirely clear to me how many additional failure cases we've bought ourselves with this patch. The particular one you've demonstrated seems pretty similar to the on we already had, although possibly the window for it is wider. It's not so much the probability of failure that is bothering me, as the variety of possible symptoms. There was exactly one failure mode before, namely no such relation. I'm not sure how many possible symptoms there are now, but there's a lot, and most of them are going to be weird what the heck was that?? behaviors. If we let 9.1 ship like this, we are going to be creating a support headache. Even worse, knowing that those bugs exist will tempt us to write off reports of weird cache lookup failures as being instances of this problem, when closer investigation might show that they're something else. Please note that this position should not be regarded as support for Simon's proposed patch. I still think the right decision is to revert the ALTER TABLE feature, mainly because I do not believe this is the last bug in it. And the fact that there's a pre-existing bug with a vaguely similar symptom is no justification for introducing more bugs. 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] ALTER TABLE lock strength reduction patch is unsafe
Simon Riggs si...@2ndquadrant.com writes: On Mon, Jun 20, 2011 at 10:56 PM, Tom Lane t...@sss.pgh.pa.us wrote: The ALTER TABLE patch has greatly expanded the scope of the issue, and that *is* a regression compared to prior releases. I agree the scope for RELOID errors increased with my 9.1 patch. I'm now happy with the locking patch (attached), which significantly reduces the scope - back to the original error scope, in my testing. I tried to solve both, but I think that's a step too far given the timing. It seems likely that there will be objections to this patch. Yup, you're right. Having read this patch, I have absolutely zero confidence in it. It introduces some locks in random places, with no rhyme or reason that I can see. There is no reason to think that this is a complete solution, and considerable reason to think that it isn't (notably, the RELOID syscache is hardly the only one at risk). Worse, it's adding more locking in performance-critical places, which seems to me to severely degrade the argument for the original feature, namely that it was supposed to give us *less* locking. 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] Re: [COMMITTERS] pgsql: Fixed string in German translation that causes segfault.
Excerpts from Magnus Hagander's message of mar jun 21 11:01:58 -0400 2011: On Tue, Jun 21, 2011 at 15:36, Alvaro Herrera alvhe...@commandprompt.com wrote: If git submodules are so cool that we still want to use them, maybe we still can -- can a submodule be submodule of more than one module? If so, we could create one submodule for each subdir that the translations are stored in (about 20 currently), and then have a pgtranslation meta-project that binds them all together as submodules. Can you? Yes. But I doubt that would be very convenient to work with that many submodules in general. If that's what we're looking at, what we have now seems a lot more convenient. Yeah, after reading the manpage, I think it would be pretty inconvenient for us. Maybe if we were starting from scratch it would make more sense. (The main pain point is that you can't have the meta-project I suggested above: a submodule is un-updatable from the including tree, you have to check it out separately. So we would have to move all the po files into a single dir, as you said.) -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Identifying no-op length coercions
Hi,
On Jun 19, 2011, at 2:10 PM, Noah Misch wrote:
On Sat, Jun 18, 2011 at 11:32:20PM -0400, Robert Haas wrote:
On Sat, Jun 18, 2011 at 11:12 PM, Robert Haas robertmh...@gmail.com wrote:
On Sat, Jun 18, 2011 at 11:06 PM, Noah Misch n...@leadboat.com wrote:
On Sat, Jun 18, 2011 at 10:57:13PM -0400, Robert Haas wrote:
On Sat, Jun 11, 2011 at 5:13 PM, Noah Misch n...@leadboat.com wrote:
Sounds good. ?Updated patch attached, incorporating your ideas. ?Before
applying
it, run this command to update the uninvolved pg_proc.h DATA entries:
?perl -pi -e 's/PGUID(\s+\d+){4}/$ 0/' src/include/catalog/pg_proc.h
This doesn't quite apply any more. ?I think the pgindent run broke it
slightly.
Hmm, I just get two one-line offsets when applying it to current master.
?Note
that you need to run the perl invocation before applying the patch. ?Could
you
provide full output of your `patch' invocation, along with any reject
files?
Ah, crap. ?You're right. ?I didn't follow your directions for how to
apply the patch. ?Sorry.
I think you need to update the comment in simplify_function() to say
that we have three strategies, rather than two. I think it would also
be appropriate to add a longish comment just before the test that
calls protransform, explaining what the charter of that function is
and why the mechanism exists.
Good idea. See attached.
Documentation issues aside, I see very little not to like about this.
Great! Thanks for reviewing.
noop-length-coercion-v3.patch
Here is my review of this patch.
The patch applies cleanly to the HEAD, produces no additional warnings. It
doesn't include additional regression tests. One can include a test, using the
commands like the ones included below.
Changes to the documentation are limited to the a description of a new
pg_class attribute. It would probably make sense to document all the
exceptions to the table's rewrite on ALTER TABLE documentation page, although
it could wait for more such exceptions.
The feature works as intended and I haven't noticed any crashes or assertion
failures, i.e.
postgres=# create table test(id integer, name varchar);
CREATE TABLE
postgres=# insert into test values(1, 'test');
INSERT 0 1
postgres=# select relfilenode from pg_class where oid='test'::regclass;
relfilenode
-
66302
(1 row)
postgres=# alter table test alter column name type varchar(10);
ALTER TABLE
postgres=# select relfilenode from pg_class where oid='test'::regclass;
relfilenode
-
66308
(1 row)
postgres=# alter table test alter column name type varchar(65535);
ALTER TABLE
postgres=# select relfilenode from pg_class where oid='test'::regclass;
relfilenode
-
66308
(1 row)
The code looks good and takes into account all the previous suggestions.
The only nitpick code-wise is these lines in varchar_transform:
+ int32 old_max = exprTypmod(source) - VARHDRSZ;
+ int32 new_max = new_typmod - VARHDRSZ;
I have a hard time understanding why VARHDRSZ is subtracted here, so I'd
assume that's a bug.
Other than that, I haven't noticed any issues w/ this patch.
Sincerely,
Alexey.
--
Command Prompt, Inc. http://www.CommandPrompt.com
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
Excerpts from Pavel Stehule's message of mar jun 21 11:04:11 -0400 2011: 2011/6/21 Tom Lane t...@sss.pgh.pa.us: AFAICS, this is only important in places where the syntax allows either a keyword or an identifier. If only a keyword is possible, there is no value in rejecting it because it's quoted. And, when you do the test, I think you'll find that it would be breaking hba files that used to work (though admittedly, it's doubtful that there are any such in the field). It should be better documented. I don't think so this is good solution, but this is not too important. On the contrary -- we should support it but not document it. I mean, what good would that do? If someone is so silly to uselessly quote keywords, let them do it, but let's not encourage it. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Table Partitioning
On Mon, Jun 20, 2011 at 5:42 PM, David Fetter da...@fetter.org wrote: I noticed that we have some nice new speed optimizations (more properly, de-pessimizations) for partitioned tables in 9.1. /me sticks tongue out at dfetter. Anybody care to look over the table partitioning stuff on the wiki and check it for relevance? http://wiki.postgresql.org/wiki/Table_partitioning Itagaki Takahiro had a patch for this about a year ago, but I wasn't happy with the system catalog representation he chose and I think there were some other issues as well. Still, I think a pretty clear way forward here is to try to figure out a way to add some explicit syntax for range partitions, so that you can say... foo_a is for all rows where foo starts with 'a' foo_b is for all rows where foo starts with 'b' ... foo_xyz is for all rows where foo starts with 'xyz' If we have that data represented explicitly in the system catalog, then we can look at doing built-in INSERT-routing and UPDATE-handling. For an added bonus, it's a more natural syntax. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TABLE lock strength reduction patch is unsafe
Excerpts from Robert Haas's message of mar jun 21 09:40:16 -0400 2011: On Mon, Jun 20, 2011 at 6:55 PM, Simon Riggs si...@2ndquadrant.com wrote: I agree the scope for RELOID errors increased with my 9.1 patch. I'm now happy with the locking patch (attached), which significantly reduces the scope - back to the original error scope, in my testing. I tried to solve both, but I think that's a step too far given the timing. It seems likely that there will be objections to this patch. All I would say is that issuing a stream of ALTER TABLEs against the same table is not a common situation; if it were we would have seen more of the pre-existing bug. ALTER TABLE command encompasses many subcommands and we should evaluate each subcommand differently when we decide what to do. Well, my principal objection is that I think heavyweight locking is an excessively expensive solution to this problem. I think the patch is simple enough that I wouldn't object to applying it on those grounds even at this late date, but I bet if we do some benchmarking on the right workload we'll find a significant performance regression. Yeah, taking a hw lock at each relcache item build is likely to be prohibitively expensive. Heck, relation extension is expensive already in some loads. (I'm guessing that things will tank when there's a relcache reset). Still, this seems to be an overall better approach to the problem than what's been proposed elsewhere. Maybe we can do something with a map of relations that are protected by a bunch of lwlocks instead. (We could use that for relation extension too; that'd rock.) The patch may be simple, but is it complete? I think you need to have lock acquisition on create rule and create index too. Right now it only has locks on trigger stuff and alter table. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Table Partitioning
On Tue, Jun 21, 2011 at 01:07:17PM -0400, Robert Haas wrote: On Mon, Jun 20, 2011 at 5:42 PM, David Fetter da...@fetter.org wrote: I noticed that we have some nice new speed optimizations (more properly, de-pessimizations) for partitioned tables in 9.1. /me sticks tongue out at dfetter. Anybody care to look over the table partitioning stuff on the wiki and check it for relevance? http://wiki.postgresql.org/wiki/Table_partitioning Itagaki Takahiro had a patch for this about a year ago, but I wasn't happy with the system catalog representation he chose and I think there were some other issues as well. In particular, I'm noticing things labeled 8.4 and 9.0 as future. Still, I think a pretty clear way forward here is to try to figure out a way to add some explicit syntax for range partitions, so that you can say... foo_a is for all rows where foo starts with 'a' foo_b is for all rows where foo starts with 'b' ... foo_xyz is for all rows where foo starts with 'xyz' If we have that data represented explicitly in the system catalog, then we can look at doing built-in INSERT-routing and UPDATE-handling. For an added bonus, it's a more natural syntax. Does someone else have such a syntax? Does The Standard™ have anything to say? Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TABLE lock strength reduction patch is unsafe
Excerpts from Tom Lane's message of mar jun 21 11:06:22 -0400 2011: Please note that this position should not be regarded as support for Simon's proposed patch. I still think the right decision is to revert the ALTER TABLE feature, mainly because I do not believe this is the last bug in it. And the fact that there's a pre-existing bug with a vaguely similar symptom is no justification for introducing more bugs. Note that this feature can be disabled by tweaking AlterTableGetLockLevel so that it always returns AccessExclusive. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TABLE lock strength reduction patch is unsafe
Alvaro Herrera alvhe...@commandprompt.com writes: Excerpts from Tom Lane's message of mar jun 21 11:06:22 -0400 2011: Please note that this position should not be regarded as support for Simon's proposed patch. I still think the right decision is to revert the ALTER TABLE feature, mainly because I do not believe this is the last bug in it. And the fact that there's a pre-existing bug with a vaguely similar symptom is no justification for introducing more bugs. Note that this feature can be disabled by tweaking AlterTableGetLockLevel so that it always returns AccessExclusive. I think Simon had also hacked a couple of other places such as CREATE TRIGGER, but yeah, I was thinking of just lobotomizing that function with an #ifdef. When and if we get these problems worked out, it'll be easy to re-enable the feature. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Identifying no-op length coercions
On Tue, Jun 21, 2011 at 06:31:44PM +0300, Alexey Klyukin wrote: Here is my review of this patch. Thanks! The patch applies cleanly to the HEAD, produces no additional warnings. It doesn't include additional regression tests. One can include a test, using the commands like the ones included below. Changes to the documentation are limited to the a description of a new pg_class attribute. It would probably make sense to document all the exceptions to the table's rewrite on ALTER TABLE documentation page, although it could wait for more such exceptions. I like the current level of detail in the ALTER TABLE page. Having EXPLAIN ALTER TABLE would help here, but that's a bigger project than this one. postgres=# alter table test alter column name type varchar(10); ALTER TABLE postgres=# select relfilenode from pg_class where oid='test'::regclass; relfilenode - 66308 (1 row) postgres=# alter table test alter column name type varchar(65535); ALTER TABLE postgres=# select relfilenode from pg_class where oid='test'::regclass; relfilenode - 66308 (1 row) A pg_regress test needs stable output, so we would do it roughly like this: CREATE TEMP TABLE relstorage AS SELECT 0::regclass AS oldnode; ... UPDATE relstorage SET oldnode = (SELECT relfilenode FROM pg_class WHERE oid = 'test'::regclass); ALTER TABLE test ALTER name TYPE varchar(65535); SELECT oldnode relfilenode AS rewritten FROM pg_class, relstorage WHERE oid = 'test'::regclass; I originally rejected that as too ugly to read. Perhaps not. The only nitpick code-wise is these lines in varchar_transform: + int32 old_max = exprTypmod(source) - VARHDRSZ; + int32 new_max = new_typmod - VARHDRSZ; I have a hard time understanding why VARHDRSZ is subtracted here, so I'd assume that's a bug. We track the varchar typmod internally as (max length) + VARHDRSZ. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch for 9.2: enhanced errors
On 11-06-20 03:44 PM, Pavel Stehule wrote: Hello You need to update config.sgml at the same time you update this format. You need to append a , after application name but before constraintName. As it stands the CSV log has something like: .nbtinsert.c:433,psqla_pkey,public,a,a fixed The CSV log seems fine now. nbtinsert.c pg_get_indrelation is named differently than everything else in this file (ie _bt...). My guess is that this function belongs somewhere else but I don't know the code well enough to say where you should move it too. I renamed this function to IndexRelationGetParentRelation and muved to relcache.c Thanks, it looks less out of place there than it did in nbtinsert.c I don't call a quote_identifier on only data error properties like table_name or schema_name (but I am open to arguments for it or against it). The quote_identifier is used for column names, because there should be a more names and comma should be used inside name - and this is consistent with pg_get_indexdef_columns. Regards Okay. Pavel Stehule I'm going to mark this as ready for a committer.
Re: [HACKERS] Libpq enhancement
On Sun, Jun 19, 2011 at 8:08 PM, Robert Haas robertmh...@gmail.com wrote: On Sun, Jun 19, 2011 at 11:04 AM, Jeff Shanab jsha...@smartwire.com wrote: I am wondering If I am missing something obvious. If not, I have a suggestion for plpgsql. Stored procedures can accept rows. Libpq can receive rows (PQResult). Wouldn’t it be a great interface if PQResult was “bi-directional”? Create a result set on the client then call the database with a command. For insert, we have something like this already - this is what copy is for. 'copy' is a *bulk* insert statement -- it's great for the very specific case when you are dumbly stuffing data into the database, especially if performance is critical and sane error handling is not. It is not suitable for anything else: feeding data into functions, update/upsert/delete, insert with join, pre-post process, etc. Also copy runs through libpq textually at the line level, not at the field level like the rest of libpq. For update, it's a bit more complex - we don't have a replace into operator... Actually, we do. 9.1 supports data modifying CTE around which it's possible to rig a perfectly reasonable upsert...barring that, you could trivially do something similar in a hand rolled backend upsert function that takes a row or a set of rows (fed in as a composite array). Point being, the server has the necessary features -- it's the client that's the (solved) problem. At the risk of sounding 'broken record repetitive', let me echo andrew's comment upthread that libpqtypes solves the OP's problem completely in a very elegant way. The basic M.O. is to: 1. register the type you are using for transport (can either be the table or a composite type) 2. for each record you want to send, PQputf that record, and if you are sending more than one, PQputf the record into it's array 3. PQparamExec() a query that might look like one of: /* straight up insert */ PQparamExec(conn, param, INSERT INTO foo SELECT (unnest(%foo[])).* FROM f, resfmt); /* send to function */ PQparamExec(conn, param, SELECT do_stuff(%foo[]) , resfmt); /* upsert -- pre 9.1 this could be done in plpgsql loop, etc */ WITH foos AS (SELECT (UNNEST(%foo[])).*) updated as (UPDATE foo SET foo.a = foos.a ... RETURNING foo.id) INSERT INTO foo SELECT foos.* FROM foos LEFT JOIN updated USING(id) WHERE updated.id IS NULL; Basically, the trick is to exploit the server's composite array type features on the client side to do exactly what the OP is gunning for. You can send anything from simple arrays to entire complex nested structures that way -- although the complex stuff would typically go a to a function. Performance wise, it's faster than traditional query methods (everything is sent in binary) but slower than 'copy'. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: hstore - Implementation and performance issues around its operators
1. Obviously the '@' has to be used in order to let use the GiST index.
Why is the '-' operator not supported by GiST ('-' is actually
mentioned in all examples of the doc.)?
There's no way to make the planner recognize something like
(col-'foo' = 'bar')
as being an indexable condition on col. (If 'foo' is constant then you
can of course create a functional (btree) index on (col-'foo').)
Whereas (col @ hstore('foo','bar')) can use a GiST or GIN index directly
as long as 'foo' and 'bar' are pseudoconstants (i.e. constants, columns of
other tables, or stable functions of those).
2. Currently the hstore elements are stored in order as they are
coming from the insert statement / constructor.
No, the elements of an hstore are stored in order of (keylength,key)
with the key comparison done bytewise (not locale-dependent).
Why are the elements not ordered i.e. why is the hstore not cached in
all hstore functions (like hstore_fetchval etc.)?
I don't understand what you think would be cached.
3. In the source code 'hstore_io.c' one finds the following enigmatic
note: ... very large hstore values can't be output. this could be
fixed, but many other data types probably have the same issue.
What is the max. length of a hstore (i.e. the max. length of the sum
of all elements in text representation)?
An hstore is internally limited to the 1GB limit that applies to all
varlenas and individual memory allocations. However, the output text
representation can be longer than the internal one, and the output is
_also_ limited to 1GB.
The most obvious example of another type with the same issue is bytea,
where a value 512MB will fail to output in hex mode, and a value 256MB
might fail to output in escape mode depending on the content.
4. Last, I don't fully understand the following note in the hstore
doc. (http://www.postgresql.org/docs/current/interactive/hstore.html
):
Notice that the old names are reversed from the convention
formerly followed by the core geometric data types!
Why names? Why not rather 'operators' or 'functions'?
It's referring to the name of the operator (e.g. @ or ~).
What does this reversed from the convention mean concretely?
In old postgres versions the operators @ and ~ were used by various types
to mean contains and contained by, but many contrib types had them
reversed in meaning from the core geometry types.
For example, if a and b are boxes, (a @ b) means a is contained in b,
but if they are cubes from contrib/cube, or hstores, then (a @ b) means
a contains b.
The inconsistency here was obviously intolerable and was resolved by
introducing @ and @ as the recommended names for contained in and
contains operators respectively, with the giving a visual hint as
to which was the larger side.
--
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] Online base backup from the hot-standby
On 11-06-14 02:52 AM, Jun Ishiduka wrote:
I still think that's headed in the wrong direction.
(http://archives.postgresql.org/pgsql-hackers/2011-05/msg01405.php)
Please check these mails, and teach the reason for content of the wrong
direction.
(http://archives.postgresql.org/pgsql-hackers/2011-06/msg00209.php)
(http://archives.postgresql.org/pgsql-hackers/2011-05/msg01566.php)
Jun, I've been reviewing these threads as a start to reviewing your
patch (I haven't yet looked at the patch).
I *think* the concern is that
1) Today you can do a backup by just calling pg_start_backup('x'); copy
the data directory and
pg_stop_backup(); You do not need to use pg_basebackup to create a
backup. The solution you are proposing would require pg_basebackup to be
used to build backups from standby servers.
2) If I run pg_basebackup but do not specify '-x' then no pg_xlog
segments are included in the output. The relevant pg_xlog segments are
in the archive from the master. I can see situations where you are
already copying the archive to the remote site that the new standby will
be created in so you don't want to have to copy the pg_xlog segments
twice over your network.
What Heikki is proposing will work both when you aren't using
pg_basebackup (as long the output of pg_stop_backup() is somehow
captured in a way that it can be read) and will also work with
pg_basebackup when '-x' isn't specified.
Steve
Jun Ishizuka
NTT Software Corporation
TEL：045-317-7018
E-Mail: ishizuka@po.ntts.co.jp
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch for 9.2: enhanced errors
2011/6/21 Steve Singer ssinger...@sympatico.ca: On 11-06-20 03:44 PM, Pavel Stehule wrote: Hello You need to update config.sgml at the same time you update this format. You need to append a , after application name but before constraintName. As it stands the CSV log has something like: .nbtinsert.c:433,psqla_pkey,public,a,a fixed The CSV log seems fine now. nbtinsert.c pg_get_indrelation is named differently than everything else in this file (ie _bt...). My guess is that this function belongs somewhere else but I don't know the code well enough to say where you should move it too. I renamed this function to IndexRelationGetParentRelation and muved to relcache.c Thanks, it looks less out of place there than it did in nbtinsert.c I don't call a quote_identifier on only data error properties like table_name or schema_name (but I am open to arguments for it or against it). The quote_identifier is used for column names, because there should be a more names and comma should be used inside name - and this is consistent with pg_get_indexdef_columns. Regards Okay. Pavel Stehule I'm going to mark this as ready for a committer. Thank you very much Regards Pavel Stehule -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Identifying no-op length coercions
On Jun 21, 2011, at 9:58 PM, Noah Misch wrote: A pg_regress test needs stable output, so we would do it roughly like this: CREATE TEMP TABLE relstorage AS SELECT 0::regclass AS oldnode; ... UPDATE relstorage SET oldnode = (SELECT relfilenode FROM pg_class WHERE oid = 'test'::regclass); ALTER TABLE test ALTER name TYPE varchar(65535); SELECT oldnode relfilenode AS rewritten FROM pg_class, relstorage WHERE oid = 'test'::regclass; I originally rejected that as too ugly to read. Perhaps not. Yes, your example is more appropriate. I think you can make it more straightforward by getting rid of the temp table: CREATE TABLE test(oldnode oid, name varchar(5)); INSERT INTO test(oldnode) SELECT relfilenode FROM pg_class WHERE oid='test'::regclass; ALTER TABLE test ALTER name TYPE varchar(10); SELECT oldnode relfilenode AS rewritten FROM pg_class, test WHERE oid='test'::regclass; The only nitpick code-wise is these lines in varchar_transform: +int32 old_max = exprTypmod(source) - VARHDRSZ; +int32 new_max = new_typmod - VARHDRSZ; I have a hard time understanding why VARHDRSZ is subtracted here, so I'd assume that's a bug. We track the varchar typmod internally as (max length) + VARHDRSZ. Oh, right, haven't thought that this is a varchar specific thing. Thank you, Alexey. -- Command Prompt, Inc. http://www.CommandPrompt.com PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
2011/6/21 Alvaro Herrera alvhe...@commandprompt.com: Excerpts from Pavel Stehule's message of mar jun 21 11:04:11 -0400 2011: 2011/6/21 Tom Lane t...@sss.pgh.pa.us: AFAICS, this is only important in places where the syntax allows either a keyword or an identifier. If only a keyword is possible, there is no value in rejecting it because it's quoted. And, when you do the test, I think you'll find that it would be breaking hba files that used to work (though admittedly, it's doubtful that there are any such in the field). It should be better documented. I don't think so this is good solution, but this is not too important. On the contrary -- we should support it but not document it. I mean, what good would that do? If someone is so silly to uselessly quote keywords, let them do it, but let's not encourage it. it is argument too :) It has not good solution - one break compatibility, second is strange and undocumented :( Actually I don't remember a issues about pg_hba.conf - probably 99% users work with default configuration, so we can leave this file in current state. I am thinking so a notice in pg_hba.conf can be redesigned - almost all people don't read it, but if someone read it, then he needs a correct information - in sense, so on quotes works only where literal or known literal can be entered. Regards Pavel Stehule -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
On Jun 21, 2011, at 12:41 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: On the contrary -- we should support it but not document it. +1. ...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] Coding style point: const in function parameter declarations
I notice that the SSI code is rather heavily invested in function declarations like this: extern bool PageIsPredicateLocked(const Relation relation, const BlockNumber blkno); I find this to be poor style, and would like to see if there's any support for getting rid of the const keywords. My objections are twofold: 1. What such a const marking actually does is to forbid the function from changing the value of its local variable that received the passed parameter value. While you may or may not think that it's good style to avoid doing so, whether the function chooses to do that or not is surely no business of its callers'. Putting such a marking into the extern declaration doesn't create any useful API contract, it just means you'll have to change the declaration if you change the function's implementation. 2. In cases such as const Relation foo where the parameter type is a typedeffed pointer, it is easy for readers to arrive at the false conclusion that this guarantees the function doesn't change the pointed-to structure. But it guarantees no such thing, because that construction is *not* equivalent to const struct RelationData *foo; rather it means struct RelationData * const foo, ie only the pointer is being const-ified, not that to which it points. The function can hack the struct contents, or pass the pointer to functions that do arbitrary things, and the compiler won't make a whimper. So I think that declarations like this are positively pernicious --- they'll mislead all but the most language-lawyerly readers. Declarations like const structtype *param are fine, because those create a real, enforced contract on what the function can do to data that is visible to its caller. But I don't see any value at all in const-ifying the parameter itself. Comments? 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] Re: patch review : Add ability to constrain backend temporary file space
On 21/06/11 02:39, Cédric Villemain wrote: 2011/6/20 Robert Haasrobertmh...@gmail.com: On Mon, Jun 20, 2011 at 9:15 AM, Cédric Villemain cedric.villemain.deb...@gmail.com wrote: The feature does not work exactly as expected because the write limit is rounded per 8kB because we write before checking. I believe if one write a file of 1GB in one pass (instead of repetitive 8kB increment), and the temp_file_limit is 0, then the server will write the 1GB before aborting. Can we rearrange thing so we check first, and then write? probably but it needs more work to catch corner cases. We may be safe to just document that (and also in the code). The only way I see so far to have a larger value than 8kB here is to have a plugin doing the sort instead of the postgresql core sort algo. Thanks guys - will look at moving the check, and adding some documentation about the possible impacts of plugins (or new executor methods) that might write in chunks bigger than blocksz. Maybe a few days - I'm home sick ATM, plus looking after these http://www.maftet.co.nz/kittens.html Cheers Mark -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Coding style point: const in function parameter declarations
On 21 June 2011 23:51, Tom Lane t...@sss.pgh.pa.us wrote: I notice that the SSI code is rather heavily invested in function declarations like this: extern bool PageIsPredicateLocked(const Relation relation, const BlockNumber blkno); I find this to be poor style, and would like to see if there's any support for getting rid of the const keywords. My objections are twofold: 1. What such a const marking actually does is to forbid the function from changing the value of its local variable that received the passed parameter value. While you may or may not think that it's good style to avoid doing so, whether the function chooses to do that or not is surely no business of its callers'. Putting such a marking into the extern declaration doesn't create any useful API contract, it just means you'll have to change the declaration if you change the function's implementation. People may be confused by that inconsistency though. I agree that it generally doesn't make sense to const-qualify a passed-by-value parameter. I've seen it done, but I struggle to recall a case where I thought that it made much sense. 2. In cases such as const Relation foo where the parameter type is a typedeffed pointer, it is easy for readers to arrive at the false conclusion that this guarantees the function doesn't change the pointed-to structure. But it guarantees no such thing, because that construction is *not* equivalent to const struct RelationData *foo; rather it means struct RelationData * const foo, ie only the pointer is being const-ified, not that to which it points. The function can hack the struct contents, or pass the pointer to functions that do arbitrary things, and the compiler won't make a whimper. So I think that declarations like this are positively pernicious --- they'll mislead all but the most language-lawyerly readers. IMHO, you don't have to be that much of a language lawyer to pick up on that, because it's not as if the typedef has very effectively encapsulated the fact that the underlying type is actually a pointer anyway. I for one find it intuitively obvious that the pointed-to data isn't declared const in your example. Declarations like const structtype *param are fine, because those create a real, enforced contract on what the function can do to data that is visible to its caller. But I don't see any value at all in const-ifying the parameter itself. +1 const is actually an example of C++ influencing C. It tends to work very well with C++, where it cascades usefully throughout an entire system, and where a distinction can be made between logical and bit-wise const-ness. In C, it is a bit wishy-washy, particularly the fact that what happens when const-ness is violated is undefined, rather than just having the code fail to compile. const is also supposed to be able to facilitate certain compiler optimisations, which is why you have the undefined behaviour thing (what if the variable is stored in read-only memory?), but that isn't very relevant in practice. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Coding style point: const in function parameter declarations
On Tue, Jun 21, 2011 at 06:51:20PM -0400, Tom Lane wrote: I find this to be poor style, and would like to see if there's any support for getting rid of the const keywords. I'm in favor of removing them too. Dan -- Dan R. K. Ports MIT CSAILhttp://drkp.net/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] WIP pgindent replacement
Attached is a WIP possible replacement for pgindent. Instead of a shell
script invoking a mishmash of awk and sed, some of which is pretty
impenetrable, it uses a single engine (perl) to do all the pre and post
indent processing. Of course, if your regex-fu and perl-fu is not up the
scratch this too might be impenetrable, but all but a couple of the
recipes are reduced to single lines, and I'd argue that they are all at
least as comprehensible as what they replace.
Attached also is a diff file showing what it does differently from the
existing script. I think that these are all things where the new script
is more correct than the existing script. Most of the changes come into
two categories:
* places where the existing script fails to combine the function
return type and the function name on a single line in function
prototypes.
* places where unwanted blank lines are removed by the new script
but not by the existing script.
Features include:
* command line compatibility with the existing script, so you can do:
find ../../.. -name '*.[ch]' -type f -print | egrep -v -f
exclude_file_patterns | xargs -n100 ./pgindent.pl typedefs.list
* a new way of doing the same thing much more nicely:
./pgindent.pl --search-base=../../.. --typedefs=typedefs.list
--excludes=exclude_file_patterns
* only passes relevant typedefs to indent, not the whole huge list
* should in principle be runnable on Windows, unlike existing script
(I haven't tested yet)
* no semantic tab literals; tabs are only generated using \t and
tested for using \t, \h or \s as appropriate. This makes debugging
the script much less frustrating. If something looks like a space
it should be a space.
In one case I used perl's extended regex mode to comment a fairly hairy
regex. This should probably be done a bit more, maybe for all of them.
If anybody is so inclined, this could be used as a basis for removing
the use of bsd indent altogether, as has been suggested before, as well
as external entab/detab.
Comments welcome.
cheers
andrew
pgindent.pl
Description: Perl program
diff --git a/contrib/btree_gist/btree_bit.c b/contrib/btree_gist/btree_bit.c
index 8675d24..f69faaa 100644
--- a/contrib/btree_gist/btree_bit.c
+++ b/contrib/btree_gist/btree_bit.c
@@ -95,7 +95,6 @@ gbt_bit_xfrm(bytea *leaf)
static GBT_VARKEY *
gbt_bit_l2n(GBT_VARKEY *leaf)
{
-
GBT_VARKEY *out = leaf;
GBT_VARKEY_R r = gbt_var_key_readable(leaf);
bytea *o;
diff --git a/contrib/btree_gist/btree_text.c b/contrib/btree_gist/btree_text.c
index 3d4f8c1..b55dc7c 100644
--- a/contrib/btree_gist/btree_text.c
+++ b/contrib/btree_gist/btree_text.c
@@ -119,7 +119,6 @@ gbt_text_compress(PG_FUNCTION_ARGS)
Datum
gbt_bpchar_compress(PG_FUNCTION_ARGS)
{
-
GISTENTRY *entry = (GISTENTRY *) PG_GETARG_POINTER(0);
GISTENTRY *retval;
diff --git a/contrib/btree_gist/btree_ts.c b/contrib/btree_gist/btree_ts.c
index 9d3a591..185bed0 100644
--- a/contrib/btree_gist/btree_ts.c
+++ b/contrib/btree_gist/btree_ts.c
@@ -380,7 +380,6 @@ gbt_ts_union(PG_FUNCTION_ARGS)
Datum
gbt_ts_penalty(PG_FUNCTION_ARGS)
{
-
tsKEY *origentry = (tsKEY *) DatumGetPointer(((GISTENTRY *) PG_GETARG_POINTER(0))-key);
tsKEY *newentry = (tsKEY *) DatumGetPointer(((GISTENTRY *) PG_GETARG_POINTER(1))-key);
float *result = (float *) PG_GETARG_POINTER(2);
diff --git a/contrib/btree_gist/btree_utils_num.c b/contrib/btree_gist/btree_utils_num.c
index a3da580..8b8cd31 100644
--- a/contrib/btree_gist/btree_utils_num.c
+++ b/contrib/btree_gist/btree_utils_num.c
@@ -134,7 +134,6 @@ gbt_num_union(GBT_NUMKEY *out, const GistEntryVector *entryvec, const gbtree_nin
bool
gbt_num_same(const GBT_NUMKEY *a, const GBT_NUMKEY *b, const gbtree_ninfo *tinfo)
{
-
GBT_NUMKEY_R b1,
b2;
@@ -156,7 +155,6 @@ gbt_num_same(const GBT_NUMKEY *a, const GBT_NUMKEY *b, const gbtree_ninfo *tinfo
void
gbt_num_bin_union(Datum *u, GBT_NUMKEY *e, const gbtree_ninfo *tinfo)
{
-
GBT_NUMKEY_R rd;
rd.lower = e[0];
diff --git a/contrib/btree_gist/btree_utils_var.c b/contrib/btree_gist/btree_utils_var.c
index e73799b..dc3ad38 100644
--- a/contrib/btree_gist/btree_utils_var.c
+++ b/contrib/btree_gist/btree_utils_var.c
@@ -54,7 +54,6 @@ gbt_var_decompress(PG_FUNCTION_ARGS)
GBT_VARKEY_R
gbt_var_key_readable(const GBT_VARKEY *k)
{
-
GBT_VARKEY_R r;
r.lower = (bytea *) (((char *) k)[VARHDRSZ]);
@@ -106,7 +105,6 @@ gbt_var_leaf2node(GBT_VARKEY *leaf, const gbtree_vinfo *tinfo)
static int32
gbt_var_node_cp_len(const GBT_VARKEY *node, const gbtree_vinfo *tinfo)
{
-
GBT_VARKEY_R r = gbt_var_key_readable(node);
int32 i = 0;
int32 l = 0;
@@ -270,7 +268,6 @@ gbt_var_bin_union(Datum *u, GBT_VARKEY *e, Oid collation,
GISTENTRY *
gbt_var_compress(GISTENTRY *entry, const gbtree_vinfo *tinfo)
{
-
GISTENTRY *retval;
if (entry-leafkey)
@@ -299,7 +296,6 @@ GBT_VARKEY *
gbt_var_union(const GistEntryVector
Re: [HACKERS] [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address
On Sun, Jun 19, 2011 at 7:40 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote: Sorry, the previous revision did not update regression test part towards the latest one. Some of the refactoring you've done here seems likely to break things, because you're basically making the relation locking happen later than it does not, and that's going to cause problems. get_object_address_relobject() is a particularly egregious rearrangement. It seems to me that the right formula is to call relation_openrv() if missing_ok is false, and try_relation_openrv() if missing_ok is true. But that's sort of a pain, so I propose to first apply the attached patch, which gets rid of try_relation_openrv() and try_heap_openrv() and instead adds a missing_ok argument to relation_openrv() and heap_openrv(). If we do this, then the missing_ok argument can just be passed through all the way down. Thoughts? Comments? Objections? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company there-is-no-try.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSI tuning points
On Sun, Jun 19, 2011 at 11:10 AM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: That does seem better. Thanks. OK, committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] date_part for infinity intervals
On Mon, Jun 20, 2011 at 5:54 AM, Vlad Arkhipov arhi...@dc.baikal.ru wrote:
The behaviour of date_part function is opaque for infinity intervals. For
example
date_part('epoch', 'infinity'::date) and date_part('year', 'infinity'::date)
return zero but is supposed to return 'infinity',
date_part('day', 'infinity'::date) returns zero, should it return 'NaN'
instead?
Dunno. It's been this way since 2001; before that, it returned NULL.
I don't see any particular justification for making the return value
different in the infinity case depending on whether epoch or day
is requested. Returning Infinity rather than 0 might have some
merit, but I'm not sure it's worth breaking backward compatibility for
it. What do our competitors do in this case?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Auto Start second postgres 8.3.15-1 instance MAC OS X
On Mon, Jun 20, 2011 at 6:14 AM, Diogo Santos d.san...@tomorrow-options.com wrote: Hi, I'm used to work with PostgreSQL on Windows but now I've moved to OS X and I'm having problems to create a service to auto start a new server (instance) of PostgreSQL. Firstly I used the PostgreSQL installer to create the first server and the postgres user, then I used initdb to create another server and until this step, everything turned fine. After that, I created a folder on StartupItems with StartupParameters and the other script. Here are the contents of the two files: I have no idea how to troubleshoot this, but you might want to take a loop at the contents of contrib/start-scripts/osx. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP pgindent replacement
Andrew Dunstan wrote: Attached is a WIP possible replacement for pgindent. Instead of a shell script invoking a mishmash of awk and sed, some of which is pretty impenetrable, it uses a single engine (perl) to do all the pre and post indent processing. Of course, if your regex-fu and perl-fu is not up the scratch this too might be impenetrable, but all but a couple of the recipes are reduced to single lines, and I'd argue that they are all at least as comprehensible as what they replace. I am excited Andrew has done this. It has been on my TODO list for a while --- I was hoping someday we could switch to GNU indent but gave up after the GNU indent report from Greg Stark that exactly matched my experience years ago: http://archives.postgresql.org/pgsql-hackers/2011-04/msg01436.php Basically, GNU indent has new bugs, but bugs that are harder to work around than the BSD indent bugs. Once I heard that I realized we were going to be using BSD indent for many more years to come and rewriting the shell script in Perl was the next logical step, which Andrew has done. This will also allow us to use pgindent on Windows that has Perl installed --- we should create a DOS binary of bsd indent so Windows users scan run it. I would also like to add a command-line way to detect our patched version of BSD indent so we know we are using the right binary. I will do that once the Perl version is committed to git. Thanks Andrew. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Identifying no-op length coercions
On Tue, Jun 21, 2011 at 5:50 PM, Alexey Klyukin al...@commandprompt.com wrote: On Jun 21, 2011, at 9:58 PM, Noah Misch wrote: A pg_regress test needs stable output, so we would do it roughly like this: CREATE TEMP TABLE relstorage AS SELECT 0::regclass AS oldnode; ... UPDATE relstorage SET oldnode = (SELECT relfilenode FROM pg_class WHERE oid = 'test'::regclass); ALTER TABLE test ALTER name TYPE varchar(65535); SELECT oldnode relfilenode AS rewritten FROM pg_class, relstorage WHERE oid = 'test'::regclass; I originally rejected that as too ugly to read. Perhaps not. Yes, your example is more appropriate. I think you can make it more straightforward by getting rid of the temp table: CREATE TABLE test(oldnode oid, name varchar(5)); INSERT INTO test(oldnode) SELECT relfilenode FROM pg_class WHERE oid='test'::regclass; ALTER TABLE test ALTER name TYPE varchar(10); SELECT oldnode relfilenode AS rewritten FROM pg_class, test WHERE oid='test'::regclass; Without wishing to foreclose the possibility of adding a suitable regression test, I've committed the main patch. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade using appname to lock out other users
Bruce Momjian wrote: Bruce Momjian wrote: I meant the PGPASSWORD environment variable: indexterm primaryenvarPGPASSWORD/envar/primary /indexterm envarPGPASSWORD/envar behaves the same as the xref linkend=libpq-connect-password connection parameter. Use of this environment variable is not recommended for security reasons, as some operating systems allow non-root users to see process environment variables via applicationps/; instead consider using the filename~/.pgpass/ file (see xref linkend=libpq-pgpass). The only other way to do this is to pass it on the command line, but some options don't allow that (pg_ctl), and PGPASSFILE is going to require me to create a dummy .pgpass password file in a valid format and use that. One interesting idea would be to write the server password in the PGPASSFILE format, and allow the server and libpq to read the same file by pointing PGPASSFILE at that file. Discussion seems to have ended on this thread without a clear direction. Let me throw out some new ideas. The advantage of writing a password file is that we can avoid the need to modify pg_hba.conf to allow access without supplying a password. The downside is that it will only apply to PG 9.2+ servers, making configuration even more complex because the new and old servers would behave differently. One simple improvement would be to set listen_addresses to '' on Unix and 'localhost' on Windows to limit who can connect. This works on old and new servers. PG 9.1 already allows only super-user connections in -b binary-upgrade mode. No one seemed to like the appname filter but it seemed like a cheap solution. Remember we are not trying to secure the server while in pg_upgrade mode --- only avoid accidental connections. And, again, using a default port number of 25432 will work for old/new servers. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] deadlock_timeout at PGC_SIGHUP?
2011/6/17 Shigeru Hanada shigeru.han...@gmail.com: (2011/06/12 6:43), Noah Misch wrote: On Wed, Mar 30, 2011 at 04:48:26PM -0400, Robert Haas wrote: Me neither. If making the deadlock timeout PGC_SUSET is independently useful, I don't object to doing that first, and then we can wait and see if anyone feels motivated to do more. Here's the patch for that. Not much to it. I've reviewed the patch following the article in the PostgreSQL wiki. It seems fine except that it needs to be rebased, so I'll mark this Ready for committers'. OK, committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
On 22 June 2011 00:47, Tom Lane t...@sss.pgh.pa.us wrote: Alvaro Herrera alvhe...@commandprompt.com writes: Excerpts from Pavel Stehule's message of mar jun 21 00:59:44 -0400 2011: because pamservice - is known keyword, but 'pamservice' is some literal without any mean. You should to use a makro token_is_keyword more often. Yeah, I wondered about this too (same with auth types, i.e. do we accept quoted hostssl and so on or should that by rejected?). I opted for leaving it alone, but maybe this needs to be fixed. (Now that I think about it, what we should do first is verify whether it works with quotes in the unpatched code). AFAICS, this is only important in places where the syntax allows either a keyword or an identifier. If only a keyword is possible, there is no value in rejecting it because it's quoted. And, when you do the test, I think you'll find that it would be breaking hba files that used to work (though admittedly, it's doubtful that there are any such in the field). Yes, I agree, and this was my thinking when I came up against this while writing the original patch. It doesn't help to treat hostssl differently than hostssl, because quoted identifiers are meaningless in that context. 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] smallserial / serial2
On Thu, Jun 9, 2011 at 10:27 PM, Josh Kupershmidt schmi...@gmail.com wrote: On Wed, Jun 8, 2011 at 6:36 PM, Brar Piening b...@gmx.de wrote: I tried to look at everything and cover everthing but please consider that this is my first review so please have a second look at it! I took a look at this as well, and I didn't encounter any problems either. The patch is surprisingly small, but it looks like all the documentation spots got covered, and I didn't see any missing pieces; pg_dump copes fine, I didn't try pg_upgrade but I wouldn't expect problems there either. Actually, the one piece I think could be added is a regression test. I didn't see any existing tests that covered bigserial/serial8, either, so I went ahead and added a few tests for smallerial and bigserial. I didn't see the testcases Brar posted at first, or I might have adopted a few from there, but this should cover basic use of smallserial. Committed the main patch, and your regression tests. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address
Robert Haas robertmh...@gmail.com writes: Some of the refactoring you've done here seems likely to break things, because you're basically making the relation locking happen later than it does not, and that's going to cause problems. get_object_address_relobject() is a particularly egregious rearrangement. It seems to me that the right formula is to call relation_openrv() if missing_ok is false, and try_relation_openrv() if missing_ok is true. But that's sort of a pain, so I propose to first apply the attached patch, which gets rid of try_relation_openrv() and try_heap_openrv() and instead adds a missing_ok argument to relation_openrv() and heap_openrv(). If we do this, then the missing_ok argument can just be passed through all the way down. Thoughts? Comments? Objections? At least the last hunk (in pltcl.c) seems to have the flag backwards. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Indication of db-shared tables
Do we do enough to show which tables are db shared, e.g. pg_database? I don't see any indication from psql \dS. Are our docs clear enough? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade using appname to lock out other users
Bruce Momjian br...@momjian.us writes: Discussion seems to have ended on this thread without a clear direction. I still think the right thing is to just use a non-default port number. That gets 90% of the benefit for 10% of the work of any other approach (except for the ones for which the ratio is even worse). 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] [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address
On Tue, Jun 21, 2011 at 11:04 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Some of the refactoring you've done here seems likely to break things, because you're basically making the relation locking happen later than it does not, and that's going to cause problems. get_object_address_relobject() is a particularly egregious rearrangement. It seems to me that the right formula is to call relation_openrv() if missing_ok is false, and try_relation_openrv() if missing_ok is true. But that's sort of a pain, so I propose to first apply the attached patch, which gets rid of try_relation_openrv() and try_heap_openrv() and instead adds a missing_ok argument to relation_openrv() and heap_openrv(). If we do this, then the missing_ok argument can just be passed through all the way down. Thoughts? Comments? Objections? At least the last hunk (in pltcl.c) seems to have the flag backwards. Ah, nuts. Sorry. I think that and parse_relation.c are the only places where the try variants are used; nobody else is willing to fail, and everyone else is passing false. Revised patch attached. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company there-is-no-try-v2.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP pgindent replacement
Andrew Dunstan and...@dunslane.net writes: Attached is a WIP possible replacement for pgindent. Instead of a shell script invoking a mishmash of awk and sed, some of which is pretty impenetrable, it uses a single engine (perl) to do all the pre and post indent processing. Hm ... this should offer the chance of running on Windows, but otherwise I'm not sure it does very much for us, if you still have to have a patched version of NetBSD indent underneath. If anybody is so inclined, this could be used as a basis for removing the use of bsd indent altogether, as has been suggested before, as well as external entab/detab. Now *that* I would get excited about. But surely it would be a huge amount more work? 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] Indication of db-shared tables
-BEGIN PGP SIGNED MESSAGE- Hash: RIPEMD160 Do we do enough to show which tables are db shared, e.g. pg_database? I don't see any indication from psql \dS. Are our docs clear enough? I don't think \dS should be indicating such a thing. I think it's documented well enough: if you are doing something that it matters enough which tables are shared, you really oughtta know about them anyway. - -- Greg Sabino Mullane g...@turnstep.com End Point Corporation http://www.endpoint.com/ PGP Key: 0x14964AC8 201106212323 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iEYEAREDAAYFAk4BYF8ACgkQvJuQZxSWSsjOYACgnDq27MbRCg4Dr7QL/p6tq1kj 3EwAoPnJCqazL+akS1Au5WoxB5RvceDu =RpBk -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade using appname to lock out other users
Tom Lane wrote: Bruce Momjian br...@momjian.us writes: Discussion seems to have ended on this thread without a clear direction. I still think the right thing is to just use a non-default port number. That gets 90% of the benefit for 10% of the work of any other approach (except for the ones for which the ratio is even worse). I agree. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Latch implementation that wakes on postmaster death on both win32 and Unix
On Tue, Jun 21, 2011 at 6:22 PM, Peter Geoghegan pe...@2ndquadrant.com wrote:
Thanks for giving this your attention Fujii. Attached patch addresses
your concerns.
Thanks for updating the patch! I have a few comments;
+WaitLatch(volatile Latch *latch, int wakeEvents, long timeout)
+WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket
sock, long timeout)
If 'wakeEvent' is zero, we cannot get out of WaitLatch(). Something like
Assert(waitEvents != 0) is required? Or, WaitLatch() should always wait
on latch even when 'waitEvents' is zero?
In unix_latch.c, select() in WaitLatchOrSocket() checks the timeout only when
WL_TIMEOUT is set in 'wakeEvents'. OTOH, in win32_latch.c,
WaitForMultipleObjects()
in WaitLatchOrSocket() always checks the timeout even if WL_TIMEOUT is not
given. Is this intentional?
+ else if (rc == WAIT_OBJECT_0 + 2
+((wakeEvents WL_SOCKET_READABLE) ||
(wakeEvents WL_SOCKET_WRITEABLE)))
Another corner case: when WL_POSTMASTER_DEATH and WL_SOCKET_READABLE
are given and 'sock' is set to PGINVALID_SOCKET, we can wrongly pass through the
above check. If this OK?
rc = WaitForMultipleObjects(numevents, events, FALSE,
(timeout = 0) ?
(timeout / 1000) : INFINITE);
- if (rc == WAIT_FAILED)
+ if ( (wakeEvents WL_POSTMASTER_DEATH)
+!PostmasterIsAlive(true))
After WaitForMultipleObjects() detects the death of postmaster,
WaitForSingleObject()
is called in PostmasterIsAlive(). In this case, what code does
WaitForSingleObject() return?
I wonder if WaitForSingleObject() returns the code other than
WAIT_TIMEOUT and really
can detect the death of postmaster.
+ if (fcntl(postmaster_alive_fds[POSTMASTER_FD_WATCH], F_GETFL) 0)
+ {
+ ereport(FATAL,
+ (errcode_for_socket_access(),
+errmsg(failed to set the postmaster death watching
fd's flags:
%s, strerror(errno;
+ }
Is the above check really required? It's harmless, but looks unnecessary.
+errmsg( pipe() call failed to create pipe to monitor
postmaster
death: %s, strerror(errno;
+errmsg(failed to set the postmaster death watching
fd's flags:
%s, strerror(errno;
+errmsg(failed to set the postmaster death watching
fd's flags:
%s, strerror(errno;
'%m' should be used instead of '%s' and 'strerror(errno)'.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP pgindent replacement
On 06/21/2011 11:15 PM, Tom Lane wrote: Andrew Dunstanand...@dunslane.net writes: Attached is a WIP possible replacement for pgindent. Instead of a shell script invoking a mishmash of awk and sed, some of which is pretty impenetrable, it uses a single engine (perl) to do all the pre and post indent processing. Hm ... this should offer the chance of running on Windows, but otherwise I'm not sure it does very much for us, if you still have to have a patched version of NetBSD indent underneath. Well, to start with it provides a nicer way to invoke pgindent for the whole repo. And it will be relatively easy to incorporate new stuff, for example what Greg Smith was doing in his wrapper script. It's true that the script contains an unfortunately large number of workarounds for the limitations and failure modes of the patched bsd indent we've been using. Some of those are currently working less than perfectly, and I found it fairly difficult to understand exactly what they were doing. I hope what I have produced will be a bit clearer and more maintainable than what it replaces, as well as working a bit better and more robustly. If anybody is so inclined, this could be used as a basis for removing the use of bsd indent altogether, as has been suggested before, as well as external entab/detab. Now *that* I would get excited about. But surely it would be a huge amount more work? Yes, which is why I decided to do this - I certainly don't currently have time to re-implement indent. 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] Fwd: Keywords in pg_hba.conf should be field-specific
2011/6/22 Brendan Jurd dire...@gmail.com: On 22 June 2011 00:47, Tom Lane t...@sss.pgh.pa.us wrote: Alvaro Herrera alvhe...@commandprompt.com writes: Excerpts from Pavel Stehule's message of mar jun 21 00:59:44 -0400 2011: because pamservice - is known keyword, but 'pamservice' is some literal without any mean. You should to use a makro token_is_keyword more often. Yeah, I wondered about this too (same with auth types, i.e. do we accept quoted hostssl and so on or should that by rejected?). I opted for leaving it alone, but maybe this needs to be fixed. (Now that I think about it, what we should do first is verify whether it works with quotes in the unpatched code). AFAICS, this is only important in places where the syntax allows either a keyword or an identifier. If only a keyword is possible, there is no value in rejecting it because it's quoted. And, when you do the test, I think you'll find that it would be breaking hba files that used to work (though admittedly, it's doubtful that there are any such in the field). Yes, I agree, and this was my thinking when I came up against this while writing the original patch. It doesn't help to treat hostssl differently than hostssl, because quoted identifiers are meaningless in that context. ook, now is clean, so this is majority opinion. Please, can you send a final patch. Regards Pavel 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] Fwd: Keywords in pg_hba.conf should be field-specific
On 22 June 2011 14:01, Pavel Stehule pavel.steh...@gmail.com wrote: ook, now is clean, so this is majority opinion. Please, can you send a final patch. I don't have any further changes to add to Alvaro's version 3, which is already up on the CF app. 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
[HACKERS] Repeated PredicateLockRelation calls during seqscan
I was looking at ExecSeqScan today and noticed that it invokes
PredicateLockRelation each time it's called, i.e. for each tuple
returned. Any reason we shouldn't skip that call if
rs_relpredicatelocked is already set, as in the attached patch?
That would save us a bit of overhead, since checking that flag is
cheaper than doing a hash lookup in the local predicate lock table
before bailing out.
Dan
--
Dan R. K. Ports MIT CSAILhttp://drkp.net/
diff --git a/src/backend/executor/nodeSeqscan.c b/src/backend/executor/nodeSeqscan.c
index f356874..32a8f56 100644
--- a/src/backend/executor/nodeSeqscan.c
+++ b/src/backend/executor/nodeSeqscan.c
@@ -113,9 +113,13 @@ SeqRecheck(SeqScanState *node, TupleTableSlot *slot)
TupleTableSlot *
ExecSeqScan(SeqScanState *node)
{
- PredicateLockRelation(node-ss_currentRelation,
- node-ss_currentScanDesc-rs_snapshot);
- node-ss_currentScanDesc-rs_relpredicatelocked = true;
+ if (!node-ss_currentScanDesc-rs_relpredicatelocked)
+ {
+ PredicateLockRelation(node-ss_currentRelation,
+ node-ss_currentScanDesc-rs_snapshot);
+ node-ss_currentScanDesc-rs_relpredicatelocked = true;
+ }
+
return ExecScan((ScanState *) node,
(ExecScanAccessMtd) SeqNext,
(ExecScanRecheckMtd) SeqRecheck);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pika buildfarm member failure on isolationCheck tests
On Tue, Jun 21, 2011 at 03:01:48PM +0300, Heikki Linnakangas wrote: Thanks, committed. Thanks. In the long term, I'm not sure this is the best way to handle this. It feels a bit silly to set the flag, release the SerializableXactHashLock, and reacquire it later to remove the transaction from the hash table. Surely that could be done in some more straightforward way. But I don't feel like fiddling with it this late in the release cycle. Yes, I suspect it can be done better. The reason it's tricky is a lock ordering issue; part of releasing a SerializableXact has to be done while holding SerializableXactHashLock and part has to be done without it (because it involves taking partition locks). Reworking the order of these things might work, but would require some careful thought since most of the code is shared with the non-abort cleanup paths. And yes, it's definitely the time for that. I've been meaning to take another look at this part of the code anyway, with an eye towards performance. SerializableXactHashLock can be a bottleneck on certain in-memory workloads. One of the prepared_xacts regression tests actually hits this bug. I removed the anomaly from the duplicate-gids test so that it fails in the intended way, and added a new test to check serialization failures with a prepared transaction. Hmm, I have ran make installcheck with default_transaction_isolation='serializable' earlier. I wonder why I didn't see that. The affected test was being run at SERIALIZABLE already, so that wouldn't have made a difference. One reason I didn't notice an issue when I looked at that test before before, is that it was intended to fail anyway, just with a different error. I didn't think too hard about which error would take precedence. Dan -- Dan R. K. Ports MIT CSAILhttp://drkp.net/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pika buildfarm member failure on isolationCheck tests
On Wed, Jun 22, 2011 at 01:31:11AM -0400, Dan Ports wrote: Yes, I suspect it can be done better. The reason it's tricky is a lock ordering issue; part of releasing a SerializableXact has to be done while holding SerializableXactHashLock and part has to be done without it (because it involves taking partition locks). Reworking the order of these things might work, but would require some careful thought since most of the code is shared with the non-abort cleanup paths. And yes, it's definitely the time for that. ...by which I mean it's definitely *not* the time for that, of course. Dan -- Dan R. K. Ports MIT CSAILhttp://drkp.net/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
