Re: [HACKERS] Regression test errors
OK, noticed how horrible this patch was (thanks for the heads up from Jaime Casanova). This happens when trying to fetch changes one made on a test copy after a day of lots of work back to a git repository: you just make very silly mistakes. Well, now I got the changes right (tested the patch, because silly changes should be tested as well ;)). 2014-03-07 21:46 GMT-03:00 Martín Marqués mar...@2ndquadrant.com: I was testing some builds I was doing and found that the regression tests fails when doing the against a Hot Standby server: $ make standbycheck [...] == running regression test queries== test hs_standby_check ... ok test hs_standby_allowed ... FAILED test hs_standby_disallowed... FAILED test hs_standby_functions ... ok == 2 of 4 tests failed. == The differences that caused some tests to fail can be viewed in the file /usr/local/postgresql-9.3.3/src/test/regress/regression.diffs. A copy of the test summary that you see above is saved in the file /usr/local/postgresql-9.3.3/src/test/regress/regression.out. The regression.diffs and patch attached. I haven't checked how far back those go. I don't think it's even important to back patch this, but it's nice for future testing. Regards, -- Martín Marqués http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Martín Marqués http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services From 82b4d69d3980ad8852bbf2de67abd4105b328d3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Marqu=C3=A9s?= mar...@2ndquadrant.com Date: Sun, 9 Mar 2014 08:58:17 -0300 Subject: [PATCH] Two Hot Standby regression tests failed for various reasons. - One error was due to the fact that it was checking for a VACUUM error on an ANALYZE call in src/test/regress/expected/hs_standby_disallowed.out - Serializable transactions won't work on a Hot Standby. --- src/test/regress/expected/hs_standby_allowed.out| 2 +- src/test/regress/expected/hs_standby_disallowed.out | 2 +- src/test/regress/sql/hs_standby_allowed.sql | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/regress/expected/hs_standby_allowed.out b/src/test/regress/expected/hs_standby_allowed.out index 1abe5f6..c26c982 100644 --- a/src/test/regress/expected/hs_standby_allowed.out +++ b/src/test/regress/expected/hs_standby_allowed.out @@ -49,7 +49,7 @@ select count(*) as should_be_1 from hs1; (1 row) end; -begin transaction isolation level serializable; +begin transaction isolation level repeatable read; select count(*) as should_be_1 from hs1; should_be_1 - diff --git a/src/test/regress/expected/hs_standby_disallowed.out b/src/test/regress/expected/hs_standby_disallowed.out index e7f4835..bc11741 100644 --- a/src/test/regress/expected/hs_standby_disallowed.out +++ b/src/test/regress/expected/hs_standby_disallowed.out @@ -124,7 +124,7 @@ unlisten *; ERROR: cannot execute UNLISTEN during recovery -- disallowed commands ANALYZE hs1; -ERROR: cannot execute VACUUM during recovery +ERROR: cannot execute ANALYZE during recovery VACUUM hs2; ERROR: cannot execute VACUUM during recovery CLUSTER hs2 using hs1_pkey; diff --git a/src/test/regress/sql/hs_standby_allowed.sql b/src/test/regress/sql/hs_standby_allowed.sql index 58e2c01..7fc2214 100644 --- a/src/test/regress/sql/hs_standby_allowed.sql +++ b/src/test/regress/sql/hs_standby_allowed.sql @@ -28,7 +28,7 @@ begin transaction read only; select count(*) as should_be_1 from hs1; end; -begin transaction isolation level serializable; +begin transaction isolation level repeatable read; select count(*) as should_be_1 from hs1; select count(*) as should_be_1 from hs1; select count(*) as should_be_1 from hs1; -- 1.8.3.1 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] db_user_namespace a temporary measure
Hi, I've noticed that db_user_namespace has had the following note attached to it since 2002: This feature is intended as a temporary measure until a complete solution is found. At that time, this option will be removed. It will be 12 years this year since this temporary measure was added. I'm just wondering, is there any complete solution that anyone had in mind yet? Or should this just be deprecated? Thanks Thom -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Selection of join algorithm.
On Sat, Mar 8, 2014 at 6:18 AM, Ishaya Bhatt ishayabh...@gmail.com wrote: Hi, I am trying to analyze join performance. But I see that even for a table having 100,000 rows and join attribute as primary key, postgres always performs hash join. Can anyone please tell me under which conditions merge join or nested loop join is invoked? Unless you trying to look into the source code of postgresql to see how the internals of the planner works, this should really go to pgsql-performa...@postgresql.org, not to hackers. A nested loop would be favored if there were some WHERE condition that filtered out nearly all of the rows of the outer table. In that case, only a small amount of the inner table needs to be accessed, and so reading the whole thing to hash it would be too expensive. A merge join would be favored if you used an ORDER BY to ask for the data to be sorted in the same order as the merge join would naturally deliver it in. If the data is too large to fit in work_mem, it might favor either the merge join or nested loop compared to the hash join. This stuff is hard to discuss in the abstract. It is probably best to use the enable_*join settings to see what it does with your actual data (or better yet a synthetic data set whose generator you can share with us). Cheers, Jeff
[HACKERS] issue log message to suggest VACUUM FULL if a table is nearly empty
Hi Enclosed is the patch to implement the requirement that issue log message to suggest VACUUM FULL if a table is nearly empty. The requirement comes from the Postgresql TODO list. [Benefit] To find which table is nearly empty and suggest using 'VACUUM FULL' to release the unused disk space this table occupied. [Analysis] A table is nearly empty include two scenario: 1. The table occupy small disk size and contains few unused rows. 2. The table occupy large disk size and contains large numbers of unused rows. Obviously the requirement is used to release the disk in the scenario2. [Solution details] A check function is added in the function 'lazy_vacuum_rel' to check if the table is large enough and contains large numbers of unused rows. If it is then issue a log message that suggesting using 'VACUUM FULL' on the table. The judgement policy is as following: If the relpage of the table RELPAGES_VALUES_THRESHOLD(default 1000) then the table is considered to be large enough. If the free_space/total_space FREESPACE_PERCENTAGE_THRESHOLD(default 0.5) then the table is considered to have large numbers of unused rows. The free_space is calculated by reading the details from the FSM pages. This may increase the IO, but expecting very less FSM pages thus it shouldn't cause Any problems. Please let me know your suggestions. [When the log message prints] When executing SQL command 'VACUUM' or 'VACUUM on a table', this function will be invoked and may issue the log message if the table reach the condition. When auto vacuum work and execute 'VACUUM on a table', this function will be invoked and may issue the log message if the table reach the condition. [Example] SELECT count(*) from t5; count --- 3000 (1 row) DELETE FROM t5 where f12900; DELETE 2899 SELECT count(*) from t5; count --- 101 (1 row) LOG: automatic vacuum of table wjdb.public.t5: index scans: 0 pages: 0 removed, 20 remain tuples: 2899 removed, 101 remain, 0 are dead but not yet removable buffer usage: 64 hits, 1 misses, 25 dirtied avg read rate: 0.130 MB/s, avg write rate: 3.261 MB/s system usage: CPU 0.00s/0.00u sec elapsed 0.05 sec LOG: Table t5 contains large numbers of unused row, suggest using VACUUM FULL on it! VACUUM t5; LOG: Table t5 contains large numbers of unused row, suggest using VACUUM FULL on it! Kind regards Jing Wang Fujitsu Australia vacuum_v1.patch Description: vacuum_v1.patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TABLE lock strength reduction patch is unsafe
On 03/06/2014 10:47 AM, Simon Riggs wrote: On 5 March 2014 09:28, Simon Riggs si...@2ndquadrant.com wrote: So that returns us to solving the catalog consistency problem in pg_dump and similar applications. No answer, guys, and time is ticking away here. Sorry, I've accumulated several days of backlog (and it'll only get worse in the coming week) so I haven't had time to look at all this in the detail I wanted to. I'd like to get a communal solution to this rather than just punting the whole patch. If we have to strip it down to the bar essentials, so be it. For me, the biggest need here is to make VALIDATE CONSTRAINT take only a ShareUpdateExclusiveLock while it runs. Almost everything else needs a full AccessExclusiveLock anyway, or doesn't run for very long so isn't a critical problem. (Perhaps we can then wrap ADD CONSTRAINT ... NOT VALID and VALIDATE into a single command using the CONCURRENTLY keyword so it runs two transactions to complete the task). Validating FKs on big tables can take hours and it really isn't acceptable for us to lock out access while we do that. FKs are *supposed* to be a major reason people use RDBMS, so keeping them in a state where they are effectively unusable is a major debilitating point against adoption of PostgreSQL. If there are issues with pg_dump we can just document them. Guide me with your thoughts. I think committing VALIDATE CONSTRAINT is essential for 9.4; the rest can be delayed until 9.5. None of the discussion in this thread has been about that subcommand, and I don't personally see a problem with it. I don't care much about ADD CONSTRAINT CONCURRENTLY. If it's there, fine. If it's not, that's fine, too. My personal use case for this, and I even started writing a patch before I realized I was re-writing this one, is adding a CHECK constraint NOT VALID so that future commits respect it, then UPDATEing the existing rows to fix them, and then VALIDATE CONSTRAINTing it. There is zero need for an AccessExclusiveLock on that last step. My original idea was to concurrently create a partial index on the bad rows, and then validate the constraint using that index. The AEL would only be held long enough to check if the index is empty or not. Obviously, reducing the lock level is a cleaner solution, so I'd like to see that happen. -- Vik -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch (v2) for updatable security barrier views
I've found an issue with updatable security barrier views. Locking is
being pushed down into the subquery. Locking is thus applied before
user-supplied quals are, so we potentially lock too many rows.
I'm looking into the code now, but in the mean time, here's a demo of
the problem:
regress= CREATE TABLE t1(x integer, y integer);
CREATE TABLE
regress= INSERT INTO t1(x,y) VALUES (1,1), (2,2), (3,3), (4,4);
INSERT 0 4
regress= CREATE VIEW v1 WITH (security_barrier) AS SELECT x, y FROM t1
WHERE x % 2 = 0;
CREATE VIEW
regress= EXPLAIN SELECT * FROM v1 FOR UPDATE;
QUERY PLAN
---
LockRows (cost=0.00..42.43 rows=11 width=40)
- Subquery Scan on v1 (cost=0.00..42.32 rows=11 width=40)
- LockRows (cost=0.00..42.21 rows=11 width=14)
- Seq Scan on t1 (cost=0.00..42.10 rows=11 width=14)
Filter: ((x % 2) = 0)
Planning time: 0.140 ms
(6 rows)
or, preventing pushdown with a wrapper function to demonstrate the problem:
regress= CREATE FUNCTION is_one(integer) RETURNS boolean AS $$ DECLARE
result integer; BEGIN SELECT $1 = 1
regress= EXPLAIN SELECT * FROM v1 WHERE is_one(x) FOR UPDATE;
QUERY PLAN
---
LockRows (cost=0.00..45.11 rows=4 width=40)
- Subquery Scan on v1 (cost=0.00..45.07 rows=4 width=40)
Filter: is_one(v1.x)
- LockRows (cost=0.00..42.21 rows=11 width=14)
- Seq Scan on t1 (cost=0.00..42.10 rows=11 width=14)
Filter: ((x % 2) = 0)
Planning time: 0.147 ms
(7 rows)
OK, so it looks like the code:
/*
* Now deal with any PlanRowMark on this RTE by requesting a
lock
* of the same strength on the RTE copied down to the subquery.
*/
rc = get_plan_rowmark(root-rowMarks, rt_index);
if (rc != NULL)
{
switch (rc-markType)
{
/* */
}
root-rowMarks = list_delete(root-rowMarks, rc);
}
isn't actually appropriate. We should _not_ be pushing locking down into
the subquery.
Instead, we should be retargeting the rowmark so it points to the new
subquery RTE, marking rows _after_filtering. We want a plan like:
regress= EXPLAIN SELECT * FROM v1 WHERE is_one(x) FOR UPDATE;
QUERY PLAN
---
LockRows (cost=0.00..45.11 rows=4 width=40)
- Subquery Scan on v1 (cost=0.00..45.07 rows=4 width=40)
Filter: is_one(v1.x)
- Seq Scan on t1 (cost=0.00..42.10 rows=11 width=14)
Filter: ((x % 2) = 0)
Planning time: 0.147 ms
(7 rows)
I'm not too sure what the best way to do that is. Time permitting I'll
see if I can work out the RowMark code and set something up.
--
Craig Ringer 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] review: psql command copy count tag
As mentioned by Pavel also, this patch will be very useful, which provides below enhancement: 1. Brings consistency between copy from “stdin” and “file”. 2. Consistent with server side copy statement. 3. Also fixes the issue related to “\copy destination file becomes default destination file for next command given in sequence”. This has been in “Ready for committer” stage for long time. Please check if this can be committed now or any other changes required. Thanks and Regards, Kumar Rajeev Rastogi -- This e-mail and its attachments contain confidential information from HUAWEI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it! From: Pavel Stehule [mailto:pavel.steh...@gmail.com] Sent: 30 January 2014 01:57 To: PostgreSQL Hackers Cc: Amit Khandekar; Rajeev rastogi Subject: review: psql command copy count tag related to http://www.postgresql.org/message-id/bf2827dcce55594c8d7a8f7ffd3ab7713ddb1...@szxeml508-mbx.china.huawei.com Hello 1. I had to rebase this patch - actualised version is attached - I merged two patches to one 2. The psql code is compiled without issues after patching 3. All regress tests are passed without errors 5. We like this feature - it shows interesting info without any slowdown - psql copy command is more consistent with server side copy statement from psql perspective. This patch is ready for commit Regards Pavel
Re: [HACKERS] review: psql command copy count tag
Rajeev rastogi rajeev.rast...@huawei.com writes: This has been in âReady for committerâ stage for long time. Yeah, I started to work on it and got distracted, but was working on it some more yesterday. As submitted, it leaks PGresults, and makes some inconsistent and mostly undocumented changes in the APIs of the psql functions involved. But I'm pretty close to having something committable. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] calculating an aspect of shared buffer state from a background worker
Dear Hackers -- I'm looking at doing a calculation to determine the number of free buffers available. A n example ratio that is based on some data structures in freelist.c as follows: (StrategyControl-lastFreeBuffer - StrategyControl-firstFreeBuffer) / (double) NBuffers Is there a way to get access to the StrategyControl pointer in the context of a background worker? Thanks, Robert
Re: [HACKERS] [review] PostgreSQL Service on Windows does not start if data directory given is relative path
While registering the service for PostgreSQL on windows, we cannot expect user to give absolute path always. So it is required to support relative path as data directory. So this patch will be very useful to handle the same. This patch has been in Ready for committer stage for long time. Please check if this can be committed now or any other changes required. Thanks and Regards, Kumar Rajeev Rastogi -Original Message- From: MauMau [mailto:maumau...@gmail.com] Sent: 24 February 2014 15:31 To: Rajeev rastogi Cc: pgsql-hackers@postgresql.org; Magnus Hagander Subject: Re: [HACKERS] [review] PostgreSQL Service on Windows does not start if data directory given is relative path From: Rajeev rastogi rajeev.rast...@huawei.com Please find the attached modified patch. Thanks, reviewed and made this patch ready for committer. Regards MauMau -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] issue log message to suggest VACUUM FULL if a table is nearly empty
On Mon, Mar 10, 2014 at 5:58 AM, Wang, Jing ji...@fast.au.fujitsu.com wrote: Enclosed is the patch to implement the requirement that issue log message to suggest VACUUM FULL if a table is nearly empty. The requirement comes from the Postgresql TODO list. [Solution details] A check function is added in the function 'lazy_vacuum_rel' to check if the table is large enough and contains large numbers of unused rows. If it is then issue a log message that suggesting using 'VACUUM FULL' on the table. The judgement policy is as following: If the relpage of the table RELPAGES_VALUES_THRESHOLD(default 1000) then the table is considered to be large enough. If the free_space/total_space FREESPACE_PERCENTAGE_THRESHOLD(default 0.5) then the table is considered to have large numbers of unused rows. The free_space is calculated by reading the details from the FSM pages. This may increase the IO, but expecting very less FSM pages thus it shouldn't cause I think it would be better if we can use some existing stats to issue warning message rather than traversing the FSM for all pages. For example after vacuuming page in lazy_scan_heap(), we update the freespace for page. You can refer below line in lazy_scan_heap(). freespace = PageGetHeapFreeSpace(page); Now it might be possible that we might not get freespace info easily as it is not accumulated for previous vacuum's. Incase there is no viable way to get it through vacuum stats, we are already updating fsm after vacuum by FreeSpaceMapVacuum(), where I think it should be possible to get freespace. In general, I think idea to log a message for Vaccum Full is okay, but it would be more viable if we can do that without any additional cost. 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] [bug fix] pg_ctl always uses the same event source
On Mon, Feb 17, 2014 at 9:17 AM, Amit Kapila amit.kapil...@gmail.com wrote: On Sat, Feb 1, 2014 at 12:31 PM, Amit Kapila amit.kapil...@gmail.com wrote: I think it's just a very minor coding style thing, so I am marking this patch as Ready For Committer. I could see that this patch has been marked as Needs Review in CF app. suggesting that it should be rejected based on Tom's rejection in below mail: http://www.postgresql.org/message-id/3315.1390836...@sss.pgh.pa.us If I understand correctly that objection was on changing Default Event Source name, and the patch now doesn't contain that change, it's just a bug fix for letting pg_ctl know the non-default event source set by user. Please clarify if I misunderstood something, else this should be changed to Ready For Committer. Tom/Andres, please let me know if you have objection for this patch, because as per my understanding all the objectionable part of patch is removed from final patch and it's a defect fix to make pg_ctl aware of Event Source name set in postgresql.conf. If there is no objection, I will again change it to Ready For Committer. 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] Little confusing things about client_min_messages.
Hi Tom, Bruce,
Thank you for your response.
(2014/03/09 2:12), Tom Lane wrote:
Bruce Momjian br...@momjian.us writes:
On Sat, Mar 8, 2014 at 11:31:22AM -0500, Tom Lane wrote:
Tomonari Katsumata t.katsumata1...@gmail.com writes:
[ client_min_messages = info is not documented ]
That's intentional, because it's not a useful setting. Even more so
for the other two.
Well, 'info' is between other settings we do document, so I am not clear
why info should be excluded. It is because we always output INFO to the
client? From elog.c:
if (ClientAuthInProgress)
output_to_client = (elevel = ERROR);
else
output_to_client = (elevel = client_min_messages ||
elevel == INFO);
Right, so if you did set it to that, it wouldn't be functionally different
from NOTICE.
I understand it's intensional.
We can set INFO but it doesn't have any difference from NOTICE
because all INFO messages are sent to client.
So it is hidden from the document.
The word valid in the document has meant meaningful.
I've misread it was setable.
So no, I don't think we ought to be advertising these as suggested
values. A saner proposed patch would be to remove them from the
valid values altogether. We probably had some good reason for leaving
them in the list back when, but I'm having a hard time reconstructing
what that would be.
Adding FATAL and PANIC to client_min_messages is done at below-commit.
8ac386226d76b29a9f54c26b157e04e9b8368606
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=8ac386226d76b29a9f54c26b157e04e9b8368606
According to the commit log, it seems that the purpose
is suppressing to be sent error message to client when DROP TABLE.
In those days(pre 8.1), we did not have DROP IF EXISTS syntax,
so it was useful.
If this was the reason, now(from 8.2) we have DROP IF EXISTS syntax,
so we could delete FATAL and PANIC from client_min_messages valid value.
Attached patch do it. Please check it.
regards,
---
Tomonari Katsumata
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 2811f11..cbaf264 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3796,8 +3796,7 @@ local0.*/var/log/postgresql
Valid values are literalDEBUG5/,
literalDEBUG4/, literalDEBUG3/, literalDEBUG2/,
literalDEBUG1/, literalLOG/, literalNOTICE/,
-literalWARNING/, literalERROR/, literalFATAL/,
-and literalPANIC/. Each level
+literalWARNING/, and literalERROR/. Each level
includes all the levels that follow it. The later the level,
the fewer messages are sent. The default is
literalNOTICE/. Note that literalLOG/ has a different
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index c76edb4..b7cfe14 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -242,8 +242,6 @@ static const struct config_enum_entry client_message_level_options[] = {
{notice, NOTICE, false},
{warning, WARNING, false},
{error, ERROR, false},
- {fatal, FATAL, true},
- {panic, PANIC, true},
{NULL, 0, false}
};
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
