[HACKERS] Re: [BUGS] BUG #14849: jsonb_build_object doesn't like VARIADIC calls very much
On Wed, Oct 25, 2017 at 5:32 PM, Michael Paquier wrote: > On Mon, Oct 23, 2017 at 6:50 AM, Michael Paquier > wrote: > > Okay, attached is what I think a fully implemented patch should look > > like. On top of what Andrew has done, I added and reworked the > > following: > > - removed duplicate error handling. > > - documented the function in funcapi.h and funcapi.c. > > - Added a new section in funcapi.h to outline that this is for support > > of VARIADIC inputs. > > I have added a commit message as well. Hope this helps. > > For the sake of the archives, the introduction of > extract_variadic_args is committed with f3c6e8a2, and the JSON fixes > with 18fc4ecf. Thanks Andrew for the commit, and thanks Tom, Andrew > and Dmitry for the reviews. Thx yo. .m
Re: [HACKERS] Query started showing wrong result after Ctrl+c
On Thu, Oct 12, 2017 at 12:03 PM, tushar wrote: > postgres=# SELECT * FROM ( SELECT n from tv where n= (select * from > (select n from tv limit 1) c)) as c ; > n > -- > 3713 > (1 row) > > This time , query is started showing wrong result. Is this an expected > behavior and if yes -then how to get the correct result ? The subquery: select n from tv limit 1 could in theory return any row due to the lack of ORDER BY. What I'm guessing happened is that you're seeing a synchronized sequential scan in follow-up queries. Add an ORDER BY. .m
Re: [HACKERS] Index expression syntax
On Fri, Sep 29, 2017 at 9:31 AM, Konstantin Knizhnik < k.knizh...@postgrespro.ru> wrote: > I wonder why syntax error is produced in this case: > > postgres=# create index metaindex on foo using gin(to_tsvector('english', > x)||to_tsvector('english',y)); > ERROR: syntax error at or near "||" > LINE 1: ...taindex on foo using gin(to_tsvector('english', x)||to_tsvec... > ^ > [ .. ] > > *expression:* An expression based on one or more columns of the table. > The expression usually must be written with surrounding parentheses, as > shown in the syntax. However, the parentheses can be omitted if the > expression has the form of a function call. > > So documentations states that sometimes it is possible to avoid > parentheses, but it is unclear why I have to use double parentheses... > I think that either grammar should be fixed, either documentation should > be updated. > Your expression is clearly not a function call, it's a concatenation of two of them. The documentation seems perfectly accurate to me? .m
Re: [HACKERS] 200 = 199 + 1?
On Wed, Sep 27, 2017 at 5:45 PM, Tom Lane wrote: > Marko Tiikkaja writes: > > I wonder if the nested loop shouldn't have some kind of a cap on its own > > estimate if it's wildly off of what you'd get by multiplying the child > > nodes' estimates with each other? > > Nonstarter I'm afraid. The join relation's size estimate is determined > long before we get to a point where we could multiply the sizes of these > particular child paths to arrive at the conclusion that it should be > something different than what we decided originally. Ah hah. Thanks for the explanation, that makes sense. > Adjusting the size > of the nestloop result at that point would merely give it an unfair > advantage over other paths for the same join relation. (I think it would > also break some assumptions about paths for the same relation all giving > the same number of rows, unless parameterized.) > With the previous paragraph in mind, I would agree; it's not a very good idea. > Looking at it another way, the main thing that the combination of hashagg > outer path + indexscan inner path knows that eqjoinsel_semi didn't account > for is that there's a unique index on foo.id. But that info is available > to eqjoinsel_semi, in the sense that it's been given a nondefault estimate > that nd1 is equal to the outer relation size. So the mistake that it's > making is to throw up its hands and use an 0.5 selectivity estimate just > because it has no info about the inner relation. I think if we'd pushed > through the nd2/nd1 calculation after setting nd2 = size of inner rel, > we'd end up with an estimate matching the product of these path sizes. > (Caution: inadequate caffeine absorbed yet, this might be all wrong.) > This sounds very reasonable to me. .m
[HACKERS] 200 = 199 + 1?
Hi, I just came across this very peculiar behavior: =# create table foo(id int primary key); CREATE TABLE =# insert into foo select generate_series(1, 100); INSERT 0 100 =# set enable_hashjoin to false; set enable_mergejoin to false; SET SET =# explain select * from foo where id in (select i from generate_series(1, 200) i limit 199); QUERY PLAN Nested Loop (cost=4.90..1648.52 rows=199 width=4) -> HashAggregate (cost=4.48..6.47 rows=199 width=4) Group Key: i.i -> Limit (cost=0.00..1.99 rows=199 width=4) -> Function Scan on generate_series i (cost=0.00..10.00 rows=1000 width=4) -> Index Only Scan using foo_pkey on foo (cost=0.42..8.24 rows=1 width=4) Index Cond: (id = i.i) (7 rows) =# explain select * from foo where id in (select i from generate_series(1, 200) i limit 200); QUERY PLAN Nested Loop (cost=4.93..1653.00 rows=50 width=4) -> HashAggregate (cost=4.50..6.50 rows=200 width=4) Group Key: i.i -> Limit (cost=0.00..2.00 rows=200 width=4) -> Function Scan on generate_series i (cost=0.00..10.00 rows=1000 width=4) -> Index Only Scan using foo_pkey on foo (cost=0.42..8.22 rows=1 width=4) Index Cond: (id = i.i) (7 rows) So it seems that once the HashAggregate estimates to return 200 rows or more, something extremely weird happens and the Nested Loop's estimate goes wild. I've recently seen numerous instances of this kind of a problem where the row estimates from a nested loop's child nodes are very reasonable but the loop itself goes absolutely nuts. I can't see how this can possibly be justified. I wonder if the nested loop shouldn't have some kind of a cap on its own estimate if it's wildly off of what you'd get by multiplying the child nodes' estimates with each other? .m
Re: [HACKERS] [PATCH] Pattern based listeners for asynchronous messaging (LISTEN/NOTIFY)
Hi Markus, On Sun, Aug 20, 2017 at 9:56 PM, Markus Sintonen wrote: > I also encountered this when I built it with different configuration. I > attached updated patch with the correct number of arguments to > 'similar_escape'. I also added preliminary documentation to the patch. > (Unfortunately unable to currently compile the documentation for testing > purpose on Windows probably because of commit https://github.com/post > gres/postgres/commit/510074f9f0131a04322d6a3d2a51c87e6db243f9. I followed > https://www.postgresql.org/docs/devel/static/install-windows > -full.html#idm45412738673840.) > > What do you think about the syntax? There was a suggestion to specify type > of the pattern (eg ltree extension) but to me this feels like a overkill. > One option here would be eg: > LISTEN PATTERN 'foo%' TYPE 'similar' > LISTEN PATTERN 'foo.*' TYPE 'ltree' > Not that my opinion matters, but I think we should pick one pattern style and stick to it. SIMILAR TO doesn't seem like the worst choice. ltree seems useless. As for the rest of the interface.. First, I think mixing patterns and non-patterns is weird. This is apparent in at least two cases: marko=# listen "foo%"; LISTEN marko=# listen similar to 'foo%'; LISTEN marko=# select * from pg_listening_channels(); pg_listening_channels --- foo% (1 row) -- Not actually listening on the pattern. Confusion. The second case being the way UNLISTEN can be used to unlisten patterns, too. It kind of makes sense given that you can't really end up with both a channel name and a pattern with the same source string, but it's still weird. I think it would be much better to keep these completely separate so that you could be listening on both the channel "foo%" and the pattern 'foo%' at the same time, and you'd use a bare UNLISTEN to unsubscribe from the former, and UNLISTEN SIMILAR TO for the latter. As you said in the original email: > Allow *UNLISTEN SIMILAR TO 'xxx'* which would unregister matching listeners. To me this feels confusing therefore it is not in the patch. I agree, starting to match the patterns themselves would be confusing. So I think we should use that syntax for unsubscribing from patterns. But others should feel free to express their opinions on this. Secondly -- and this is a kind of continuation to my previous point of conflating patterns and non-patterns -- I don't think you can get away with not changing the interface for pg_listening_channels(). Not knowing which ones are compared byte-for-byte and which ones are patterns just seems weird. As for the patch itself, I have a couple of comments. I'm writing this based on the latest commit in your git branch, commit fded070f2a56024f931b9a0f174320eebc362458. In queue_listen(), the new variables would be better declared at the innermost scope possible. The datum is only used if isSimilarToPattern is true, errormsg only if compile_regex didn't return REG_OKAY, etc.. I found this comment confusing at first: "If compiled RE was not applied as a listener then it is freed at transaction commit." The past tense makes it seem like something that has already happened when that code runs, when in reality it happens later in the transaction. I'm not a fan of the dance you're doing with pcompreg. I think it would be better to optimistically allocate the ListenAction struct and compile directly into actrec->compiledRegex. The changed DEBUG1 line in Async_Listen should include whether it's a pattern or not. I don't understand why the return value of Exec_UnlistenAllCommit() was changed at all. Why do we need to do something different based on whether listenChannels was empty or not? The same goes for Exec_UnlistenCommit. This looks wrong in isolationtester.c: @@ -487,7 +488,7 @@ run_permutation(TestSpec *testspec, int nsteps, Step **steps) res = PQexec(conns[0], testspec->setupsqls[i]); if (PQresultStatus(res) == PGRES_TUPLES_OK) { - printResultSet(res); + printResultSet(res, conns[i + 1]); (conns[0] vs. conns[i + 1]). Moving to Waiting on Author. .m
Re: [HACKERS] INSERT .. ON CONFLICT DO SELECT [FOR ..]
On Mon, Sep 4, 2017 at 7:46 PM, Peter Geoghegan wrote: > On Mon, Sep 4, 2017 at 10:05 AM, Marko Tiikkaja wrote: > But I'm generally against > > interfaces which put arbitrary restrictions on what power users can do on > > the basis that less experienced people might misuse the interface. > > I agree with that as a broad principle, but disagree that it applies > here. Or if it does, then you have yet to convince me of it. In all > sincerity, if you think that I just don't understand your perspective, > then please try to make me understand it. Would a power user actually > miss ON CONFLICT DO SELECT? And if that's the case, would it really be > something they'd remember 5 minutes later? > I don't know, but I don't want to be limiting what people can do just because I can't come up with a use case. In any case, others, feel free to chime in. .m
Re: [HACKERS] INSERT .. ON CONFLICT DO SELECT [FOR ..]
On Mon, Sep 4, 2017 at 4:09 AM, Peter Geoghegan wrote: > On Tue, Aug 15, 2017 at 12:17 AM, Marko Tiikkaja wrote: > > On Tue, Aug 15, 2017 at 7:43 AM, Peter Geoghegan wrote: > >> > >> On Mon, Aug 14, 2017 at 6:23 PM, Marko Tiikkaja wrote: > >> > Attached is a patch for $SUBJECT. It might still be a bit rough > around > >> > the > >> > edges and probably light on docs and testing, but I thought I'd post > it > >> > anyway so I won't forget. > >> > >> Is it possible for ON CONFLICT DO SELECT to raise a cardinality > >> violation error? Why or why not? > > > > > > No. I don't see when that would need to happen. But I'm guessing you > have > > a case in mind? > > Actually, no, I didn't. But I wondered if you did. I think that it > makes some sense not to, now that I think about it. ON CONFLICT DO > NOTHING doesn't have cardinality violations, because it cannot affect > a row twice if there are duplicates proposed for insertion (at least, > not through any ON CONFLICT related codepath). But, this opinion only > applies to ON CONFLICT DO SELECT, not ON CONFLICT DO SELECT FOR > UPDATE. And I have other reservations, which I'll go in to > momentarily, about ON CONFLICT DO SELECT in general. So, the upshot is > that I think we need cardinality violations in all cases for this > feature. Why would a user ever not want to know that the row was > locked twice? > I had to look at the patch to see what I'd done, and the tests suggest that we already complain about this with if a FOR [lock level] clause is present: +begin transaction isolation level read committed; +insert into selfconflict values (10,1), (10,2) on conflict(f1) do select for update returning *; +ERROR: ON CONFLICT command cannot affect row a second time +HINT: Ensure that no rows proposed for insertion within the same command have duplicate constrained values. +commit; (in expected/insert_conflict.out.) > On to the subject of my more general reservation: Why did you include > ON CONFLICT DO SELECT at all? Why not just ON CONFLICT DO SELECT FOR > UPDATE (and FOR SHARE, ...) ? > If you don't intend to actually do anything with the row in the same database transaction, locking seems unnecessary. For example, you might want to provide an idempotent method in your API which inserts the data and returns the ID to the caller. The transaction is committed by the time the client sees the data, so locking is just extra overhead. > I think I know what you're going to say about it: ON CONFLICT DO > NOTHING doesn't lock the conflicting row, so why should I insist on it > here (why not have an ON CONFLICT DO SELECT variant, too)? > I wasn't going to say that, no. > In other words, while ON CONFLICT DO NOTHING may have set a precedent > here, it at least has semantics that limit the consequences of not > locking the row; and it *feels* a little bit dirty to use it > indifferently, even where that makes sense. ON CONFLICT DO SELECT is > probably going to be used within wCTEs some of the time. I'm not sure > that a plain ON CONFLICT DO SELECT variant won't allow unpredictable, > complicated problems when composed within a more complicated query. > Yeah, in most cases you'd probably do a SELECT FOR KEY SHARE. And I wouldn't mind that being the default, but it would mean inventing special syntax with no previous precedent (as far as I know) for not locking the row in cases where the user doesn't want that. And that doesn't seem too attractive, either. Maybe it would be better to make the default sensible for people who are not super familiar with postgres. I don't know. And the more I think about the use case above, the less I care about it. But I'm generally against interfaces which put arbitrary restrictions on what power users can do on the basis that less experienced people might misuse the interface. .m
Re: [HACKERS] INSERT .. ON CONFLICT DO SELECT [FOR ..]
On Tue, Aug 15, 2017 at 7:43 AM, Peter Geoghegan wrote: > On Mon, Aug 14, 2017 at 6:23 PM, Marko Tiikkaja wrote: > > Attached is a patch for $SUBJECT. It might still be a bit rough around > the > > edges and probably light on docs and testing, but I thought I'd post it > > anyway so I won't forget. > > Is it possible for ON CONFLICT DO SELECT to raise a cardinality > violation error? Why or why not? No. I don't see when that would need to happen. But I'm guessing you have a case in mind? .m
[HACKERS] INSERT .. ON CONFLICT DO SELECT [FOR ..]
Hi, Attached is a patch for $SUBJECT. It might still be a bit rough around the edges and probably light on docs and testing, but I thought I'd post it anyway so I won't forget. .m insert_conflict_select_v1.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] Allow INSTEAD OF DELETE triggers to modify the tuple for RETURNING
On Fri, Jul 1, 2016 at 2:12 AM, I wrote: > Currently the tuple returned by INSTEAD OF triggers on DELETEs is only > used to determine whether to pretend that the DELETE happened or not, which > is often not helpful enough; for example, the actual tuple might have been > concurrently UPDATEd by another transaction and one or more of the columns > now hold values different from those in the planSlot tuple. Attached is an > example case which is impossible to properly implement under the current > behavior. For what it's worth, the current behavior seems to be an > accident; before INSTEAD OF triggers either the tuple was already locked > (in case of BEFORE triggers) or the actual pre-DELETE version of the tuple > was fetched from the heap. > > So I'm suggesting to change this behavior and allow INSTEAD OF DELETE > triggers to modify the OLD tuple and use that for RETURNING instead of > returning the tuple in planSlot. Attached is a WIP patch implementing that. > > Is there any reason why we wouldn't want to change the current behavior? Since nobody seems to have came up with a reason, here's a patch for that with test cases and some documentation changes. I'll also be adding this to the open commit fest, as is customary. .m instead_of_delete_returning_v2.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table
On Tue, Jun 6, 2017 at 10:25 PM, Kevin Grittner wrote: > Nice as it would be to add a SQL standard feature and advance the > effort to get to incremental maintenance of materialized views, and > much as I really appreciate the efforts Thomas has put into trying > to solve these problems, I agree that it is best to revert the > feature. It took years to get an in-depth review, then I was asked > not to commit it because others were working on patches that would > conflict. That just doesn't leave enough time to address these > issues before release. Fundamentally, I'm not sure that there is a > level of interest sufficient to support the effort. > > I'll give it a few days for objections before reverting. > I can only say that the lack of this feature comes up on a weekly basis on IRC, and a lot of people would be disappointed to see it reverted. .m
[HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table
Since the subject of transition tables came up, I thought I'd test how this case works: =# create table qwr(a int); CREATE TABLE =# create function qq() returns trigger as $$ begin raise notice '%', (select count(*) from oo); return null; end $$ language plpgsql; CREATE FUNCTION =# create trigger xx after insert on qwr referencing new table as oo for each statement execute procedure qq(); CREATE TRIGGER =# with t as (insert into qwr values (1)) insert into qwr values (2), (3); NOTICE: 3 NOTICE: 3 INSERT 0 2 to me, this means that it doesn't work. Surely one of the trigger invocations should say 1, and the other 2. Or was this intentional? .m
Re: [HACKERS] COMPRESS VALUES feature request
On Tue, May 9, 2017 at 8:18 PM, Erez Segal wrote: > In the IRC channel - johto suggested an implementation: > > if you want to get really fancy you could have two columns where > only one of set; one would be the value (if reasonably unique) and the > other the id (if not) > I only suggested how to do this in an application if one really wants the feature so badly. I don't think this would fly as a native implementation. > I'd like to add that an ENUM can be used instead of the id+lookup table in > the 2nd column for non unique values. > An ENUM in postgres *is* an id + lookup table.. .m
Re: [HACKERS] Why type coercion is not performed for parameters?
On Fri, May 5, 2017 at 10:58 AM, Konstantin Knizhnik < k.knizh...@postgrespro.ru> wrote: > If I evaluate expression typename('literal'), then type coercion is > performed and the function is successfully resolved, i.e. > > SELECT regnamespace('"pg_catalog"'); > > But if I want to prepare this query, I get the error: > > postgres=# prepare foo as SELECT regnamespace($1); > ERROR: function regnamespace(unknown) does not exist > LINE 1: prepare foo as SELECT regnamespace($1); > > Certainly, I can explicitly specify parameter type: > > prepare foo (text) as SELECT regnamespace($1); > > and it will work. But it is not always possible. > There are other similar examples which have even bigger issues, such as now() - interval '6 hours'. now() - interval $1 won't even parse. > Why do I need it? I want to implement autoprepare. > My original intention was to let parse_analyze_varparams to infer type of > parameters from the context. > But it is not always possible and sometime leads to different behavior of > query. > For example if the query: > > select count(*) from test_range_gist where ir @> 10; > > is replaced with > > select count(*) from test_range_gist where ir @> $1; > > then type of parameter will be int4range rather then int, which > corresponds to the different operator. > But you know that the type of the literal "10" is int. If you're throwing that information away, surely that's a bug in your code. .m
Re: [HACKERS] PG 10 release notes
On Thu, Apr 27, 2017 at 4:13 PM, Bruce Momjian wrote: > On Thu, Apr 27, 2017 at 08:00:28AM +0530, Amit Kapila wrote: > > > Oh, so non-correlated subqueries can be run in parallel. Yes, that is > > > something we should have in the release notes. How is this? > > > > > > Author: Robert Haas > > > 2017-02-14 [5e6d8d2bb] Allow parallel workers to execute > subplans. > > > > > > Allow non-correlated subqueries to be run in parallel (Amit > Kapila) > > > > > > > Looks good to me. It's not clear from this item what the previous behavior was. How about adding something like "Correlated subqueries can not yet be parallelized."? (I'm guessing that's the change, anyway, please correct me if I'm mistaken.) .m
[HACKERS] Logical replication SnapBuildInitalSnapshot spelling
Hi, Commit 7c4f52409a8c7d85ed169bbbc1f6092274d03920 seems to have introduced an alternative spelling of "initial". Fixed in the attached. .m logical_inital.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] Contrib: pqasyncnotifier.c -- a shell command client for LISTEN
On Tue, Jan 24, 2017 at 12:40 AM, Nico Williams wrote: > psql(1) does not output notifications asynchronously, as it does not > check for them when idle. This makes it difficult to script handling of > NOTIFYs. > > Attached is pqasyncnotifier.c, a simple command that allows one to > handle NOTIFYs asynchronously. > Did you forget the attachment? .m
Re: [HACKERS] plpgsql - additional extra checks
On Fri, Jan 13, 2017 at 2:46 AM, Jim Nasby wrote: > On 1/11/17 5:54 AM, Pavel Stehule wrote: > >> +too_many_rows >> + >> + >> + When result is assigned to a variable by INTO >> clause, >> + checks if query returns more than one row. In this case the >> assignment >> + is not deterministic usually - and it can be signal some issues in >> design. >> > > Shouldn't this also apply to > > var := blah FROM some_table WHERE ...; > > ? > > AIUI that's one of the beefs the plpgsql2 project has. > No, not at all. That syntax is undocumented and only works because PL/PgSQL is a hack internally. We don't use it, and frankly I don't think anyone should. .m
Re: [HACKERS] plpgsql - additional extra checks
On Wed, Jan 11, 2017 at 2:54 PM, Pavel Stehule wrote: > 1. strict_multi_assignment - checks the number of target variables and > source values. > I've proposed this before (maybe around a year ago), except the checks were done at parse time, rather than runtime. I much prefer that approach. If I recall correctly, the patch was considered to be good, but not good enough since it didn't handle all contexts (perhaps FOR loop were missing, or something; I forget). > 2. too_many_rows - checks if returned rows is more than one > I've also proposed this, and it was rejected because it was a runtime check, and some people don't like runtime checks. .m
Re: [HACKERS] merging some features from plpgsql2 project
On Tue, Jan 10, 2017 at 2:26 PM, Peter Eisentraut < peter.eisentr...@2ndquadrant.com> wrote: > It's not like PL/pgSQL is the king of brevity. This is essentially saying "PL/PgSQL isn't perfect, so we shouldn't try and make it better". I hear this argument a lot, and as long as people keep rejecting improvements for this reason they can keep saying it. It's a self-fulfilling prophecy. .m
Re: [HACKERS] merging some features from plpgsql2 project
On Tue, Jan 10, 2017 at 1:03 AM, Jim Nasby wrote: > On 1/9/17 5:53 PM, Marko Tiikkaja wrote: > >> My idea was that the currently unsupported combination of NOT >> NULL and >> no DEFAULT would mean "has to be assigned to a non-NULL value >> before it >> can be read from, or an exception is thrown". Solves the most >> common >> use case and is backwards compatible. >> >> >> That won't allow you to use a variable in multiple places though... >> is there a reason we couldn't support something like IS DEFINED and >> UNSET? >> >> >> I don't understand what your use case is. Could you demonstrate that >> with some code you'd write if these features were in? >> > > One use case is NEW and OLD in triggers. Checking to see if one or the > other is set is easier than checking TG_OP. It's also going to be faster > (probably MUCH faster; IIRC the comparison currently happens via SPI). > This sounds useless. > Another case is selecting into a record: > > EXECUTE ... INTO rec; > IF rec IS DEFINED THEN > ELSE > EXECUTE INTO rec; > IF rec IS DEFINED THEN > And this a workaround for non-functional FOUND. I can't get excited about this idea based on these examples. .m
Re: [HACKERS] merging some features from plpgsql2 project
On Tue, Jan 10, 2017 at 12:47 AM, Jim Nasby wrote: > On 1/9/17 5:30 PM, Marko Tiikkaja wrote: > My idea was that the currently unsupported combination of NOT NULL and >> no DEFAULT would mean "has to be assigned to a non-NULL value before it >> can be read from, or an exception is thrown". Solves the most common >> use case and is backwards compatible. >> > > That won't allow you to use a variable in multiple places though... is > there a reason we couldn't support something like IS DEFINED and UNSET? > I don't understand what your use case is. Could you demonstrate that with some code you'd write if these features were in? .m
Re: [HACKERS] merging some features from plpgsql2 project
On Mon, Jan 9, 2017 at 12:37 AM, Jim Nasby wrote: > If we're going to create a brand new language then I think it would be > extremely foolish to keep *any* of the current pain points around. Off the > top of my head: > > - variables must have an identifier (what $ in most languages does). The > steps you have to go through to avoid simple naming collisions are insane. > This is exactly what we did not want to do with this project. The idea is to create a language which is really close to PL/PgSQL, but removes some of the brain diarrhoea currently present. Now, this *is* a problem, and the solution we had (well I, mostly, at this point) in mind is to use the underscore prefix for all input variables and make OUT parameters invisible to queries inside function bodies unless explicitly prefixed with OUT. As far as I can tell this eliminates most if not all collisions while staying almost completely compatible with arguably well-written PL/PgSQL 1. - Support for the notion of a variable being unset (which is NOT the same > thing as NULL). > My idea was that the currently unsupported combination of NOT NULL and no DEFAULT would mean "has to be assigned to a non-NULL value before it can be read from, or an exception is thrown". Solves the most common use case and is backwards compatible. .m
Re: [HACKERS] SELECT FOR UPDATE regression in 9.5
On 07/09/16 7:29 PM, Alvaro Herrera wrote: Marko, does this fix your reported problem too? Both the assertion and the overall test case that causes it to fire? The test case never realized anything was wrong, but the assertion is gone. So yup, problem solved on this end, at least. .m -- 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] kqueue
On 2016-06-03 01:45, Thomas Munro wrote: On Fri, Jun 3, 2016 at 4:02 AM, Alvaro Herrera wrote: Tom Lane wrote: Andres Freund writes: pg_strtoi? I think that's what Thomas did upthread. Are you taking this one then? I'd go with just "strtoint". We have "strtoint64" elsewhere. For closure of this subthread: this rename was committed by Tom as 0ab3595e5bb5. Thanks. And here is a new version of the kqueue patch. The previous version doesn't apply on top of recent commit a3b30763cc8686f5b4cd121ef0bf510c1533ac22, which sprinkled some MAXALIGN macros nearby. I've now done the same thing with the kevent struct because it's cheap, uniform with the other cases and could matter on some platforms for the same reason. I've tested and reviewed this, and it looks good to me, other than this part: + /* +* kevent guarantees that the change list has been processed in the EINTR +* case. Here we are only applying a change list so EINTR counts as +* success. +*/ this doesn't seem to be guaranteed on old versions of FreeBSD or any other BSD flavors, so I don't think it's a good idea to bake the assumption into this code. Or what do you think? .m -- 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] SELECT FOR UPDATE regression in 9.5
On 2016-09-06 6:02 PM, Marti Raudsepp wrote: This issue is also reproducible on the current master branch. In an assertions-enabled build, it traps an assertion in HeapTupleHeaderGetCmax called by heap_lock_tuple. The following test case demonstrates the issue... I think you found a reproducible test case for my bug in 48d3eade-98d3-8b9a-477e-1a8dc32a7...@joh.to. Thanks. .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Change error code for hstore syntax error
Hi Sherrylyn, On 2016-05-09 19:42, Sherrylyn Branchaw wrote: I'm attaching a revised patch; please let me know if there are any other issues before I submit to the commitfest. I think this is mostly good, but these two should be changed: errmsg("unexpected end of string: \"%s\"", state->begin) errmsg("syntax error at position %d: \"%s\"", ...) Right now, aside from the error code, these two look like they're reporting about an error in the SQL statement itself, and not in an input value for a type. I think they should look more like this: errmsg("invalid input syntax for type hstore: \"%s\"", string), errdetail("Unexpected end of input.") If possible, it might also make sense to provide more information than "unexpected end of string". For example: what character were you expecting to find, or what were you scanning? I didn't look too closely what exactly could be done here. I'll leave that part to you. .m -- 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] INSERT .. SET syntax
Hello hello, Here's a rebased and updated patch for $SUBJECT for the September commit fest. .m *** a/doc/src/sgml/ref/insert.sgml --- b/doc/src/sgml/ref/insert.sgml *** *** 22,33 PostgreSQL documentation [ WITH [ RECURSIVE ] with_query [, ...] ] ! INSERT INTO table_name [ AS alias ] [ ( column_name [, ...] ) ] ! { DEFAULT VALUES | VALUES ( { expression | DEFAULT } [, ...] ) [, ...] | query } [ ON CONFLICT [ conflict_target ] conflict_action ] [ RETURNING * | output_expression [ [ AS ] output_name ] [, ...] ] ! where conflict_target can be one of: ( { index_column_name | ( index_expression ) } [ COLLATE collation ] [ opclass ] [, ...] ) [ WHERE index_predicate ] ON CONSTRAINT constraint_name --- 22,42 [ WITH [ RECURSIVE ] with_query [, ...] ] ! INSERT INTO table_name [ AS alias ] ! { ! [ column_list ] VALUES ( { expression | DEFAULT } [, ...] ) [, ...] | ! [ column_list ] query | ! DEFAULT VALUES | ! SET column_name = { expression | DEFAULT } [, ...] ! } [ ON CONFLICT [ conflict_target ] conflict_action ] [ RETURNING * | output_expression [ [ AS ] output_name ] [, ...] ] ! where column_list is: ! ! ( column_name [, ...] ) ! ! and conflict_target can be one of: ( { index_column_name | ( index_expression ) } [ COLLATE collation ] [ opclass ] [, ...] ) [ WHERE index_predicate ] ON CONSTRAINT constraint_name *** *** 53,65 INSERT INTO table_name [ AS !The target column names can be listed in any order. If no list of !column names is given at all, the default is all the columns of the !table in their declared order; or the first N column !names, if there are only N columns supplied by the !VALUES clause or query. The values !supplied by the VALUES clause or query are !associated with the explicit or implicit column list left-to-right. --- 62,87 !The target column names in a column_list can be !listed in any order. If no column_list is given at !all (and the SET syntax is not used), the default is all !the columns of the table in their declared order; or the first !N column names, if there are only N !columns supplied by the VALUES clause or !query. The values supplied by the VALUES !clause or query are associated with the explicit or !implicit column list left-to-right. ! ! ! ! Instead of a column_list and a VALUES ! clause, a SET clause similar to that of an ! UPDATE can be used instead. The advantage of the ! SET clause is that instead of matching the elements in ! the two lists by ordinal position, the column name and the ! expression to assign to that column are visually next to each other. ! This can make long column assignment lists significantly more ! readable. *** *** 690,702 INSERT INTO distributors (did, dname) VALUES (10, 'Conrad International') INSERT conforms to the SQL standard, except that the RETURNING clause is a !PostgreSQL extension, as is the ability !to use WITH with INSERT, and the ability to !specify an alternative action with ON CONFLICT. !Also, the case in !which a column name list is omitted, but not all the columns are !filled from the VALUES clause or query, !is disallowed by the standard. --- 712,724 INSERT conforms to the SQL standard, except that the RETURNING clause is a !PostgreSQL extension, as is the !SET clause when used instead of a VALUES clause, the !ability to use WITH with INSERT, and the !ability to specify an alternative action with ON !CONFLICT. Also, the case in which a column name list is omitted, !but not all the columns are filled from the VALUES clause !or query, is disallowed by the standard. *** a/src/backend/parser/analyze.c --- b/src/backend/parser/analyze.c *** *** 467,474 transformInsertStmt(ParseState *pstate, InsertStmt *stmt) stmt->onConflictClause->action == ONCONFLICT_UPDATE); /* ! * We have three cases to deal with: DEFAULT VALUES (selectStmt == NULL), ! * VALUES list, or general SELECT input. We special-case VALUES, both for * efficiency and so we can handle DEFAULT specifications. * * The grammar allows attaching ORDER BY, LIMIT, FOR UPDATE, or WITH to a --- 467,475 stmt->onConflictClause->action == ONCONFLICT_UPDATE); /* ! * We have four cases to deal with: DEFAULT VALUES (selectStmt == NULL and ! * cols == NIL), SET syntax (selectStmt == NULL but cols != NIL), VALUES ! * list, or general SELECT input. We special-case VALUES, both for * efficiency and so we can handle DEFAULT specifications. * * The grammar allows attaching ORDER BY, LIMIT, FOR UPDATE, or WITH to a *** *** 523,529 transfo
Re: [HACKERS] Assertion failure in REL9_5_STABLE
On 11/08/16 8:48 AM, Michael Paquier wrote: On Thu, Aug 11, 2016 at 8:09 AM, Marko Tiikkaja wrote: On 2016-08-11 12:09 AM, Alvaro Herrera wrote: BTW this is not a regression, right? It's been broken all along. Or am I mistaken? You're probably right. I just hadn't realized I could run our app against 9.5 with --enable-cassert until last week. Just wondering... If you revert 1f9534b4 and/or b33e81cb do you still see a problem? Yeah, no effect. .m -- 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] Assertion failure in REL9_5_STABLE
On 2016-08-11 12:09 AM, Alvaro Herrera wrote: BTW this is not a regression, right? It's been broken all along. Or am I mistaken? You're probably right. I just hadn't realized I could run our app against 9.5 with --enable-cassert until last week. .m -- 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] Assertion failure in REL9_5_STABLE
On 2016-08-10 11:01 PM, Alvaro Herrera wrote: Oh, I see ... so there's an update chain, and you're updating a earlier tuple. But the later tuple has a multixact and one of the members is the current transaction. I wonder how can you lock a tuple that's not the latest, where that update chain was produced by your own transaction. I suppose this is because of plpgsql use of cursors. There's a rolled back subtransaction that also did some magic on the rows AFAICT. I can try and spend some time producing a smaller test case over the weekend. No hurry since this missed the today's point release anyway. .m -- 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] Assertion failure in REL9_5_STABLE
On 2016-08-10 19:28, Alvaro Herrera wrote: Umm. AFAICS HeapTupleSatisfiesUpdate() only returns SelfUpdated after already calling HeapTupleHeaderGetCmax() (which obviously hasn't caught the same assertion). Something is odd there ... HeapTupleSatisfiesUpdate() returns HeapTupleBeingUpdated in this case. HeapTupleSelfUpdated comes from here (line 4749): /* if there are updates, follow the update chain */ if (follow_updates && !HEAP_XMAX_IS_LOCKED_ONLY(infomask)) { HTSU_Result res; res = heap_lock_updated_tuple(relation, tuple, &t_ctid, GetCurrentTransactionId(), mode); if (res != HeapTupleMayBeUpdated) { result = res; /* recovery code expects to have buffer lock held */ LockBuffer(*buffer, BUFFER_LOCK_EXCLUSIVE); goto failed; } } Can you share the test case? Not at this time, sorry. .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Assertion failure in REL9_5_STABLE
Hi, Running one specific test from our application against REL9_5_STABLE (current as of today) gives me this: #2 0x7effb59595be in ExceptionalCondition ( conditionName=conditionName@entry=0x7effb5b27a88 "!(CritSectionCount > 0 || TransactionIdIsCurrentTransactionId(( (!((tup)->t_infomask & 0x0800) && ((tup)->t_infomask & 0x1000) && !((tup)->t_infomask & 0x0080)) ? HeapTupleGetUpdateXid(tup) : ( (tup)-"..., errorType=errorType@entry=0x7effb599b74b "FailedAssertion", fileName=fileName@entry=0x7effb5b2796c "combocid.c", lineNumber=lineNumber@entry=132) at assert.c:54 #3 0x7effb598e19b in HeapTupleHeaderGetCmax (tup=0x7effa85714c8) at combocid.c:131 #4 0x7effb55fb0c1 in heap_lock_tuple (relation=0x7effb5bf5d90, tuple=tuple@entry=0x7fffcee73690, cid=346, mode=mode@entry=LockTupleExclusive, wait_policy=LockWaitBlock, follow_updates=follow_updates@entry=1 '\001', buffer=buffer@entry=0x7fffcee7367c, hufd=hufd@entry=0x7fffcee73680) at heapam.c:4813 #5 0x7effb5753e82 in ExecLockRows (node=node@entry=0x7effb6cebb00) at nodeLockRows.c:179 #6 0x7effb573cbc8 in ExecProcNode (node=node@entry=0x7effb6cebb00) at execProcnode.c:516 #7 0x7effb5739432 in ExecutePlan (dest=0x7effb5dd8160 , direction=, numberTuples=0, sendTuples=1 '\001', operation=CMD_SELECT, planstate=0x7effb6cebb00, estate=0x7effb6ceb8f8) at execMain.c:1549 #8 standard_ExecutorRun (queryDesc=0x7effb6ae3b40, direction=out>, count=0) at execMain.c:337 #9 0x7effb57661db in _SPI_pquery (tcount=0, fire_triggers=1 '\001', queryDesc=0x7effb6ae3b40) at spi.c:2411 The failure is a number of levels down a call stack of PL/PgSQL functions, but I can reproduce it at will by calling the function. I unfortunately can't narrow it down to a smaller test case, but attached is an xlogdump of the session. The query that finally breaks the elephant's back is a SELECT .. FOR UPDATE from relid=54511. Any ideas on how to debug this? .m rmgr: Standby len (rec/tot): 24/50, tx: 0, lsn: 0/2428, prev 0/23000108, desc: RUNNING_XACTS nextXid 8495 latestCompletedXid 8494 oldestRunningXid 8495 rmgr: Transaction len (rec/tot): 12/38, tx: 8496, lsn: 0/2460, prev 0/2428, desc: ASSIGNMENT xtop 8495: subxacts: 8496 rmgr: Heaplen (rec/tot): 7/ 1042, tx: 8496, lsn: 0/2488, prev 0/2460, desc: LOCK off 10: xid 8496 LOCK_ONLY EXCL_LOCK KEYS_UPDATED , blkref #0: rel 1663/47473/50461 blk 0 FPW rmgr: Heaplen (rec/tot): 7/ 8246, tx: 8496, lsn: 0/240004A0, prev 0/2488, desc: LOCK off 48: xid 8496 LOCK_ONLY EXCL_LOCK KEYS_UPDATED , blkref #0: rel 1663/47473/51018 blk 81 FPW rmgr: Heaplen (rec/tot): 7/53, tx: 8496, lsn: 0/240024F0, prev 0/240004A0, desc: LOCK off 49: xid 8496 LOCK_ONLY EXCL_LOCK KEYS_UPDATED , blkref #0: rel 1663/47473/51018 blk 81 rmgr: Sequencelen (rec/tot):158/ 204, tx: 8496, lsn: 0/24002528, prev 0/240024F0, desc: LOG rel 1663/47473/49387, blkref #0: rel 1663/47473/49387 blk 0 rmgr: Sequencelen (rec/tot):158/ 204, tx: 0, lsn: 0/240025F8, prev 0/24002528, desc: LOG rel 1663/47473/49992, blkref #0: rel 1663/47473/49992 blk 0 rmgr: Heaplen (rec/tot): 3/ 4404, tx: 8497, lsn: 0/240026C8, prev 0/240025F8, desc: INSERT off 74, blkref #0: rel 1663/47473/49994 blk 18 FPW rmgr: Btree len (rec/tot): 2/ 6961, tx: 8497, lsn: 0/24003800, prev 0/240026C8, desc: INSERT_LEAF off 78, blkref #0: rel 1663/47473/59412 blk 2 FPW rmgr: Btree len (rec/tot): 2/ 6933, tx: 8497, lsn: 0/24005350, prev 0/24003800, desc: INSERT_LEAF off 342, blkref #0: rel 1663/47473/59414 blk 8 FPW rmgr: Heaplen (rec/tot): 7/ 4910, tx: 8497, lsn: 0/24006E80, prev 0/24005350, desc: LOCK off 3: xid 8497 LOCK_ONLY KEYSHR_LOCK , blkref #0: rel 1663/47473/50005 blk 0 FPW rmgr: Heaplen (rec/tot): 3/87, tx: 8498, lsn: 0/240081C8, prev 0/24006E80, desc: INSERT off 75, blkref #0: rel 1663/47473/49994 blk 18 rmgr: Btree len (rec/tot): 2/ 4273, tx: 8498, lsn: 0/24008220, prev 0/240081C8, desc: INSERT_LEAF off 76, blkref #0: rel 1663/47473/59412 blk 13 FPW rmgr: Btree len (rec/tot): 2/64, tx: 8498, lsn: 0/240092D8, prev 0/24008220, desc: INSERT_LEAF off 343, blkref #0: rel 1663/47473/59414 blk 8 rmgr: Heaplen (rec/tot): 7/53, tx: 8498, lsn: 0/24009318, prev 0/240092D8, desc: LOCK off 29: xid 8498 LOCK_ONLY KEYSHR_LOCK , blkref #0: rel 1663/47473/50005 blk 0 rmgr: Heaplen (rec/tot): 3/ 2397, tx: 8496, lsn: 0/24009350, prev 0/24009318, desc: INSERT off 16, blkref #0: rel 1663/47473/49389 blk 16 FPW rmgr: Btree len (rec/tot): 2/ 5773, tx: 8496, lsn: 0/24009CB0, prev 0/24009350, desc: INSERT_LEAF off 6, blkref #0: rel 1663/47473/49400 blk 1 FPW rmgr: Btree len (rec/tot): 2
Re: [HACKERS] Oddity with NOT IN
On 2016-08-04 11:23 PM, Jim Nasby wrote: I've got a customer that discovered something odd... SELECT f1 FROM v1 WHERE f2 not in (SELECT bad FROM v2 WHERE f3 = 1); does not error, even though bad doesn't exist, but I'm guessing there's a v1.bad? This is a common mistake, and also why I recommend always table qualifying column references when there's more than one table in scope. .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] INSERT .. SET syntax
Hi, Here's a patch for $SUBJECT. I'll probably work on the docs a bit more before the next CF, but I thought I'd post it anyway. .m diff --git a/doc/src/sgml/ref/insert.sgml b/doc/src/sgml/ref/insert.sgml index e710cf4..33e577b 100644 --- a/doc/src/sgml/ref/insert.sgml +++ b/doc/src/sgml/ref/insert.sgml @@ -22,12 +22,21 @@ PostgreSQL documentation [ WITH [ RECURSIVE ] with_query [, ...] ] -INSERT INTO table_name [ AS alias ] [ ( column_name [, ...] ) ] -{ DEFAULT VALUES | VALUES ( { expression | DEFAULT } [, ...] ) [, ...] | query } +INSERT INTO table_name [ AS alias ] +{ +[ column_list ] VALUES ( { expression | DEFAULT } [, ...] ) [, ...] | +[ column_list ] query | +DEFAULT VALUES | +SET column_name = { expression | DEFAULT } [, ...] +} [ ON CONFLICT [ conflict_target ] conflict_action ] [ RETURNING * | output_expression [ [ AS ] output_name ] [, ...] ] -where conflict_target can be one of: +where column_list is: + +( column_name [, ...] ) + +and conflict_target can be one of: ( { index_column_name | ( index_expression ) } [ COLLATE collation ] [ opclass ] [, ...] ) [ WHERE index_predicate ] ON CONSTRAINT constraint_name @@ -53,13 +62,26 @@ INSERT INTO table_name [ AS - The target column names can be listed in any order. If no list of - column names is given at all, the default is all the columns of the - table in their declared order; or the first N column - names, if there are only N columns supplied by the - VALUES clause or query. The values - supplied by the VALUES clause or query are - associated with the explicit or implicit column list left-to-right. + The target column names in a column_list can be + listed in any order. If no column_list is given at + all (and the SET syntax is not used), the default is all + the columns of the table in their declared order; or the first + N column names, if there are only N + columns supplied by the VALUES clause or + query. The values supplied by the VALUES + clause or query are associated with the explicit or + implicit column list left-to-right. + + + +Instead of a column_list and a VALUES +clause, a SET clause similar to that of an +UPDATE can be used instead. The advantage of the +SET clause is that instead of matching the elements in +the two lists by ordinal position, the column name and the +expression to assign to that column are visually next to each other. +This can make long column assignment lists significantly more +readable. @@ -691,13 +713,13 @@ INSERT INTO distributors (did, dname) VALUES (10, 'Conrad International') INSERT conforms to the SQL standard, except that the RETURNING clause is a - PostgreSQL extension, as is the ability - to use WITH with INSERT, and the ability to - specify an alternative action with ON CONFLICT. - Also, the case in - which a column name list is omitted, but not all the columns are - filled from the VALUES clause or query, - is disallowed by the standard. + PostgreSQL extension, as is the + SET clause when used instead of a VALUES clause, the + ability to use WITH with INSERT, and the + ability to specify an alternative action with ON + CONFLICT. Also, the case in which a column name list is omitted, + but not all the columns are filled from the VALUES clause + or query, is disallowed by the standard. diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 29c8c4e..55c4cb3 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -466,8 +466,9 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) stmt->onConflictClause->action == ONCONFLICT_UPDATE); /* -* We have three cases to deal with: DEFAULT VALUES (selectStmt == NULL), -* VALUES list, or general SELECT input. We special-case VALUES, both for +* We have four cases to deal with: DEFAULT VALUES (selectStmt == NULL and +* cols == NIL), SET syntax (selectStmt == NULL but cols != NIL), VALUES +* list, or general SELECT input. We special-case VALUES, both for * efficiency and so we can handle DEFAULT specifications. * * The grammar allows attaching ORDER BY, LIMIT, FOR UPDATE, or WITH to a @@ -522,7 +523,7 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) /* * Determine which variant of INSERT we have. */ - if (selectStmt == NULL) + if (selectStmt == NULL && stmt->cols == NIL) { /* * We have INSERT ... DEFAULT VALUES. We can handle this case by @@ -531,6 +532,25 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) */ exprList = NIL; } + else if (selectStmt == NULL) + { + /* +
[HACKERS] Column COMMENTs in CREATE TABLE?
Hi, Currently we have CREATE TABLE statements in a git repository that look roughly like this: CREATE TABLE foo( -- the first field f1 int NOT NULL, -- the second field f2 int NOT NULL, ... ); But the problem is that those comments don't obviously make it all the way to the database, so e.g. \d+ tblname won't show you that precious information. If you want them to make it all the way to the database, you'd have to add COMMENT ON statements *after* the CREATE TABLE, which means that either column comments have to be maintained twice, or the CREATE TABLE statement won't have them, so you have to go back and forth in your text editor to see the comments. Both solutions are suboptimal. What I would prefer is something like this: CREATE TABLE foo( f1 int NOT NULL COMMENT 'the first field', f2 int NOT NULL COMMENT 'the second field', ... ); which would ensure the comments are both next to the field definition they're documenting and that they make it all the way to the database. I looked into the biggest products, and MySQL supports this syntax. I couldn't find any similar syntax in any other product. The downside is that this would require us to make COMMENT a fully reserved keyword, which would quite likely break at least one application out in the wild. Another option would be to make the syntax something like [ COLUMN COMMENT '...' ], but that's not exactly a beautiful solution either. I still think this would be a really valuable feature if we can come up with a decent syntax for it. Does anyone have any ideas? Or does anybody want to shoot this proposal down right off the bat? .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Allow INSTEAD OF DELETE triggers to modify the tuple for RETURNING
Hi, Currently the tuple returned by INSTEAD OF triggers on DELETEs is only used to determine whether to pretend that the DELETE happened or not, which is often not helpful enough; for example, the actual tuple might have been concurrently UPDATEd by another transaction and one or more of the columns now hold values different from those in the planSlot tuple. Attached is an example case which is impossible to properly implement under the current behavior. For what it's worth, the current behavior seems to be an accident; before INSTEAD OF triggers either the tuple was already locked (in case of BEFORE triggers) or the actual pre-DELETE version of the tuple was fetched from the heap. So I'm suggesting to change this behavior and allow INSTEAD OF DELETE triggers to modify the OLD tuple and use that for RETURNING instead of returning the tuple in planSlot. Attached is a WIP patch implementing that. Is there any reason why we wouldn't want to change the current behavior? .m BEGIN; CREATE OR REPLACE FUNCTION foof() RETURNS TRIGGER AS $$ BEGIN -- imagine someone concurrently incremented counter here OLD.counter := OLD.counter + 1; RETURN OLD; END $$ LANGUAGE plpgsql; CREATE TABLE foo(counter int NOT NULL); CREATE VIEW foov AS SELECT * FROM foo; CREATE TRIGGER foov_instead INSTEAD OF DELETE ON foov FOR EACH ROW EXECUTE PROCEDURE foof(); INSERT INTO foo VALUES (0); DELETE FROM foov RETURNING counter; ROLLBACK; *** a/src/backend/commands/trigger.c --- b/src/backend/commands/trigger.c *** *** 2295,2307 ExecARDeleteTriggers(EState *estate, ResultRelInfo *relinfo, } } ! bool ExecIRDeleteTriggers(EState *estate, ResultRelInfo *relinfo, !HeapTuple trigtuple) { TriggerDesc *trigdesc = relinfo->ri_TrigDesc; TriggerData LocTriggerData; ! HeapTuple rettuple; int i; LocTriggerData.type = T_TriggerData; --- 2295,2307 } } ! TupleTableSlot * ExecIRDeleteTriggers(EState *estate, ResultRelInfo *relinfo, !HeapTuple trigtuple, TupleTableSlot *slot) { TriggerDesc *trigdesc = relinfo->ri_TrigDesc; TriggerData LocTriggerData; ! HeapTuple rettuple = trigtuple; int i; LocTriggerData.type = T_TriggerData; *** *** 2333,2343 ExecIRDeleteTriggers(EState *estate, ResultRelInfo *relinfo, relinfo->ri_TrigInstrument, GetPerTupleMemoryContext(estate)); if (rettuple == NULL) ! return false; /* Delete was suppressed */ ! if (rettuple != trigtuple) ! heap_freetuple(rettuple); } ! return true; } void --- 2333,2359 relinfo->ri_TrigInstrument, GetPerTupleMemoryContext(estate)); if (rettuple == NULL) ! return NULL;/* Delete was suppressed */ } ! ! if (rettuple != trigtuple) ! { ! /* !* Return the modified tuple using the es_trig_tuple_slot. We assume !* the tuple was allocated in per-tuple memory context, and therefore !* will go away by itself. The tuple table slot should not try to !* clear it. !*/ ! TupleTableSlot *newslot = estate->es_trig_tuple_slot; ! TupleDesc tupdesc = RelationGetDescr(relinfo->ri_RelationDesc); ! ! if (newslot->tts_tupleDescriptor != tupdesc) ! ExecSetSlotDescriptor(newslot, tupdesc); ! ExecStoreTuple(rettuple, newslot, InvalidBuffer, false); ! slot = newslot; ! } ! ! return slot; } void *** a/src/backend/executor/nodeModifyTable.c --- b/src/backend/executor/nodeModifyTable.c *** *** 573,585 ExecDelete(ItemPointer tupleid, if (resultRelInfo->ri_TrigDesc && resultRelInfo->ri_TrigDesc->trig_delete_instead_row) { ! booldodelete; ! Assert(oldtuple != NULL); ! dodelete = ExecIRDeleteTriggers(estate, resultRelInfo, oldtuple); ! if (!dodelete) /* "do nothing" */ return NULL; } else if (resultRelInfo->ri_FdwRoutine) { --- 573,595 if (resultRelInfo->ri_TrigDesc && resultRelInfo->ri_TrigDesc->trig_delete_instead_row) { ! /* !* Store the heap tuple into the tuple table slot, making sure
Re: [HACKERS] Feature suggestions: "dead letter"-savepoint.
On 2016-06-23 12:34, Terje Elde wrote: Typically the flow would be something like: BEGIN; SELECT id FROM targets WHERE status=‘scheduled’ FOR UPDATE SKIP LOCKED LIMIT 1; UPDATE targets SET status=‘in-flight’ WHERE id =%(id); COMMIT; — Do the work. BEGIN; UPDATE targets SET status=‘completed’ WHERE id = %(id); — or status=‘failed-foo’, if it fails for reason foo COMMIT; What I’m suggesting would be something along the lines of; BEGIN; SELECT id FROM targets WHERE status=‘scheduled’ FOR UPDATE SKIP LOCKED LIMIT 1; UPDATE targets SET status=‘failed-unknown’ WHERE id =%(id); SAVEPOINT deadletter ON FAILURE COMMIT; — Do the work. UPDATE targets SET status=‘completed’ WHERE id = %(id); — or status=‘failed-foo' COMMIT; Comparing these two; how is the latter any better? It's the same number of commands, except it's holding a transaction open for longer, it's using a non-standard concept and it's arguably more complex. .m -- 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_dump -j against standbys
On 25/05/16 15:59, Magnus Hagander wrote: On Tue, May 24, 2016 at 5:39 PM, Tom Lane wrote: This patch will cause pg_dump to fail entirely against pre-9.0 servers. You need to execute the test conditionally. Ugh. can I blame coding while too jetlagged? You could try blaming Magnus. Oh, wait.. .m -- 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] Calling json_* functions with JSONB data
On 2016-05-23 18:55, Peter van Hardenberg wrote: I talked this over with Andrew who had no objections and suggested I float it on the list before writing a patch. Looks pretty straightforward, just a few new data rows in pg_proc.h. Anyone have any concerns or suggestions? What about cases like json_whatever($1) which previously worked but will now be ambiguous? (Or will they somehow not be ambiguous?) .m -- 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] Alter or rename enum value
On 2016-03-27 19:30, Dagfinn Ilmari Mannsåker wrote: ilm...@ilmari.org (Dagfinn Ilmari Mannsåker) writes: I was bored and thought "how hard could it be?", and a few hours' hacking later, I have something that seems to work. It doesn't do IF NOT EXISTS yet, and the error messaging could do with some improvement, and there are no docs. The patch is attached, as well as at https://github.com/ilmari/postgres/commit/enum-alter-value Here's v3 of the patch of the patch, which I consider ready for proper review. A couple of trivial comments below. +ALTER VALUE [ IF EXISTST ] TO [ IF NOT EXISTS ] typo EXISTST + If IF EXISTS or is IF NOT EXISTS + is specified, it is not an error if the type doesn't contain the old double is + if the old value is not alreeady present or the new value is. typo alreeady + * RenameEnumLabel + * Add a new label to the enum set. By default it goes at copypaste-o? + if (!stmt->oldVal) { not project curly brace style .m -- 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] pl/pgsql exported functions
On 11/02/16 18:29, Magnus Hagander wrote: Most of the pl/pgsql functions and variables are prefixed plpgsql_, so they don't risk conflicting with other shared libraries loaded. There are a couple that are not prefixed. Attached patch fixes that. It's mostly a cleanup, but I think it's something we should do since it's only 2 variables and 2 functions. AFAICT these are clearly meant to be internal. (the plugin variable is accessed through find_rendezvous_variable) Comments? Looks good to me. .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: schema PL session variables
On 08/02/16 14:16, Pavel Stehule wrote: 2016-02-08 13:53 GMT+01:00 Marko Tiikkaja : Yeah, and that's exactly what I don't want, because that means that CREATE SCHEMA VARIABLE suddenly breaks existing code. theoretically yes, but this conflict can be 100% detected - so no quiet bug is possible, and plpgsql_check can find this issue well. If you don't use schema variable, then your code will be correct. You have to explicitly create the variable, and if there will be any problem, then the problem will be only in PL functions in one schema. And you can identify it by statical analyse. I'm sorry, but I think you've got your priorities completely backwards. You're saying that it's OK to add a footgun because blown-off pieces of feet can be found by using a third party static analyzer barely anyone uses. And at best, that footgun is only a very minor convenience (though I'd argue that omitting it actually hurts readability). That makes absolutely no sense to me at all. .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: schema PL session variables
On 08/02/16 13:41, Pavel Stehule wrote: 2016-02-08 13:22 GMT+01:00 Marko Tiikkaja : Personally I find that undesirable. I don't know what oracle does, but variables being visible without schema-qualifying them can introduce variable conflicts in PL/PgSQL. I'd prefer if you could only refer to them by prefixing them with the schema name (or maybe allow search_path to be used). I hope so there are not new conflicts - schema variable is not directly visible from SQL (in this iteration) - they are visible only from functions - and the behave is same like global plpgsql variable. So schema variable can be in conflict with SQL identifier only exactly identically as plpgsql variable Yeah, and that's exactly what I don't want, because that means that CREATE SCHEMA VARIABLE suddenly breaks existing code. But prefix can be used. Sure, but I don't see the point. Is there a reason not to require such variable references to be prefixed with the schema name? Or explicitly bring them into scope in the DECLARE section somehow. .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: schema PL session variables
On 08/02/16 13:17, Pavel Stehule wrote: 2016-02-08 13:03 GMT+01:00 Marko Tiikkaja : How does this function know which schema variables are visible? function see all schema variables from same schema as function's schema Personally I find that undesirable. I don't know what oracle does, but variables being visible without schema-qualifying them can introduce variable conflicts in PL/PgSQL. I'd prefer if you could only refer to them by prefixing them with the schema name (or maybe allow search_path to be used). .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: schema PL session variables
On 08/02/16 09:16, Pavel Stehule wrote: Usage = DROP SCHEMA IF EXISTS test_schema CASCADE; SET SCHEMA test_schema; CREATE SCHEMA VARIABLE local_counter AS int DEFAULT 0; CREATE OR REPLACE FUNCTION increment_counter() RETURNS void AS $$ BEGIN local_counter := local_counter + 1; END; $$ LANGUAGE plpgsql; How does this function know which schema variables are visible? .m -- 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] count_nulls(VARIADIC "any")
On 2016-02-05 05:06, Tom Lane wrote: I wrote: Pavel Stehule writes: [ num_nulls_v6.patch ] I started looking through this. It seems generally okay, but I'm not very pleased with the function name "num_notnulls". I think it would be better as "num_nonnulls", as I see Oleksandr suggested already. Not hearing any complaints, I pushed it with that change and some other cosmetic adjustments. Thanks Tom and Pavel and everyone who provided feedback. .m -- 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 behind-the-scenes effects' CONTEXT
On 2016-01-21 04:17, Simon Riggs wrote: Marko, I was/am waiting for an updated patch. Could you comment please? Sorry, I've not found time to work on this recently. Thanks for everyone's comments so far. I'll move this to the next CF and try and get an updated patch done in time for that one. .m -- 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] count_nulls(VARIADIC "any")
On 25/01/16 19:57, Pavel Stehule wrote: Marco is a author of this patch, so - Marco, please, send final version of this patch I don't really care about the tests. Can we not use the v5 patch already in the thread? As far as I could tell there were no reviewer's comments on it anymore. .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: function parse_ident
Hi Pavel, Sorry for the lack of review here. I didn't realize I was still the reviewer for this after it had been moved to another commitfest. That said, I think I've exhausted my usefulness here as a reviewer. Marking ready for committer. .m -- 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] SET syntax in INSERT
On 2016-01-14 20:50, Vitaly Burovoy wrote: On 1/14/16, Tom Lane wrote: Assume a table with an int-array column, and consider INSERT INTO foo SET arraycol[2] = 7, arraycol[4] = 11; Right part is a column name, not an expression. Isn't it? So "arraycol[2]" is not possible there. I think the idea here was that it's allowed in UPDATE. But I don't see the point of allowing that in an INSERT. .m -- 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] SET syntax in INSERT
On 2016-01-14 20:33, Tom Lane wrote: Pavel Stehule writes: Probably there is less risk than 7 years ago, but still creating own syntax isn't the best idea. This is syntactic sugar only and different from ANSi SQL or common standard. It's more than syntactic sugar; you are going to have to invent semantics, as well, because it's less than clear what partial-field assignments should do. I don't really care for such. In my opinion it would be fine if this simply was only "syntactic sugar", and trying to do any tricks like this would simply raise an exception. .m -- 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] SET syntax in INSERT
On 2016-01-14 8:06 PM, Pavel Stehule wrote: Probably there is less risk than 7 years ago, but still creating own syntax isn't the best idea. This is syntactic sugar only and different from ANSi SQL or common standard. So is RETURNING, UPSERT, PL/PgSQL and many other useful features. .m -- 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] SET syntax in INSERT
Hi, SET syntax for INSERT was brought up a few years ago here: http://www.postgresql.org/message-id/2c5ef4e30908251010s46d9d566m1da21357891ba...@mail.gmail.com From the discussion it seems that one committer was against, one committer was not against, and one committer saw something good in the proposal. Personally, I believe this would be a huge readability improvement to INSERTs with more than a few columns. I'm willing to put in some work to make this happen for 9.7, but I'd like to know that I'm not wasting my time. What do we think? .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: [JDBC] Re: 9.4-1207 behaves differently with server side prepared statements compared to 9.2-1102
On 13/01/16 15:34, Vladimir Sitnikov wrote: This completely screws over PL/PgSQL, among other things. Can you elaborate a bit? You just write a query like this: SELECT * FROM foo WHERE bar = _Variable; so you don't get to (or want to) have any control over the underlying prepared statement. .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: [JDBC] Re: 9.4-1207 behaves differently with server side prepared statements compared to 9.2-1102
On 13/01/16 15:26, Vladimir Sitnikov wrote: 2) It is likely to be more performant. We just need to explain users that "if different plans required, just use different statements". This completely screws over PL/PgSQL, among other things. .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: [JDBC] Re: 9.4-1207 behaves differently with server side prepared statements compared to 9.2-1102
On 13/01/16 14:36, Vladimir Sitnikov wrote: Say you already have a plan which looks like this: Now the plan gets invoked with $1 = 5. What exactly in your mind would happen here? A sequential scan with $1=5 condition. What else could be there? I don't know, it's your proposal :-) But it looks like I misunderstood. Note: I do not suggest changing already cached plans yet. I suggest looking into "6th bind values" when building a cached plan. But that wouldn't have helped your case. The custom plan is *more expensive*; I'm guessing because the generic plan gambles on a better average case instead of preparing for the worst case. .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: [JDBC] Re: 9.4-1207 behaves differently with server side prepared statements compared to 9.2-1102
On 13/01/16 14:27, Vladimir Sitnikov wrote: TL;DR: I suggest to create "generic plan" with regard to current bind values. What's wrong with that approach? I don't understand how this is supposed to work. Say you already have a plan which looks like this: Seq Scan on foo (cost=0.00..100.00 rows=1 width=630) Filter: (bar = $1) Now the plan gets invoked with $1 = 5. What exactly in your mind would happen here? .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: [JDBC] Re: 9.4-1207 behaves differently with server side prepared statements compared to 9.2-1102
On 13/01/16 14:12, Pavel Stehule wrote: so the strategy - if cost of generic plan is less than some MAGIC CONSTANT (can be specified by GUC), then use generic plan. Elsewhere use a custom plan everytime. It allow to controll the plan reusing. When MAGIC CONSTANT = 0 .. use custom plan everytime, When MAGIC CONSTANT = M, then use generic plan always. I don't think that would solve even the original problem without effectively disabling generic plans, despite the problem being relatively simple. The generic plan appears to be really cheap, because the planner doesn't have the concept of a "worst case". .m -- 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] count_nulls(VARIADIC "any")
On 03/01/16 22:49, Jim Nasby wrote: In the unit test, I'd personally prefer just building a table with the test cases and the expected NULL/NOT NULL results, at least for all the calls that would fit that paradigm. That should significantly reduce the size of the test. Not a huge deal though... I don't really see the point. "The size of the test" doesn't seem like a worthwhile optimization target, unless the test scripts are somehow really unnecessarily large. Further, if you were developing code related to this, previously you could just copy-paste the defective test case in order to easily reproduce a problem. But now suddenly you need a ton of different setup. I don't expect to really have a say in this, but I think the tests are now worse than they were before. .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fwd: [JDBC] Re: 9.4-1207 behaves differently with server side prepared statements compared to 9.2-1102
On 12/01/16 13:00, Dave Cramer wrote: We have an interesting problem, and the reporter has been kind enough to provide logs for which we can't explain. I'd be interested to hear any plausible explanations for a prepared plan suddenly going from 2ms to 60ms for the same input values ? This is a new feature in 9.2, where on the fifth (or sixth, not sure) execution the planner might choose to use a generic plan. From the 9.2 release notes (though I'm fairly certain this is documented somewhere in the manual as well): In the past, a prepared statement always had a single "generic" plan that was used for all parameter values, which was frequently much inferior to the plans used for non-prepared statements containing explicit constant values. Now, the planner attempts to generate custom plans for specific parameter values. A generic plan will only be used after custom plans have repeatedly proven to provide no benefit. This change should eliminate the performance penalties formerly seen from use of prepared statements (including non-dynamic statements in PL/pgSQL). .m -- 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] Question about DROP TABLE
On 12/01/16 12:17, Pavel Stehule wrote: 2016-01-12 12:14 GMT+01:00 Michal Novotny : Hi Pavel, thanks for the information. I've been doing more investigation of this issue and there's autovacuum running on the table however it's automatically starting even if there is "autovacuum = off" in the postgresql.conf configuration file. Real autovacuum is automatically cancelled. It looks like VACUUM started by cron, maybe? Not if it's to prevent wraparound, which isn't unlikely if autovacuum=off. .m -- 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 numeric_trim(numeric)
On 2016-01-07 1:11 AM, Tom Lane wrote: A different approach is that I'm not real sure why we want a function that returns a modified numeric value at all. To the extent I understood Marko's original use case, it seems like what you'd invariably do with the result is extract its scale(). Well, no, both are useful. I think as far as the interface goes, having both a scale() and a way to "rtrim" a numeric is optimal. rtrim() can also be used before storing and/or displaying values to get rid of unnecessary zeroes. As for what the actual function should be called, I don't much care. I wanted to avoid making trim() work because I thought it would interfere with the trim('foo') case where the input argument's type is unknown, but after some testing it appears that that would not be interfered with. .m -- 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] Passing initially_valid values instead of !skip_validation to StoreRelCheck() in AddRelationNewConstraints()
On 12/3/15 12:44 PM, amul sul wrote: On Thursday, 3 December 2015 4:36 PM, Amit Langote wrote: The user will have to separately validate the constraint by issuing a ALTER TABLE VALIDATE CONSTRAINT command at a time of their choosing. This could be time consuming operation for big table, If I am pretty much sure that my constraint will be valid, simply I could set both flag(initially_valid & skip_validation) to true. I'm confused here. It sounds like you're suggesting an SQL level feature, but you're really focused on a single line of code for some reason. Could you take a step back and explain the high level picture of what you're trying to achieve? .m -- 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 Functionally Dependent GROUP BY Columns
On 2015-12-01 05:00, David Rowley wrote: We already allow a SELECT's target list to contain non-aggregated columns in a GROUP BY query in cases where the non-aggregated column is functionally dependent on the GROUP BY clause. For example a query such as; SELECT p.product_id,p.description, SUM(s.quantity) FROM product p INNER JOIN sale s ON p.product_id = s.product_id GROUP BY p.product_id; is perfectly fine in PostgreSQL, as p.description is functionally dependent on p.product_id (assuming product_id is the PRIMARY KEY of product). This has come up before (on other forums, at least), and my main concern has been that unlike the case where we go from throwing an error to allowing a query, this has a chance to make the planning of currently legal queries slower. Have you tried to measure the impact of this on queries where there's no runtime gains to be had? .m -- 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] count_nulls(VARIADIC "any")
On 2015-11-22 21:17, Jim Nasby wrote: On 11/22/15 11:34 AM, Marko Tiikkaja wrote: On 2015-11-22 18:29, Jim Nasby wrote: Only if you know how many columns there already are. Or does this not work if you hand it a row? It "works" in the sense that it tells you whether the row is NULL or not. I.e. the answer will always be 0 or 1. Hrm, I was hoping something like count_nulls(complex_type.*) would work. Nope: =# select num_nulls((f).*) from (select '(1,2,3)'::foo) ss(f); ERROR: row expansion via "*" is not supported here .m -- 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] custom function for converting human readable sizes to bytes
On 2015-11-22 18:30, Jim Nasby wrote: On 11/21/15 12:49 AM, Pavel Stehule wrote: I propose inversion function to pg_size_pretty function - like pg_human_size. Usage: SELECT * FROM pg_class WHERE pg_table_size(oid) > pg_human_size('2GB'); I'm not a big fan of the name, but +1 on the general idea. Maybe pg_size_pretty(text)? pg_ytterp_ezis(text) -- 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] count_nulls(VARIADIC "any")
On 2015-11-22 18:29, Jim Nasby wrote: Only if you know how many columns there already are. Or does this not work if you hand it a row? It "works" in the sense that it tells you whether the row is NULL or not. I.e. the answer will always be 0 or 1. .m -- 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 (INSERT/UPDATE/DELETE .. RETURNING ..)
On 2015-11-16 08:24, Michael Paquier wrote: On Sun, Nov 1, 2015 at 2:49 AM, Marko Tiikkaja wrote: Attached is a patch for being able to do $SUBJECT without a CTE. The reasons this is better than a CTE version are: 1) It's not obvious why a CTE version works but a plain one doesn't 2) This one has less overhead (I measured a ~12% improvement on a not-too-unreasonable test case) Would you mind sharing this test case as well as numbers? Attached the test case I used; removing a batch of old rows from a table through an index. Best out of three runs, I get 13.1 seconds versus 15.0 seconds, which should amount to an improvement of around 12.7%. Also attached a v3 of the patch which adds an Assert on top of the test case changes suggested by you. .m drop table if exists tbl; create table tbl(a serial, b int not null, c text not null); insert into tbl (b,c) select i, random()::text from generate_series(1, 1) i; alter table tbl add primary key (a); analyze tbl; checkpoint; \timing -- new code --\copy (delete from tbl where a <= 500 returning *) to foo.dat -- old code --SET work_mem TO '1GB'; --\copy (with t as (delete from tbl where a <= 500 returning *) select * from t) to foo.dat *** a/doc/src/sgml/ref/copy.sgml --- b/doc/src/sgml/ref/copy.sgml *** *** 112,121 COPY { table_name [ ( query ! A or !command ! whose results are to be copied. ! Note that parentheses are required around the query. --- 112,128 query ! A , , ! , or !command whose results are to be ! copied. Note that parentheses are required around the query. ! ! ! For INSERT, UPDATE and ! DELETE queries a RETURNING clause must be provided, ! and the target relation must not have a conditional rule, nor ! an ALSO rule, nor an INSTEAD rule ! that expands to multiple statements. *** a/src/backend/commands/copy.c --- b/src/backend/commands/copy.c *** *** 201,207 typedef struct CopyStateData int raw_buf_len; /* total # of bytes stored */ } CopyStateData; ! /* DestReceiver for COPY (SELECT) TO */ typedef struct { DestReceiver pub; /* publicly-known function pointers */ --- 201,207 int raw_buf_len; /* total # of bytes stored */ } CopyStateData; ! /* DestReceiver for COPY (query) TO */ typedef struct { DestReceiver pub; /* publicly-known function pointers */ *** *** 772,778 CopyLoadRawBuf(CopyState cstate) * * Either unload or reload contents of table , depending on . * ( = TRUE means we are inserting into the table.) In the "TO" case ! * we also support copying the output of an arbitrary SELECT query. * * If is false, transfer is between the table and the file named * . Otherwise, transfer is between the table and our regular --- 772,779 * * Either unload or reload contents of table , depending on . * ( = TRUE means we are inserting into the table.) In the "TO" case ! * we also support copying the output of an arbitrary SELECT, INSERT, UPDATE ! * or DELETE query. * * If is false, transfer is between the table and the file named * . Otherwise, transfer is between the table and our regular *** *** 1374,1384 BeginCopy(bool is_from, Assert(!is_from); cstate->rel = NULL; ! /* Don't allow COPY w/ OIDs from a select */ if (cstate->oids) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), ! errmsg("COPY (SELECT) WITH OIDS is not supported"))); /* * Run parse analysis and rewrite. Note this also acquires sufficient --- 1375,1385 Assert(!is_from); cstate->rel = NULL; ! /* Don't allow COPY w/ OIDs from a query */ if (cstate->oids) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), ! errmsg("COPY (query) WITH OIDS is not supported"))); /* * Run parse analysis and rewrite. Note this also acquires sufficient *** *** 1393,1401 BeginCopy(bool is_from, rewritten = pg_analyze_and_rewrite((Node *) copyObject(raw_query), queryString, NULL, 0); ! /* We don't expect more or less than one result query */ ! if (list_length(rewritten) != 1) ! elog(ERROR, "unexpected rewrite result"); query = (Query *) linitial(rewritten); --- 1394,1429 rewritten = pg_analyze_and_rewrite((Node *) copyObject(raw_query), queryString, NULL, 0); ! /* check that we got back something we can work with */ ! if (rewritten == NIL) ! { ! ereport(ERROR, ! (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), ! errmsg("DO INSTEAD NOTHING rules are not supported for COPY"))); ! } ! else if (list_length(rewritten) >
Re: [HACKERS] count_nulls(VARIADIC "any")
On 2015-11-21 06:52, Jim Nasby wrote: On 11/20/15 11:12 PM, Marko Tiikkaja wrote: On 2015-11-21 06:02, I wrote: Here's a patch implementing this under the name num_nulls(). For January's CF, of course. I forgot to update the some references in the documentation. Fixed in v3, attached. I thought there was going to be a not-null equivalent as well? I've definitely wanted both variations in the past. I'm not sure that's necessary. It's quite simple to implement yourself using the int - int operator. .m -- 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] count_nulls(VARIADIC "any")
On 2015-11-21 06:02, I wrote: Here's a patch implementing this under the name num_nulls(). For January's CF, of course. I forgot to update the some references in the documentation. Fixed in v3, attached. .m *** a/doc/src/sgml/func.sgml --- b/doc/src/sgml/func.sgml *** *** 182,188 !Comparison Operators comparison --- 182,188 !Comparison Functions and Operators comparison *** *** 191,200 The usual comparison operators are available, shown in . ! Comparison Operators --- 191,200 The usual comparison operators are available, shown in . ! Comparison Operators *** *** 437,442 --- 437,470 --> + + Comparison Functions + + + +Function +Description +Example +Example Result + + + + + + + num_nulls + + num_nulls(VARIADIC "any") + +Returns the number of NULL input arguments +num_nulls(1, NULL, 2) +1 + + + + + + *** *** 10307,10313 table2-mapping The standard comparison operators shown in are available for jsonb, but not for json. They follow the ordering rules for B-tree operations outlined at . --- 10335,10341 The standard comparison operators shown in are available for jsonb, but not for json. They follow the ordering rules for B-tree operations outlined at . *** a/src/backend/utils/adt/misc.c --- b/src/backend/utils/adt/misc.c *** *** 45,50 --- 45,118 /* + * num_nulls() + * Count the number of NULL input arguments + */ + Datum + pg_num_nulls(PG_FUNCTION_ARGS) + { + int32 count = 0; + int i; + + if (get_fn_expr_variadic(fcinfo->flinfo)) + { + ArrayType *arr; + int ndims, nitems, *dims; + bits8 *bitmap; + int bitmask; + + /* Should have just the one argument */ + Assert(PG_NARGS() == 1); + + /* num_nulls(VARIADIC NULL) is defined as NULL */ + if (PG_ARGISNULL(0)) + PG_RETURN_NULL(); + + /* + * Non-null argument had better be an array. We assume that any call + * context that could let get_fn_expr_variadic return true will have + * checked that a VARIADIC-labeled parameter actually is an array. So + * it should be okay to just Assert that it's an array rather than + * doing a full-fledged error check. + */ + Assert(OidIsValid(get_base_element_type(get_fn_expr_argtype(fcinfo->flinfo, 0; + + /* OK, safe to fetch the array value */ + arr = PG_GETARG_ARRAYTYPE_P(0); + + ndims = ARR_NDIM(arr); + dims = ARR_DIMS(arr); + nitems = ArrayGetNItems(ndims, dims); + + bitmap = ARR_NULLBITMAP(arr); + if (!bitmap) + PG_RETURN_INT32(0); + bitmask = 1; + + for (i = 0; i < nitems; i++) + { + if ((*bitmap & bitmask) == 0) + count++; + + bitmask <<= 1; + if (bitmask == 0x100) + { + bitmap++; + bitmask = 1; + } + } + PG_RETURN_INT32(count); + } + + for (i = 0; i < PG_NARGS(); i++) + { + if (PG_ARGISNULL(i)) + count++; + } + PG_RETURN_INT32(count); + } + + /* * current_database() * Expose the current database to the user */ *** a/src/include/catalog/pg_proc.h --- b/src/include/catalog/pg_proc.h *** *** 2963,2968 DESCR("adjust time with time zone precision"); --- 2963,2970 DATA(insert OID = 2003 ( textanycat PGNSP PGUID 14 1 0 0 0 f f f f t f s s 2 0 25 "25 2776" _null_ _null_ _null_ _null_ _null_ "select $1 || $2::pg_catalog.text" _null_ _null_ _null_ )); DATA(insert OID = 2004 ( anytextcat PGNSP PGUID 14 1 0 0 0 f f f f t f s s 2 0 25 "2776 25" _null_ _null_ _null_ _null_ _null_ "select $1::pg_catalog.text || $2" _null_ _null_ _null_ )); + DATA(insert OID = 4400 ( num_nullsPGNSP PGUID 12 1 0 2276 0 f f f f f f i s 1 0 23 "2276" "{2276}" "{v}" _null_ _null_ _null_pg_num_nulls _null_ _null_ _null_ )); + DESCR("count the number of NULL input arguments"); DATA(insert OID = 2005 ( bytealike PGNSP PGUID 12 1 0 0 0 f f f f t f i s 2 0 16 "17 17" _null_ _null_ _null_ _null_ _null_ bytealike _null_ _null_ _null_ )); DATA(insert OID = 2006 ( byteanlike PGNSP PGUID 12 1 0 0 0 f f f f t f i s 2 0 16 "17 17" _null_ _null_ _null_ _null_ _null_ byteanlike _null_ _null_ _null_ )); *** a/src/include/utils/builtins.h --- b/src/include/utils/builtins.h *** *** 481,486 extern Datum pg_ls_dir(PG_FUNCTION_ARGS); --- 481,487 extern Datum pg_ls_dir_1arg(PG_FUNCTION_ARGS); /* misc.c */ + extern Datum pg_num_nulls(PG_FUNCTION_ARGS); extern Datum current_database(PG_FUNCTION_ARGS); extern Datum current_query(PG_FUNCTION_ARGS); extern Datum pg_cancel_backend(PG_FUNCTION_ARGS); *** /dev/n
Re: [HACKERS] count_nulls(VARIADIC "any")
On 2015-11-21 06:06, Tom Lane wrote: Marko Tiikkaja writes: Here's a patch implementing this under the name num_nulls(). For January's CF, of course. What's this do that "count(*) - count(x)" doesn't? This is sort of a lateral version of count(x); the input is a list of expressions rather than an expression executed over a bunch of input rows. .m -- 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] count_nulls(VARIADIC "any")
Hello, Here's a patch implementing this under the name num_nulls(). For January's CF, of course. .m *** a/doc/src/sgml/func.sgml --- b/doc/src/sgml/func.sgml *** *** 182,188 !Comparison Operators comparison --- 182,188 !Comparison Functions and Operators comparison *** *** 194,200 linkend="functions-comparison-table">. ! Comparison Operators --- 194,200 linkend="functions-comparison-table">. ! Comparison Operators *** *** 437,442 --- 437,470 --> + + Comparison Functions + + + +Function +Description +Example +Example Result + + + + + + + num_nulls + + num_nulls(VARIADIC "any") + +Returns the number of NULL input arguments +num_nulls(1, NULL, 2) +1 + + + + + + *** a/src/backend/utils/adt/misc.c --- b/src/backend/utils/adt/misc.c *** *** 45,50 --- 45,118 /* + * num_nulls() + * Count the number of NULL input arguments + */ + Datum + pg_num_nulls(PG_FUNCTION_ARGS) + { + int32 count = 0; + int i; + + if (get_fn_expr_variadic(fcinfo->flinfo)) + { + ArrayType *arr; + int ndims, nitems, *dims; + bits8 *bitmap; + int bitmask; + + /* Should have just the one argument */ + Assert(PG_NARGS() == 1); + + /* num_nulls(VARIADIC NULL) is defined as NULL */ + if (PG_ARGISNULL(0)) + PG_RETURN_NULL(); + + /* + * Non-null argument had better be an array. We assume that any call + * context that could let get_fn_expr_variadic return true will have + * checked that a VARIADIC-labeled parameter actually is an array. So + * it should be okay to just Assert that it's an array rather than + * doing a full-fledged error check. + */ + Assert(OidIsValid(get_base_element_type(get_fn_expr_argtype(fcinfo->flinfo, 0; + + /* OK, safe to fetch the array value */ + arr = PG_GETARG_ARRAYTYPE_P(0); + + ndims = ARR_NDIM(arr); + dims = ARR_DIMS(arr); + nitems = ArrayGetNItems(ndims, dims); + + bitmap = ARR_NULLBITMAP(arr); + if (!bitmap) + PG_RETURN_INT32(0); + bitmask = 1; + + for (i = 0; i < nitems; i++) + { + if ((*bitmap & bitmask) == 0) + count++; + + bitmask <<= 1; + if (bitmask == 0x100) + { + bitmap++; + bitmask = 1; + } + } + PG_RETURN_INT32(count); + } + + for (i = 0; i < PG_NARGS(); i++) + { + if (PG_ARGISNULL(i)) + count++; + } + PG_RETURN_INT32(count); + } + + /* * current_database() * Expose the current database to the user */ *** a/src/include/catalog/pg_proc.h --- b/src/include/catalog/pg_proc.h *** *** 2963,2968 DESCR("adjust time with time zone precision"); --- 2963,2970 DATA(insert OID = 2003 ( textanycat PGNSP PGUID 14 1 0 0 0 f f f f t f s s 2 0 25 "25 2776" _null_ _null_ _null_ _null_ _null_ "select $1 || $2::pg_catalog.text" _null_ _null_ _null_ )); DATA(insert OID = 2004 ( anytextcat PGNSP PGUID 14 1 0 0 0 f f f f t f s s 2 0 25 "2776 25" _null_ _null_ _null_ _null_ _null_ "select $1::pg_catalog.text || $2" _null_ _null_ _null_ )); + DATA(insert OID = 4400 ( num_nullsPGNSP PGUID 12 1 0 2276 0 f f f f f f i s 1 0 23 "2276" "{2276}" "{v}" _null_ _null_ _null_pg_num_nulls _null_ _null_ _null_ )); + DESCR("count the number of NULL input arguments"); DATA(insert OID = 2005 ( bytealike PGNSP PGUID 12 1 0 0 0 f f f f t f i s 2 0 16 "17 17" _null_ _null_ _null_ _null_ _null_ bytealike _null_ _null_ _null_ )); DATA(insert OID = 2006 ( byteanlike PGNSP PGUID 12 1 0 0 0 f f f f t f i s 2 0 16 "17 17" _null_ _null_ _null_ _null_ _null_ byteanlike _null_ _null_ _null_ )); *** a/src/include/utils/builtins.h --- b/src/include/utils/builtins.h *** *** 481,486 extern Datum pg_ls_dir(PG_FUNCTION_ARGS); --- 481,487 extern Datum pg_ls_dir_1arg(PG_FUNCTION_ARGS); /* misc.c */ + extern Datum pg_num_nulls(PG_FUNCTION_ARGS); extern Datum current_database(PG_FUNCTION_ARGS); extern Datum current_query(PG_FUNCTION_ARGS); extern Datum pg_cancel_backend(PG_FUNCTION_ARGS); *** /dev/null --- b/src/test/regress/expected/misc_functions.out *** *** 0 --- 1,68 + -- + -- num_nulls() + -- + SELECT num_nulls(); + ERROR: function num_nulls() does not exist + LINE 1: SELECT num_nulls(); +^ + HINT: No function matches the given name and argument types. You might need to add explicit type casts. + SELECT num_nulls(NULL); + num_nulls + --- + 1 + (1 row) + + SELECT num_nulls('1'); + num_nulls + --- + 0 + (1 row) + + SELECT num_nulls(NULL::text); + num_nulls + --- + 1 + (1 row) + + SE
Re: [HACKERS] onlyvalue aggregate (was: First Aggregate Funtion?)
Hi Dean, Here's v2 of the patch. How's this look? .m *** a/doc/src/sgml/func.sgml --- b/doc/src/sgml/func.sgml *** *** 12635,12640 NULL baz(3 rows) --- 12635,12660 + single_value + + + single_value(expression) + + + +any type for which the equality operator has been defined + + +same as argument type + + returns the single distinct value from the input values; + if more than one distinct value exists (while considering NULLs equal), + an exception is raised + + + + + string_agg *** a/src/backend/utils/adt/misc.c --- b/src/backend/utils/adt/misc.c *** *** 40,45 --- 40,46 #include "utils/acl.h" #include "utils/builtins.h" #include "utils/timestamp.h" + #include "utils/typcache.h" #define atooid(x) ((Oid) strtoul((x), NULL, 10)) *** *** 598,600 pg_column_is_updatable(PG_FUNCTION_ARGS) --- 599,702 PG_RETURN_BOOL((events & REQ_EVENTS) == REQ_EVENTS); } + + struct single_value_agg_stype + { + FunctionCallInfoData fcinfo; + Datum datum; + bool isnull; + }; + + Datum + single_value_agg_transfn(PG_FUNCTION_ARGS) + { + MemoryContext aggcontext; + struct single_value_agg_stype *state; + + if (!AggCheckCallContext(fcinfo, &aggcontext)) + { + /* cannot be called directly because of internal-type argument */ + elog(ERROR, "single_value_agg_transfn called in non-aggregate context"); + } + + if (PG_ARGISNULL(0)) + { + TypeCacheEntry *typentry; + Oid arg1_typeid = get_fn_expr_argtype(fcinfo->flinfo, 1); + + if (arg1_typeid == InvalidOid) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("could not determine input data type"))); + + typentry = lookup_type_cache(arg1_typeid, + TYPECACHE_EQ_OPR_FINFO); + if (!OidIsValid(typentry->eq_opr_finfo.fn_oid)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_FUNCTION), + errmsg("could not identify an equality operator for type %s", + format_type_be(arg1_typeid; + + state = (struct single_value_agg_stype *) MemoryContextAlloc(aggcontext, sizeof(struct single_value_agg_stype)); + + if (PG_ARGISNULL(1)) + { + state->datum = (Datum) 0; + state->isnull = true; + memset(&state->fcinfo, 0, sizeof(state->fcinfo)); + } + else + { + state->datum = PG_GETARG_DATUM(1); + state->isnull = false; + InitFunctionCallInfoData(state->fcinfo, &typentry->eq_opr_finfo, 2, + InvalidOid, NULL, NULL); + } + } + else + { + bool oprresult; + + state = (struct single_value_agg_stype *) PG_GETARG_POINTER(0); + + if (state->isnull) + oprresult = PG_ARGISNULL(1); + else + { + state->fcinfo.argnull[0] = false; + state->fcinfo.argnull[1] = false; + state->fcinfo.arg[0] = state->datum; + state->fcinfo.arg[1] = PG_GETARG_DATUM(1); + state->fcinfo.isnull = false; + oprresult = DatumGetBool(FunctionCallInvoke(&state->fcinfo)); + } + if (!oprresult) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("more than one distinct value passed to single_value"))); + } + + /* + * The transition type for single_value() is declared to be "internal", + * which is a pass-by-value type the same size as a pointer. So we can + * safely pass the pointer through nodeAgg.c's machinations. + */ + PG_RETURN_POINTER(state); + } + + Datum + single_value_agg_finalfn(PG_FUNCTION_ARGS) + { + struct single_value_agg_stype *state; + + if (PG_ARGISNULL(0)) + PG_RETURN_NULL(); + + /* cannot be called directly because of internal-type argument */ + Assert(AggCheckCallContext(fcinfo, NULL)); + + state = (struct single_value_agg_stype *) PG_GETARG_POINTER(0); + if (state->isnull) + PG_RETURN_NULL(); + PG_RETURN_DATUM(state->datum); + } *** a/src/include/catalog/pg_aggregate.h --- b/src/include/catalog/pg_aggregate.h *** *** 292,297 DATA(insert ( 3197 n 0 json_object_agg_transfn json_object_agg_finalfn -- --- 292,300 DATA(insert ( 3267 n 0 jsonb_agg_transfn jsonb_agg_finalfn ---f f 0 2281 0 0 0 _null_ _null_ )); DATA(insert ( 3270 n 0 jsonb_object_agg_transfn jsonb_object_agg_finalfn ---f f 0 2281 0 0 0 _null_ _null_ )); + /* single_value */ + DATA(insert ( 4202 n 0 single_value_agg_transfnsingle_value_agg_finalfn- - - t f 0 22810 0 0 _null_ _null_ )); + /* ordered-set and hypothetical-set aggregates */ DATA(insert ( 3972 o 1 ordered_set_transition percentile_disc_final - - - t f 0 2281 0 0 0 _null_ _null_ )); DATA(insert ( 3974 o 1 ordered_set_transition percentile_cont_float8_final - - - f f 0 2281 0 0 0 _null_ _null_ )); *** a/src/include/catalog/pg_proc.h --- b/src/include/catalog/pg_pr
Re: [HACKERS] COPY (INSERT/UPDATE/DELETE .. RETURNING ..)
On 11/19/15 7:39 PM, Michael Paquier wrote: On Thu, Nov 19, 2015 at 9:22 PM, Marko Tiikkaja wrote: Of course, something might break if we added a new statement type which supported RETURNING, but I'm really not worried about that. I'm not dead set against adding some Assert(IsA()) calls here, but I don't see the point. gram.y has a long comment before select_no_parens regarding why we shouldn't do it this way, did you notice it? It talks about not doing '(' SelectStmt ')' "in the expression grammar". I don't see what that has to do with this patch. .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: LISTEN *
On 11/19/15 5:32 PM, Tom Lane wrote: Marko Tiikkaja writes: On 11/19/15 4:21 PM, Tom Lane wrote: ... and then you gotta get the notifications to the clients, so it seems like this just leaves the performance question hanging. I'm not sure which performance question you think is left hanging. If every client is connected to postgres, you're waking up tens if not hundreds of processes tens if not hundreds of times per second. Each of them start a transaction, check which notifications are visible in the queue from clog and friends, go through the tails of every other process to see whether they should advance the tail pointer of the queue, commit the transaction and go back to sleep only to be immediately woken up again. If they're not connected to postgres directly, this kind of complex processing only happens once, and then the notification server just unconditionally serves all notifications to the clients based on a simple map lookup. It should be trivial to see how the overhead is avoided. Meh. You've gotten rid of one single-point-of-contention by creating another one. Essentially, your argument why this is an improvement is that any random application developer using Postgres is smarter than the Postgres development team and can therefore produce something better-performing off the cuff. It's not about who's smarter and who's not. All a notification server has to do is wait for notifications incoming from one socket and make sure they're delivered to the correct sockets in an epoll loop. There's only one process being woken up, no need to synchronize with other processes, no need to see where the pointers of other processes are, no overhead of transaction visibility, transaction BEGIN, or transaction COMMIT. The total amount of work done is trivially smaller. Which may indeed be true, but shall we say it's unproven? Well it's not proof, but every time we moved an application from LISTENing in postgres to the notification server our CPU usage halved. The last time we did that (from ~100 connections to exactly 1), s_lock went down from 30% to 0.16% in our CPU profiles, and our CPU usage is only a fraction of what it used to be. And this is with the notification server running on the same server with postgres, so it's not cheating, either. There's no way we could keep running postgres if all 400+ clients interested in notifications had a LISTEN open in the database. .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: LISTEN *
On 11/19/15 4:21 PM, Tom Lane wrote: Marko Tiikkaja writes: I've in the past wanted to listen on all notification channels in the current database for debugging purposes. But recently I came across another use case. Since having multiple postgres backends listening for notifications is very inefficient (one Thursday I found out 30% of our CPU time was spent spinning on s_locks around the notification code), it makes sense to implement a notification server of sorts which then passes on notifications from postgres to interested clients. ... and then you gotta get the notifications to the clients, so it seems like this just leaves the performance question hanging. I'm not sure which performance question you think is left hanging. If every client is connected to postgres, you're waking up tens if not hundreds of processes tens if not hundreds of times per second. Each of them start a transaction, check which notifications are visible in the queue from clog and friends, go through the tails of every other process to see whether they should advance the tail pointer of the queue, commit the transaction and go back to sleep only to be immediately woken up again. If they're not connected to postgres directly, this kind of complex processing only happens once, and then the notification server just unconditionally serves all notifications to the clients based on a simple map lookup. It should be trivial to see how the overhead is avoided. Why don't we find and fix the actual performance issue (assuming that 07e4d03fb wasn't it)? 07e4d03fb wasn't it, no. The reason I'm not terribly enthused about this proposal is that some implementations of LISTEN (for example, our pre-9.0 one) would be unable to support LISTEN *. It's not too hard to imagine that at some point we might wish to redo the existing implementation to reduce the overhead of all listeners seeing all messages, and then having promised we could do LISTEN * would be a serious restriction on our flexibility. So while I'm not necessarily trying to veto the idea, I think it has significant opportunity cost, and I'd like to see a more solid rationale than this one before we commit to it. A reasonable point. In any case, it would be good to understand exactly what's the performance issue that's biting you. Can you provide a test case that reproduces that behavior? I've attached a Go program which simulates quite accurately the LISTEN/NOTIFY-part of our setup. What it does is: 1) Open 50 connections, and issue a LISTEN in all of them 2) Open another 50 connections, and deliver one notification every 750 milliseconds from each of them (My apologies for the fact that it's written in Go. It's the only thing I can produce without spending significant amount of time working on this.) On the test server I'm running on, it doesn't look quite as bad as the profiles we had in production, but s_lock is still the worst offender in the profiles, called from: - 80.33% LWLockAcquire + 48.34% asyncQueueReadAllNotifications + 23.09% SIGetDataEntries + 16.92% SimpleLruReadPage_ReadOnly + 10.21% TransactionIdIsInProgress + 1.27% asyncQueueAdvanceTail which roughly looks like what I recall from our actual production profiles. .m package main import ( "github.com/lib/pq" "database/sql" "fmt" "log" "time" ) const connInfo = "host=/var/run/postgresql sslmode=disable" func listener(i int) { l := pq.NewListener(connInfo, time.Second, time.Second, nil) err := l.Listen(fmt.Sprintf("channel%d", i)) if err != nil { log.Fatal(err) } for { <-l.Notify } } func notifier(dbh *sql.DB, i int) { for { _, err := dbh.Exec(fmt.Sprintf("NOTIFY channel%d", i)) if err != nil { log.Fatal(err) } time.Sleep(750 * time.Millisecond) } } func main() { openDb := func() *sql.DB { db, _ := sql.Open("postgres", connInfo) err := db.Ping() if err != nil { log.Fatal(err) } db.SetMaxIdleConns(2) db.SetMaxOpenConns(2) return db } for i := 0; i < 50; i++ { go listener(i) go notifier(openDb(), i) time.Sleep(20 * time.Millisecond) } select{} } -- 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 (INSERT/UPDATE/DELETE .. RETURNING ..)
On 11/19/15 1:17 PM, Michael Paquier wrote: On Thu, Nov 19, 2015 at 11:04 AM, Marko Tiikkaja wrote: Further, if someone's going to add new stuff to PreparableStmt, she should probably think about whether it would make sense to add it to COPY and CTEs from day one, too, and in that case not splitting them up is actually a win. Personally, I would take it on the safe side and actually update it. If someone were to add a new node type in PreparableStmt I am pretty sure that we are going to forget to update the COPY part, leading us to unwelcome bugs. And that would not be cool. They'd have to get past this: + if (query->commandType != CMD_SELECT && + query->returningList == NIL) + { + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), +errmsg("COPY query must have a RETURNING clause"))); + } Of course, something might break if we added a new statement type which supported RETURNING, but I'm really not worried about that. I'm not dead set against adding some Assert(IsA()) calls here, but I don't see the point. .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: LISTEN *
On 11/19/15 4:48 AM, Pavel Stehule wrote: 2015-11-19 4:40 GMT+01:00 Marko Tiikkaja : I've in the past wanted to listen on all notification channels in the current database for debugging purposes. But recently I came across another use case. Since having multiple postgres backends listening for notifications is very inefficient (one Thursday I found out 30% of our CPU time was spent spinning on s_locks around the notification code), it makes sense to implement a notification server of sorts which then passes on notifications from postgres to interested clients. A server like this would be a lot more simple to implement if there was a way to announce that the client wants to see all notifications, regardless of the name of the channel. Attached is a very crude proof-of-concept patch implementing this. Any thoughts on the idea? It is looking much more like workaround. Proposed feature isn't bad, but optimization of current implementation should be better. Isn't possible to fix internal implementation? It's probably possible to improve the internal implementation. But the way I see it, it's always going to be less efficient than a dedicated process outside the system (or maybe as a background worker?) handing out notifications, so I don't see any point in spending my time on that. .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] proposal: LISTEN *
Hi, I've in the past wanted to listen on all notification channels in the current database for debugging purposes. But recently I came across another use case. Since having multiple postgres backends listening for notifications is very inefficient (one Thursday I found out 30% of our CPU time was spent spinning on s_locks around the notification code), it makes sense to implement a notification server of sorts which then passes on notifications from postgres to interested clients. A server like this would be a lot more simple to implement if there was a way to announce that the client wants to see all notifications, regardless of the name of the channel. Attached is a very crude proof-of-concept patch implementing this. Any thoughts on the idea? .m *** a/src/backend/commands/async.c --- b/src/backend/commands/async.c *** *** 294,299 static SlruCtlData AsyncCtlData; --- 294,304 static List *listenChannels = NIL;/* list of C strings */ /* + * If listenWildcard is set, we're listening on all channels. + */ + static bool listenWildcard = false; + + /* * State for pending LISTEN/UNLISTEN actions consists of an ordered list of * all actions requested in the current transaction. As explained above, * we don't actually change listenChannels until we reach transaction commit. *** *** 306,311 static List *listenChannels = NIL; /* list of C strings */ --- 311,317 typedef enum { LISTEN_LISTEN, + LISTEN_LISTEN_ALL, LISTEN_UNLISTEN, LISTEN_UNLISTEN_ALL } ListenActionKind; *** *** 373,378 static void queue_listen(ListenActionKind action, const char *channel); --- 379,385 static void Async_UnlistenOnExit(int code, Datum arg); static void Exec_ListenPreCommit(void); static void Exec_ListenCommit(const char *channel); + static void Exec_ListenAllCommit(void); static void Exec_UnlistenCommit(const char *channel); static void Exec_UnlistenAllCommit(void); static bool IsListeningOn(const char *channel); *** *** 598,604 Async_Notify(const char *channel, const char *payload) /* * queue_listen ! *Common code for listen, unlisten, unlisten all commands. * *Adds the request to the list of pending actions. *Actual update of the listenChannels list happens during transaction --- 605,611 /* * queue_listen ! *Common code for listen, listen all, unlisten, unlisten all commands. * *Adds the request to the list of pending actions. *Actual update of the listenChannels list happens during transaction *** *** 613,620 queue_listen(ListenActionKind action, const char *channel) /* * Unlike Async_Notify, we don't try to collapse out duplicates. It would * be too complicated to ensure we get the right interactions of !* conflicting LISTEN/UNLISTEN/UNLISTEN_ALL, and it's unlikely that there !* would be any performance benefit anyway in sane applications. */ oldcontext = MemoryContextSwitchTo(CurTransactionContext); --- 620,627 /* * Unlike Async_Notify, we don't try to collapse out duplicates. It would * be too complicated to ensure we get the right interactions of !* conflicting LISTEN/LISTEN_ALL/UNLISTEN/UNLISTEN_ALL, and it's unlikely !* that there would be any performance benefit anyway in sane applications. */ oldcontext = MemoryContextSwitchTo(CurTransactionContext); *** *** 644,649 Async_Listen(const char *channel) --- 651,671 } /* + * Async_ListenAll + * + *This is executed by the LISTEN * command. + */ + void + Async_ListenAll(void) + { + if (Trace_notify) + elog(DEBUG1, "Async_ListenAll(%d)", MyProcPid); + + queue_listen(LISTEN_LISTEN_ALL, ""); + } + + + /* * Async_Unlisten * *This is executed by the SQL unlisten command. *** *** 790,795 PreCommit_Notify(void) --- 812,818 switch (actrec->action) { case LISTEN_LISTEN: + case LISTEN_LISTEN_ALL: Exec_ListenPreCommit(); break; case LISTEN_UNLISTEN: *** *** 895,900 AtCommit_Notify(void) --- 918,926 case LISTEN_LISTEN: Exec_ListenCommit(actrec->channel); break; + case LISTEN_LISTEN_ALL: + Exec_ListenAllCommit(); + break; case LISTEN_UNLISTEN: Exec_UnlistenCommit(actrec->channel); break;
[HACKERS] Add numeric_trim(numeric)
Hi, Here's a patch for the second function suggested in 5643125e.1030...@joh.to. This is my first patch trying to do anything with numerics, so please be gentle. I'm sure it's full of stupid. January's commit fest, feedback welcome, yada yada.. .m *** a/doc/src/sgml/func.sgml --- b/doc/src/sgml/func.sgml *** *** 782,787 --- 782,800 + numeric_trim + + numeric_trim(numeric) + +numeric +remove trailing decimal zeroes after the decimal point +numeric_trim(8.4100) +8.41 + + + + + pi pi() *** a/src/backend/utils/adt/numeric.c --- b/src/backend/utils/adt/numeric.c *** *** 2825,2830 numeric_power(PG_FUNCTION_ARGS) --- 2825,2904 PG_RETURN_NUMERIC(res); } + /* + * numeric_trim() - + * + *Remove trailing decimal zeroes after the decimal point + */ + Datum + numeric_trim(PG_FUNCTION_ARGS) + { + Numeric num = PG_GETARG_NUMERIC(0); + NumericVar arg; + Numeric res; + int dscale; + int ndigits; + + if (NUMERIC_IS_NAN(num)) + PG_RETURN_NUMERIC(make_result(&const_nan)); + + init_var_from_num(num, &arg); + + ndigits = arg.ndigits; + /* for simplicity in the loop below, do full NBASE digits at a time */ + dscale = ((arg.dscale + DEC_DIGITS - 1) / DEC_DIGITS) * DEC_DIGITS; + /* trim unstored significant trailing zeroes right away */ + if (dscale > (ndigits - arg.weight - 1) * DEC_DIGITS) + dscale = (ndigits - arg.weight - 1) * DEC_DIGITS; + while (dscale > 0 && ndigits > 0) + { + NumericDigit dig = arg.digits[ndigits - 1]; + + #if DEC_DIGITS == 4 + if ((dig % 10) != 0) + break; + if (--dscale == 0) + break; + if ((dig % 100) != 0) + break; + if (--dscale == 0) + break; + if ((dig % 1000) != 0) + break; + if (--dscale == 0) + break; + #elif DEC_DIGITS == 2 + if ((dig % 10) != 0) + break; + if (--dscale == 0) + break; + #elif DEC_DIGITS == 1 + /* nothing to do */ + #else + #error unsupported NBASE + #endif + if (dig != 0) + break; + --dscale; + --ndigits; + } + arg.dscale = dscale; + arg.ndigits = ndigits; + + /* If it's zero, normalize the sign and weight */ + if (ndigits == 0) + { + arg.sign = NUMERIC_POS; + arg.weight = 0; + arg.dscale = 0; + } + + res = make_result(&arg); + free_var(&arg); + + PG_RETURN_NUMERIC(res); + } + /* -- * *** a/src/include/catalog/pg_proc.h --- b/src/include/catalog/pg_proc.h *** *** 2361,2366 DESCR("exponentiation"); --- 2361,2368 DATA(insert OID = 2169 ( powerPGNSP PGUID 12 1 0 0 0 f f f f t f i s 2 0 1700 "1700 1700" _null_ _null_ _null_ _null_ _null_ numeric_power _null_ _null_ _null_ )); DESCR("exponentiation"); DATA(insert OID = 1739 ( numeric_powerPGNSP PGUID 12 1 0 0 0 f f f f t f i s 2 0 1700 "1700 1700" _null_ _null_ _null_ _null_ _null_ numeric_power _null_ _null_ _null_ )); + DATA(insert OID = ( numeric_trim PGNSP PGUID 12 1 0 0 0 f f f f t f i s 1 0 1700 "1700" _null_ _null_ _null_ _null_ _null_ numeric_trim _null_ _null_ _null_ )); + DESCR("remove trailing decimal zeroes after the decimal point"); DATA(insert OID = 1740 ( numeric PGNSP PGUID 12 1 0 0 0 f f f f t f i s 1 0 1700 "23" _null_ _null_ _null_ _null_ _null_ int4_numeric _null_ _null_ _null_ )); DESCR("convert int4 to numeric"); DATA(insert OID = 1741 ( log PGNSP PGUID 14 1 0 0 0 f f f f t f i s 1 0 1700 "1700" _null_ _null_ _null_ _null_ _null_ "select pg_catalog.log(10, $1)" _null_ _null_ _null_ )); *** a/src/include/utils/builtins.h --- b/src/include/utils/builtins.h *** *** 1022,1027 extern Datum numeric_exp(PG_FUNCTION_ARGS); --- 1022,1028 extern Datum numeric_ln(PG_FUNCTION_ARGS); extern Datum numeric_log(PG_FUNCTION_ARGS); extern Datum numeric_power(PG_FUNCTION_ARGS); + extern Datum numeric_trim(PG_FUNCTION_ARGS); extern Datum int4_numeric(PG_FUNCTION_ARGS); extern Datum numeric_int4(PG_FUNCTION_ARGS); extern Datum int8_numeric(PG_FUNCTION_ARGS); *** a/src/test/regress/expected/numeric.out --- b/src/test/regress/expected/numeric.out *** *** 1852,1854 select l
[HACKERS] Add scale(numeric)
Hi, As suggested in 5643125e.1030...@joh.to, here's a patch for extracting the scale out of a numeric. This is 2016-01 CF material, but if someone wants to bas^H^H^Hsay nice things in the meanwhile, feel free. .m *** a/doc/src/sgml/func.sgml --- b/doc/src/sgml/func.sgml *** *** 852,857 --- 852,870 + scale + + scale(numeric) + +numeric +scale of the argument (the number of decimal digits in the fractional part) +scale(8.41) +2 + + + + + sign sign(dp or numeric) *** a/src/backend/utils/adt/numeric.c --- b/src/backend/utils/adt/numeric.c *** *** 2825,2830 numeric_power(PG_FUNCTION_ARGS) --- 2825,2847 PG_RETURN_NUMERIC(res); } + /* + * numeric_scale() - + * + *Returns the scale, i.e. the count of decimal digits in the fractional part + */ + Datum + numeric_scale(PG_FUNCTION_ARGS) + { + Numeric num = PG_GETARG_NUMERIC(0); + + if (NUMERIC_IS_NAN(num)) + PG_RETURN_NULL(); + + PG_RETURN_INT32(NUMERIC_DSCALE(num)); + } + + /* -- * *** a/src/include/catalog/pg_proc.h --- b/src/include/catalog/pg_proc.h *** *** 2361,2366 DESCR("exponentiation"); --- 2361,2368 DATA(insert OID = 2169 ( powerPGNSP PGUID 12 1 0 0 0 f f f f t f i s 2 0 1700 "1700 1700" _null_ _null_ _null_ _null_ _null_ numeric_power _null_ _null_ _null_ )); DESCR("exponentiation"); DATA(insert OID = 1739 ( numeric_powerPGNSP PGUID 12 1 0 0 0 f f f f t f i s 2 0 1700 "1700 1700" _null_ _null_ _null_ _null_ _null_ numeric_power _null_ _null_ _null_ )); + DATA(insert OID = ( scale PGNSP PGUID 12 1 0 0 0 f f f f t f i s 1 0 23 "1700" _null_ _null_ _null_ _null_ _null_ numeric_scale _null_ _null_ _null_ )); + DESCR("returns the number of decimal digits in the fractional part"); DATA(insert OID = 1740 ( numeric PGNSP PGUID 12 1 0 0 0 f f f f t f i s 1 0 1700 "23" _null_ _null_ _null_ _null_ _null_ int4_numeric _null_ _null_ _null_ )); DESCR("convert int4 to numeric"); DATA(insert OID = 1741 ( log PGNSP PGUID 14 1 0 0 0 f f f f t f i s 1 0 1700 "1700" _null_ _null_ _null_ _null_ _null_ "select pg_catalog.log(10, $1)" _null_ _null_ _null_ )); *** a/src/include/utils/builtins.h --- b/src/include/utils/builtins.h *** *** 1022,1027 extern Datum numeric_exp(PG_FUNCTION_ARGS); --- 1022,1028 extern Datum numeric_ln(PG_FUNCTION_ARGS); extern Datum numeric_log(PG_FUNCTION_ARGS); extern Datum numeric_power(PG_FUNCTION_ARGS); + extern Datum numeric_scale(PG_FUNCTION_ARGS); extern Datum int4_numeric(PG_FUNCTION_ARGS); extern Datum numeric_int4(PG_FUNCTION_ARGS); extern Datum int8_numeric(PG_FUNCTION_ARGS); *** a/src/test/regress/expected/numeric.out --- b/src/test/regress/expected/numeric.out *** *** 1852,1854 select log(3.1954752e47, 9.4792021e-73); --- 1852,1911 -1.51613372350688302142917386143459361608600157692779164475351842333265418126982165 (1 row) + -- + -- Tests for scale() + -- + select scale(numeric 'NaN'); + scale + --- + + (1 row) + + select scale(NULL::numeric); + scale + --- + + (1 row) + + select scale(1.12); + scale + --- + 2 + (1 row) + + select scale(0); + scale + --- + 0 + (1 row) + + select scale(0.00); + scale + --- + 2 + (1 row) + + select scale(1.12345); + scale + --- + 5 + (1 row) + + select scale(110123.12475871856128); + scale + --- + 14 + (1 row) + + select scale(-1123.12471856128); + scale + --- + 11 + (1 row) + + select scale(-13.000); + scale + --- + 15 + (1 row) + *** a/src/test/regress/sql/numeric.sql --- b/src/test/regress/sql/numeric.sql *** *** 983,985 select log(1.23e-89, 6.4689e45); --- 983,999 select log(0.99923, 4.58934e34); select log(1.16, 8.452010e18); select log(3.1954752e47, 9.4792021e-73); + + -- + -- Tests for scale() + -- + + select scale(numeric 'NaN'); + select scale(NULL::numeric); + select scale(1.12); + select scale(0); + select scale(0.00); + select scale(1.12345); + select scale(110123.12475871856128); + select scale(-1123.12471856128); + select scale(-13.000); -- 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 (INSERT/UPDATE/DELETE .. RETURNING ..)
On 2015-11-16 08:24, Michael Paquier wrote: On Sun, Nov 1, 2015 at 2:49 AM, Marko Tiikkaja wrote: Attached is a patch for being able to do $SUBJECT without a CTE. The reasons this is better than a CTE version are: 1) It's not obvious why a CTE version works but a plain one doesn't 2) This one has less overhead (I measured a ~12% improvement on a not-too-unreasonable test case) Would you mind sharing this test case as well as numbers? I'll try and re-puzzle it together tomorrow. It looks like I wasn't smart enough to actually save the test case anywhere. Regression tests are broken when copyselect is run in parallel because test3 is a table used there as well. A simple idea to fix this is to rename the table test3 to something else or to use a schema dedicated to this test, I just renamed it to copydml_test in the patch attached. Seems reasonable. - | COPY select_with_parens TO opt_program copy_file_name opt_with copy_options + | COPY '(' PreparableStmt ')' TO opt_program copy_file_name opt_with copy_options This does not look right, PreparableStmt is used for statements that are part of PREPARE queries, any modifications happening there would impact COPY with this implementation. I think that we had better have a new option query_with_parens. Please note that the patch attached has not changed that. This was discussed in 2010 when CTEs got the same treatment, and I agree with what was decided back then. If someone needs to make PreparableStmt different from what COPY and CTEs support, we can split them up. But they're completely identical after this patch, so splitting them up right now is somewhat pointless. Further, if someone's going to add new stuff to PreparableStmt, she should probably think about whether it would make sense to add it to COPY and CTEs from day one, too, and in that case not splitting them up is actually a win. .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: function parse_ident
On 9/11/15 12:25 PM, Pavel Stehule wrote: new update of parse_ident function patch Nice! I've certainly wanted something like this a number of times. Some comments about the v2 of the patch: - The patch doesn't apply anymore, so it should be rebased. - The docs don't even try and explain what the "strictmode" parameter does. - The comment before the parse_ident function is not up to date anymore, since "the rest" was removed from the interface. - I can't immediately grep for any uses of do { .. } while (true) from our code base. AFAICT the majority look like for (;;); I see no reason not to be consistent here. - What should happen if the input is a string like 'one.two.three.four.five.six'? Do we want to accept input like that? - I haven't reviewed the actual parsing code carefully, but didn't we already have a function which splits identifiers up? I of course can't find one with my grepping right now, so I might be wrong. .m -- 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: add \pset true/false
On 11/12/15 1:50 PM, Michael Paquier wrote: FWIW, I am -1 on the concept of enforcing output values for particular datatypes because that's not the job of psql In my view, the job of psql is to make working with a postgres database easy for us human beings. That means (among other things) formatting and aligning query output for readability. I don't see how reformatting unreadable boolean values meant for computers is that big of a stretch. .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] proposal: numeric scale functions
Hi, Dealing with "numeric"s right now in cases where it's really important that the scale is correct is quite painful. For example, if I want to accept a EUR amount as an input, I often want to reject values such as '21.413', but I'd be fine with e.g. '21.41'. My suggestion is to add two functions: one to return the number of decimal places in a numeric, and another one to remove non-significant decimal places. These two functions are useful on their own, and when combined allow you to look at the number of significant decimal places in a numeric. Any thoughts, objections? .m -- 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: Fix parallel workers connection bug in pg_dump (Bug #13727)
On 11/5/15 4:11 PM, Zeus Kronion wrote: On Nov 1, 2015 5:04 PM, "Marko Tiikkaja" wrote: However, I don't quite like the way the password cache is kept up to date in the old *or* the new code. It seems to me that it should instead look like: if (PQconnectionUsedPassword(AH->connection)) AH->savedPassword = PQpass(AH->connection); What do you think? I don't understand why this logic is preferable. Is your concern that AH->savedPassword may contain a password even when none is needed? The opposite, sort of. If the first connection uses a password, the second one doesn't, and the third one does again, you need to ask for a password again because you emptied the cache on the second attempt since it didn't use a password. Granted, this situation is quite unlikely to occur in practice, but I find the "correct" code *also* more readable. To me it reads like "if the what we're caching was applied during the connection attempt, update the cache; otherwise keep the previous value in case it's useful in the future". .m -- 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] onlyvalue aggregate (was: First Aggregate Funtion?)
On 11/2/15 12:40 PM, Dean Rasheed wrote: I'm not sure what you mean when you say accepting NULLs can hide bugs. I think that if the input values to the aggregate were 1,1,1,NULL,1,1,1 then it should raise an error. ITSM that that is more likely to reveal problems with your underlying data or the query. If you want to ignore NULLs, you can always add a FILTER(WHERE val IS NOT NULL) clause. Ah, I see. So you're arguing that the aggregate should accept NULLs as input, but consider them distinct from any non-NULL values. I thought you meant accepting NULLs and *not* considering them distinct, which could easily hide problems. In that case, I don't oppose to changing the behavior. I'll make the necessary changes. .m -- 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] onlyvalue aggregate (was: First Aggregate Funtion?)
On 11/2/15 9:32 AM, Dean Rasheed wrote: On 28 October 2015 at 16:50, Marko Tiikkaja wrote: Here's a patch for the aggregate function outlined by Corey Huinker in CADkLM=foA_oC_Ri23F9PbfLnfwXFbC3Lt8bBzRu3=cb77g9...@mail.gmail.com . +1. I've wanted something like this a few times. Of the names suggested so far, I think I prefer "single_value", and yes I think it should work with NULLs. This was actually a last-minute design change I made before submitting the patch. The two times I've previously written this aggregate both accepted NULLs and only enforced that there must not be more than one non-NULL value, but that's only because I was thinking about the "poor man's FILTER" case, which is obsolete since version 9.4. The reason I changed in this version is that accepting NULLs can also hide bugs, and it's (usually) easy to filter them out with FILTER. Did you have some specific use case in mind where accepting NULLs would be beneficial? .m -- 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: Fix parallel workers connection bug in pg_dump (Bug #13727)
On 10/25/15 10:55 PM, Zeus Kronion wrote: Parallel workers were failing to connect to the database when running pg_dump with a connection string. The first of the following two commands runs without errors, while the second one fails: pg_dump "postgres://my-user:my-passw...@my.hostname.com:5432/my-db" -Fd -f my-dump pg_dump "postgres://my-user:my-passw...@my.hostname.com:5432/my-db" -Fd --jobs=9 -f my-dump The error message: pg_dump: [parallel archiver] connection to database "my-db" failed: fe_sendauth: no password supplied The password is not being stored correctly in the PGconn object when connecting with a connection string. Yeah, the current code is definitely broken for this case. However, I don't feel like this patch is quite there yet, either. _connectDB has similar logic in it which might be hit in case e.g. a a user's HBA is changed from a non-password-requiring method to a password-requiring one after the one or more connections has been initiated. That one needs changing as well. However, I don't quite like the way the password cache is kept up to date in the old *or* the new code. It seems to me that it should instead look like: if (PQconnectionUsedPassword(AH->connection)) AH->savedPassword = PQpass(AH->connection); What do you think? .m -- 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] September 2015 Commitfest
On 11/1/15 11:36 AM, Michael Paquier wrote: On Sun, Nov 1, 2015 at 1:53 AM, Marko Tiikkaja wrote: Are we doing these in an Australian time zone now? It was quite unpleasant to find that the 2015-11 is "in progress" already and two of my patches will not be in there. AFAIR the submission deadline used to be around UTC midnight of the first day of the month the CF nominally begins on. Er, well. Sorry for that... I did all this stuff on Friday evening before leaving back for Japan with not much time on the table. I have switched the CF back to an open status for now. And I'll switch it back to in-progress in 24 hours. If there are patches you would like attached to the CF app don't hesitate to ping me. Thanks. I managed to add the two patches just fine now. .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] COPY (INSERT/UPDATE/DELETE .. RETURNING ..)
Hi, Attached is a patch for being able to do $SUBJECT without a CTE. The reasons this is better than a CTE version are: 1) It's not obvious why a CTE version works but a plain one doesn't 2) This one has less overhead (I measured a ~12% improvement on a not-too-unreasonable test case) With regard to RULEs, similar restrictions apply as the ones on data-modifying statements in WITH. I can't add this to November's commit fest, but I'm posting this anyway in case someone is thinking about implementing this feature. .m diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml index 2850b47..07e2f45 100644 --- a/doc/src/sgml/ref/copy.sgml +++ b/doc/src/sgml/ref/copy.sgml @@ -112,10 +112,17 @@ COPY { table_name [ ( query - A or - command - whose results are to be copied. - Note that parentheses are required around the query. + A , , + , or + command whose results are to be + copied. Note that parentheses are required around the query. + + + For INSERT, UPDATE and + DELETE queries a RETURNING clause must be provided, + and the target relation must not have a conditional rule, nor + an ALSO rule, nor an INSTEAD rule + that expands to multiple statements. diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 9a52ec6..eb2f126 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -201,7 +201,7 @@ typedef struct CopyStateData int raw_buf_len; /* total # of bytes stored */ } CopyStateData; -/* DestReceiver for COPY (SELECT) TO */ +/* DestReceiver for COPY (query) TO */ typedef struct { DestReceiver pub; /* publicly-known function pointers */ @@ -772,7 +772,8 @@ CopyLoadRawBuf(CopyState cstate) * * Either unload or reload contents of table , depending on . * ( = TRUE means we are inserting into the table.) In the "TO" case - * we also support copying the output of an arbitrary SELECT query. + * we also support copying the output of an arbitrary SELECT, INSERT, UPDATE + * or DELETE query. * * If is false, transfer is between the table and the file named * . Otherwise, transfer is between the table and our regular @@ -1374,11 +1375,11 @@ BeginCopy(bool is_from, Assert(!is_from); cstate->rel = NULL; - /* Don't allow COPY w/ OIDs from a select */ + /* Don't allow COPY w/ OIDs from a query */ if (cstate->oids) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("COPY (SELECT) WITH OIDS is not supported"))); + errmsg("COPY (query) WITH OIDS is not supported"))); /* * Run parse analysis and rewrite. Note this also acquires sufficient @@ -1393,9 +1394,36 @@ BeginCopy(bool is_from, rewritten = pg_analyze_and_rewrite((Node *) copyObject(raw_query), queryString, NULL, 0); - /* We don't expect more or less than one result query */ - if (list_length(rewritten) != 1) - elog(ERROR, "unexpected rewrite result"); + /* check that we got back something we can work with */ + if (rewritten == NIL) + { + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("DO INSTEAD NOTHING rules are not supported for COPY"))); + } + else if (list_length(rewritten) > 1) + { + ListCell *lc; + + /* examine queries to determine which error message to issue */ + foreach(lc, rewritten) + { +Query *q = (Query *) lfirst(lc); + +if (q->querySource == QSRC_QUAL_INSTEAD_RULE) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("conditional DO INSTEAD rules are not supported for COPY"))); +if (q->querySource == QSRC_NON_INSTEAD_RULE) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("DO ALSO rules are not supported for the COPY"))); + } + + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("multi-statement DO INSTEAD rules are not supported for COPY"))); + } query = (Query *) linitial(rewritten); @@ -1406,9 +1434,20 @@ BeginCopy(bool is_from, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("COPY (SELECT INTO) is not supported"))); - Assert(query->commandType == CMD_SELECT); Assert(query->utilityStmt == NULL); + /* + * Similarly the grammar doesn't enforce he presence of a RETURNING + * clause, but we require it. + */ + if (query->commandType != CMD_SELECT && + query->returningList == NIL) + { + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("COPY query must have a RETURNING clause"))); + } + /* plan the query */ plan = pg_plan_query(query, 0, NULL); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index fba91d5..d87ae93 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -2561,9 +2561,12 @@ ClosePortalStmt: * * QUERY : *COPY relname [(columnList)] FROM/TO file [WITH] [(options)] - *COPY ( SELE
Re: [HACKERS] September 2015 Commitfest
On 10/31/15 12:42 AM, Michael Paquier wrote: So, seeing nothing happening I have done the above, opened 2015-11 CF and closed the current one. Are we doing these in an Australian time zone now? It was quite unpleasant to find that the 2015-11 is "in progress" already and two of my patches will not be in there. AFAIR the submission deadline used to be around UTC midnight of the first day of the month the CF nominally begins on. .m -- 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: add \pset true/false
On 10/29/15 11:51 AM, Daniel Verite wrote: Marko Tiikkaja wrote: Since the default t/f output for booleans is not very user friendly, attached is a patch which enables you to do for example the following: Personally I think it would be worth having, but how about booleans inside ROW() or composite types ? There's not enough information sent over to do that in the client. Note that this works the same way as \pset null with SELECT ROW(NULL), so I don't consider it a show stopper for the patch. .m -- 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: add \pset true/false
On 10/28/15 11:52 PM, Robert Haas wrote: -0 on this concept from me. I'm not going to vigorously oppose it, but: 1. You can always do it in the query if you really want it. True, but not always practical. 2. If you're the sort of person liable to be confused by t/f, you probably aren't in the target audience for psql anyway. Really? The difference between t/f is that the vertical squiggle is flipped, THAT'S IT. Consider: t t f f f f t f t f Saying that I'm not target audience for not being able to see WTF is going on above I find offending. 3. I really don't want to end up with a bunch of features of this type for a variety of different data types. Fine. Then let's not add it for a different variety of data types. But boolean is quite essential and it has a really small number of valid output values. .m -- 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] onlyvalue aggregate (was: First Aggregate Funtion?)
On 10/28/15 5:53 PM, Pavel Stehule wrote: what is use case for this function and why it should be in core? Corey had one example in his email, but I can offer another one which came up this week at $work. The query looked something like this: SELECT a, sum(amount), onlyvalue(rolling_count) FROM ( SELECT a, amount, count(*) OVER (ORDER BY a) AS rolling_count FROM tbl ) ss GROUP BY a; We know that all the values for the column are going to be the same value for every "a", so we could use min() or max(). But the advantage of "onlyvalue" is that it actually checks that, so if someone went and changed the window frame to do something slightly different, the query would blow up instead of silently returning the (now likely incorrect) minimum or maximum value. It's also self-documenting for the reader of such queries. In my experience this problem comes up often enough that it would be make sense to have this aggregate in core. .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] onlyvalue aggregate (was: First Aggregate Funtion?)
Hi, Here's a patch for the aggregate function outlined by Corey Huinker in CADkLM=foA_oC_Ri23F9PbfLnfwXFbC3Lt8bBzRu3=cb77g9...@mail.gmail.com . I called it "onlyvalue", which is a horrible name, but I have nothing better to offer. (Corey called it "only", but that doesn't really work since ONLY is a fully reserved keyword.) I'll add this to September's commit fest, but if you want to bash me or the patch in the meanwhile, go ahead. .m diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 2946122..6edc220 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -12631,6 +12631,26 @@ NULL baz(3 rows) +onlyvalue + + + onlyvalue(expression) + + + + any type for which the equality operator has been defined + + + same as argument type + + returns the single distinct non-NULL value from the input + values; if any of the input values is NULL or more than one distinct + value exists, an exception is raised + + + + + string_agg diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c index c0495d9..72bb55c 100644 --- a/src/backend/utils/adt/misc.c +++ b/src/backend/utils/adt/misc.c @@ -40,6 +40,7 @@ #include "utils/acl.h" #include "utils/builtins.h" #include "utils/timestamp.h" +#include "utils/typcache.h" #define atooid(x) ((Oid) strtoul((x), NULL, 10)) @@ -598,3 +599,93 @@ pg_column_is_updatable(PG_FUNCTION_ARGS) PG_RETURN_BOOL((events & REQ_EVENTS) == REQ_EVENTS); } + +struct onlyvalue_agg_stype +{ + FunctionCallInfoData fcinfo; + Datum datum; +}; + +Datum +onlyvalue_agg_transfn(PG_FUNCTION_ARGS) +{ + Oid arg1_typeid = get_fn_expr_argtype(fcinfo->flinfo, 1); + MemoryContext aggcontext; + struct onlyvalue_agg_stype *state; + + if (arg1_typeid == InvalidOid) + ereport(ERROR, +(errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("could not determine input data type"))); + + if (!AggCheckCallContext(fcinfo, &aggcontext)) + { + /* cannot be called directly because of internal-type argument */ + elog(ERROR, "onlyvalue_agg_transfn called in non-aggregate context"); + } + + if (PG_ARGISNULL(1)) + { + ereport(ERROR, +(errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("NULL value passed to onlyvalue"))); + } + + if (PG_ARGISNULL(0)) + { + TypeCacheEntry *typentry; + + state = (struct onlyvalue_agg_stype *) MemoryContextAlloc(aggcontext, sizeof(struct onlyvalue_agg_stype)); + state->datum = PG_GETARG_DATUM(1); + + typentry = lookup_type_cache(arg1_typeid, + TYPECACHE_EQ_OPR_FINFO); + if (!OidIsValid(typentry->eq_opr_finfo.fn_oid)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_FUNCTION), + errmsg("could not identify an equality operator for type %s", + format_type_be(arg1_typeid; + + InitFunctionCallInfoData(state->fcinfo, &typentry->eq_opr_finfo, 2, + InvalidOid, NULL, NULL); + } + else + { + bool oprresult; + + state = (struct onlyvalue_agg_stype *) PG_GETARG_POINTER(0); + + state->fcinfo.argnull[0] = false; + state->fcinfo.argnull[1] = false; + state->fcinfo.arg[0] = state->datum; + state->fcinfo.arg[1] = PG_GETARG_DATUM(1); + state->fcinfo.isnull = false; + oprresult = DatumGetBool(FunctionCallInvoke(&state->fcinfo)); + if (!oprresult) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("more than one distinct value passed to onlyvalue"))); + } + + /* + * The transition type for onlyvalue() is declared to be "internal", which + * is a pass-by-value type the same size as a pointer. So we can safely + * pass the pointer through nodeAgg.c's machinations. + */ + PG_RETURN_POINTER(state); +} + +Datum +onlyvalue_agg_finalfn(PG_FUNCTION_ARGS) +{ + struct onlyvalue_agg_stype *state; + + if (PG_ARGISNULL(0)) + PG_RETURN_NULL(); + + /* cannot be called directly because of internal-type argument */ + Assert(AggCheckCallContext(fcinfo, NULL)); + + state = (struct onlyvalue_agg_stype *) PG_GETARG_POINTER(0); + PG_RETURN_DATUM(state->datum); +} diff --git a/src/include/catalog/pg_aggregate.h b/src/include/catalog/pg_aggregate.h index dd6079f..9d6c44a 100644 --- a/src/include/catalog/pg_aggregate.h +++ b/src/include/catalog/pg_aggregate.h @@ -292,6 +292,9 @@ DATA(insert ( 3197 n 0 json_object_agg_transfn json_object_agg_finalfn -- DATA(insert ( 3267 n 0 jsonb_agg_transfn jsonb_agg_finalfn ---f f 0 2281 0 0 0 _null_ _null_ )); DATA(insert ( 3270 n 0 jsonb_object_agg_transfn jsonb_object_agg_finalfn ---f f 0 2281 0 0 0 _null_ _null_ )); +/* onlyvalue */ +DATA(insert ( 4202 n 0 onlyvalue_agg_transfnonlyvalue_agg_finalfn- - - t f 0 22810 0 0 _null_ _null_ )); + /* ordered-set and hypothetical-set aggregates */ DATA(insert ( 3972 o 1 ordered_set_transition percentile_disc_final - - - t f 0 2281 0 0
[HACKERS] psql: add \pset true/false
Hello hello, Since the default t/f output for booleans is not very user friendly, attached is a patch which enables you to do for example the following: =# \pset true TRUE Boolean TRUE display is "TRUE". =# \pset false FALSE Boolean FALSE display is "FALSE". =# select true, false; bool | bool --+--- TRUE | FALSE (1 row) (For anyone reviewing: I didn't touch the parts of describe.c which do this for nullPrint: myopt.nullPrint = NULL; since I thought it was dubious in the first place, and I don't think we output booleans in the describe commands.) .m diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 212dbfa..2048ba3 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -2229,6 +2229,26 @@ lo_import 152801 + true + + + Sets the string to be printed in place of a boolean TRUE value. + The default is to print a 't' character. + + + + + + false + + + Sets the string to be printed in place of a boolean FALSE value. + The default is to print an 'f' character. + + + + + null diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 5163c76..7fadb0a 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -1151,7 +1151,7 @@ exec_command(const char *cmd, int i; static const char *const my_list[] = { "border", "columns", "expanded", "fieldsep", "fieldsep_zero", -"footer", "format", "linestyle", "null", +"footer", "format", "linestyle", "true", "false", "null", "numericlocale", "pager", "pager_min_lines", "recordsep", "recordsep_zero", "tableattr", "title", "tuples_only", @@ -2591,6 +2591,26 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) } } + /* boolean TRUE display */ + else if (strcmp(param, "true") == 0) + { + if (value) + { + free(popt->truePrint); + popt->truePrint = pg_strdup(value); + } + } + + /* boolean FALSE display */ + else if (strcmp(param, "false") == 0) + { + if (value) + { + free(popt->falsePrint); + popt->falsePrint = pg_strdup(value); + } + } + /* field separator for unaligned text */ else if (strcmp(param, "fieldsep") == 0) { @@ -2782,6 +2802,20 @@ printPsetInfo(const char *param, struct printQueryOpt *popt) popt->nullPrint ? popt->nullPrint : ""); } + /* show true display */ + else if (strcmp(param, "true") == 0) + { + printf(_("Boolean TRUE display is \"%s\".\n"), + popt->truePrint ? popt->truePrint : "t"); + } + + /* show false display */ + else if (strcmp(param, "false") == 0) + { + printf(_("Boolean FALSE display is \"%s\".\n"), + popt->falsePrint ? popt->falsePrint : "f"); + } + /* show locale-aware numeric output */ else if (strcmp(param, "numericlocale") == 0) { @@ -2953,6 +2987,14 @@ pset_value_string(const char *param, struct printQueryOpt *popt) return psprintf("%s", _align2string(popt->topt.format)); else if (strcmp(param, "linestyle") == 0) return psprintf("%s", get_line_style(&popt->topt)->name); + else if (strcmp(param, "true") == 0) + return pset_quoted_string(popt->truePrint + ? popt->truePrint + : "t"); + else if (strcmp(param, "false") == 0) + return pset_quoted_string(popt->falsePrint + ? popt->falsePrint + : "f"); else if (strcmp(param, "null") == 0) return pset_quoted_string(popt->nullPrint ? popt->nullPrint diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c index 5b63e76..d0bf418 100644 --- a/src/bin/psql/help.c +++ b/src/bin/psql/help.c @@ -258,7 +258,7 @@ slashUsage(unsigned short int pager) fprintf(output, _(" \\H toggle HTML output mode (currently %s)\n"), ON(pset.popt.topt.format == PRINT_HTML)); fprintf(output, _(" \\pset [NAME [VALUE]] set table output option\n" - " (NAME := {format|border|expanded|fieldsep|fieldsep_zero|footer|null|\n" + " (NAME := {format|border|expanded|fieldsep|fieldsep_zero|footer|true|false|null|\n" " numericlocale|recordsep|recordsep_zero|tuples_only|title|tableattr|pager|\n" " unicode_border_linestyle|unicode_column_linestyle|unicode_header_linestyle})\n")); fprintf(output, _(" \\t [on|off]show only rows (currently %s)\n"), @@ -371,6 +371,8 @@ helpVariables(unsigned short int pager) fprintf(output, _(" format set output format [unaligned, aligned, wrapped, html, asciidoc, ...]\n")); fprintf(output, _(" footer enable or disable display of the table footer [on, off]\n")); fprintf(output, _(" linestyle set the border line drawing style [ascii, old-ascii, unicode]\n")); + fprintf(output, _(" true
Re: [HACKERS] Questionable behavior regarding aliasing
On 2015-10-09 10:31 PM, Jim Nasby wrote: I was about to report this as a bug until Marko Tiikkaja pointed out on IRC that now was being treated as an alias for relname. I'm not sure if this is required by the spec, but can we at least emit a WARNING if not reject this case outright? I think it'd be OK to leave the AS syntax alone... presumably it's a lot harder to fumble that. But I'm not wed to that. I'd be happy to turn on a GUC disabling this misfeature. It's only ever brought pain and unhappiness to me and my family. .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers