Re: [HACKERS] plpgsql_check_function - rebase for 9.3
There is still valid a variant to move check function to contrib for fist
moment.
Regards
Pavel
2013/12/7 Pavel Stehule
> Hello
>
> thank you for review
>
>
> 2013/12/7 Steve Singer
>
>> On 08/22/2013 02:02 AM, Pavel Stehule wrote:
>>
>>> rebased
>>>
>>> Regards
>>>
>>> Pavel
>>>
>>> This patch again needs a rebase, it no longer applies cleanly.
>> plpgsql_estate_setup now takes a shared estate parameter and the call in
>> pl_check needs that. I passed NULL in and things seem to work.
>>
>>
>>
>> I think this is another example where are processes aren't working as
>> we'd like.
>>
>> The last time this patch got a review was in March when Tom said
>> http://www.postgresql.org/message-id/27661.1364267...@sss.pgh.pa.us
>>
>> Shortly after Pavel responded with a new patch that changes the output
>> format. http://www.postgresql.org/message-id/
>> CAFj8pRAqpAEWqL465xkTmwe-DaZ8+7N-oEQ=uv_jjop21fj...@mail.gmail.com
>>
>> I don't much has progress in the following 9 months on this patch.
>>
>> In Tom's review the issue of code duplication and asks:
>>
>> "do we want to accept that this is the implementation pathway we're going
>> to settle for, or are we going to hold out for a better one? I'd be the
>> first in line to hold out if I had a clue how to move towards a better
>> implementation, but I have none."
>>
>
> yes, I am waiting still to a) have some better idea, or b) have
> significantly more free time for larger plpgsql refactoring. Nothing of it
> happens :(
>
>
>> This question is still outstanding.
>>
>> Here are my comments on this version of the patch:
>>
>>
>>
>> This version of the patch gives output as a table with the structure:
>>
>> functionid | lineno | statement | sqlstate | message | detail | hint |
>> level | position | query | context
>> ++---+--+-+-
>> ---+--+---+--+---+-
>>
>> I like this much better than the previous format.
>>
>> I think the patch needs more documentation.
>>
>> The extent of the documentation is
>>
>> Checking of embedded SQL
>> +
>> + The SQL statements inside PL/pgSQL functions are
>> + checked by validator for semantic errors. These errors
>> + can be found by plpgsql_check_function:
>>
>>
>>
>> I a don't think that this adequately describes the function. It isn't
>> clear to me what we mean by 'embedded' SQL.
>> and 'semantic errors'.
>>
>>
>> I think this would read better as
>>
>>
>>Checking SQL Inside Functions
>>
>> The SQL Statements inside of PL/pgsql functions can
>> be
>> checked by a validation function for semantic errors. You can check
>> PL/pgsl functions by passing the name of
>> the
>> function to plpgsql_check_function.
>>
>>
>>The plpgsql_check_function will check the SQL
>>inside of a PL/pgsql function for certain
>>types of errors and warnings.
>>
>>
>> but that needs to be expanded on.
>>
>> I was expecting something like:
>>
>> create function x() returns void as $$ declare a int4; begin a='f'; end
>> $$ language plpgsql;
>>
>> to return an error but it doesn't
>>
>
> This is expected. PL/pgSQL use a late casting - so a := '10'; is
> acceptable. And in this moment plpgsql check doesn't evaluate constant and
> doesn't try to cast to target type. But this check can be implemented
> sometime. In this moment, we have no simple way how to identify constant
> expression in plpgsql. So any constants are expressions from plpgsql
> perspective.
>
>
>>
>> but
>>
>> create temp table x(a int4);
>> create or replace function x() returns void as $$ declare a int4; begin
>> insert into x values('h'); end $$ language plpgsql;
>>
>
> it is expected too. SQL doesn't use late casting - casting is controlled
> by available casts and constants are evaluated early - due possible
> optimization.
>
>
>>
>> does give an error when I pass it to the validator. Is this the
>> intended behavior of the patch? If so we need to explain why the first
>> function doesn't generate an error.
>>
>>
>> I feel we need to document what each of the columns in the output means.
>> What is the difference between statement, query and context?
>>
>> Some random comments on the messages you return:
>>
>> Line 816:
>>
>> if (is_expression && tupdesc->natts != 1)
>> ereport(ERROR,
>> (errcode(ERRCODE_SYNTAX_ERROR),
>> errmsg("qw",
>> query->query,
>> tupdesc->natts)));
>>
>> Should this be "query \"%s\" returned %d columns but only 1 is allowed"
>> or something similar?
>>
>> Line 837
>>
>> else
>> /* XXX: should be a warning? */
>> ereport(ERROR,
>> (errmsg("cannot to identify real
>> type for record type varia
Re: [HACKERS] Feature request: Logging SSL connections
On Fri, Dec 06, 2013 at 02:53:27PM +0100, Dr. Andreas Kunert wrote: > >>Anything else missing? > > > >Functionally it's fine now, but I see few style problems: > > > >- "if (port->ssl > 0)" is wrong, ->ssl is pointer. So use just > > "if (port->ssl)". > >- Deeper indentation would look nicer with braces. > >- There are some duplicated message, could you restructure it so that > > each message exists only once. > > New version is attached. I could add braces before and after the > ereport()-calls too, but then I need two more #ifdefs to catch the > closing braces. Thank you. Looks good now. I added it to next commitfest: https://commitfest.postgresql.org/action/patch_view?id=1324 -- marko -- 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] shared memory message queues
2013/12/6 Kohei KaiGai : > What will happen if sender tries to send a large chunk that needs to > be split into multiple sub-chunks and receiver concurrently detaches > itself from the queue during the writes by sender? > It seems to me the sender gets SHM_MQ_DETACHED and only > earlier half of the chunk still remains on the queue even though > its total length was already in the message queue. > It may eventually lead infinite loop on the receiver side when another > receiver appeared again later, then read incomplete chunk. > Does it a feasible scenario? If so, it might be a solution to prohibit > enqueuing something without receiver, and reset queue when a new > receiver is attached. > Doesn't it an intended usage to attach a peer process on a message queue that had once detached, does it? If so, it may be a solution to put ereport() on shm_mq_set_receiver() and shm_mq_set_sender() to prohibit to assign a process on the message queue with mq_detached = true. It will make the situation simplified. Regarding to the test-shm-mq-v1.patch, setup_background_workers() tries to launch nworkers of background worker processes, however, may fail during the launching if max_worker_processes is not enough. Is it a situation to attach the BGWORKER_EPHEMERAL flag when your patch gets committed, isn't it? I think it is waste of efforts to add error handling here instead of the core support to be added, however, it makes sense to put a source code comment not to forget to add this flag when it came. Also, test_shm_mq_setup() waits for completion of starting up of background worker processes. I'm uncertain whether it is really needed, because this shared memory message queue allows to send byte stream without receiver, and also blocks until byte stream will come from the peer to be set later. This module is designed for test purpose, so I think it makes more sense if test condition is more corner case. Thanks, -- KaiGai Kohei -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_stat_statements: calls under-estimation propagation
On Sun, Dec 8, 2013 at 1:00 AM, Peter Geoghegan wrote: > On Sat, Dec 7, 2013 at 3:50 PM, Peter Eisentraut wrote: > > 32-bit buildfarm members are having problems with this patch. > > This should fix that problem. Thanks. > Applied. I also noted on http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=frogmouth&dt=2013-12-08%2007%3A30%3A01&stg=make-contrib that there are compiler warnings being generated in pgss. But from a quick look that looks like something pre-existing and not caused by the latest patch. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] [bug fix] pg_ctl always uses the same event source
On Sun, Dec 8, 2013 at 3:41 AM, MauMau wrote: > From: "MauMau" > > I've removed a limitation regarding event log on Windows with the attached >> patch. I hesitate to admit this is a bug fix and want to regard this an >> improvement, but maybe it's a bug fix from users' perspective. Actually, >> I >> received problem reports from some users. >> > > I've done a small correction. I removed redundant default value from the > pg_ctl.c. > Not having looked at it in detail yet, but this seems to completely remove the default value. What happens if the error that needs to be logged is the one saying that it couldn't exec postgres to find out the value in the config file? AFAICT it's going to try to register an eventsource with whatever random garbage happens to be in the variable. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] pg_stat_statements: calls under-estimation propagation
On Sat, 2013-12-07 at 16:00 -0800, Peter Geoghegan wrote: > On Sat, Dec 7, 2013 at 3:50 PM, Peter Eisentraut wrote: > > 32-bit buildfarm members are having problems with this patch. > > This should fix that problem. Thanks. This is incidentally the same problem that was reported here about another pg_stat_statements patch: http://www.postgresql.org/message-id/bf2827dcce55594c8d7a8f7ffd3ab7713ddac...@szxeml508-mbx.china.huawei.com Can we make this more robust so that we don't accidentally keep breaking 32-bit systems? Maybe a static assert? -- 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] CLUSTER FREEZE
On Tue, 2013-11-19 at 18:24 +0100, Andres Freund wrote: > On 2013-11-19 12:23:30 -0500, Robert Haas wrote: > > On Mon, Nov 18, 2013 at 11:45 AM, Andres Freund > > wrote: > > >> Yes, we probably should make a decision, unless Robert's idea can be > > >> implemented. We have to balance the ease of debugging MVCC failures > > >> with the interface we give to the user community. > > > > > > Imo that patch really doesn't need too much further work. > > > > Would you care to submit a version you're happy with? > > I'll give it a spin sometime this week. I'm setting the CLUSTER FREEZE patch as returned with feedback, until this other patch has been considered. -- 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] Completing PL support for Event Triggers
On Tue, 2013-11-26 at 06:59 -0500, Peter Eisentraut wrote: > Attached is my "final" patch. Let me know if it's OK for you. Dimitri, will you have a change to review this? -- 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] plpgsql_check_function - rebase for 9.3
In my opinion, the idea of having a separate lint checker for a language is antiquated. If there are problems, they should be diagnosed at compile time or run time. You can add options about warning levels or strictness if there are concerns about which diagnostics are appropriate. > > > > > > > -- 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] plpgsql_check_function - rebase for 9.3
On Sun, 2013-12-08 at 09:45 +0100, Pavel Stehule wrote: > There is still valid a variant to move check function to contrib for > fist moment. If we are not happy with the code or the interface, then moving it to contrib is not a solution. We are still committed to main things in contrib indefinitely. -- 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] Add %z support to elog/ereport?
On Sun, 2013-12-08 at 01:32 +0100, Andres Freund wrote: > Patch 02 converts some elog/ereport() callers to using the z modifier, > some were wrong at least on 64 bit windows. This is making the assumption that Size is the same as size_t. If we are going to commit to that, we might as well get rid of Size. -- 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] Add %z support to elog/ereport?
On 2013-12-08 11:43:46 -0500, Peter Eisentraut wrote: > On Sun, 2013-12-08 at 01:32 +0100, Andres Freund wrote: > > Patch 02 converts some elog/ereport() callers to using the z modifier, > > some were wrong at least on 64 bit windows. > > This is making the assumption that Size is the same as size_t. If we > are going to commit to that, we might as well get rid of Size. Well, we're unconditionally defining it as such in c.h and its usage is pretty much what size_t is for. Given how widespread Size's use is, I am not seing a realistic way of getting rid of it though. 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] plpgsql_check_function - rebase for 9.3
2013/12/8 Peter Eisentraut > In my opinion, the idea of having a separate lint checker for a language > is antiquated. If there are problems, they should be diagnosed at > compile time or run time. You can add options about warning levels or > strictness if there are concerns about which diagnostics are > appropriate. > There are two points, that should be solved a) introduction a dependency to other object in schema - now CREATE FUNCTION is fully independent on others b) slow start - if we check all paths on start, then start can be slower - and some functions should not work due dependency on temporary tables. I am thinking about possible marking a function by #option (we have same idea) some like #option check_on_first_start #option check_on_create #option check_newer But still I have no idea, how to push check without possible slowdown execution with code duplication Pavel > > > > > > > > > > > > > > > >
Re: [HACKERS] WITHIN GROUP patch
Andrew Gierth writes: > There's also the question of ungrouped vars, maybe. Consider these two > queries: > select array(select a+sum(x) from (values (0.3),(0.7)) v(a) group by a) > from generate_series(1,5) g(x); > select array(select percentile_disc(a) within group (order by x) >from (values (0.3),(0.7)) v(a) group by a) > from generate_series(1,5) g(x); > In both cases the aggregation query is the outer one; but while the first > can return a value, I think the second one has to fail (at least I can't > see any reasonable way of executing it). Hm, interesting. So having decided that the agg has level 1, we need to reject any level-0 vars in the direct parameters, grouped or not. We could alternatively decide that the agg has level 0, but that doesn't seem terribly useful, and I think it's not per spec either. SQL:2008 section 6.9 seems pretty clear that only aggregated arguments should be considered when determining the semantic level of an aggregate. OTOH, I don't see any text there restricting what can be in the non-aggregated arguments, so maybe the committee thinks this case is sensible? Or they just missed it. 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] dblink performance regression
Joe Conway writes: > I don't think it makes sense to create a new function in dblink either > -- we're only talking about two lines of added redundancy which is > less lines of code than a new function would add. Indeed, and I think the claim that such a function "encapsulates" anything useful is pretty silly as well. I think the committed patch is fine. 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] ANALYZE sampling is too good
On Sun, Dec 8, 2013 at 12:06 AM, Mark Kirkwood wrote: > > bench=# ANALYZE pgbench_accounts; > NOTICE: acquire sample will need 3 blocks > NOTICE: sampled 3 blocks > ANALYZE > Time: 10059.446 ms > bench=# \q I did some experimenting here as well. I hacked up a version of analyze.c that has a guc for rows_per_block to sample. Setting it to 1 doesn't modify the behaviour at all whereas setting it to 4 divides the number of blocks to sample by 4 which causes it to do less I/O and use more rows from each block. I then initialized pgbench with scale factor 100 but modified the code to run the actual pgbench with scale factor 1. In other words I ran a lot of updates on 1% of the database but left the other 99% untouched from the initial load. Then I ran "ANALYZE VERBOSE accounts" with rows_per_block set to 1, 4, 16, and 64. The latter is slightly larger than the average number of tuples per block so the resulting sample is actually slightly short. The whole accounts table is 1.2GB and contains 10 million rows. As expected with rows_per_block set to 1 it reads 240MB of that containing nearly 2 million rows (and takes nearly 20s -- doing a full table scan for select count(*) only takes about 5s): stark=# analyze verbose pgbench_accounts; INFO: analyzing "public.pgbench_accounts" INFO: "pgbench_accounts": scanned 3 of 158756 pages, containing 1889701 live rows and 0 dead rows; 3 rows in sample, 1036 estimated total rows ANALYZE Time: 19468.987 ms With rows_per_block=4 it reads only a quarter as many blocks but it's not much faster: stark=# analyze verbose pgbench_accounts; INFO: analyzing "public.pgbench_accounts" INFO: "pgbench_accounts": scanned 7501 of 158756 pages, containing 472489 live rows and 0 dead rows; 3 rows in sample, 1037 estimated total rows ANALYZE Time: 17062.331 ms But with rows_per_block=16 it's much faster, 6.7s stark=# set statistics_rows_per_block = 16; SET Time: 1.583 ms stark=# analyze verbose pgbench_accounts; INFO: analyzing "public.pgbench_accounts" INFO: "pgbench_accounts": scanned 1876 of 158756 pages, containing 118163 live rows and 0 dead rows; 3 rows in sample, 1031 estimated total rows ANALYZE Time: 6694.331 ms And with rows_per_block=64 it's under 2s: stark=# set statistics_rows_per_block = 64; SET Time: 0.693 ms stark=# analyze verbose pgbench_accounts; INFO: analyzing "public.pgbench_accounts" INFO: "pgbench_accounts": scanned 469 of 158756 pages, containing 29544 live rows and 0 dead rows; 29544 rows in sample, 1033 estimated total rows ANALYZE Time: 1937.055 ms The estimates for the total rows is just as accurate in every case. (It seems to be consistently sightly high though which is a bit disconcerting) However looking at the actual pg_stats entries the stats are noticeably less accurate for the "blockier" samples. The "bid" column actually has 100 distinct values and so with a statistics_target of 100 each value should appear in the MCV list with a frequency of about .01. With rows_per_block=1 the MCV frequency list ranges from .0082 to .0123 With rows_per_block=4 the MCV frequency list ranges from .0063 to .0125 With rows_per_block=16 the MCV frequency list ranges from .0058 to .0164 With rows_per_block=64 the MCV frequency list ranges from .0021 to .0213 I'm not really sure if this is due to the blocky sample combined with the skewed pgbench run or not. It doesn't seem to be consistently biasing towards or against bid 1 which I believe are the only rows that would have been touched by pgbench. Still it's suspicious that they seem to be consistently getting less accurate as the blockiness increases. I've attached the results of pg_stats following the analyze with the various levels of "blockiness". -- greg stark=# set statistics_rows_per_block = 1; SET Time: 1.363 ms stark=# analyze verbose pgbench_accounts; INFO: analyzing "public.pgbench_accounts" INFO: "pgbench_accounts": scanned 3 of 158756 pages, containing 1889701 live rows and 0 dead rows; 3 rows in sample, 1036 estimated total rows ANALYZE Time: 19468.987 ms stark=# select * from pg_stats where tablename = 'pgbench_accounts'; -[ RECORD 1 ]--+
Re: [HACKERS] Re: [BUGS] BUG #7873: pg_restore --clean tries to drop tables that don't exist
2013/12/8 Dean Rasheed > On 7 December 2013 21:34, Pavel Stehule wrote: > >> Well I was basically proposing that does_not_exist_skipping() be > >> enhanced to report on non-existent types that form part of the object > >> specification. I think this would affect the CAST, FUNCTION, AGGREGATE > >> and OPERATOR cases, but should be a fairly trivial extension to the > >> code that you've already added. > > > > > > ok, updated patch is in attachment > > > > Cool. This looks good to me, except I found a corner case --- the type > name for an operator may be "NONE", in which case the typeName in the > list will be NULL, so that needs to be guarded against. Updated patch > attached. > > I think this is a good patch. It makes all the DROP...IF EXISTS > commands consistently fault-tolerant, instead of the current 50/50 > mix, and all the resulting NOTICEs give useful information about why > objects don't exist and are being skipped. > > I think this is now ready for committer. > thank you :) Pavel > > Nice work! > > Regards, > Dean >
Re: [HACKERS] ANALYZE sampling is too good
On 12/08/2013 10:14 AM, Greg Stark wrote: > With rows_per_block=1 the MCV frequency list ranges from .0082 to .0123 > With rows_per_block=4 the MCV frequency list ranges from .0063 to .0125 > With rows_per_block=16 the MCV frequency list ranges from .0058 to .0164 > With rows_per_block=64 the MCV frequency list ranges from .0021 to .0213 > > I'm not really sure if this is due to the blocky sample combined with > the skewed pgbench run or not. It doesn't seem to be consistently > biasing towards or against bid 1 which I believe are the only rows > that would have been touched by pgbench. Still it's suspicious that > they seem to be consistently getting less accurate as the blockiness > increases. They will certainly do so if you don't apply any statistical adjustments for selecting more rows from the same pages. So there's a set of math designed to calculate for the skew introduced by reading *all* of the rows in each block. That's what I meant by "block-based sampling"; you read, say, 400 pages, you compile statistics on *all* of the rows on those pages, you apply some algorithms to adjust for groupings of rows based on how grouped they are. And you have a pretty good estimate of how grouped they are, because you just looked a complete sets of rows on a bunch of nonadjacent pages. Obviously, you need to look at more rows than you would with a pure-random sample. Like I said, the 80%+ accurate point in the papers seemed to be at a 5% sample. However, since those rows come from the same pages, the cost of looking at more rows is quite small, compared to the cost of looking at 64 times as many disk pages. My ACM subscription has lapsed, though; someone with a current ACM subscription could search for this; there are several published papers, with math and pseudocode. -- 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] ANALYZE sampling is too good
On 12/08/2013 08:14 PM, Greg Stark wrote: The whole accounts table is 1.2GB and contains 10 million rows. As expected with rows_per_block set to 1 it reads 240MB of that containing nearly 2 million rows (and takes nearly 20s -- doing a full table scan for select count(*) only takes about 5s): One simple thing we could do, without or in addition to changing the algorithm, is to issue posix_fadvise() calls for the blocks we're going to read. It should at least be possible to match the speed of a plain sequential scan that way. - Heikki -- 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] GIN improvements part 1: additional information
On Fri, Nov 29, 2013 at 11:17 PM, Heikki Linnakangas < hlinnakan...@vmware.com> wrote: > On 11/29/2013 11:41 AM, Heikki Linnakangas wrote: > >> On 11/28/2013 09:19 AM, Alexander Korotkov wrote: >> >>> On Wed, Nov 27, 2013 at 1:14 AM, Heikki Linnakangas >>> >> wrote: >>> >>> On 11/26/13 15:34, Alexander Korotkov wrote: What's your plans about GIN now? I tried to rebase packed posting lists > with head. But I found that you've changed interface of placeToPage > function. That's conflicts with packed posting lists, because > dataPlaceToPageLeaf needs not only offset number to describe place to > insert item pointer. Do you like to commit rework of handling GIN > incomplete splits before? > Yeah, I'm planning to get back to this patch after committing the incomplete splits patch. I think the refactoring of the WAL-logging that I did in that patch will simplify this patch, too. I'll take a look at Michael's latest comments on the incomplete splits patch tomorrow, so I should get back to this on Thursday or Friday. >>> >>> Should I try to rebase this patch now or you plan to do it yourself? Some >>> changes like "void *insertdata" argument make me think you have some >>> particular plan to rebase this patch, but I didn't get it exactly. >>> >> >> Here's rebased version. I'll continue reviewing it now.. >> > > Another update. Fixes a bunch of bugs. Mostly introduced by me, but a > couple were present in your v16: > > * When allocating the entry->list buffer in a scan, it must be large > enough for the max number of items that can fit on a compressed page, > whether the current page is compressed or not. That's because the same > buffer is reused on subsequent pages, which might be compressed. > > * When splitting a leaf page during index creation, missed the trick > that's present in current master, to choose the split point so that left > page is packed as full as possible. I put that back, it makes newly-built > indexes somewhat smaller. (I wonder if we should leave some free space for > future updates. But that's a separate patch, let's keep the current > behavior in this patch) > > I'll continue reviewing next week.. Good. Thanks for debug and fixing bugs. Can I do anything for this patch now? -- With best regards, Alexander Korotkov.
Re: [HACKERS] [bug fix] pg_ctl always uses the same event source
From: "Magnus Hagander" Not having looked at it in detail yet, but this seems to completely remove the default value. What happens if the error that needs to be logged is the one saying that it couldn't exec postgres to find out the value in the config file? AFAICT it's going to try to register an eventsource with whatever random garbage happens to be in the variable. Thank you for commenting, Magnus san. The variable is global and contains an empty string, so even in the unlikely situation where postgres -C fails, the event source simply becomes blank. 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] Re: [RFC] Shouldn't we remove annoying FATAL messages from server log?
From: "David Johnston" 5. FATAL: terminating walreceiver process due to administrator command 6. FATAL: terminating background worker \"%s\" due to administrator command 5 and 6: I don't fully understand when they would happen but likely fall into the same "the DBA should know what is going on with their server and confirm any startup/shutdown activity it is involved with". They might be better categorized "NOTICE" level if they were in response to a administrator action, versus in response to a crashed process, but even for the user-initiated situation making sure they hit the log but using FATAL is totally understandable and IMO desirable. #5 is output when the DBA shuts down the replication standby server. #6 is output when the DBA shuts down the server if he is using any custom background worker. These messages are always output. What I'm seeing as a problem is that FATAL messages are output in a normal situation, which worries the DBA, and those messages don't help the DBA with anything. They merely worry the DBA. While "worry" is something to be avoided not logging messages is not the correct solution. On the project side looking for ways and places to better communicate to the user is worthwhile in the absence of adding additional complexity to the logging system. The user/DBA, though, is expected to learn how the proces works, ideally in a testing environment where worry is much less a problem, and understand that some of what they are seeing are client-oriented messages that they should be aware of but that do not indicate server-level issues by themselves. I see, It might makes sense to make the DBA learn how the system works, so that more DBAs can solve problems by themselves. However, I wonder how those messages (just stopping server process by admin request) are useful for the DBA. If they are useful, why don't other background processes (e.g. archiver, writer, walsender, etc.) output such a message when stopping? For information, #5 and #6 are not client-oriented. If we want to keep the messages, I think LOG or DEBUG would be appropriate. How about altering "ereport(FATAL, ...)" to "ereport(LOG, ...); proc_exit(1)"? 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] [RFC] Shouldn't we remove annoying FATAL messages from server log?
From: "Greg Stark" On the client end the FATAL is pretty logical but in the logs it makes it sound like the entire server died. Especially in this day of multithreaded servers. I was suggesting that that was the origin of the confusion here. Anyone who has seen these messages on the client end many times might interpret them correctly in the server logs but someone who has only been a DBA, not a database user might never have seen them except in the server logs and without the context might not realize that FATAL is a term of art peculiar to Postgres. I think so, too. My customers and colleagues, who are relatively new to PostgreSQL, asked like "Is this FATAL message a sign of some problem? What does it mean?" I think it's natural to show concern when finding FATAL messages. I find it unnatural for a normal administration operation to emit a FATAL message. 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] ANALYZE sampling is too good
On Sun, Dec 8, 2013 at 7:03 PM, Josh Berkus wrote: > They will certainly do so if you don't apply any statistical adjustments > for selecting more rows from the same pages. > > So there's a set of math designed to calculate for the skew introduced > by reading *all* of the rows in each block. I just think this is an oversimplification. There's no skew introduced just by reading all the rows in each block unless there's some kind of dependency between the block a row is placed on and the data in it. So I don't believe there can be some single set of math that automatically removes any skew automatically. The math will depend on what the dependency is. Just to be clear, you have to think pretty hard about the way Postgres internals work to see what kinds of skew might be appearing here. Due to the way Postgres updates work and HOT cleanups work "hot" tuples will be weighted less than "cold" tuples. That's not going to be something someone in ACM knew to design into their maths. I do have access to ACM or other academic articles if you remember any author names or any keywords but if it's a database journal I would worry about patent issues. Do you remember if it was over 17 years old? > Obviously, you need to look at more rows than you would with a > pure-random sample. Like I said, the 80%+ accurate point in the papers > seemed to be at a 5% sample. I really don't believe the 5% thing. It's not enough for n_distinct and it's *far* too high a value for linear properties like histograms or nullfrac etc. From a computer point of view it's too high to be worth bothering. If we have to read 5% of the table we might as well do a full scan anyways, it'll be marginally slower but much better quality results. -- greg -- 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] ANALYZE sampling is too good
On 09/12/13 08:03, Josh Berkus wrote: So there's a set of math designed to calculate for the skew introduced by reading *all* of the rows in each block. That's what I meant by "block-based sampling"; you read, say, 400 pages, you compile statistics on *all* of the rows on those pages, you apply some algorithms to adjust for groupings of rows based on how grouped they are. And you have a pretty good estimate of how grouped they are, because you just looked a complete sets of rows on a bunch of nonadjacent pages. Obviously, you need to look at more rows than you would with a pure-random sample. Like I said, the 80%+ accurate point in the papers seemed to be at a 5% sample. However, since those rows come from the same pages, the cost of looking at more rows is quite small, compared to the cost of looking at 64 times as many disk pages. My ACM subscription has lapsed, though; someone with a current ACM subscription could search for this; there are several published papers, with math and pseudocode. This makes more sense! Unfortunately you had confused everyone (well me at least) by decreeing that we needed block based sampling - when we started using that in 2004 - there is more than one way to sample blocks it seems :-) What you are actually suggesting is a way to do block based sampling that requires reading fewer blocks, and that is a great idea! Of course as Greg has just suggested - the details are gonna be the clincher. Can you supply authors or titles of papers? Also with reference to Greenplum - their use of postgres is fairly special case, and an approach that works well for them is not always suitable for more general purpose use. Regards Mark -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Why we are going to have to go DirectIO
On 12/5/13 9:59 AM, Tom Lane wrote: Greg Stark writes: I think the way to use mmap would be to mmap very large chunks, possibly whole tables. We would need some way to control page flushes that doesn't involve splitting mappings and can be efficiently controlled without having the kernel storing arbitrarily large tags on page tables or searching through all the page tables to mark pages flushable. I might be missing something, but AFAICS mmap's API is just fundamentally wrong for this. The kernel is allowed to write-back a modified mmap'd page to the underlying file at any time, and will do so if say it's under memory pressure. You can tell the kernel to sync now, but you can't tell it *not* to sync. I suppose you are thinking that some wart could be grafted onto that API to reverse that, but I wouldn't have a lot of confidence in it. Any VM bug that caused the kernel to sometimes write too soon would result in nigh unfindable data consistency hazards. Something else to ponder on... a Segate researcher gave a talk on upcoming hard drive technology it RICON East this spring. The interesting bit is that 1 or 2 generations down the road HDs will start using "shingling": The write head has to be bigger than the read head, so they're going to set it up so you can not modify a range of tracks after they've been written. They'll do this by keeping a journal inside the HD. This is somewhat similar to how SSDs work too (you can only erase large pages of data, you can't update individual bytes/sectors/filesystem blocks. So long-term, random access updates to permanent storage will be less efficient than today. (Of course, non-volatile memory could turn all this on it's head..) -- 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
[HACKERS] Possible work-around for 9.1 partial vacuum bug?
If I'm understanding the vacuum truncate bug correctly, it can be avoided if every 2^31 transactions[1] you: SET vacuum_freeze_table_age = 0; VACUUM FREEZE; table_age = 0 disables partial vacuum and then everything[1] gets frozen, eliminating the risk. Or am I missing something? [1]: Obviously open transactions mean 2^31 isn't exact. -- 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] Possible work-around for 9.1 partial vacuum bug?
On Mon, Dec 9, 2013 at 6:33 AM, Jim Nasby wrote: > If I'm understanding the vacuum truncate bug correctly, it can be avoided if > every 2^31 transactions[1] you: > > SET vacuum_freeze_table_age = 0; > VACUUM FREEZE; > > table_age = 0 disables partial vacuum and then everything[1] gets frozen, > eliminating the risk. Or am I missing something? Yes, this will fix any latent error by forcing a freeze on all the rows of all tables, but not the ones that already happened if your system has already done more than 2^31 transactions. In this case take care of any constraint violation. Regards, -- Michael -- 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] WITHIN GROUP patch
> "Tom" == Tom Lane writes: Tom> We could alternatively decide that the agg has level 0, but that Tom> doesn't seem terribly useful, and I think it's not per spec Tom> either. SQL:2008 section 6.9 seems Tom> pretty clear that only aggregated arguments should be considered Tom> when determining the semantic level of an aggregate. OTOH, I Tom> don't see any text there restricting what can be in the Tom> non-aggregated arguments, so maybe the committee thinks this Tom> case is sensible? Or they just missed it. My bet is that they missed it, because there's another obvious oversight there; it doesn't define column references in the FILTER clause applied to an ordered set function as being aggregated column references, yet it's clear that this must be the case (since they filter the set of rows that the aggregated column references refer to). -- Andrew (irc:RhodiumToad) -- 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] plpgsql_check_function - rebase for 9.3
On Sun, Dec 8, 2013 at 10:31 PM, Pavel Stehule wrote: > > > > 2013/12/8 Peter Eisentraut >> >> In my opinion, the idea of having a separate lint checker for a language >> is antiquated. If there are problems, they should be diagnosed at >> compile time or run time. You can add options about warning levels or >> strictness if there are concerns about which diagnostics are >> appropriate. > > > There are two points, that should be solved > > a) introduction a dependency to other object in schema - now CREATE FUNCTION > is fully independent on others > > b) slow start - if we check all paths on start, then start can be slower - > and some functions should not work due dependency on temporary tables. > > I am thinking about possible marking a function by #option (we have same > idea) > > some like > > #option check_on_first_start > #option check_on_create > #option check_newer what exactly check_newer means, does it mean whenever a function is replaced (changed)? > But still I have no idea, how to push check without possible slowdown > execution with code duplication 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, Dec 9, 2013 at 2:25 AM, MauMau wrote: > From: "Magnus Hagander" > >> Not having looked at it in detail yet, but this seems to completely remove >> the default value. What happens if the error that needs to be logged is >> the >> one saying that it couldn't exec postgres to find out the value in the >> config file? AFAICT it's going to try to register an eventsource with >> whatever random garbage happens to be in the variable. > > > Thank you for commenting, Magnus san. > The variable is global and contains an empty string, so even in the unlikely > situation where postgres -C fails, the event source simply becomes blank. 1. isn't it better to handle as it is done in write_eventlog() which means if string is empty then use PostgreSQL. "evtHandle = RegisterEventSource(NULL, event_source ? event_source : "PostgreSQL");" 2. What will happen if user doesn't change the name in "event_source" or kept the same name, won't it hit the same problem again? So shouldn't it try to generate different name by appending version string to it? 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] plpgsql_check_function - rebase for 9.3
2013/12/9 Amit Kapila > On Sun, Dec 8, 2013 at 10:31 PM, Pavel Stehule > wrote: > > > > > > > > 2013/12/8 Peter Eisentraut > >> > >> In my opinion, the idea of having a separate lint checker for a language > >> is antiquated. If there are problems, they should be diagnosed at > >> compile time or run time. You can add options about warning levels or > >> strictness if there are concerns about which diagnostics are > >> appropriate. > > > > > > There are two points, that should be solved > > > > a) introduction a dependency to other object in schema - now CREATE > FUNCTION > > is fully independent on others > > > > b) slow start - if we check all paths on start, then start can be slower > - > > and some functions should not work due dependency on temporary tables. > > > > I am thinking about possible marking a function by #option (we have same > > idea) > > > > some like > > > > #option check_on_first_start > > #option check_on_create > > #option check_newer > > what exactly check_newer means, does it mean whenever a function is > replaced (changed)? > > no, it means, so request for check will be ignored ever - some functions cannot be deeply checked due using dynamic SQL or dynamic created data types - temporary tables created in functions. Regards Pavel > > But still I have no idea, how to push check without possible slowdown > > execution with code duplication > > > With Regards, > Amit Kapila. > EnterpriseDB: http://www.enterprisedb.com >
Re: [HACKERS] Why we are going to have to go DirectIO
(2013/12/05 23:42), Greg Stark wrote: On Thu, Dec 5, 2013 at 8:35 AM, KONDO Mitsumasa wrote: Yes. And using something efficiently DirectIO is more difficult than BufferedIO. If we change write() flag with direct IO in PostgreSQL, it will execute hardest ugly randomIO. Using DirectIO presumes you're using libaio or threads to implement prefetching and asynchronous I/O scheduling. I think in the long term there are only two ways to go here. Either a) we use DirectIO and implement an I/O scheduler in Postgres or b) We use mmap and use new system calls to give the kernel all the information Postgres has available to it to control the I/O scheduler. I agree with part of (b) method. I think MMAP API isn't purpose for controling I/O as others saying. And I think posix_fadivse(), sync_file_range() and fallocate() is easier way to be realized better I/O sheduler in Postgres. These systemcall doesn't cause data corruption at all, and we can just use existing implementaion. They effect only perfomance. My survey of posix_fadvise() and sync_file_range() is here. It's simple rule. #Almost my explaining is written in linux man:-) * Optimize readahead in OS [ posix_fadvise() ] These options is for mainly read perfomance. - POSIX_FADV_SEQUENTIAL flag -> Readahead parameter in OS becomes maximum. - POSIX_FADV_RANDOM flag -> Don't use readahead parameter in OS. It can calculate the file cache frequency and efficiency for using the file cache. - POSIX_FADV_NORMAL -> Readahead parameter in OS optimized dynamically in each situasions. If you doesn't judge strategy of disk controlling, we can select this option. It might be good working in almost cases. * Contorol dirty or clean buffer in OS [ posix_fadvise() and sync_file_range() ] These optinos is for write and read perfomance controling in OS file caches. - POSIX_FADV_DONTNEED -> Drop the file cache. If it is dirty, write disk and drop file cache. If it isn't dirty, it only drop from OS file cache. - sync_file_range() -> If you want to write dirty buffer to disk and remain file cache in OS, you can select this system-call. And it can contorol amount of write size. - POSIX_FADV_NOREUSE -> If you think that the file cache will not be needed, we can set this option. The file cache will be drop soon. - POSIX_FADV_WILLNEED -> If you think that the file cache will be important, we can set this option. The file cache will be tend to remain in OS file caches. That's all. Kernel in OS cannot predict IO pattern perfectly in each midlleware, therefore it is optimized by general heuristic algorithms. I think it is right way. However, PostgreSQL can predict IO pattern in part of planner, executer and checkpointer, so we had better set optimum posix_fadvise() flag or sync_file_range() before/after execute general IO systemcall. I think that they will be good IO contoroling and scheduling method without unreliable implementations. 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] Show lossy heap block info in EXPLAIN ANALYZE for bitmap heap scan
Robert Haas wrote: > On Fri, Dec 6, 2013 at 5:02 AM, Etsuro Fujita > wrote: > > Though at first I agreed on this, while working on this I start to > > think information about (2) is enough for tuning work_mem. Here are > > examples using a version under development, where "Bitmap Memory > > Usage" means (peak) memory space used by a TIDBitmap, and "Desired" > > means the memory required to guarantee non-lossy storage of a TID set, > > which is shown only when the TIDBitmap has been lossified. (work_mem > > = 1MB.) > I'd be wary of showing a desired value unless it's highly likely to be > accurate. Thank you for the comments! The desired value is accurately estimated based on (a) the total number of exact/lossy pages stored in the TIDBitmap and (b) the following equation in tbm_create(), except for the GIN case where lossy pages are added to the TIDBitmap by tbm_add_page(). /* * Estimate number of hashtable entries we can have within maxbytes. ... */ nbuckets = maxbytes / (MAXALIGN(sizeof(HASHELEMENT)) + MAXALIGN(sizeof(PagetableEntry)) + sizeof(Pointer) + sizeof(Pointer)); In the GIN case, however, the version under development has a risk of the overestimation. (And in that case, in my understanding, we can't guarantee non-lossy storage of the TIDBitmap any more.) So, for that case, I think to change the message for the desired value a bit. I'll submit the patch later. Thanks, Best regards, Etsuro Fujita -- 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] COPY table FROM STDIN doesn't show count tag
On 29 November 2013 19:20, Rajeev rastogi wrote: > On 26 November 2013, Amit Khandelkar wrote: > > >Can you please submit the \COPY patch as a separate patch ? Since these > are two different issues, I would like to have these two fixed and > committed separately. You can always test the \COPY issue using \COPY TO > followed by INSERT. > > > > Please find the attached two separate patches: > Thanks. > > 1. slashcopyissuev1.patch :- This patch fixes the \COPY issue. > You have removed the if condition in this statement, mentioning that it is always true now: - if (copystream == pset.cur_cmd_source) - pset.lineno++; + pset.lineno++; But copystream can be different than pset.cur_cmd_source , right ? + FILE *copyStream; /* Stream to read/write for copy command */ There is no tab between FILE and *copystream, hence it is not aligned. 2. initialcopyissuev1_ontopofslashcopy.patch : Fix for “COPY table > FROM STDIN/STDOUT doesn't show count tag”. > The following header comments of ProcessResult() need to be modified: * Changes its argument to point to the last PGresult of the command string, * or NULL if that result was for a COPY FROM STDIN or COPY TO STDOUT. Regression results show all passed. Other than this, the patch needs a new regression test. I don't think we need to do any doc changes, because the doc already mentions that COPY should show the COUNT tag, and does not mention anything specific to client-side COPY. > > > Thanks and Regards, > > Kumar Rajeev Rastogi > > > > >
Re: [HACKERS] [GENERAL] pg_upgrade ?deficiency
BTW, thanks to all who helped in improving this issue. Karsten -- GPG key ID E4071346 @ gpg-keyserver.de E167 67FD A291 2BEA 73BD 4537 78B9 A9F9 E407 1346 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
