[HACKERS] Planner issue
Hi
I developed a new character string type, named myvarchar.
Also an operator class for btree is added.
I created a table with two columns, first have myvarchar(100) and other is
varchar(100).
CREATE TABLE test_myvarchar (mine myvarchar(100), plain varchar(100));
CREATE INDEX test_myvarchar_i_mine ON test_myvarchar USING btree (mine);
CREATE INDEX test_myvarchar_i_plain ON test_myvarchar USING btree (plain);
Two same random strings to both of columns are inserted, and the operation
repeated until 32K rows are in the table.
INSERT INTO test_myvarchar VALUES ('example', 'example');
PROBLEM:
When I executed a query with where clause on 'mine' column, PG does not use
index.
But after I changed where clause to be on 'plain' column, PG uses index!
EXPLAIN SELECT * FROM test_myvarchar WHERE 'zagftha' = mine ORDER BY 1;
--
Sort (cost=3038.39..3065.00 rows=10642 width=197)
Sort Key: mine
- Seq Scan on test_myvarchar (cost=0.00..1308.08 rows=10642 width=197)
Filter: ('zagftha'::myvarchar = mine)
##
EXPLAIN SELECT * FROM test_myvarchar WHERE 'zagftha' = plain ORDER BY 2;
Index Scan using test_myvarchar_i_plain on test_myvarchar
(cost=0.41..6099.0
8 rows=31175 width=197)
Index Cond: ('zagftha'::text = (plain)::text)
Why planner does not choose the lowest cost path?
Is there any problem with my new type? How can I fix it?
Any help would be appreciated.
Regards,
Soroosh Sardari
Sharif University of Technology
Re: [HACKERS] Patch for reserved connections for replication users
On Sun, Oct 13, 2013 at 2:08 PM, Gibheer gibh...@zero-knowledge.org wrote: On Sun, 13 Oct 2013 11:38:17 +0530 Amit Kapila amit.kapil...@gmail.com wrote: On Thu, Oct 10, 2013 at 3:17 AM, Gibheer gibh...@zero-knowledge.org wrote: On Mon, 7 Oct 2013 11:39:55 +0530 Amit Kapila amit.kapil...@gmail.com wrote: Robert Haas wrote: On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund andres(at)2ndquadrant(dot)com wrote: Hmm. It seems like this match is making MaxConnections no longer mean the maximum number of connections, but rather the maximum number of non-replication connections. I don't think I support that definitional change, and I'm kinda surprised if this is sufficient to implement it anyway (e.g. see InitProcGlobal()). I don't think the implementation is correct, but why don't you like the definitional change? The set of things you can do from replication connections are completely different from a normal connection. So using separate pools for them seems to make sense. That they end up allocating similar internal data seems to be an implementation detail to me. Because replication connections are still connections. If I tell the system I want to allow 100 connections to the server, it should allow 100 connections, not 110 or 95 or any other number. I think that to reserve connections for replication, mechanism similar to superuser_reserved_connections be used rather than auto vacuum workers or background workers. This won't change the definition of MaxConnections. Another thing is that rather than introducing new parameter for replication reserved connections, it is better to use max_wal_senders as it can serve the purpose. Review for replication_reserved_connections-v2.patch, considering we are going to use mechanism similar to superuser_reserved_connections and won't allow definition of MaxConnections to change. Hi, I took the time and reworked the patch with the feedback till now. Thank you very much Amit! So this patch uses max_wal_senders together with the idea of the first patch I sent. The error messages are also adjusted to make it obvious, how it is supposed to be and all checks work, as far as I could tell. If I understand correctly, now the patch has implementation such that a. if the number of connections left are (ReservedBackends + max_wal_senders), then only superusers or replication connection's will be allowed b. if the number of connections left are ReservedBackend, then only superuser connections will be allowed. That is correct. So it will ensure that max_wal_senders is used for reserving connection slots from being used by non-super user connections. I find new usage of max_wal_senders acceptable, if anyone else thinks otherwise, please let us know. 1. +varnamesuperuser_reserved_connections/varname +varnamemax_wal_senders/varname only superuser and WAL connections +are allowed. Here minus seems to be missing before max_wal_senders and I think it will be better to use replication connections rather than WAL connections. This is fixed. 2. -new replication connections will be accepted. +new WAL or other connections will be accepted. I think as per new implementation, we don't need to change this line. I reverted that change. 3. + * reserved slots from max_connections for wal senders. If the number of free + * slots (max_connections - max_wal_senders) is depleted. Above calculation (max_connections - max_wal_senders) needs to include super user reserved connections. My first thought was, that I would not add it here. When superuser reserved connections are not set, then only max_wal_senders would count. But you are right, it has to be set, as 3 connections are reserved by default for superusers. + * slots (max_connections - superuser_reserved_connections - max_wal_senders) here it should be ReservedBackends rather than superuser_reserved_connections. 4. + /* + * Although replication connections currently require superuser privileges, we + * don't allow them to consume the superuser reserved slots, which are + * intended for interactive use. */ if ((!am_superuser || am_walsender) ReservedBackends 0 !HaveNFreeProcs(ReservedBackends)) ereport(FATAL, (errcode(ERRCODE_TOO_MANY_CONNECTIONS), - errmsg(remaining connection slots are reserved for non-replication superuser connections))); + errmsg(remaining connection slots are reserved for superuser connections))); Will there be any problem if we do the above check before the check for wal senders and reserved replication connections (+ !HaveNFreeProcs(max_wal_senders + ReservedBackends))) and don't change the error message in this check. I think this will ensure that users who doesn't enable max_wal_senders will see the same error message as before and the purpose to reserve connections for replication
Re: [HACKERS] Planner issue
Hello
pls, send a output of EXPLAIN ANALYZE statement,
there can be different reasons why optimizer doesn't choose some index
Regards
Pavel Stehule
2013/10/14 Soroosh Sardari soroosh.sard...@gmail.com
Hi
I developed a new character string type, named myvarchar.
Also an operator class for btree is added.
I created a table with two columns, first have myvarchar(100) and other is
varchar(100).
CREATE TABLE test_myvarchar (mine myvarchar(100), plain varchar(100));
CREATE INDEX test_myvarchar_i_mine ON test_myvarchar USING btree (mine);
CREATE INDEX test_myvarchar_i_plain ON test_myvarchar USING btree (plain);
Two same random strings to both of columns are inserted, and the
operation repeated until 32K rows are in the table.
INSERT INTO test_myvarchar VALUES ('example', 'example');
PROBLEM:
When I executed a query with where clause on 'mine' column, PG does not
use index.
But after I changed where clause to be on 'plain' column, PG uses index!
EXPLAIN SELECT * FROM test_myvarchar WHERE 'zagftha' = mine ORDER BY 1;
--
Sort (cost=3038.39..3065.00 rows=10642 width=197)
Sort Key: mine
- Seq Scan on test_myvarchar (cost=0.00..1308.08 rows=10642
width=197)
Filter: ('zagftha'::myvarchar = mine)
##
EXPLAIN SELECT * FROM test_myvarchar WHERE 'zagftha' = plain ORDER BY 2;
Index Scan using test_myvarchar_i_plain on test_myvarchar
(cost=0.41..6099.0
8 rows=31175 width=197)
Index Cond: ('zagftha'::text = (plain)::text)
Why planner does not choose the lowest cost path?
Is there any problem with my new type? How can I fix it?
Any help would be appreciated.
Regards,
Soroosh Sardari
Sharif University of Technology
Fwd: [HACKERS] Planner issue
2013/10/14 Soroosh Sardari soroosh.sard...@gmail.com
Hi
I developed a new character string type, named myvarchar.
Also an operator class for btree is added.
I created a table with two columns, first have myvarchar(100) and other
is
varchar(100).
CREATE TABLE test_myvarchar (mine myvarchar(100), plain varchar(100));
CREATE INDEX test_myvarchar_i_mine ON test_myvarchar USING btree (mine);
CREATE INDEX test_myvarchar_i_plain ON test_myvarchar USING btree (plain);
Two same random strings to both of columns are inserted, and the
operation repeated until 32K rows are in the table.
INSERT INTO test_myvarchar VALUES ('example', 'example');
PROBLEM:
When I executed a query with where clause on 'mine' column, PG does not
use index.
But after I changed where clause to be on 'plain' column, PG uses index!
EXPLAIN SELECT * FROM test_myvarchar WHERE 'zagftha' = mine ORDER BY 1;
--
Sort (cost=3038.39..3065.00 rows=10642 width=197)
Sort Key: mine
- Seq Scan on test_myvarchar (cost=0.00..1308.08 rows=10642
width=197)
Filter: ('zagftha'::myvarchar = mine)
##
EXPLAIN SELECT * FROM test_myvarchar WHERE 'zagftha' = plain ORDER BY 2;
Index Scan using test_myvarchar_i_plain on test_myvarchar
(cost=0.41..6099.0
8 rows=31175 width=197)
Index Cond: ('zagftha'::text = (plain)::text)
Why planner does not choose the lowest cost path?
Is there any problem with my new type? How can I fix it?
Any help would be appreciated.
Regards,
Soroosh Sardari
Sharif University of Technology
On Mon, Oct 14, 2013 at 10:29 AM, Pavel Stehule pavel.steh...@gmail.com
wrote:
Hello
pls, send a output of EXPLAIN ANALYZE statement,
there can be different reasons why optimizer doesn't choose some index
Regards
Pavel Stehule
The output of EXPLAIN ANALYSE for the two queries come in the blow.
Sort (cost=3038.39..3065.00 rows=10642 width=197) (actual
time=938.564..1168.1
18 rows=31070 loops=1)
Sort Key: mine
Sort Method: external merge Disk: 6304kB
- Seq Scan on test_myvarchar (cost=0.00..1308.08 rows=10642
width=197) (a
ctual time=0.072..78.545 rows=31070 loops=1)
Filter: ('zagftha'::myvarchar = mine)
Rows Removed by Filter: 856
Total runtime: 1176.822 ms
Index Scan using test_myvarchar_i_plain on test_myvarchar
(cost=0.41..6099.0
8 rows=31175 width=197) (actual time=0.124..61.417 rows=31054 loops=1)
Index Cond: ('zagftha'::text = (plain)::text)
Total runtime: 67.918 ms
Re: [HACKERS] Planner issue
Soroosh Sardari soroosh.sard...@gmail.com writes: I developed a new character string type, named myvarchar. Also an operator class for btree is added. PROBLEM: When I executed a query with where clause on 'mine' column, PG does not use index. Most likely you got the opclass definition wrong. Since you've shown us no details of what you did, it's hard to speculate about just how. But note that varchar itself is a pretty bad model for a user-added datatype, because it has a special symbiotic relationship with type text (to wit, it has no operators of its own but uses text's operators via implicit casts). To get to a working independent datatype like this, you'd need to pick the right aspects of each of text and varchar to clone. So my unfounded speculation is you didn't do that just right. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add use of asprintf()
On Sat, Oct 12, 2013 at 2:58 AM, Alvaro Herrera alvhe...@2ndquadrant.comwrote: Peter Eisentraut escribió: On Tue, 2013-09-17 at 15:13 +0500, Asif Naeem wrote: I did put some time review the patch, please see my findings below i.e. Updated patch for this. Looks good to me. http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=5b6d08cd2992922b667564a49f19580f11676050 This commit is has broken the visual studios windows build. I'm looking into it now 31 errors similar to: D:\Postgres\c\pgsql.sln (default target) (1) - D:\Postgres\c\pg_archivecleanup.vcxproj (default target) (56) - libpgport.lib(asprintf.obj) : error LNK2019: unresolved external symbol _va_copy referenced in function _vasprintf [D:\Postgres\c\pg_archivecleanup.vcxproj] .\Release\pg_archivecleanup\pg_archivecleanup.exe : fatal error LNK1120: 1 unresolved externals [D:\Postgres\c\pg_archivecleanup.vcxproj] Regards David Rowley -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] using testlo example of postgres source code
I work in linux I want use testlo.c in client side that testlo.c is in src/test/example/ address of postgresql source code. I run make in /src/test/example/ then copy executable file(testlo) in lib directory of installation directory of postgresql. now I don't know how do I can use this execute library? how did I can an large object with this library(testlo)?
Re: [HACKERS] [PATCH] Add use of asprintf()
On Mon, Oct 14, 2013 at 9:45 PM, David Rowley dgrowle...@gmail.com wrote: On Sat, Oct 12, 2013 at 2:58 AM, Alvaro Herrera alvhe...@2ndquadrant.comwrote: Peter Eisentraut escribió: On Tue, 2013-09-17 at 15:13 +0500, Asif Naeem wrote: I did put some time review the patch, please see my findings below i.e. Updated patch for this. Looks good to me. http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=5b6d08cd2992922b667564a49f19580f11676050 This commit is has broken the visual studios windows build. I'm looking into it now Looks like something like: #ifndef WIN32 #define HAVE_VA_COPY 1 #endif would need to be added to asprintf.c, but also some work needs to be done with mcxt.c as it uses va_copy unconditionally. Perhaps just defining a macro for va_copy would be better for windows. I was not quite sure the best header file for such a macro so I did not write a patch to fix it. Regards David Rowley 31 errors similar to: D:\Postgres\c\pgsql.sln (default target) (1) - D:\Postgres\c\pg_archivecleanup.vcxproj (default target) (56) - libpgport.lib(asprintf.obj) : error LNK2019: unresolved external symbol _va_copy referenced in function _vasprintf [D:\Postgres\c\pg_archivecleanup.vcxproj] .\Release\pg_archivecleanup\pg_archivecleanup.exe : fatal error LNK1120: 1 unresolved externals [D:\Postgres\c\pg_archivecleanup.vcxproj] Regards David Rowley -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] dynamic shared memory
On Sun, Oct 13, 2013 at 3:07 AM, Amit Kapila amit.kapil...@gmail.com wrote:
1. Do you think we should add information about pg_dynshmem file at link:
http://www.postgresql.org/docs/devel/static/storage-file-layout.html
It contains information about all files/folders in data directory
2.
+/*
+ * Forget that a temporary file is owned by a ResourceOwner
+ */
+void
+ResourceOwnerForgetDSM(ResourceOwner owner, dsm_segment *seg)
+{
Above function description should use 'dynamic shmem segment' rather
than temporary file.
Forget that a dynamic shmem segment is owned by a ResourceOwner
Good catches, will fix.
--
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] Long paths for tablespace leads to uninterruptible hang in Windows
On Thu, Oct 10, 2013 at 9:34 AM, Amit Kapila amit.kapil...@gmail.com wrote: On further analysis, I found that hang occurs in some of Windows API(FindFirstFile, RemoveDirectroy) when symlink path (pg_tblspc/spcoid/TABLESPACE_VERSION_DIRECTORY) is used in these API's. For above testcase, it will hang in path destroy_tablespace_directories-ReadDir-readdir-FindFirstFile Well, that sucks. So it's a Windows bug. Some of the ways to resolve the problem are described as below: 1. I found that if the link path is accessed as a full path during readdir or stat, it works fine. For example in function destroy_tablespace_directories(), the path used to access tablespace directory is of form pg_tblspc/16235/PG_9.4_201309051 by using below sprintf sprintf(linkloc_with_version_dir, pg_tblspc/%u/%s,tablespaceoid,TABLESPACE_VERSION_DIRECTORY); Now when it tries to access this path it is assumed in code that corresponding OS API will take care of considering this path w.r.t current working directory, which is right as per specs, however as it hangs in OS API (FindFirstFile) if path length 130 for symlink and if try to use full path instead of starting with pg_tblspc, it works fine. So one way to resolve this issue is to use full path for symbolic link path access instead of relying on OS to use full path. I'm not sure how we'd implement this, except by doing #2. 2. Resolve symbolic link to actual path in code whenever we tries to access it using pgreadlink. It is already used in pg_basebackup. This seems reasonable. 3. One another way is to check in code (initdb and create tablespace) to not allow path of length more than 100 or 120 I don't think we could consider back-patching this, because it'd break installations that might be working fine now with longer pathnames. And I'd be a little reluctant to change the behavior in master, either, because it would create a dump-and-reload hazard, when users of older versions try to upgrade. -- 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] Cmpact commits and changeset extraction
On Fri, Oct 11, 2013 at 4:05 PM, Andres Freund and...@2ndquadrant.com wrote: Maybe. The original reason we added compact commits was because we thought that making unlogged tables logged and visca/versa was going to require adding still more stuff to the commit record. I'm no longer sure that's the case and, in any case, nobody's worked out the design details. But I can't help thinking more stuff's likely to come up in the future. I'm certainly willing to entertain proposals for restructuring that, but I don't really want to just throw it out. Well, what I am thinking of - including reading data depending on a flag in -xinfo - would give you extensibility without requiring different types of commits. And it would only blow up the size by whatever needs to be included. Hard to comment without seeing the patch. Sounds like it could be reasonable, if it's not too ugly. Maybe you should just skip replay of transactions with no useful content. Yes, I have thought about that as well. But I dislike it - how do we define no useful content? The only action we detected for that XID was the commit itself. What if the transaction was intentionally done to get an xid + timestamp in a multimaster system? What if it includes DDL but no logged data? Do we replay a transaction because of the pg_shdepend entry when creating a table in another database? None of these things seem particularly alarming to me. I don't know whether that represents a failure of imagination on my part or undue alarm on your part. :-) -- 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] background workers, round three
On Fri, Oct 11, 2013 at 9:27 AM, Michael Paquier michael.paqu...@gmail.com wrote: Few comments about the code: 1) In postmaster.c, what about adding a comment here telling that we can forget about this bgworker as it has already been requested for a termination: + if (rw-rw_worker.bgw_restart_time == BGW_NEVER_RESTART + || rw-rw_terminate) I think that'd be just repeating what the code already says. 2) Trying to think about this patch as an independent feature, I think that it would have been nice to demonstrate the capacity of TerminateBackgroundWorker directly with an example in worker_spi to show a direct application of it, with for example the addition of a function like worker_spi_terminate. However this needs: - either an additional API using as input the PID, pid used to fetch a bgworker handle terminating the worker afterwards. This is basically what I did in the patch attached when playing with your patch. You might find it useful... Or not! It introduces as well I think this is kind of missing the point of this interface. If you've already got the PID, you're can just kill the PID yourself. But suppose you've just registered a background worker in the parent process, and then there's an ERROR. You want the background worker to die, since you don't need it any more, but the postmaster might not have even started it yet, so there's no PID. That's the case in which this is really useful. It's not a good match for what worker_spi needs, because in the worker_spi, you're intending to start a process that you *expect* to outlive the user backend's transaction, and even the user backend's lifetime. I agree we need better testing for this code, but the problem is that this is all pretty low-level plumbing. This patch set was prompted by problems that came up when I tried to build an application using this facility; and I don't know that this is the last patch that will be needed to solve all of those problems. I'm working on another patch set that adds a shared-memory message queue through which messages can be pushed, and I think that will lend itself well to testing of both background workers and dynamic shared memory. I hope to have that ready in time for the next CommitFest. 3) Documentation is needed, following comment 2). Good point. -- 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] background workers, round three
On Sat, Oct 12, 2013 at 4:53 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: I briefly checked these patches. Let me add some comments. Thanks! * terminate-worker-v1.patch TerminateBackgroundWorker() turns on slot-terminate flag under LW_SHARED lock. Is it reasonable because all the possible caller is the background worker process itself, isn't it? Hmm. It's probably harmless the way it is, just because if two processes do that at the same time, it's not going to change the outcome. But it might be better to use LW_EXCLUSIVE just to make it easy to reason about the logic. I think I cut-and-pasted without thinking carefully about that. * ephemeral-precious-v1.patch AtEOXact_BackgroundWorker() is located around other AtEOXact_* routines. Doesn't it makes resource management complicated? In case when main process goes into error handler but worker process is still running in health, it may continue to calculate something and put its results on shared memory segment, even though main process suggest postmaster to kill it. Since I wrote this patch set, I've been thinking a lot more about error recovery. Obviously, one of the big problems as we think about parallel query is that you've now got multiple backends floating around, and if the transaction aborts (in any backend), the other backends don't automatically know that; they need some way to know that they, too, short abort processing. There are a lot of details to get right here, and the time I've spent on it so far convinces me that the problem is anything but easy. Having said that, I'm not too concerned about the particular issue that you raise here. The resources that need to be cleaned up during transaction abort are backend-private resources. If, for example, the user backend detaches a dynamic shared memory segment that is being used for a parallel computation, they're not actually *destroying* the segment; they are just detaching it *from their address space*. The last process to detach it will also destroy it. So the ordering in which the various processes detach it doesn't matter much. One of the things I do this is necessary is a set of on_dsm_detach callbacks that work pretty much the way that on_shmem_exit callbacks work today. Just as we can't detach from the main shared memory segment without releasing locks and buffer pins and lwlocks and our PGXACT, we can't release from a dynamic shared memory segment without performing any similar cleanup that is needed. I'm currently working on a patch for that. All the ResourceOwnerRelease() callbacks are located prior to AtEOXact_BackgroundWorker(), it is hard to release resources being in use by background worker, because they are healthy running until it receives termination signal, but sent later. In addition, it makes implementation complicated if we need to design background workers to release resources if and when it is terminated. I don't think it is a good coding style, if we need to release resources in different location depending on context. Which specific resources are you concerned about? So, I'd like to propose to add a new invocation point of ResourceOwnerRelease() after all AtEOXact_* jobs, with new label something like RESOURCE_RELEASE_FINAL. In addition, AtEOXact_BackgroundWorker() does not synchronize termination of background worker processes being killed. Of course it depends on situation, I think it is good idea to wait for completion of worker processes to be terminated, to ensure resource to be released is backed to the main process if above ResourceOwnerRelease() do the job. Again, which resources are we talking about here? I tend to think it's an essential property of the system that we *shouldn't* have to care about the order in which processes are terminated. First, that will be difficult to control; if an ERROR or FATAL condition has occurred and we need to terminate, then there are real limits to what guarantees we can provide after that point. Second, it's also *expensive*. The point of parallelism is to make things faster; any steps we add that involve waiting for other processes to do things will eat away at the available gains. For a query that'll run for an hour that hardly matters, but for short queries it's important to avoid unnecessary overhead. -- 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-tuning work_mem and maintenance_work_mem
On Sat, Oct 12, 2013 at 3:07 AM, Magnus Hagander mag...@hagander.net wrote: On Oct 11, 2013 10:23 PM, Josh Berkus j...@agliodbs.com wrote: On 10/11/2013 01:11 PM, Bruce Momjian wrote: In summary, I think we need to: * decide on new defaults for work_mem and maintenance_work_mem * add an initdb flag to allow users/packagers to set shared_bufffers? * add an autovacuum_work_mem setting? * change the default for temp_buffers? If we're changing defaults, bgwriter_lru_maxpages and vacuum_cost_limit could also use a bump; those thresholds were set for servers with 1GB of RAM Uh, those are there to limit io and not memory, right? More memory isn't the reason to increase them, more io is. For people deploying on modern server hardware then yes it's often low, but for all those deploying in virtualized environments with io performance reminding you of the 1990ies, I'm not so sure it is... bgwriter_lru_maxpages is clearly related to the size of shared_buffers, although confusingly it is expressed as a number of buffers, while shared_buffers is expressed as a quantity of memory. I think we might have done better to call the GUC bgwriter_lru_maxpercent and make it a percentage of shared buffers. -- 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] buildfarm failures on smew and anole
On Fri, Oct 11, 2013 at 4:03 PM, Andrew Dunstan and...@dunslane.net wrote:
Can the owners of these buildfarm machines please check whether there
are extra semaphores allocated and if so free them? Or at least
reboot, to see if that unbreaks the build?
It is possible to set the buildfarm config
build_env= {MAX_CONNECTIONS = 10 },
and the tests will run with that constraint.
Not sure if this would help.
Maybe I didn't explain that well. The problem is that the regression
tests require at least 20 connections to run, and those two machines
are currently auto-selecting 10 connections, so make check is failing.
--
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] buildfarm failures on smew and anole
On 2013-10-14 09:12:09 -0400, Robert Haas wrote:
On Fri, Oct 11, 2013 at 4:03 PM, Andrew Dunstan and...@dunslane.net wrote:
Can the owners of these buildfarm machines please check whether there
are extra semaphores allocated and if so free them? Or at least
reboot, to see if that unbreaks the build?
It is possible to set the buildfarm config
build_env= {MAX_CONNECTIONS = 10 },
and the tests will run with that constraint.
Not sure if this would help.
Maybe I didn't explain that well. The problem is that the regression
tests require at least 20 connections to run, and those two machines
are currently auto-selecting 10 connections, so make check is failing.
I think pg_regress has support for spreading groups to fewer connections
if max_connections is set appropriately. I guess that's what Andrew is
referring to.
That said, I don't think that's the solution here. The machine clearly
worked with more connections until recently.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training 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] buildfarm failures on smew and anole
On 10/14/2013 09:12 AM, Robert Haas wrote:
On Fri, Oct 11, 2013 at 4:03 PM, Andrew Dunstan and...@dunslane.net wrote:
Can the owners of these buildfarm machines please check whether there
are extra semaphores allocated and if so free them? Or at least
reboot, to see if that unbreaks the build?
It is possible to set the buildfarm config
build_env= {MAX_CONNECTIONS = 10 },
and the tests will run with that constraint.
Not sure if this would help.
Maybe I didn't explain that well. The problem is that the regression
tests require at least 20 connections to run, and those two machines
are currently auto-selecting 10 connections, so make check is failing.
Why do they need 20 connections? pg_regress has code in it to limit the
degree of parallelism of tests, and has done for years, specifically to
cater for buildfarm machines that are unable to handle the defaults.
Using this option in the buildfarm client config triggers use of this
feature.
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] buildfarm failures on smew and anole
On Mon, Oct 14, 2013 at 9:22 AM, Andrew Dunstan and...@dunslane.net wrote: Maybe I didn't explain that well. The problem is that the regression tests require at least 20 connections to run, and those two machines are currently auto-selecting 10 connections, so make check is failing. Why do they need 20 connections? pg_regress has code in it to limit the degree of parallelism of tests, and has done for years, specifically to cater for buildfarm machines that are unable to handle the defaults. Using this option in the buildfarm client config triggers use of this feature. Hmm, I wasn't aware of that. I thought they needed 20 connections because parallel_schedule says: # By convention, we put no more than twenty tests in any one parallel group; # this limits the number of connections needed to run the tests. If it's not supposed to matter how many connections are available, then that comment is misleading. But I think it does matter, at least in some situations, because otherwise these machines wouldn't be failing with sorry, too many clients already. Anyway, as Andres said, the machines were working fine until recently, so I think we just need to get them un-broken. -- 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] buildfarm failures on smew and anole
On 2013-10-14 09:28:04 -0400, Robert Haas wrote: # By convention, we put no more than twenty tests in any one parallel group; # this limits the number of connections needed to run the tests. If it's not supposed to matter how many connections are available, then that comment is misleading. But I think it does matter, at least in some situations, because otherwise these machines wouldn't be failing with sorry, too many clients already. Well, you need to explicitly pass --max-connections to pg_regress. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] logical changeset generation v6.2
On Fri, Oct 11, 2013 at 12:57 PM, Andres Freund and...@2ndquadrant.com wrote: I don't see any need for SQL syntax. I was just thinking that the _PG_init function could fill in a structure and then call RegisterLogicalReplicationOutputPlugin(mystruct). Hm. We can do that, but what'd be the advantage of that? The current model will correctly handle things like a'shared_preload_libraries'ed output plugin, because its _PG_init() will not register it. With the handling done in _PG_init() there would be two. Being able to use the same .so for output plugin handling and some other replication solution specific stuff is imo useful. Well, I just think relying on specific symbol names in the .so file is kind of unfortunate. It means that, for example, you can't have multiple output plugins provided by a single .so. And in general I think it's something that we've tried to minimize. I don't see why you're so pessimistic about that. I know you haven't worked it out yet, but what makes this harder than sitting down and designing something? Because every replication solution has different requirements for the format and they will want filter the output stream with regard to their own configuration. E.g. bucardo will want to include the transaction timestamp for conflict resolution and such. But there's only so much information available here. Why not just have a format that logs it all? Sure, that's no problem. Do I understand correctly that you'd like wal_decoding: Add information about a tables primary key to struct RelationData wal_decoding: Add wal_level = logical and log data required for logical decoding earlier? Yes. I'd really like to do so. I am travelling atm, but I will be back tomorrow evening and will push an updated patch this weekend. The issue I know of in the latest patches at http://www.postgresql.org/message-id/20131007133232.ga15...@awork2.anarazel.de is renaming from http://www.postgresql.org/message-id/20131008194758.gb3718...@alap2.anarazel.de I'm a bit nervous about the way the combo CID logging. I would have thought that you would emit one record per combo CID, but what you're apparently doing is emitting one record per heap tuple that uses a combo CID. I thought and implemented that in the beginning. Unfortunately it's not enough :(. That's probably the issue that took me longest to understand in this patchseries... Combocids can only fix the case where a transaction actually has create a combocid: 1) TX1: INSERT id = 1 at 0/1: (xmin = 1, xmax=Invalid, cmin = 55, cmax = Invalid) 2) TX2: DELETE id = 1 at 0/1: (xmin = 1, xmax=2, cmin = Invalid, cmax = 1) So, if we're decoding data that needs to lookup those rows in TX1 or TX2 we both times need access to cmin and cmax, but neither transaction will have created a multixact. That can only be an issue in transaction with catalog modifications. Oh, yuck. So that means you have to write an extra WAL record for EVERY heap insert, update, or delete to a catalog table? OUCH. Couldn't measure anything either, which is not surprising that I couldn't measure the overhead in the first place. I've done some parallel INSERT/DELETE pgbenching around the wal_level=logical and I couldn't measure any overhead with it disabled. With wal_level = logical, UPDATEs and DELETEs do get a bit slower, but that's to be expected. It'd probably not hurt to redo those benchmarks to make sure... Yes, I think it would be good to characterize it more precisely than a bit, so people know what to expect. -- 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] removing old ports and architectures
On Sun, Oct 13, 2013 at 5:34 AM, Tom Lane t...@sss.pgh.pa.us wrote: - PA-RISC. I think Tom was the remaining user there? Maybe just !gcc. Until pretty recently, there was a PA-RISC machine (not mine) in the buildfarm. I don't see it in the list today though. In any case, HP's compiler has always been a PITA, so no objection to requiring gcc for this platform. I object to requiring gcc on that platform; we've had recent interest in aCC-compiled builds on that platform. -- 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] removing old ports and architectures
On 2013-10-14 09:40:10 -0400, Robert Haas wrote: On Sun, Oct 13, 2013 at 5:34 AM, Tom Lane t...@sss.pgh.pa.us wrote: - PA-RISC. I think Tom was the remaining user there? Maybe just !gcc. Until pretty recently, there was a PA-RISC machine (not mine) in the buildfarm. I don't see it in the list today though. In any case, HP's compiler has always been a PITA, so no objection to requiring gcc for this platform. I object to requiring gcc on that platform; we've had recent interest in aCC-compiled builds on that platform. On PA-RISC or Itanic? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] removing old ports and architectures
On Mon, Oct 14, 2013 at 9:41 AM, Andres Freund and...@2ndquadrant.com wrote: On 2013-10-14 09:40:10 -0400, Robert Haas wrote: On Sun, Oct 13, 2013 at 5:34 AM, Tom Lane t...@sss.pgh.pa.us wrote: - PA-RISC. I think Tom was the remaining user there? Maybe just !gcc. Until pretty recently, there was a PA-RISC machine (not mine) in the buildfarm. I don't see it in the list today though. In any case, HP's compiler has always been a PITA, so no objection to requiring gcc for this platform. I object to requiring gcc on that platform; we've had recent interest in aCC-compiled builds on that platform. On PA-RISC or Itanic? Oh, sorry. I think it was Itanium. -- 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] psql tab completion for updatable foreign tables
On Fri, Sep 20, 2013 at 7:54 PM, Dean Rasheed dean.a.rash...@gmail.comwrote: On 20 September 2013 11:29, Samrat Revagade revagade.sam...@gmail.com wrote: Okay, are you adding this to the september commitfest? OK, I've done that. I think that it's too late for 9.3. +1 for idea. I have tested patch and got surprising results with Cent-OS Patch is working fine for Cent-OS 6.2 and RHEL 6.3 But is is giving problem on Cent-OS 6.3 (tab complete for local tables but not for foreign tables) I have used following script: Two postgres servers are running by using ports 5432 and 5433. Server with port 5432 has postgres_fdw installed and will interact with the remote server running under port 5433. psql -p 5433 -c CREATE TABLE aa_remote (a int, b int) postgres postgres=# CREATE EXTENSION postgres_fdw; postgres=# CREATE SERVER postgres_server FOREIGN DATA WRAPPER postgres_fdw OPTIONS (host 'localhost', port '5433', dbname 'postgres'); postgres=# CREATE USER MAPPING FOR PUBLIC SERVER postgres_server OPTIONS (password ''); postgres=# CREATE FOREIGN TABLE aa_foreign (a int, b int) SERVER postgres_server OPTIONS (table_name 'aa_remote'); postgres=# INSERT into aa_foreign values (1,2); But while doing any operation on aa_foreign tab do not complete on Cent-OS 6.3 (tab complete for local tables but not for foreign tables) Is that a problem ? Hmm. It works for me. What does pg_relation_is_updatable() return for your foreign table? Sorry .my environment has some problem. when I compiled it with fresh installation of Cent-OS 6.3 it worked. Patch : 1. Applies cleanly to git master 2. Has necessary source code comments 3. Follows coding standards 4. Solves use case.
Re: [HACKERS] removing old ports and architectures
On 2013-10-14 09:42:46 -0400, Robert Haas wrote: On Mon, Oct 14, 2013 at 9:41 AM, Andres Freund and...@2ndquadrant.com wrote: On 2013-10-14 09:40:10 -0400, Robert Haas wrote: On Sun, Oct 13, 2013 at 5:34 AM, Tom Lane t...@sss.pgh.pa.us wrote: - PA-RISC. I think Tom was the remaining user there? Maybe just !gcc. Until pretty recently, there was a PA-RISC machine (not mine) in the buildfarm. I don't see it in the list today though. In any case, HP's compiler has always been a PITA, so no objection to requiring gcc for this platform. I object to requiring gcc on that platform; we've had recent interest in aCC-compiled builds on that platform. On PA-RISC or Itanic? Oh, sorry. I think it was Itanium. Good. Not that Itanium is nice, but it certainly supports all required atomic ops. I think somebody with access to that will have to figure out what intrinsics are provided then - I haven't found reference documentation for aCC's ia64/sys/inline.h. I think Tom's stance that people who want a platform to be supported need to help is reasonable. It's really hard to develop not entirely trivial code for platforms you don't have access to and that don't even have a buildfarm member. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] logical changeset generation v6.2
On 2013-10-14 09:36:03 -0400, Robert Haas wrote: On Fri, Oct 11, 2013 at 12:57 PM, Andres Freund and...@2ndquadrant.com wrote: I don't see any need for SQL syntax. I was just thinking that the _PG_init function could fill in a structure and then call RegisterLogicalReplicationOutputPlugin(mystruct). Hm. We can do that, but what'd be the advantage of that? The current model will correctly handle things like a'shared_preload_libraries'ed output plugin, because its _PG_init() will not register it. With the handling done in _PG_init() there would be two. Being able to use the same .so for output plugin handling and some other replication solution specific stuff is imo useful. Well, I just think relying on specific symbol names in the .so file is kind of unfortunate. It means that, for example, you can't have multiple output plugins provided by a single .so. And in general I think it's something that we've tried to minimize. But that's not really different when you rely on _PG_init doing it's thing, right? I don't see why you're so pessimistic about that. I know you haven't worked it out yet, but what makes this harder than sitting down and designing something? Because every replication solution has different requirements for the format and they will want filter the output stream with regard to their own configuration. E.g. bucardo will want to include the transaction timestamp for conflict resolution and such. But there's only so much information available here. Why not just have a format that logs it all? Because we do not know what all is? Also, how would we handle replication sets and such that all of the existing replication solutions have generically? Sure, that's no problem. Do I understand correctly that you'd like wal_decoding: Add information about a tables primary key to struct RelationData wal_decoding: Add wal_level = logical and log data required for logical decoding earlier? Yes. That's done. Hope the new order makes sense. So, if we're decoding data that needs to lookup those rows in TX1 or TX2 we both times need access to cmin and cmax, but neither transaction will have created a multixact. That can only be an issue in transaction with catalog modifications. Oh, yuck. So that means you have to write an extra WAL record for EVERY heap insert, update, or delete to a catalog table? OUCH. Yes. We could integrate it into the main record without too many problems, but it didn't seem like an important optimization and it would have higher chances of slowing down wal_level logical. It'd probably not hurt to redo those benchmarks to make sure... Yes, I think it would be good to characterize it more precisely than a bit, so people know what to expect. A bit was below the 3% range for loops of adding columns. So, any tests you'd like to see? * loop around CREATE TABLE/DROP TABLE * loop around ALTER TABLE ... ADD COLUMN * loop around CREATE FUNCTION/DROP FUNCTION Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] Long paths for tablespace leads to uninterruptible hang in Windows
On Mon, Oct 14, 2013 at 2:28 PM, Robert Haas robertmh...@gmail.com wrote: On Thu, Oct 10, 2013 at 9:34 AM, Amit Kapila amit.kapil...@gmail.com wrote: On further analysis, I found that hang occurs in some of Windows API(FindFirstFile, RemoveDirectroy) when symlink path (pg_tblspc/spcoid/TABLESPACE_VERSION_DIRECTORY) is used in these API's. For above testcase, it will hang in path destroy_tablespace_directories-ReadDir-readdir-FindFirstFile Well, that sucks. So it's a Windows bug. Some of the ways to resolve the problem are described as below: 1. I found that if the link path is accessed as a full path during readdir or stat, it works fine. For example in function destroy_tablespace_directories(), the path used to access tablespace directory is of form pg_tblspc/16235/PG_9.4_201309051 by using below sprintf sprintf(linkloc_with_version_dir, pg_tblspc/%u/%s,tablespaceoid,TABLESPACE_VERSION_DIRECTORY); Now when it tries to access this path it is assumed in code that corresponding OS API will take care of considering this path w.r.t current working directory, which is right as per specs, however as it hangs in OS API (FindFirstFile) if path length 130 for symlink and if try to use full path instead of starting with pg_tblspc, it works fine. So one way to resolve this issue is to use full path for symbolic link path access instead of relying on OS to use full path. I'm not sure how we'd implement this, except by doing #2. If we believe it's a Windows bug, perhaps a good start would be to report it to Microsoft? There might be an official workaround for it, or in fact, there might already exist a fix for it.. We're *probably* going to have to end up deploying a workaround, but it would be a good idea to check first if they have a suggestion for how... -- 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] dynamic shared memory
On Mon, Oct 14, 2013 at 5:11 PM, Robert Haas robertmh...@gmail.com wrote:
On Sun, Oct 13, 2013 at 3:07 AM, Amit Kapila amit.kapil...@gmail.com wrote:
1. Do you think we should add information about pg_dynshmem file at link:
http://www.postgresql.org/docs/devel/static/storage-file-layout.html
It contains information about all files/folders in data directory
2.
+/*
+ * Forget that a temporary file is owned by a ResourceOwner
+ */
+void
+ResourceOwnerForgetDSM(ResourceOwner owner, dsm_segment *seg)
+{
Above function description should use 'dynamic shmem segment' rather
than temporary file.
Forget that a dynamic shmem segment is owned by a ResourceOwner
Good catches, will fix.
During test, I found one issue in Windows implementation.
During startup, when it tries to create new control segment for
dynamic shared memory, it loops until an unused identifier is found,
but for Windows implementation (dsm_impl_windows()), it was returning
error for EEXIST. This error will convert into FATAL as it is during
postmaster startup and will not allow server to start.
Please find attached patch to fix the problem.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
fix_error_handling_eexist_windows.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] Long paths for tablespace leads to uninterruptible hang in Windows
On Mon, Oct 14, 2013 at 8:40 PM, Magnus Hagander mag...@hagander.net wrote: On Mon, Oct 14, 2013 at 2:28 PM, Robert Haas robertmh...@gmail.com wrote: On Thu, Oct 10, 2013 at 9:34 AM, Amit Kapila amit.kapil...@gmail.com wrote: On further analysis, I found that hang occurs in some of Windows API(FindFirstFile, RemoveDirectroy) when symlink path (pg_tblspc/spcoid/TABLESPACE_VERSION_DIRECTORY) is used in these API's. For above testcase, it will hang in path destroy_tablespace_directories-ReadDir-readdir-FindFirstFile Well, that sucks. So it's a Windows bug. Some of the ways to resolve the problem are described as below: 1. I found that if the link path is accessed as a full path during readdir or stat, it works fine. For example in function destroy_tablespace_directories(), the path used to access tablespace directory is of form pg_tblspc/16235/PG_9.4_201309051 by using below sprintf sprintf(linkloc_with_version_dir, pg_tblspc/%u/%s,tablespaceoid,TABLESPACE_VERSION_DIRECTORY); Now when it tries to access this path it is assumed in code that corresponding OS API will take care of considering this path w.r.t current working directory, which is right as per specs, however as it hangs in OS API (FindFirstFile) if path length 130 for symlink and if try to use full path instead of starting with pg_tblspc, it works fine. So one way to resolve this issue is to use full path for symbolic link path access instead of relying on OS to use full path. I'm not sure how we'd implement this, except by doing #2. If we believe it's a Windows bug, perhaps a good start would be to report it to Microsoft? I had tried it on Windows forums, but didn't got any answer from them till now. The links where I posted this are as below: http://answers.microsoft.com/en-us/windows/forum/windows_7-performance/stat-hangs-on-windows-7-when-used-for-symbolic/f7c4573e-be28-4bbf-ac9f-de990a3f5564 http://social.technet.microsoft.com/Forums/windows/en-US/73af1516-baaf-4d3d-914c-9b22c465e527/stat-hangs-on-windows-7-when-used-for-symbolic-link?forum=TechnetSandboxForum There might be an official workaround for it, or in fact, there might already exist a fix for it.. The only workaround I could find is to use absolute path, and one of the ways to fix it is that in functions like pgwin32_safestat(), call make_absolute_path() before using path. The other way to fix is whereever in code we use path as pg_tblspc/, change it to absolute path, but it is used at quite a few places and trying to change there might make code dirty. We're *probably* going to have to end up deploying a workaround, but it would be a good idea to check first if they have a suggestion for how... With Regards, Amit Kapila. 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] Planner issue
On Mon, Oct 14, 2013 at 10:55 AM, Tom Lane t...@sss.pgh.pa.us wrote: Soroosh Sardari soroosh.sard...@gmail.com writes: I developed a new character string type, named myvarchar. Also an operator class for btree is added. PROBLEM: When I executed a query with where clause on 'mine' column, PG does not use index. Most likely you got the opclass definition wrong. Since you've shown us no details of what you did, it's hard to speculate about just how. But note that varchar itself is a pretty bad model for a user-added datatype, because it has a special symbiotic relationship with type text (to wit, it has no operators of its own but uses text's operators via implicit casts). To get to a working independent datatype like this, you'd need to pick the right aspects of each of text and varchar to clone. So my unfounded speculation is you didn't do that just right. regards, tom lane As Tom said, I did something wrong when I was creating new operators. The RESTRICT parameter is forgotten. Since all varchar operations redirected to text operators, hence my operators must be like operators of type text. I used following command to find text operator: select * from pg_operator where oprleft = 25 and oprright = 25 P.S : 25 is oid of text type. Cheers, Soroosh Sardari
Re: [HACKERS] dynamic shared memory
On Mon, Oct 14, 2013 at 11:11 AM, Amit Kapila amit.kapil...@gmail.com wrote: During test, I found one issue in Windows implementation. During startup, when it tries to create new control segment for dynamic shared memory, it loops until an unused identifier is found, but for Windows implementation (dsm_impl_windows()), it was returning error for EEXIST. This error will convert into FATAL as it is during postmaster startup and will not allow server to start. Please find attached patch to fix the problem. Committed, thanks. -- 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] Patch for reserved connections for replication users
On 10/13/2013 01:38 AM, Gibheer wrote: So it will ensure that max_wal_senders is used for reserving connection slots from being used by non-super user connections. I find new usage of max_wal_senders acceptable, if anyone else thinks otherwise, please let us know. I think otherwise. Changing max_wal_senders requires a restart. As such, we currently advise users to set the setting generously: as many replication connections as you think you'll ever need, plus two. If max_wal_senders is a reservation which could cause the user to run out of other connections sooner than expected, then the user is faced with a new hard to set parameter: they don't want to set it too high *or* too low. This would result in a lot of user frustration as they try to get thier connection configuration right and have to restart the server multiple times. I find few new features worth making it *harder* to configure PostgreSQL, and reserved replication connections certainly don't qualify. If it's worth having reserved replication connections (and I can see some reasons to want it), then we need a new GUC for this: reserved_walsender_connections -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Long paths for tablespace leads to uninterruptible hang in Windows
Robert Haas robertmh...@gmail.com writes: On Thu, Oct 10, 2013 at 9:34 AM, Amit Kapila amit.kapil...@gmail.com wrote: On further analysis, I found that hang occurs in some of Windows API(FindFirstFile, RemoveDirectroy) when symlink path (pg_tblspc/spcoid/TABLESPACE_VERSION_DIRECTORY) is used in these API's. For above testcase, it will hang in path destroy_tablespace_directories-ReadDir-readdir-FindFirstFile Well, that sucks. So it's a Windows bug. It's not clear to me that we should do anything about this at all, except perhaps document that people should avoid long tablespace path names on an unknown set of Windows versions. We should not be in the business of working around any and every bug coming out of Redmond. 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] buildfarm failures on smew and anole
Robert Haas robertmh...@gmail.com writes: Anyway, as Andres said, the machines were working fine until recently, so I think we just need to get them un-broken. I think you're talking past each other. What would be useful here is to find out *why* these machines are now failing, when they didn't before. There might or might not be anything useful to be done about it, but if we don't have that information, we can't tell. 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] [SQL] Comparison semantics of CHAR data type
On Fri, Oct 11, 2013 at 3:44 PM, Bruce Momjian br...@momjian.us wrote: You have some good questions here, though there are two interrelated things going on here. First is collation, and the second is the trimming of spaces from char() comparisons. Sorry, I should have probably mentioned more explicitly that I'm only concerned about the latter. I might get yelled at for saying this, but I think the rather ubiquitous right-trimming of bpchar values is wrong, and does - in combination with memcmp() for comparison - lead to some non-standard behavior. Now, on to the trailing space issue using the default TEXT value for strings, first in UTF8: test= SHOW lc_collate; lc_collate - en_US.UTF-8 (1 row) test= select E'ab\n' E'ab '; ?column? -- f (1 row) then in C: test2= SHOW lc_collate; lc_collate C (1 row) test2= select E'ab\n' E'ab '; ?column? -- t (1 row) This matches the \n/space issue we saw above. Now, here is where CHAR() starts to show the unusual behavior you saw, first in UTF8: test= SHOW lc_collate; lc_collate - en_US.UTF-8 (1 row) test= select E'ab\n'::CHAR(10) E'ab '::CHAR(10); ?column? -- f (1 row) then in C: test2= SHOW lc_collate; lc_collate C (1 row) test2= select E'ab\n'::CHAR(10) E'ab '::CHAR(10); ?column? -- f (1 row) You can see the UTF8 case is fine because \n is considered greater than space, but in the C locale, where \n is less than space, the false return value shows the problem with internal_bpchar_pattern_compare() trimming the string and first comparing on lengths. This is exactly the problem you outline, where space trimming assumes everything is less than a space. For collations other than C some of those issues that have to do with string comparisons might simply be hidden, depending on how strcoll() handles inputs off different lengths: If strcoll() applies implicit space padding to the shorter value, there won't be any visible difference in ordering between bpchar and varchar values. If strcoll() does not apply such space padding, the right-trimming of bpchar values causes very similar issues even in a en_US collation. For example, this seems to be the case on OS X: select 'ab '::char(10) collate en_US E'ab\n'::char(10) collate en_US; ?column? -- t (1 row) select 'ab '::char(10) collate C E'ab\n'::char(10) collate C; ?column? -- t (1 row) select 'ab '::varchar(10) collate en_US E'ab\n'::varchar(10) collate en_US; ?column? -- f (1 row) select 'ab '::varchar(10) collate C E'ab\n'::varchar(10) collate C; ?column? -- f (1 row) So here there's actually not only the same \n/space issue as in the C collation (which would go away if the bpchar value weren't trimmed). It also shows that there might be slight differences in behavior, depending which platform you're running on. On Fri, Oct 11, 2013 at 4:58 PM, Kevin Grittner kgri...@ymail.com wrote: What matters in general isn't where the characters fall when comparing individual bytes, but how the strings containing them sort according to the applicable collation. That said, my recollection of the spec is that when two CHAR(n) values are compared, the shorter should be blank-padded before making the comparison. Not necessarily. The SQL Standard actually ties this to the collation sequence that is in use. Without a lot of context, this is from Subclause 8.2, comparison predicate, General Rule 3)b): b) If the length in characters of X is not equal to the length in characters of Y, then the shorter string is effectively replaced, for the purposes of comparison, with a copy of itself that has been extended to the length of the longer string by concatenation on the right of one or more pad characters, where the pad character is chosen based on CS. If CS has the NO PAD characteristic, then the pad character is an implementation-dependent character different from any character in the character set of X and Y that collates less than any string under CS. Otherwise, the pad character is a space. In my opinion, that's just a lot of handwaving, to the extent that in practice different vendors interpret this clause differently. It seems that SQLServer and DB2 do PAD semantics across the board, whereas Oracle has uses NO PAD semantics whenever there's a VARCHAR type involved in the comparison. But
Re: [HACKERS] Patch for reserved connections for replication users
On 2013-10-14 10:26:25 -0700, Josh Berkus wrote: On 10/13/2013 01:38 AM, Gibheer wrote: So it will ensure that max_wal_senders is used for reserving connection slots from being used by non-super user connections. I find new usage of max_wal_senders acceptable, if anyone else thinks otherwise, please let us know. I think otherwise. Changing max_wal_senders requires a restart. As such, we currently advise users to set the setting generously: as many replication connections as you think you'll ever need, plus two. If max_wal_senders is a reservation which could cause the user to run out of other connections sooner than expected, then the user is faced with a new hard to set parameter: they don't want to set it too high *or* too low. This would result in a lot of user frustration as they try to get thier connection configuration right and have to restart the server multiple times. I find few new features worth making it *harder* to configure PostgreSQL, and reserved replication connections certainly don't qualify. If it's worth having reserved replication connections (and I can see some reasons to want it), then we need a new GUC for this: reserved_walsender_connections Imo the complications around this prove my (way earlier) point that it'd be much better to treat replication connections as something entirely different to normal SQL connections. There's really not much overlap here and while there's some philosophical point to be made about it all being connections, from a practical POV treating them separately seems better. If we were designing on a green field I'd just rename max_connections to something explicitly talking about client database connections, but renaming it seems like it'd cause unnecessary pain. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] Patch for reserved connections for replication users
On 10/14/2013 10:51 AM, Andres Freund wrote: Imo the complications around this prove my (way earlier) point that it'd be much better to treat replication connections as something entirely different to normal SQL connections. There's really not much overlap here and while there's some philosophical point to be made about it all being connections, from a practical POV treating them separately seems better. Given that replication connections don't even appear in pg_stat_activity now, I'd agree with you. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] buildfarm failures on smew and anole
On Mon, Oct 14, 2013 at 1:33 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Anyway, as Andres said, the machines were working fine until recently, so I think we just need to get them un-broken. I think you're talking past each other. What would be useful here is to find out *why* these machines are now failing, when they didn't before. There might or might not be anything useful to be done about it, but if we don't have that information, we can't tell. Well, my OP had a working theory which I think fits the facts, and some suggested troubleshooting steps. How about that for a start? The real problem here is that neither of the buildfarm owners has responded to this thread. -- 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
[HACKERS] CF 2013-09 Wrap Up
CF 2013-09 will be wrapping up this week. As a reminder, beginning on the official CF end date (11/15), patches Waiting for Author will be Returned with Feedback. Authors are welcome to add their patch to the next CF (2013-11). Any patches marked Needs Review will be automatically moved to the next CF. We will try to make sure that all patches in the current CF have received at least one review. If you have any questions or comments about the CF process, feel free to send a note to me or David Fetter (CFM). ^_^ Thanks to all who have submitted or reviewed patches this time around! Mike Blackwell
Re: [HACKERS] buildfarm failures on smew and anole
On Fri, 2013-10-11 at 15:33 -0400, Robert Haas wrote: Can the owners of these buildfarm machines please check whether there are extra semaphores allocated and if so free them? Or at least reboot, to see if that unbreaks the build? I cleaned the semaphores on smew, but they came back. Whatever is crashing is leaving the semaphores lying around. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add use of asprintf()
On Mon, 2013-10-14 at 23:08 +1300, David Rowley wrote: Looks like something like: #ifndef WIN32 #define HAVE_VA_COPY 1 #endif would need to be added to asprintf.c, but also some work needs to be done with mcxt.c as it uses va_copy unconditionally. Perhaps just defining a macro for va_copy would be better for windows. I was not quite sure the best header file for such a macro so I did not write a patch to fix it. Does Windows not have va_copy? What do they use instead? -- Sent 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-tuning work_mem and maintenance_work_mem
On 10/14/13 8:18 AM, Robert Haas wrote: On Sat, Oct 12, 2013 at 3:07 AM, Magnus Hagander mag...@hagander.net wrote: On Oct 11, 2013 10:23 PM, Josh Berkus j...@agliodbs.com wrote: On 10/11/2013 01:11 PM, Bruce Momjian wrote: In summary, I think we need to: * decide on new defaults for work_mem and maintenance_work_mem * add an initdb flag to allow users/packagers to set shared_bufffers? * add an autovacuum_work_mem setting? * change the default for temp_buffers? If we're changing defaults, bgwriter_lru_maxpages and vacuum_cost_limit could also use a bump; those thresholds were set for servers with 1GB of RAM Uh, those are there to limit io and not memory, right? More memory isn't the reason to increase them, more io is. For people deploying on modern server hardware then yes it's often low, but for all those deploying in virtualized environments with io performance reminding you of the 1990ies, I'm not so sure it is... bgwriter_lru_maxpages is clearly related to the size of shared_buffers, although confusingly it is expressed as a number of buffers, while shared_buffers is expressed as a quantity of memory. I think we might have done better to call the GUC bgwriter_lru_maxpercent and make it a percentage of shared buffers. Also, more memory generally means more filesystem cache which means you can do more vacuum work per round. FWIW, on our 512G servers... cnuapp_p...@postgres11.obr=# select name, setting from pg_settings where name ~ 'vacuum_cost'; name | setting --+- autovacuum_vacuum_cost_delay | 10 autovacuum_vacuum_cost_limit | -1 vacuum_cost_delay| 10 vacuum_cost_limit| 2000 vacuum_cost_page_dirty | 10 vacuum_cost_page_hit | 1 vacuum_cost_page_miss| 10 (7 rows) The page_hit cost is intentionally the same as the page_dirty limit because writes to the SAN are generally far cheaper than reads that actually hit spindles. Of course with the amount of FS cache we have (512G-8G shared buffers at most) reads are often very likely to hit the FS cache, but tuning of these settings while watching IO stats has shown these settings to be minimally disruptive. -- Jim C. Nasby, Data Architect j...@nasby.net 512.569.9461 (cell) http://jim.nasby.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] logical changeset generation v6.2
On 2013-10-14 15:51:14 +0200, Andres Freund wrote: It'd probably not hurt to redo those benchmarks to make sure... Yes, I think it would be good to characterize it more precisely than a bit, so people know what to expect. A bit was below the 3% range for loops of adding columns. So, any tests you'd like to see? * loop around CREATE TABLE/DROP TABLE * loop around ALTER TABLE ... ADD COLUMN * loop around CREATE FUNCTION/DROP FUNCTION So, see the attatched benchmark skript. I've always done using a disk bound and a memory bound (using eatmydata, preventing fsyncs) run. * unpatched run, wal_level = hot_standby, eatmydata * unpatched run, wal_level = hot_standby * patched run, wal_level = hot_standby, eatmydata * patched run, wal_level = hot_standby * patched run, wal_level = logical, eatmydata * patched run, wal_level = logical Based on those results, there's no difference above noise for wal_level=hot_standby, with or without the patch. With wal_level=logical there's a measurable increase in wal traffic (~12-17%), but no performance decrease above noise. From my POV that's ok, those are really crazy catalog workloads. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services $ eatmydata /tmp/bench.sh createtable single tps = 320.293146 (including connections establishing) tps = 320.334981 (excluding connections establishing) 262152 /tmp/perf/pg_xlog createtable parallel tps = 1516.095491 (including connections establishing) tps = 1516.488664 (excluding connections establishing) 1163272 /tmp/perf/pg_xlog altertable single tps = 139.474141 (including connections establishing) tps = 139.491721 (excluding connections establishing) 163848 /tmp/perf/pg_xlog altertable parallel tps = 774.868029 (including connections establishing) tps = 775.136842 (excluding connections establishing) 851976 /tmp/perf/pg_xlog createfunction single tps = 2938.380801 (including connections establishing) tps = 2938.711692 (excluding connections establishing) 81928 /tmp/perf/pg_xlog createfunction parallel tps = 20603.023567 (including connections establishing) tps = 20608.799987 (excluding connections establishing) 458760 /tmp/perf/pg_xlog $ /tmp/bench.sh createtable single tps = 51.014096 (including connections establishing) tps = 51.020329 (excluding connections establishing) 49160 /tmp/perf/pg_xlog createtable parallel tps = 171.012045 (including connections establishing) tps = 171.054406 (excluding connections establishing) 147464 /tmp/perf/pg_xlog altertable single tps = 9.138758 (including connections establishing) tps = 9.139863 (excluding connections establishing) 32776 /tmp/perf/pg_xlog altertable parallel tps = 45.109269 (including connections establishing) tps = 45.122066 (excluding connections establishing) 65544 /tmp/perf/pg_xlog createfunction single tps = 131.192719 (including connections establishing) tps = 131.209112 (excluding connections establishing) 16392 /tmp/perf/pg_xlog createfunction parallel tps = 624.830173 (including connections establishing) tps = 625.017525 (excluding connections establishing) 32776 /tmp/perf/pg_xlog -- patch applied -- $ eatmydata /tmp/bench.sh createtable single tps = 329.063474 (including connections establishing) tps = 329.104147 (excluding connections establishing) 262152 /tmp/perf/pg_xlog createtable parallel tps = 1462.524932 (including connections establishing) tps = 1462.872552 (excluding connections establishing) 1130504 /tmp/perf/pg_xlog altertable single tps = 141.091905 (including connections establishing) tps = 141.108352 (excluding connections establishing) 163848 /tmp/perf/pg_xlog altertable parallel tps = 812.810544 (including connections establishing) tps = 813.026734 (excluding connections establishing) 901128 /tmp/perf/pg_xlog createfunction single tps = 3384.068190 (including connections establishing) tps = 3384.460953 (excluding connections establishing) 81928 /tmp/perf/pg_xlog createfunction parallel tps = 20744.363972 (including connections establishing) tps = 20750.135978 (excluding connections establishing) 458760 /tmp/perf/pg_xlog $ /tmp/bench.sh createtable single tps = 42.522505 (including connections establishing) tps = 42.527233 (excluding connections establishing) 49160 /tmp/perf/pg_xlog createtable parallel tps = 148.753762 (including connections establishing) tps = 148.794361 (excluding connections establishing) 131080 /tmp/perf/pg_xlog altertable single tps = 9.524616 (including connections establishing) tps = 9.525757 (excluding connections establishing) 32776 /tmp/perf/pg_xlog altertable parallel tps = 49.209278 (including connections establishing) tps = 49.223312 (excluding connections establishing) 65544 /tmp/perf/pg_xlog createfunction single tps = 132.325526 (including connections establishing) tps = 132.340677 (excluding connections establishing) 16392 /tmp/perf/pg_xlog createfunction parallel
Re: [HACKERS] Triggers on foreign tables
2013/10/13 Ronan Dunklau rdunk...@gmail.com: Le samedi 12 octobre 2013 07:30:35 Kohei KaiGai a écrit : 2013/10/10 Ronan Dunklau rdunk...@gmail.com: Sorry, I'm uncertain the point above. Are you saying FDW driver may be able to handle well a case when a remote tuple to be updated is different from a remote tuple being fetched on the first stage? Or, am I misunderstanding? I was not clear in the point above: what I meant was that, from my understanding, the FDW driver has no obligation to use the system-attribute tupleid to identify the remote tuple. IIRC, one of the early API proposal involved had the tupleid passed as an argument to the ExecForeign* hooks. Now, these hooks receive the whole planslot instead. This slot can store additional resjunks attributes (thanks to the AddForeignUpdateTargets) which shouldn't be altered during the execution and are carried on until modify stage. So, these additional attributes can be used as identifiers instead of the tupleid. In fact, this is the approach I implemented for the multicorn fdw, and I believe it to be used in other FDWs as well (HadoopFDW does that, if I understand it correctly). So, it doesn't make sense to me to assume that the tupleid will be filled as valid remote identifier in the context of triggers. Sorry, I might call it something like primary key, instead of 'tupleid'. Apart from whether we can uniquely identify a particular remote record with an attribute, what FDW needs here is something to identify a remote record. So, we were talking about same concept with different names. Does the incomplete tuple mean a tuple image but some of columns are replaced with NULL due to optimization, as postgres_fdw is doing, doesn't it? If so, a solution is to enforce FDW driver to fetch all the columns if its managing foreign table has row level triggers for update / delete. What I meant is that a DELETE statement, for example, does not build a scan- stage reltargetlist consisting of the whole set of attributes: the only attributes that are fetched are the ones built by addForeignUpdateTargets, and whatever additional attributes are involved in the DELETE statement (in the WHERE clause, for example). DELETE statement indeed does not (need to) construct a complete tuple image on scan stage, however, it does not prohibit FDW driver to keep the latest record being fetched from remote server. FDW driver easily knows whether relation has row-level trigger preliminary, so here is no matter to keep a complete image internally if needed. Or, it is perhaps available to add additional target-entries that is needed to construct a complete tuple using AddForeignUpdateTargets() callback. As I noted above, 2nd round trip is not a suitable design to support after-row trigger. Likely, GetTupleForTrigger() hook shall perform to return a cached older image being fetched on the first round of remote scan. So, I guess every FDW driver will have similar implementation to handle these the situation, thus it makes sense that PostgreSQL core has a feature to support this handling; that keeps a tuple being fetched last and returns older image for row-level triggers. How about your opinion? I like this idea, but I don't really see all the implications of such a design. Where would this tuple be stored ? From what I understand, after-triggers are queued for later execution, using the old/new tupleid for later retrieval (in the AfterTriggerEventData struct). This would need a major change to work with foreign tables. Would that involve a special path for executing those triggers ASAP ? Ops, I oversight here. AfterTriggerSaveEvent() indeed saves only ctid of tuples in regular tables, because it can re-construct a complete tuple image from a particular ctid any time. On the other hand, it is not available on foreign table due to its nature. All we can do seems to me, probably, to save copy of before/after tuple image on local memory, even though it consumes much memory than regular tables. The feature we need here might become a bit clear little by little. A complete tuple image shall be fetched on the scan stage, if foreign table has row-level trigger. The planSlot may make sense if it contains (junk) attributes that is sufficient to re-construct an old tuple image. Target-list of DELETE statement contains a junk ctid attribute. Here is no reason why we cannot add junk attributes that reference each regular attribute, isn't it? Also, target-list of UPDATE statement contains new tuple image, however, it is available to add junk attributes that just reference old value, as a carrier of old values from scan stage to modify stage. I want core PostgreSQL to support a part of these jobs. For example, it may be able to add junk attribute to re-construct old tuple image on rewriteTargetListUD(), if target relation has row-level triggers. Also, it may be able to extract these old values from planSlot to construct an old tuple image on
Re: [HACKERS] Re: [BUGS] BUG #7873: pg_restore --clean tries to drop tables that don't exist
On Thu, Oct 10, 2013 at 12:54 PM, Andrew Dunstan and...@dunslane.net wrote: This thread seems to have gone cold, but I'm inclined to agree with Pavel. If the table doesn't exist, neither does the trigger, and the whole point of the 'IF EXISTS' variants is to provide the ability to issue DROP commands that don't fail if their target is missing. +1 from me as well. Also, Pavel, this patch is still listed as 'Needs Review' in the CF app, but I haven't seen a response to the concerns in my last message. Josh -- Sent 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: [BUGS] BUG #7873: pg_restore --clean tries to drop tables that don't exist
On 2013-10-10 12:54:23 -0400, Andrew Dunstan wrote: On 09/19/2013 06:12 PM, Pavel Stehule wrote: 2013/9/16 Satoshi Nagayasu sn...@uptime.jp mailto:sn...@uptime.jp I'm looking at this patch, and I have a question here. Should DROP TRIGGER IF EXISTS ignore error for non-existing trigger and non-existing table? Or just only for non-existing trigger? My opinion is so, both variants should be ignored - it should be fully fault tolerant in this use case. This thread seems to have gone cold, but I'm inclined to agree with Pavel. If the table doesn't exist, neither does the trigger, and the whole point of the 'IF EXISTS' variants is to provide the ability to issue DROP commands that don't fail if their target is missing. -1, this seems to likely to just hide typos. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] Re: [BUGS] BUG #7873: pg_restore --clean tries to drop tables that don't exist
On 10/14/2013 05:44 PM, Andres Freund wrote: On 2013-10-10 12:54:23 -0400, Andrew Dunstan wrote: On 09/19/2013 06:12 PM, Pavel Stehule wrote: 2013/9/16 Satoshi Nagayasu sn...@uptime.jp mailto:sn...@uptime.jp I'm looking at this patch, and I have a question here. Should DROP TRIGGER IF EXISTS ignore error for non-existing trigger and non-existing table? Or just only for non-existing trigger? My opinion is so, both variants should be ignored - it should be fully fault tolerant in this use case. This thread seems to have gone cold, but I'm inclined to agree with Pavel. If the table doesn't exist, neither does the trigger, and the whole point of the 'IF EXISTS' variants is to provide the ability to issue DROP commands that don't fail if their target is missing. -1, this seems to likely to just hide typos. So if I say DROP TRIGGER IF EXISTS foo ON bar; you're ok with succeeding if foo is a typo, but not if bar is? 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] Re: [BUGS] BUG #7873: pg_restore --clean tries to drop tables that don't exist
On 2013-10-14 17:59:25 -0400, Andrew Dunstan wrote: On 10/14/2013 05:44 PM, Andres Freund wrote: On 2013-10-10 12:54:23 -0400, Andrew Dunstan wrote: On 09/19/2013 06:12 PM, Pavel Stehule wrote: 2013/9/16 Satoshi Nagayasu sn...@uptime.jp mailto:sn...@uptime.jp I'm looking at this patch, and I have a question here. Should DROP TRIGGER IF EXISTS ignore error for non-existing trigger and non-existing table? Or just only for non-existing trigger? My opinion is so, both variants should be ignored - it should be fully fault tolerant in this use case. This thread seems to have gone cold, but I'm inclined to agree with Pavel. If the table doesn't exist, neither does the trigger, and the whole point of the 'IF EXISTS' variants is to provide the ability to issue DROP commands that don't fail if their target is missing. -1, this seems to likely to just hide typos. So if I say DROP TRIGGER IF EXISTS foo ON bar; you're ok with succeeding if foo is a typo, but not if bar is? Yes. Normally you won't do DROP TRIGGER if you don't need the table in the first place, in that case you can just DROP TABLE. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] Improving avg performance for numeric
On 24.9.2013 17:57, Pavel Stehule wrote: 2013/9/24 Robert Haas robertmh...@gmail.com mailto:robertmh...@gmail.com On Mon, Sep 23, 2013 at 4:15 PM, Tomas Vondra t...@fuzzy.cz mailto:t...@fuzzy.cz wrote: Seems ready for commiter to me. I'll wait a few days for others to comment, and then I'll update the commitfest page. Some thoughts: The documentation doesn't reflect the restriction to type internal. On a related note, why restrict this to type internal? Now, for almost all types Postgres well estimate size of state value. Only arrays with unknown size can be a different. When we enable this value for all types, then users can specify some bad values for scalar buildin types. Next argument is simply and bad - I don't see a good use case for customization this value for other than types than internal type. I have no strong position here - prefer joining with internal type due little bit higher robustness. I share this oppinion. I was not able to come up with a single use case benefiting from allowing this for types other than internal. Seems like a footgun to me, with no potential benefit. So +1 to keeping the patch 'type internal only' from me. With the formatting issues now fixed, I believe the patch is ready for committer (and someone already switched it to that state). Tomas -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] buildfarm failures on smew and anole
On Mon, Oct 14, 2013 at 4:29 PM, Peter Eisentraut pete...@gmx.net wrote: On Fri, 2013-10-11 at 15:33 -0400, Robert Haas wrote: Can the owners of these buildfarm machines please check whether there are extra semaphores allocated and if so free them? Or at least reboot, to see if that unbreaks the build? I cleaned the semaphores on smew, but they came back. Whatever is crashing is leaving the semaphores lying around. Ugh. When did you do that exactly? I thought I fixed the problem that was causing that days ago, and the last 4 days worth of runs all show the too many clients error. -- 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] Re: [BUGS] BUG #7873: pg_restore --clean tries to drop tables that don't exist
Hi, On 14.10.2013 23:44, Andres Freund wrote: On 2013-10-10 12:54:23 -0400, Andrew Dunstan wrote: On 09/19/2013 06:12 PM, Pavel Stehule wrote: 2013/9/16 Satoshi Nagayasu sn...@uptime.jp mailto:sn...@uptime.jp I'm looking at this patch, and I have a question here. Should DROP TRIGGER IF EXISTS ignore error for non-existing trigger and non-existing table? Or just only for non-existing trigger? My opinion is so, both variants should be ignored - it should be fully fault tolerant in this use case. This thread seems to have gone cold, but I'm inclined to agree with Pavel. If the table doesn't exist, neither does the trigger, and the whole point of the 'IF EXISTS' variants is to provide the ability to issue DROP commands that don't fail if their target is missing. -1, this seems to likely to just hide typos. Not sure I agree with your reasoning. Isn't that equally true for 'IF EXISTS' clause with all commands in general? Why should we use likely to hide typos argument in this case and not the others? Or do you object only to extending DROP TRIGGER IF EXISTS to if table and trigger exists? It seems natural to me that no table = no trigger so I'm fine with this interpretation, just like Andrew. The purpose of this patch was to add support for quiet pg_restore --clean and pg_restore should not do typos (if it does, we're in much deeper troubles I guess). If you're concerned about users doing typos, they may as well do typos in the trigger name with exactly the same result (trigger not dropped without any kind of error message). I see no reason to support DROP TRIGGER IF EXISTS but restrict the IF EXISTS clause only to the trigger on the grounds of typos. So +1 from me, both to the patch and graceful handling of missing table. kind regards Tomas -- Sent 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: [BUGS] BUG #7873: pg_restore --clean tries to drop tables that don't exist
On 2013-10-15 00:23:15 +0200, Tomas Vondra wrote: Hi, On 14.10.2013 23:44, Andres Freund wrote: On 2013-10-10 12:54:23 -0400, Andrew Dunstan wrote: On 09/19/2013 06:12 PM, Pavel Stehule wrote: 2013/9/16 Satoshi Nagayasu sn...@uptime.jp mailto:sn...@uptime.jp I'm looking at this patch, and I have a question here. Should DROP TRIGGER IF EXISTS ignore error for non-existing trigger and non-existing table? Or just only for non-existing trigger? My opinion is so, both variants should be ignored - it should be fully fault tolerant in this use case. This thread seems to have gone cold, but I'm inclined to agree with Pavel. If the table doesn't exist, neither does the trigger, and the whole point of the 'IF EXISTS' variants is to provide the ability to issue DROP commands that don't fail if their target is missing. -1, this seems to likely to just hide typos. Not sure I agree with your reasoning. Isn't that equally true for 'IF EXISTS' clause with all commands in general? Why should we use likely to hide typos argument in this case and not the others? Because there simply is no reason to issue a DROP TRIGGER IF EXISTS if you don't need the contents of the table. In that case you can just issue a DROP TABLE IF EXISTS and start anew. The purpose of this patch was to add support for quiet pg_restore --clean and pg_restore should not do typos (if it does, we're in much deeper troubles I guess). Why does that even have to do anything for triggers? Emitting DROP TABLE should be enough. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] Compression of full-page-writes
(2013/10/13 0:14), Amit Kapila wrote: On Fri, Oct 11, 2013 at 10:36 PM, Andres Freund and...@2ndquadrant.com wrote: But maybe pglz is just not a good fit for this, it really isn't a very good algorithm in this day and aage. +1. This compression algorithm is needed more faster than pglz which is like general compression algorithm, to avoid the CPU bottle-neck. I think pglz doesn't have good performance, and it is like fossil compression algorithm. So we need to change latest compression algorithm for more better future. Do you think that if WAL reduction or performance with other compression algorithm (for ex. snappy) is better, then chances of getting the new compression algorithm in postresql will be more? Latest compression algorithms papers(also snappy) have indecated. I think it is enough to select algorithm. It may be also good work in postgres. Regards, -- Mitsumasa KONDO 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] Patch for reserved connections for replication users
On Mon, Oct 14, 2013 at 10:56 PM, Josh Berkus j...@agliodbs.com wrote: On 10/13/2013 01:38 AM, Gibheer wrote: So it will ensure that max_wal_senders is used for reserving connection slots from being used by non-super user connections. I find new usage of max_wal_senders acceptable, if anyone else thinks otherwise, please let us know. I think otherwise. Changing max_wal_senders requires a restart. As such, we currently advise users to set the setting generously: as many replication connections as you think you'll ever need, plus two. If max_wal_senders is a reservation which could cause the user to run out of other connections sooner than expected, then the user is faced with a new hard to set parameter: they don't want to set it too high *or* too low. It is documented in the patch that configuration max_wal_senders is reserved from max_connections. So it will not cause the user to run out of other connections sooner than expected, if both the variables are configured properly. This would result in a lot of user frustration as they try to get thier connection configuration right and have to restart the server multiple times. I find few new features worth making it *harder* to configure PostgreSQL, and reserved replication connections certainly don't qualify. If it's worth having reserved replication connections (and I can see some reasons to want it), then we need a new GUC for this: reserved_walsender_connections Having new variable also might make users life difficult, as with new variable he needs to take care of setting 3 parameters (max_wal_senders, reserved_walsender_connections, max_connections) appropriately for this purpose and then choosing value for reserved_walsender_connections will be another thing for which he has to consult. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On Mon, Oct 14, 2013 at 11:21 PM, Andres Freund and...@2ndquadrant.com wrote: On 2013-10-14 10:26:25 -0700, Josh Berkus wrote: On 10/13/2013 01:38 AM, Gibheer wrote: So it will ensure that max_wal_senders is used for reserving connection slots from being used by non-super user connections. I find new usage of max_wal_senders acceptable, if anyone else thinks otherwise, please let us know. I think otherwise. Changing max_wal_senders requires a restart. As such, we currently advise users to set the setting generously: as many replication connections as you think you'll ever need, plus two. If max_wal_senders is a reservation which could cause the user to run out of other connections sooner than expected, then the user is faced with a new hard to set parameter: they don't want to set it too high *or* too low. This would result in a lot of user frustration as they try to get thier connection configuration right and have to restart the server multiple times. I find few new features worth making it *harder* to configure PostgreSQL, and reserved replication connections certainly don't qualify. If it's worth having reserved replication connections (and I can see some reasons to want it), then we need a new GUC for this: reserved_walsender_connections Imo the complications around this prove my (way earlier) point that it'd be much better to treat replication connections as something entirely different to normal SQL connections. There's really not much overlap here and while there's some philosophical point to be made about it all being connections, from a practical POV treating them separately seems better. If we were designing on a green field I'd just rename max_connections to something explicitly talking about client database connections, but renaming it seems like it'd cause unnecessary pain. If we think this way, then may be we should have max_user_connections instead of max_connections and then max_wal_connections. But still there are other's like pg_basebackup who needs connections and tomorrow there can be new such entities which need connection. Also we might need to have different infrastructure in code to make these options available to users. I think having different parameters to configure maximum connections for different entities can complicate both code as well as user's job. With Regards, Amit Kapila. 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] psql tab completion for updatable foreign tables
On Mon, Oct 14, 2013 at 7:20 PM, Samrat Revagade revagade.sam...@gmail.comwrote: Sorry .my environment has some problem. May be you were using old version of psql ? IIRC tab-completion relies heavily on the psql side, Thanks, Pavan -- Pavan Deolasee http://www.linkedin.com/in/pavandeolasee
Re: [HACKERS] Compression of full-page-writes
On Tue, Oct 15, 2013 at 6:30 AM, KONDO Mitsumasa kondo.mitsum...@lab.ntt.co.jp wrote: (2013/10/13 0:14), Amit Kapila wrote: On Fri, Oct 11, 2013 at 10:36 PM, Andres Freund and...@2ndquadrant.com wrote: But maybe pglz is just not a good fit for this, it really isn't a very good algorithm in this day and aage. +1. This compression algorithm is needed more faster than pglz which is like general compression algorithm, to avoid the CPU bottle-neck. I think pglz doesn't have good performance, and it is like fossil compression algorithm. So we need to change latest compression algorithm for more better future. Do you think that if WAL reduction or performance with other compression algorithm (for ex. snappy) is better, then chances of getting the new compression algorithm in postresql will be more? Latest compression algorithms papers(also snappy) have indecated. I think it is enough to select algorithm. It may be also good work in postgres. Snappy is good mainly for un-compressible data, see the link below: http://www.postgresql.org/message-id/CAAZKuFZCOCHsswQM60ioDO_hk12tA7OG3YcJA8v=4yebmoa...@mail.gmail.com I think it is bit difficult to prove that any one algorithm is best for all kind of loads. With Regards, Amit Kapila. 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] Release note fix for timeline item
Sorry for my reply late... (2013/10/08 23:26), Bruce Momjian wrote: First, I want to apologize for not completing the release notes earlier so that others could review them. I started working on the release notes on Friday, but my unfamiliarity with the process and fear of making a mistake caused many delays. I have improved the documentation on the process which will hopefully help next time. There isn't anything in particular that I was dissatisfied about it. You are right that there is alot of details skipped in the release note text. I have developed the attached patch which I think does a better job. Is it OK? Yes, off course! Thanks for your sincere action! Regards, -- Mitsumasa KONDO 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] [PATCH] Add use of asprintf()
On Tue, Oct 15, 2013 at 2:18 AM, Peter Eisentraut pete...@gmx.net wrote:
On Mon, 2013-10-14 at 23:08 +1300, David Rowley wrote:
Looks like something like:
#ifndef WIN32
#define HAVE_VA_COPY 1
#endif
would need to be added to asprintf.c, but also some work needs to be
done with mcxt.c as it uses va_copy unconditionally. Perhaps just
defining a macro for va_copy would be better for windows. I was not
quite sure the best header file for such a macro so I did not write a
patch to fix it.
Does Windows not have va_copy? What do they use instead?
No, Windows doesn't have va_copy, instead they use something like below:
#define va_copy(dest, src) (dest = src)
Please refer below link for details of porting va_copy() on Windows:
http://stackoverflow.com/questions/558223/va-copy-porting-to-visual-c
I could see that there is similar handling in code of vasprintf(),
such that if va_copy is not available then directly assign src to dst.
#if defined(HAVE_VA_COPY)
va_copy(ap2, ap);
#define my_va_end(ap2) va_end(ap2)
#elif defined(HAVE___BUILTIN_VA_COPY)
__builtin_va_copy(ap2, ap);
#define my_va_end(ap2) __builtin_va_end(ap2)
#else
ap2 = ap;
#define my_va_end(ap2) do {} while (0)
#endif
I think rather than having writing code like above at places where
va_copy is used, we can use something like:
#ifdef WIN32
#define va_copy(dest, src) (dest = src)
#endif
and define HAVE_VA_COPY to 1 for non-windows platform.
With Regards,
Amit Kapila.
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] CREATE FOREIGN TABLE ( ... LIKE ... )
On Mon, Oct 07, 2013 at 11:16:56PM -0700, David Fetter wrote:
Folks,
Please find attached a patch implementing and documenting, to some
extent, $subject. I did this in aid of being able to import SQL
standard catalogs and other entities where a known example could
provide a template for a foreign table.
Should there be errhint()s, too? Should we pile up all such errors
and mention them at the end rather than simply bailing on the first
one?
TBD: regression tests.
Now included: regression tests for disallowed LIKE options.
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
diff --git a/doc/src/sgml/ref/create_foreign_table.sgml
b/doc/src/sgml/ref/create_foreign_table.sgml
index 1ef4b5e..4a8e265 100644
--- a/doc/src/sgml/ref/create_foreign_table.sgml
+++ b/doc/src/sgml/ref/create_foreign_table.sgml
@@ -20,6 +20,7 @@
synopsis
CREATE FOREIGN TABLE [ IF NOT EXISTS ] replaceable
class=PARAMETERtable_name/replaceable ( [
replaceable class=PARAMETERcolumn_name/replaceable replaceable
class=PARAMETERdata_type/replaceable [ OPTIONS ( replaceable
class=PARAMETERoption/replaceable 'replaceable
class=PARAMETERvalue/replaceable' [, ... ] ) ] [ COLLATE
replaceablecollation/replaceable ] [ replaceable
class=PARAMETERcolumn_constraint/replaceable [ ... ] ]
+| LIKE replaceablesource_table/replaceable [
replaceablelike_option/replaceable ... ] }
[, ... ]
] )
SERVER replaceable class=parameterserver_name/replaceable
@@ -114,6 +115,15 @@ CREATE FOREIGN TABLE [ IF NOT EXISTS ] replaceable
class=PARAMETERtable_name
/varlistentry
varlistentry
+termliteralLIKE
replaceablesource_table/replaceable/literal/term
+listitem
+ para
+ The literalLIKE/literal clause specifies a table from which
+ the new foreign table automatically copies all column names and their
data types.
+ /para
+ /varlistentry
+
+ varlistentry
termliteralNOT NULL//term
listitem
para
diff --git a/src/backend/parser/parse_utilcmd.c
b/src/backend/parser/parse_utilcmd.c
index 19d19e5f..219c910 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -649,7 +649,7 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint
*constraint)
/*
* transformTableLikeClause
*
- * Change the LIKE srctable portion of a CREATE TABLE statement into
+ * Change the LIKE srctable portion of a CREATE [FOREIGN] TABLE statement
into
* column definitions which recreate the user defined column portions of
* srctable.
*/
@@ -668,12 +668,6 @@ transformTableLikeClause(CreateStmtContext *cxt,
TableLikeClause *table_like_cla
setup_parser_errposition_callback(pcbstate, cxt-pstate,
table_like_clause-relation-location);
- /* we could support LIKE in many cases, but worry about it another day
*/
- if (cxt-isforeign)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-errmsg(LIKE is not supported for creating
foreign tables)));
-
relation = relation_openrv(table_like_clause-relation,
AccessShareLock);
if (relation-rd_rel-relkind != RELKIND_RELATION
@@ -689,6 +683,25 @@ transformTableLikeClause(CreateStmtContext *cxt,
TableLikeClause *table_like_cla
cancel_parser_errposition_callback(pcbstate);
/*
+* For foreign tables, disallow some options.
+*/
+ if (cxt-isforeign)
+ {
+ if (table_like_clause-options CREATE_TABLE_LIKE_CONSTRAINTS)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+errmsg(ERROR: foreign tables do not
support LIKE INCLUDING CONSTRAINTS)));
+ else if (table_like_clause-options CREATE_TABLE_LIKE_INDEXES)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+errmsg(ERROR: foreign tables do not
support LIKE INCLUDING INDEXES)));
+ else if (table_like_clause-options CREATE_TABLE_LIKE_STORAGE)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+errmsg(ERROR: foreign tables do not
support LIKE INCLUDING STORAGE)));
+ }
+
+ /*
* Check for privileges
*/
if (relation-rd_rel-relkind == RELKIND_COMPOSITE_TYPE)
diff --git a/src/test/regress/expected/foreign_data.out
b/src/test/regress/expected/foreign_data.out
Re: [HACKERS] [PATCH] Add use of asprintf()
+1
I think you can safely use va_list without copy on Windows. va_copy is
available in Visual Studio 2013 as part of support for C99, previous
versions don't have it.
Regards,
Muhammad Asif Naeem
On Tue, Oct 15, 2013 at 10:33 AM, Amit Kapila amit.kapil...@gmail.comwrote:
On Tue, Oct 15, 2013 at 2:18 AM, Peter Eisentraut pete...@gmx.net wrote:
On Mon, 2013-10-14 at 23:08 +1300, David Rowley wrote:
Looks like something like:
#ifndef WIN32
#define HAVE_VA_COPY 1
#endif
would need to be added to asprintf.c, but also some work needs to be
done with mcxt.c as it uses va_copy unconditionally. Perhaps just
defining a macro for va_copy would be better for windows. I was not
quite sure the best header file for such a macro so I did not write a
patch to fix it.
Does Windows not have va_copy? What do they use instead?
No, Windows doesn't have va_copy, instead they use something like below:
#define va_copy(dest, src) (dest = src)
Please refer below link for details of porting va_copy() on Windows:
http://stackoverflow.com/questions/558223/va-copy-porting-to-visual-c
I could see that there is similar handling in code of vasprintf(),
such that if va_copy is not available then directly assign src to dst.
#if defined(HAVE_VA_COPY)
va_copy(ap2, ap);
#define my_va_end(ap2) va_end(ap2)
#elif defined(HAVE___BUILTIN_VA_COPY)
__builtin_va_copy(ap2, ap);
#define my_va_end(ap2) __builtin_va_end(ap2)
#else
ap2 = ap;
#define my_va_end(ap2) do {} while (0)
#endif
I think rather than having writing code like above at places where
va_copy is used, we can use something like:
#ifdef WIN32
#define va_copy(dest, src) (dest = src)
#endif
and define HAVE_VA_COPY to 1 for non-windows platform.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Re: [HACKERS] [PATCH] Add use of asprintf()
On Tue, Oct 15, 2013 at 9:48 AM, Peter Eisentraut pete...@gmx.net wrote: On Mon, 2013-10-14 at 23:08 +1300, David Rowley wrote: Looks like something like: #ifndef WIN32 #define HAVE_VA_COPY 1 #endif would need to be added to asprintf.c, but also some work needs to be done with mcxt.c as it uses va_copy unconditionally. Perhaps just defining a macro for va_copy would be better for windows. I was not quite sure the best header file for such a macro so I did not write a patch to fix it. Does Windows not have va_copy? What do they use instead? Not quite sure what is used instead as I've never had the need to use it before, but Mircosoft do seem to be getting around to implementing the C99 standard for Visual Studios 2013. See here. http://msdn.microsoft.com/en-us/library/vstudio/kb57fad8(v=vs.120).aspx If we skip back to VS2012, it does not exist: http://msdn.microsoft.com/en-us/library/vstudio/kb57fad8(v=vs.110).aspx So maybe this is the fix for it, which I think should be forwards compatible for vs2013 and beyond when we go there. diff --git a/src/include/c.h b/src/include/c.h index 8916310..30e68ff 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -63,6 +63,11 @@ #undef errcode #endif +/* Visual Studios 2012 and earlier don't have va_copy() */ +#if _MSC_VER = 1700 +#define va_copy(dest, src) ((dest) = (src)) +#endif + /* * We have to include stdlib.h here because it defines many of these macros * on some platforms, and we only want our definitions used if stdlib.h doesn't Though this is not yet enough to get the windows build work with me... I'm still getting link failures for isolationtester.c D:\Postgres\c\pgsql.sln (default target) (1) - D:\Postgres\c\isolationtester.vcxproj (default target) (89) - (Link target) - isolationtester.obj : error LNK2019: unresolved external symbol _pg_strdup referenced in function _try_complete_step [D:\Postgres\c\isolationtester.vcxproj] isolationtester.obj : error LNK2019: unresolved external symbol _pg_asprintf referenced in function _try_complete_step [D:\Postgres\c\isolationtester.vcxproj ] .\Release\isolationtester\isolationtester.exe : fatal error LNK1120: 2 unresolved externals [D:\Postgres\c\isolationtester.vcxproj] 1 Warning(s) I guess this is down to a make file error somewhere. David
