Re: [HACKERS] Precedence of standard comparison operators
Robert Haas robertmh...@gmail.com writes: On Tue, Mar 10, 2015 at 12:45 PM, David G. Johnston david.g.johns...@gmail.com wrote: I would vote for Auto meaning On in the .0 release. I don't think users will appreciate an auto value whose meaning might change at some point, and I doubt we've have much luck identifying the correct point, either. Users will upgrade over the course of years, not months, and will not necessarily complete their application retesting within any particular period of time thereafter. Yeah, I think that's too cute by far. And people do not like things like this changing in point releases. If we do this, I envision it as being on by default in 9.5 and then changing the default in 9.6 or 9.7 or so. Another possibility is to leave it on through beta testing with the intent to turn it off before 9.5 final; that would give us more data about whether there are real issues than we're likely to get otherwise. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: disallow operator = and use it for named parameters
Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Mar 10, 2015 at 11:50 AM, Tom Lane t...@sss.pgh.pa.us wrote: Was there any consideration given to whether ruleutils should start printing NamedArgExprs with =? Or do we think that needs to wait? I have to admit that I didn't consider that. What do you think? I guess I'd be tentatively in favor of changing that to match, but I could be convinced otherwise. Presumably we are going to change it at some point; maybe we should just do it rather than waiting another 5 years. +1 It has been deprecated long enough that I don't see the point of waiting. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: disallow operator = and use it for named parameters
Kevin Grittner kgri...@ymail.com writes: Tom Lane t...@sss.pgh.pa.us wrote: Presumably we are going to change it at some point; maybe we should just do it rather than waiting another 5 years. +1 It has been deprecated long enough that I don't see the point of waiting. Uh, just to clarify, this has nothing to do with how long the operator has been deprecated. The issue is whether pg_dump should dump a function-call syntax that will not be recognized by any pre-9.5 release, when there is an alternative that will be recognized back to 9.0. BTW, I just noticed another place that probably should be changed: regression=# select foo(x = 1); ERROR: 42883: function foo(x := integer) does not exist LINE 1: select foo(x = 1); ^ HINT: No function matches the given name and argument types. You might need to add explicit type casts. LOCATION: ParseFuncOrColumn, parse_func.c:516 regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal : REINDEX xxx VERBOSE
On Tue, Mar 10, 2015 at 5:05 PM, Jim Nasby jim.na...@bluetreble.com wrote:
On 3/9/15 9:43 PM, Sawada Masahiko wrote:
On Fri, Mar 6, 2015 at 11:07 AM, Jim Nasby jim.na...@bluetreble.com
wrote:
On 3/2/15 10:58 AM, Sawada Masahiko wrote:
On Wed, Feb 25, 2015 at 4:58 PM, Jim Nasby jim.na...@bluetreble.com
wrote:
On 2/24/15 8:28 AM, Sawada Masahiko wrote:
According to the above discussion, VACUUM and REINDEX should have
trailing options. Tom seems (to me) suggesting that SQL-style
(bare word preceded by WITH) options and Jim suggesting '()'
style options? (Anyway VACUUM gets the*third additional* option
sytle, but it is the different discussion from this)
Well, almost everything does a trailing WITH. We need to either stick
with
that for consistency, or add leading () as an option to those WITH
commands.
Does anyone know why those are WITH? Is it ANSI?
As a refresher, current commands are:
VACUUM (ANALYZE, VERBOSE) table1 (col1);
REINDEX INDEX index1 FORCE;
COPY table1 FROM 'file.txt' WITH (FORMAT csv);
CREATE MATERIALIZED VIEW mv1 WITH (storageparam, ...) AS qry WITH
DATA;
CREATE EXTENSION ext1 WITH SCHEMA s1 VERSION v1 FROM over;
CREATE ROLE role WITH LOGIN;
GRANT WITH GRANT OPTION;
CREATE VIEW v1 AS qry WITH CASCADED CHECK OPTION;
ALTER DATABASE db1 WITH CONNECTION LIMIT 50;
DECLARE c1 INSENSITIVE SCROLL CURSOR WITH HOLD;
BTW, I'm fine with Tom's bare-word with WITH idea. That seems to be the
most
consistent with everything else. Is there a problem with doing that? I
know
getting syntax is one of the hard parts of new features, but it seems
like
we reached consensus here...
Attached is latest version patch based on Tom's idea as follows.
REINDEX { INDEX | ... } name WITH ( options [, ...] )
Are the parenthesis necessary? No other WITH option requires them, other
than create table/matview (COPY doesn't actually require them).
We have discussed about this option including FORCE option, but I
think there are not user who want to use both FORCE and VERBOSE option
at same time.
I find that very hard to believe... I would expect a primary use case for
VERBOSE to be I ran REINDEX, but it doesn't seem to have done
anything...
what's going on? and that's exactly when you might want to use FORCE.
In currently code, nothing happens even if FORCE option is specified.
This option completely exist for backward compatibility.
But this patch add new syntax including FORCE option for now.
I forgot that. There's no reason to support it with the new stuff then.
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com
Attached patch is latest version patch changed syntax a little.
This patch adds following syntax for now.
REINDEX { INDEX | ... } name WITH (VERBOSE);
But we are under the discussion regarding parenthesis, so there is
possibility of change.
Regards,
---
Sawada Masahiko
000_reindex_verbose_v4.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] Doing better at HINTing an appropriate column within errorMissingColumn()
On Tue, Mar 10, 2015 at 4:03 PM, Peter Geoghegan p...@heroku.com wrote: On Tue, Mar 10, 2015 at 11:28 AM, Robert Haas robertmh...@gmail.com wrote: I'm prepared to commit this version if nobody finds a problem with it. It passes the additional regression tests you wrote. Looks good to me. Thanks. OK, committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] timestamp format escape rules
Currently Postgres uses DATE WHITESPACE TIME for timestamp/tz datatype. Due whitespace it needs to be escaped when returned as record. How likely is to change WHITESPACE to something more ISO like, like 'T'? This would allow it to be serialized without the need for escaping. I'm asking this since we often return nested records directly from Postgres and when reading deeply nested records, we are mostly skipping on quotes/escapes. Regards, Rikard -- Rikard Pavelic https://dsl-platform.com/ http://templater.info/ -- 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] EvalPlanQual behaves oddly for FDW queries involving system columns
On 2015/03/12 13:35, Tom Lane wrote:
I don't like the execMain.c changes much at all. They look somewhat
like they're intended to allow foreign tables to adopt a different
locking strategy,
I didn't intend such a thing. My intention is, foreign tables have
markType = ROW_MARK_COPY as ever, but I might not have correctly
understood what you pointed out. Could you elaborate on that?
The changes are not all consistent
either, eg this:
! if (erm-relation
! erm-relation-rd_rel-relkind != RELKIND_FOREIGN_TABLE)
is not necessary if this Assert is accurate:
! if (erm-relation)
! {
! Assert(erm-relation-rd_rel-relkind ==
RELKIND_FOREIGN_TABLE);
I modified InitPlan so that relations get opened for foreign tables
as well as local tables. So, I think the change would be necessary.
I don't see the need for the change in nodeForeignscan.c. If the FDW has
returned a physical tuple, it can fix that for itself, while if it has
returned a virtual tuple, the ctid will be garbage in any case.
If you leave nodeForeignscan.c unchanged, file_fdw would cause the
problem that I pointed out upthread. Here is an example.
[Create a test environment]
postgres=# create foreign table foo (a int) server file_server options
(filename '/home/pgsql/foo.csv', format 'csv');
CREATE FOREIGN TABLE
postgres=# select tableoid, ctid, * from foo;
tableoid | ctid | a
--++---
16459 | (4294967295,0) | 1
(1 row)
postgres=# create table tab (a int, b int);
CREATE TABLE
postgres=# insert into tab values (1, 1);
INSERT 0 1
[Run concurrent transactions]
In terminal1:
postgres=# begin;
BEGIN
postgres=# update tab set b = b * 2;
UPDATE 1
In terminal2:
postgres=# select foo.tableoid, foo.ctid, foo.* from foo, tab where
foo.a = tab.a for update;
In terminal1:
postgres=# commit;
COMMIT
In terminal2:(When committing the UPDATE query in terminal1, psql in
terminal2 will display the result for the SELECT-FOR-UPDATE query as
shown below.)
tableoid | ctid | a
--+---+---
16459 | (0,0) | 1
(1 row)
Note the value of the ctid has changed!
Rather than changing nodeForeignscan.c, it might be better to change
heap_form_tuple to set the t_ctid of a formed tuple to be invalid.
Thanks for the review!
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Install shared libs in lib/ and bin/ with MSVC (Was: install libpq.dll in bin directory on Windows / Cygwin)
Thank you Michael overall v3 patch looks good to me, There is one
observation that it is not installing following lib files that are required
for dev work i.e.
inst\lib\libpq.lib
inst\lib\libpgtypes.lib
inst\lib\libecpg_compat.lib
inst\lib\libecpg.lib
Please do let me know if I missed something but was not there a need to
avoid installing related .dll files in lib (duplicate) along with bin
directory e.g.
src\tools\msvc\Install.pm
if ($is_sharedlib)
{
@dirs = qw(bin);
}
else
{
@dirs = qw(lib);
}
Thanks.
On Mon, Mar 9, 2015 at 1:16 PM, Michael Paquier michael.paqu...@gmail.com
wrote:
On Wed, Mar 4, 2015 at 4:13 AM, Michael Paquier wrote:
Those entries are removed as well in the patch.
Please find attached a new version of the patch, fixing a failure for
plperl installation that contains GNUmakefile instead of Makefile.
--
Michael
Re: [HACKERS] Turning off HOT/Cleanup sometimes
On 2015-03-11 20:55:18 -0400, Peter Eisentraut wrote: I don't think so. Andres basically wanted a nontrival algorithm to determine how much pruning to do during a read-only scan. And Robert basically said, that's not really possible. I don't think either of us made really strong statements. We have seen some benchmarks that show significant improvements. We have seen some (constructed ones) that show problems. FWIW, it's not that constructed. It's just a mixture of read with write load. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] One question about security label command
On Tue, Mar 10, 2015 at 6:58 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: ERRCODE_FEATURE_NOT_SUPPORTED is suitable error code here. Please see the attached one. Committed. I did not bother back-patching this, but I can do that if people think it's important. The sepgsql regression tests don't seem to pass for me any more; I wonder if some expected-output changes are needed as a result of core changes. I'm guessing these tests are not running in an automated fashion anywhere? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company regression.diffs 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
[HACKERS] shebang for tcl postgresql modules
Dear hackers, I would like to ask you if is there any reason to pretend tcl scripts are shell scripts? I am speaking namely about these files http://git.postgresql.org/gitweb/?p=postgresql.git;a=blob_plain;f=src/pl/tcl/modules/pltcl_delmod.in;hb=HEAD http://git.postgresql.org/gitweb/?p=postgresql.git;a=blob_plain;f=src/pl/tcl/modules/pltcl_listmod.in;hb=HEAD http://git.postgresql.org/gitweb/?p=postgresql.git;a=blob_plain;f=src/pl/tcl/modules/pltcl_loadmod.in;hb=HEAD Here is the part of code I am speaking about: #! /bin/sh # Start tclsh \ exec @TCLSH@ $0 $@ instead of #! /usr/bin/tclsh or #! @TCLSH@ I am asking because our test suite is triggering errors on this [1]. In this case, it seems easier to modify code rather then test suite. Please apply attached patch if there is no particular reason for use of /bin/sh. [1] https://bugzilla.redhat.com/show_bug.cgi?id=1199464 -- Jozef Mlich jml...@redhat.com Associate Software Engineer - EMEA ENG Developer Experience Mobile: +420 604 217 719 http://cz.redhat.com/ Red Hat, Inc. diff --git a/src/pl/tcl/modules/pltcl_delmod.in b/src/pl/tcl/modules/pltcl_delmod.in index daa4fac..6b8cda4 100644 --- a/src/pl/tcl/modules/pltcl_delmod.in +++ b/src/pl/tcl/modules/pltcl_delmod.in @@ -1,8 +1,6 @@ -#! /bin/sh +#! @TCLSH@ # src/pl/tcl/modules/pltcl_delmod.in # -# Start tclsh \ -exec @TCLSH@ $0 $@ # # Code still has to be documented diff --git a/src/pl/tcl/modules/pltcl_listmod.in b/src/pl/tcl/modules/pltcl_listmod.in index 7d930ff..c3ea138 100644 --- a/src/pl/tcl/modules/pltcl_listmod.in +++ b/src/pl/tcl/modules/pltcl_listmod.in @@ -1,8 +1,6 @@ -#! /bin/sh +#! @TCLSH@ # src/pl/tcl/modules/pltcl_listmod.in # -# Start tclsh \ -exec @TCLSH@ $0 $@ # # Code still has to be documented diff --git a/src/pl/tcl/modules/pltcl_loadmod.in b/src/pl/tcl/modules/pltcl_loadmod.in index 645c6bb..06b459b 100644 --- a/src/pl/tcl/modules/pltcl_loadmod.in +++ b/src/pl/tcl/modules/pltcl_loadmod.in @@ -1,6 +1,5 @@ -#! /bin/sh -# Start tclsh \ -exec @TCLSH@ $0 $@ +#! @TCLSH@ +# # # Code still has to be documented -- 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] How about to have relnamespace and relrole?
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed Looks good. Passing it to committer. The new status of this patch is: Ready for Committer -- 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] Parallel Seq Scan
On 10-03-2015 PM 01:09, Amit Kapila wrote: On Tue, Mar 10, 2015 at 6:50 AM, Haribabu Kommi kommi.harib...@gmail.com Is this patch handles the cases where the re-scan starts without finishing the earlier scan? Do you mean to say cases like ANTI, SEMI Join (in nodeNestLoop.c) where we scan the next outer tuple and rescan inner table without completing the previous scan of inner table? I have currently modelled it based on existing rescan for seqscan (ExecReScanSeqScan()) which means it will begin the scan again. Basically if the workers are already started/initialized by previous scan, then re-initialize them (refer function ExecReScanFunnel() in patch). From Robert's description[1], it looked like the NestLoop with Funnel would have Funnel as either outer plan or topmost plan node or NOT a parameterised plan. In that case, would this case arise or am I missing something? Thanks, Amit [1] http://www.postgresql.org/message-id/CA+TgmobM7X6jgre442638b+33h1EWa=vczqnsvzedx057zh...@mail.gmail.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] OOM-killer issue when updating a inheritance table which has large number of child tables
Hi
In my test(PG9.3.4), i found when update a parent table which has a large
number of child tables, the execute plan will consume lots of memory. And
possibly cause OOM.
For example:
create table maintb(id int,name char(10));
create table childtb_1 (CHECK ( id BETWEEN 1 AND 200)) inherits(maintb);
create table childtb_2 (CHECK ( id BETWEEN 201 AND 400)) inherits(maintb);
...
create table childtb_n ...
When there are 100 child tables,the following update statement will consume
about 8MB memory when invoking pg_plan_queries()
update maintb set name = 'a12345' where id=1;
And, when there are 1000 child tables,the same update statement will consume
717MB memory when invoking pg_plan_queries().
Does this a known problem, and could that be improved in the future?
BTW:
The following comment is according my debuging when update the parent table
with 1000 child tables
src/backend/optimizer/plan/planner.c
static Plan *
inheritance_planner(PlannerInfo *root)
{
...
foreach(lc, root-append_rel_list)//### loop 1001 time
{
...
subroot.parse = (Query *)
adjust_appendrel_attrs(root,
(Node *) parse,
appinfo);//### allocate about 300KB memory a
time.
...
subroot.append_rel_list = (List *)
copyObject(root-append_rel_list);//### allocate about 400KB memory a time.
...
}
...
}
Best Regards
Chen Huajun
Re: [HACKERS] Precedence of standard comparison operators
On Wed, Mar 11, 2015 at 8:36 PM, Kevin Grittner kgri...@ymail.com wrote: Right; and they may have millions of lines of code which have been carefully tested and in production for years, and which may suddenly start to generate incorrect results on queries *or mangle existing data* with the fix unless they change their SQL code. Well not suddenly. When doing a major upgrade. And we provide a tool to help them identify the problems. But having a warning enabled by default means the problem is effectively not fixed at all. People will not be able to write code that's valid according to the docs and the spec without extra parentheses or disabling the warning. -- greg
Re: [HACKERS] alter user/role CURRENT_USER
Thank you for completing this and very sorry not to respond these
days.
I understood that it is committed after I noticed that rebasing
my code failed..
Although after committed, I found some issues as I looked on
it. Please forgive me to comment it now after all this time.
Several changes in docs according to the changed syntax and one
change in code itself to allow CURRENT_USER in GRANT roleid TO
roleid syntax.
At Mon, 9 Mar 2015 15:50:32 -0300, Alvaro Herrera alvhe...@2ndquadrant.com
wrote in 20150309185032.gq3...@alvh.no-ip.org
Alvaro Herrera wrote:
With this patch applied, doing
\h ALTER ROLE
in psql looks quite odd: note how wide it has become. Maybe we should
be doing this differently? (Hmm, why don't we accept ALL in the first SET
line? Maybe that's just a mistake and the four lines should be all
identical in the first half ...)
I have fixed the remaining issues, completed the doc changes, and
pushed. Given the lack of feedback I had to follow my gut on the best
way to change the docs. I added the regression test you submitted with
some additional changes, mainly to make sure they don't fail immediately
when other databases exist; maybe some more concurrency or platform
issues will show up there, but let's see what the buildfarm says.
Thanks Horiguchi-san for the patch and everyone for the reviews. (It's
probably worthwhile giving things an extra look.)
| =# alter role current_user rename to PubLic;
| ERROR: CURRENT_USER cannot be used as a role name
| LINE 1: alter role current_user rename to PubLic;
|^
The error message sounds somewhat different from the intention. I
think the following message would be clearer.
| ERROR: CURRENT_USER cannot be used as a role name here
The document sql-altergroup.html says
| ALTER GROUP role_specification ADD USER user_name [, ... ]
But current_user is also usable in user_name list. So the doc
should be as following, but it would not be necessary to be fixed
because it is an obsolete commnand..
| ALTER GROUP role_specification ADD USER role_specification [, ... ]
ALTER GROUP role_spec ADD/DROP USER role_spec is naturally
denied so I think no additional description is needed.
sql-alterpolicy.html
ALTER POLICY name ON table_name TO also accepts current_user
and so as the role to which the policy applies.
# As a different topic, the syntax ALTER POLICY pname ON
# tname TO user looks a bit wired, it might be better be to
# be ON tname APPLY TO user but I shouldn't try to fix it
# since it is a long standing syntax..
sql-createtablespace.html
OWNER username should be OWNER user_name | (CURRENT|SESSION)_USER
sql-drop-owned.html, sql-reassign-owned.html
name should be {name | (CURRENT|SESSION)_USER}
For REASSIGN OWNED, TO clause also needs the same fix.
==
sql-grant.html, sql-revoke.html,
GRANT roles TO roles and REVOKE roles FROM roles are
the modern equivalents of the deprecated syntaxes ALTER roles
ADD USER roles and ALTER roles DROP USER roles
respectively. But the current parser infrastructure doesn't allow
coexistence of the two following syntaxes but I couldn't find the
way to their coexistence.
# more precisely, I guess the GRANT followed by both
# privelege_list and role_list will steps out of the realm of
# LALR(1), which I don't know well of..
GRANT privs ON ...
GRANT roles TO ...
After some struggle, I decided to add special rules
(CURRENT|SESSION)_USER to the non-terminal privilege and make a
room to store RoleSpec in AccessPriv. This is quite ugly but
there seems no way other than that to accomplish it.. (AccessPriv
already conveys other than the information different from what
its name represents:p)
After this fix, the commands like following are processed
properly. public and none are simply handled as nonexistent
names.
GRANT test1 TO CURRENT_USER;
GRANT priv ON table TO role properly rejects CURRENT_USER
as priv.
But the change in gram.y cuases changes in preproc.y as following,
privilege:
SELECT opt_column_list
{
...
| ColId opt_column_list
{
$$ = cat_str(2,$1,$2);
}
+ | CURRENT_USER
+ {
+ $$ = mm_strdup(current_user);
+ }
+ | SESSION_USER
+ {
+ $$ = mm_strdup(session_user);
+ }
;
I don't understand what this change causes...
=
I haven't looked on the docs for syntaxes related to
AlterOwnerStatement. But perhaps they don't be wrong.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
From 26afc656576c8778921ff44519e3de86866ab138 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi horiguchi.kyot...@lab.ntt.co.jp
Date: Thu, 12 Mar 2015 20:56:14 +0900
Subject: [PATCH] Some additional changes for ALTER ROLE/USER CURRENT_USER.
Some documents are not edit according to new specs. Addition to it,
GRANT roles TO roles and REVOKE roles FROM roles syntaxes,
which are the modern replacement of ALTER GROUP ADD/DROP USER
syntax, are not accepted by the previous patch.
This patch fixes some docs and changes
Re: [HACKERS] OOM-killer issue when updating a inheritance table which has large number of child tables
On Thu, Mar 12, 2015 at 06:55:48PM +0800, chenhj wrote: Hi In my test(PG9.3.4), i found when update a parent table which has a large number of child tables, the execute plan will consume lots of memory. And possibly cause OOM. At the moment, partitioning into thousands of tables is not supported. If you can reproduce the problem in PostgreSQL 9.3.6, or whichever happens to be the most recent minor version by the time you do the test, that will help. Just generally, it helps to provide a complete test case which reproduces the problem if at all possible. Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical column ordering
Hi, On 2015-03-11 22:16:52 -0400, Tom Lane wrote: I agree though that it's worth considering defining pg_attribute.attnum as the logical column position so as to minimize the effects on client-side code. I actually wonder if it'd not make more sense to define it as the physical column number. That'd reduce the invasiveness and risk of the patch considerably. It means that most existing code doesn't have to be changed and can just continue to refer to attnum like today. There's much less risk of it being wrongly used to refer to the physical offset instead of creation order. Queries against attnum would still give a somewhat sane response. It would make some ALTER TABLE commands a bit more complex if we want to allow reordering the physical order. But that seems like a much more localized complexity than previous patches in this thread (although I've not looked at the last version). Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_dump: CREATE TABLE + CREATE RULE vs. relreplident
Hi, On 2015-03-12 14:25:24 +0100, Marko Tiikkaja wrote: My colleague Per Lejontand brought to my attention that when dumping views with circular dependencies from a postgres version older than 9.4 using a recent pg_dump, the SQL looks something like the following: create table qwr(); create rule _RETURN as on select to qwr do instead select; In this case the relreplident column in pg_class for the view ends up being 'd', instead of the 'n' normally used for views. Patch to update relreplident when turning a table into a view is attached; this makes sure that the identity is NOTHING regardless of how the view was created. I think that's a good idea. I consider this a bug fix, and suggest back patching to 9.4. I agree on backpatching it. Arguably we could additionally avoid emitting the ALTER TABLE ... REPLICA IDENTITY for views that have already been created with identity set like this. But I doubt it's worth it. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical column ordering
Tomas Vondra wrote: On 12.3.2015 03:16, Tom Lane wrote: I agree though that it's worth considering defining pg_attribute.attnum as the logical column position so as to minimize the effects on client-side code. I doubt there is much stuff client-side that cares about column creation order, but there is plenty that cares about logical column order. OTOH this would introduce confusion into the backend code, since Alvaro's definition of attnum is what most of the backend should care about. IMHO reusing attnum for logical column order would actually make it more complex, especially if we allow users to modify the logical order using ALTER TABLE. Because if you change it, you have to walk through all the places where it might be referenced and update those too (say, columns referenced in indexes and such). Keeping attnum immutable makes this much easier and simpler. I think you're misunderstanding. The suggestion, as I understand it, is to rename the attnum column to something else (maybe, say, attidnum), and rename attlognum to attnum. That preserves the existing property that ORDER BY attnum gives you the correct view of the table from the point of view of the user. That's very useful because it means clients looking at pg_attribute need less changes, or maybe none at all. I think this wouldn't be too difficult to implement, because there aren't that many places that refer to the column-identity attribute by name; most of them just grab the TupleDesc-attrs array in whatever order is appropriate and scan that in a loop. Only a few of these use att-attnum inside the loop --- that's what would need to be changed, and it should be pretty mechanical. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_dump: CREATE TABLE + CREATE RULE vs. relreplident
Marko == Marko Tiikkaja ma...@joh.to writes: Marko Hi, Marko My colleague Per Lejontand brought to my attention that when Marko dumping views with circular dependencies from a postgres version Marko older than 9.4 using a recent pg_dump, the SQL looks something Marko like the following: Marko create table qwr(); Marko create rule _RETURN as on select to qwr do instead select; I've wondered for a while whether this wouldn't have been better handled as: create view qwr(colnames...) as select null::type1, null::type2, ...; /* ... */ create or replace view qwr as ...; -- Andrew (irc:RhodiumToad) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] OOM-killer issue when updating a inheritance table which has large number of child tables
chenhj chjis...@163.com writes: In my test(PG9.3.4), i found when update a parent table which has a large number of child tables, the execute plan will consume lots of memory. And possibly cause OOM. See file:///net/sss1/home/postgres/pgsql/doc/src/sgml/html/ddl-partitioning.html#DDL-PARTITIONING-CAVEATS particularly the last paragraph. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_dump: CREATE TABLE + CREATE RULE vs. relreplident
Andrew Gierth and...@tao11.riddles.org.uk writes: Marko == Marko Tiikkaja ma...@joh.to writes: Marko create table qwr(); Marko create rule _RETURN as on select to qwr do instead select; I've wondered for a while whether this wouldn't have been better handled as: create view qwr(colnames...) as select null::type1, null::type2, ...; /* ... */ create or replace view qwr as ...; Yeah, possibly. The existing pg_dump coding dates from before we had CREATE OR REPLACE VIEW. But we'll have to live with pg_dump files that do this for the indefinite future, so I agree some fix is needed on the backend side. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical column ordering
Alvaro Herrera alvhe...@2ndquadrant.com writes: However, there's a difference between making a query silently given different results, and breaking it completely forcing the user to re-study how to write it. I think the latter is better. In that light we should just drop attnum as a column name, and use something else: maybe (attidnum, attlognum, attphysnum). So all queries in the wild would be forced to be updated, but we would not silently change semantics instead. Hm. I'm on board with renaming like that inside the backend, but I'm very much less excited about forcibly breaking client queries. I think there is little if any client-side code that will care about either attidnum or attphysnum, so forcing people to think about it will just create make-work. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] recovery_target_action doesn't work for anything but shutdown
Hi, Unless I'm missing something recovery_target_action = promote/pause don't work. There's the following block of code in readRecoveryCommandFile(): /* * Override any inconsistent requests. Not that this is a change * of behaviour in 9.5; prior to this we simply ignored a request * to pause if hot_standby = off, which was surprising behaviour. */ if (recoveryTargetAction == RECOVERY_TARGET_ACTION_PAUSE recoveryTargetActionSet standbyState == STANDBY_DISABLED) recoveryTargetAction = RECOVERY_TARGET_ACTION_SHUTDOWN; the problem is that when the recovery command file is read standbyState will always still be STANDBY_DISABLED. Which makes sense, because we can't even know we're in recovery before readRecoveryCommandFile(). I guess what you actually intended to test was StandbyModeRequested? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] recovery_target_action doesn't work for anything but shutdown
On 2015-03-12 15:52:02 +0100, Andres Freund wrote: I guess what you actually intended to test was StandbyModeRequested? Err, EnableHotStandby. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical column ordering
On 12.3.2015 14:17, Alvaro Herrera wrote: Tomas Vondra wrote: On 12.3.2015 03:16, Tom Lane wrote: I agree though that it's worth considering defining pg_attribute.attnum as the logical column position so as to minimize the effects on client-side code. I doubt there is much stuff client-side that cares about column creation order, but there is plenty that cares about logical column order. OTOH this would introduce confusion into the backend code, since Alvaro's definition of attnum is what most of the backend should care about. IMHO reusing attnum for logical column order would actually make it more complex, especially if we allow users to modify the logical order using ALTER TABLE. Because if you change it, you have to walk through all the places where it might be referenced and update those too (say, columns referenced in indexes and such). Keeping attnum immutable makes this much easier and simpler. I think you're misunderstanding. The suggestion, as I understand it, is to rename the attnum column to something else (maybe, say, attidnum), and rename attlognum to attnum. That preserves the existing property that ORDER BY attnum gives you the correct view of the table from the point of view of the user. That's very useful because it means clients looking at pg_attribute need less changes, or maybe none at all. Hmm ... I understood it as a suggestion to drop attlognum and just define (attnum, attphysnum). I think this wouldn't be too difficult to implement, because there aren't that many places that refer to the column-identity attribute by name; most of them just grab the TupleDesc-attrs array in whatever order is appropriate and scan that in a loop. Only a few of these use att-attnum inside the loop --- that's what would need to be changed, and it should be pretty mechanical. I think it's way more complicated. We may fix all the pieces of the code, but that's not all - attnum is referenced in various system views, catalogs and such. For example pg_stats view does this: FROM pg_statistic s JOIN pg_class c ON (c.oid = s.starelid) JOIN pg_attribute a ON (c.oid = attrelid AND attnum = s.staattnum) LEFT JOIN pg_namespace n ON (n.oid = c.relnamespace) WHERE NOT attisdropped AND has_column_privilege(c.oid, a.attnum, 'select'); information_schema also uses attnum on many places too. I see the catalogs as a kind of public API, and redefining the meaning of an existing column this way seems tricky, especially when we reference it from other catalogs - I'm pretty sure there's plenty of SQL queries in various tools that rely on this. Just google for pg_indexes indkeys unnest and you'll find posts like this one from Craig: http://stackoverflow.com/questions/18121103/how-to-get-the-index-column-orderasc-desc-nulls-first-from-postgresql specifically tell people to do this: SELECT ... FROM ( SELECT pg_class.relname, ... unnest(pg_index.indkey) AS k FROM pg_index INNER JOIN pg_class ON pg_index.indexrelid = pg_class.oid ) i ... INNER JOIN pg_attribute ON (pg_attribute.attrelid = i.indrelid AND pg_attribute.attnum = k); which specifically tells people to match attnum vs. indkeys. If we redefine the meaning of attnum, and instead match indkeys against a different column (say, attidnum), all those queries will be broken. Which actually breaks the catalog definition as specified here: http://www.postgresql.org/docs/devel/static/catalog-pg-index.html which explicitly says that indkey references pg_attribute.attnum. But maybe we don't really care about breaking this API and it is a good approach - I need to think about it and try it. -- Tomas Vondrahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pg_dump: CREATE TABLE + CREATE RULE vs. relreplident
Hi, My colleague Per Lejontand brought to my attention that when dumping views with circular dependencies from a postgres version older than 9.4 using a recent pg_dump, the SQL looks something like the following: create table qwr(); create rule _RETURN as on select to qwr do instead select; In this case the relreplident column in pg_class for the view ends up being 'd', instead of the 'n' normally used for views. Patch to update relreplident when turning a table into a view is attached; this makes sure that the identity is NOTHING regardless of how the view was created. I consider this a bug fix, and suggest back patching to 9.4. .m diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c index f540432..a88d73e 100644 --- a/src/backend/rewrite/rewriteDefine.c +++ b/src/backend/rewrite/rewriteDefine.c @@ -597,6 +597,7 @@ DefineQueryRewrite(char *rulename, classForm-relhaspkey = false; classForm-relfrozenxid = InvalidTransactionId; classForm-relminmxid = InvalidMultiXactId; + classForm-relreplident = REPLICA_IDENTITY_NOTHING; simple_heap_update(relationRelation, classTup-t_self, classTup); CatalogUpdateIndexes(relationRelation, classTup); -- 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] Install shared libs in lib/ and bin/ with MSVC (Was: install libpq.dll in bin directory on Windows / Cygwin)
On Wed, Mar 11, 2015 at 10:03 PM, Asif Naeem anaeem...@gmail.com wrote:
Thank you Michael overall v3 patch looks good to me, There is one
observation that it is not installing following lib files that are required
for dev work i.e.
Thanks for your input.
inst\lib\libpq.lib
inst\lib\libpgtypes.lib
inst\lib\libecpg_compat.lib
inst\lib\libecpg.lib
Please do let me know if I missed something but was not there a need to
avoid installing related .dll files in lib (duplicate) along with bin
directory e.g?
Yeas, right. Sorry for missing something like that. In HEAD, the
following things are installed regarding the shared libraries:
- In lib/, all .dll and .lib
- In bin/, only libpq.dll
So I have recoded the patch to use an hash of arrays (makes the code
more readable IMO) to be able to track more easily what to install
where, and process now does the following for shared libraries:
- In lib/, install all .dll and .lib
- In bin/, install all .dll
I hope that's fine to you.
Regards,
--
Michael
diff --git a/src/tools/msvc/Install.pm b/src/tools/msvc/Install.pm
index eba9aa0..c35a9dd 100644
--- a/src/tools/msvc/Install.pm
+++ b/src/tools/msvc/Install.pm
@@ -91,7 +91,6 @@ sub Install
}
CopySolutionOutput($conf, $target);
- lcopy($target . '/lib/libpq.dll', $target . '/bin/libpq.dll');
my $sample_files = [];
my @top_dir = (src);
@top_dir = (src\\bin, src\\interfaces) if ($insttype eq client);
@@ -108,12 +107,8 @@ sub Install
$target . '/lib/',
$conf\\,
postgres\\postgres.lib,
- libpq\\libpq.lib,
- libecpg\\libecpg.lib,
libpgcommon\\libpgcommon.lib,
- libpgport\\libpgport.lib,
- libpgtypes\\libpgtypes.lib,
- libecpg_compat\\libecpg_compat.lib);
+ libpgport\\libpgport.lib);
CopyContribFiles($config, $target);
CopyIncludeFiles($target);
@@ -236,8 +231,16 @@ sub CopySolutionOutput
while ($sln =~ $rem)
{
my $pf = $1;
- my $dir;
- my $ext;
+
+ # Hash of array to list things to install. This has the
+ # shape similar to that for each project to indicate
+ # what should be installed where. The hash key indicates
+ # the installation location, and the array elements define
+ # the file type to install:
+ # 'bin' = [ 'dll', 'lib' ]
+ # 'lib' = [ 'lib' ]
+ my %install_list;
+ my $is_sharedlib = 0;
$sln =~ s/$rem//;
@@ -247,22 +250,47 @@ sub CopySolutionOutput
my $proj = read_file($pf.$vcproj)
|| croak Could not open $pf.$vcproj\n;
+
+ # Check if this project uses a shared library by looking if
+ # SO_MAJOR_VERSION is defined in its Makefile, whose path
+ # can be found using the resource file of this project.
+ if (($vcproj eq 'vcxproj'
+ $proj =~ qr{ResourceCompile\s*Include=([^]+)}) ||
+ ($vcproj eq 'vcproj'
+ $proj =~ qr{File\s*RelativePath=([^\]+)\.rc}))
+ {
+ my $projpath = dirname($1);
+ my $mfname = -e $projpath/GNUmakefile ?
+$projpath/GNUmakefile : $projpath/Makefile;
+ my $mf = read_file($mfname) ||
+croak Could not open $mfname\n;
+
+ if ($mf =~ /^SO_MAJOR_VERSION\s*=\s*(.*)$/mg)
+ {
+$is_sharedlib = 1;
+ }
+ }
+
if ($vcproj eq 'vcproj' $proj =~ qr{ConfigurationType=([^]+)})
{
if ($1 == 1)
{
-$dir = bin;
-$ext = exe;
+push( @{ $install_list { 'bin' } }, exe);
}
elsif ($1 == 2)
{
-$dir = lib;
-$ext = dll;
+push( @{ $install_list { 'lib' } }, dll);
+if ($is_sharedlib)
+{
+ push( @{ $install_list { 'bin' } }, dll);
+ push( @{ $install_list { 'lib' } }, lib);
+}
}
else
{
-# Static lib, such as libpgport, only used internally during build, don't install
+# Static libraries, such as libpgport, only used internally
+# during build, don't install.
next;
}
}
@@ -271,18 +299,21 @@ sub CopySolutionOutput
{
if ($1 eq 'Application')
{
-$dir = bin;
-$ext = exe;
+push( @{ $install_list { 'bin' } }, exe);
}
elsif ($1 eq 'DynamicLibrary')
{
-$dir = lib;
-$ext = dll;
+push( @{ $install_list { 'lib' } }, dll);
+if ($is_sharedlib)
+{
+ push( @{ $install_list { 'bin' } }, dll);
+ push( @{ $install_list { 'lib' } }, lib);
+}
}
else# 'StaticLibrary'
{
-
-# Static lib, such as libpgport, only used internally during build, don't install
+# Static lib, such as libpgport, only used internally
+# during build, don't install.
next;
}
}
@@ -290,8 +321,16 @@ sub CopySolutionOutput
{
croak Could not parse $pf.$vcproj\n;
}
- lcopy($conf\\$pf\\$pf.$ext, $target\\$dir\\$pf.$ext)
- || croak Could not copy $pf.$ext\n;
+
+ # Install each element
+ foreach my $dir ( keys %install_list )
+ {
+ foreach my $ext ( @{ $install_list{ $dir } } )
+ {
+lcopy($conf\\$pf\\$pf.$ext, $target\\$dir\\$pf.$ext)
+ || croak Could not copy $pf.$ext\n;
+ }
+ }
lcopy($conf\\$pf\\$pf.pdb, $target\\symbols\\$pf.pdb)
|| croak Could not copy $pf.pdb\n;
print .;
--
Sent via pgsql-hackers mailing list
Re: [HACKERS] logical column ordering
Tomas Vondra wrote: On 12.3.2015 14:17, Alvaro Herrera wrote: Tomas Vondra wrote: On 12.3.2015 03:16, Tom Lane wrote: I agree though that it's worth considering defining pg_attribute.attnum as the logical column position so as to minimize the effects on client-side code. I doubt there is much stuff client-side that cares about column creation order, but there is plenty that cares about logical column order. OTOH this would introduce confusion into the backend code, since Alvaro's definition of attnum is what most of the backend should care about. IMHO reusing attnum for logical column order would actually make it more complex, especially if we allow users to modify the logical order using ALTER TABLE. Because if you change it, you have to walk through all the places where it might be referenced and update those too (say, columns referenced in indexes and such). Keeping attnum immutable makes this much easier and simpler. I think you're misunderstanding. The suggestion, as I understand it, is to rename the attnum column to something else (maybe, say, attidnum), and rename attlognum to attnum. That preserves the existing property that ORDER BY attnum gives you the correct view of the table from the point of view of the user. That's very useful because it means clients looking at pg_attribute need less changes, or maybe none at all. Hmm ... I understood it as a suggestion to drop attlognum and just define (attnum, attphysnum). Pretty sure it wasn't that. I think this wouldn't be too difficult to implement, because there aren't that many places that refer to the column-identity attribute by name; most of them just grab the TupleDesc-attrs array in whatever order is appropriate and scan that in a loop. Only a few of these use att-attnum inside the loop --- that's what would need to be changed, and it should be pretty mechanical. I think it's way more complicated. We may fix all the pieces of the code, but that's not all - attnum is referenced in various system views, catalogs and such. For example pg_stats view does this: FROM pg_statistic s JOIN pg_class c ON (c.oid = s.starelid) JOIN pg_attribute a ON (c.oid = attrelid AND attnum = s.staattnum) LEFT JOIN pg_namespace n ON (n.oid = c.relnamespace) WHERE NOT attisdropped AND has_column_privilege(c.oid, a.attnum, 'select'); information_schema also uses attnum on many places too. Those can be fixed with relative ease to refer to attidnum instead. I see the catalogs as a kind of public API, and redefining the meaning of an existing column this way seems tricky, especially when we reference it from other catalogs - I'm pretty sure there's plenty of SQL queries in various tools that rely on this. That's true, but then we've never promised that system catalogs remain unchanged forever. That would essentially stop development. However, there's a difference between making a query silently given different results, and breaking it completely forcing the user to re-study how to write it. I think the latter is better. In that light we should just drop attnum as a column name, and use something else: maybe (attidnum, attlognum, attphysnum). So all queries in the wild would be forced to be updated, but we would not silently change semantics instead. Which actually breaks the catalog definition as specified here: http://www.postgresql.org/docs/devel/static/catalog-pg-index.html which explicitly says that indkey references pg_attribute.attnum. That's a simple doc fix. But maybe we don't really care about breaking this API and it is a good approach - I need to think about it and try it. Yeah, thanks. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] shebang for tcl postgresql modules
Jozef Mlich jml...@redhat.com writes: I would like to ask you if is there any reason to pretend tcl scripts are shell scripts? That is the time-honored standard formatting for tcl scripts, see for example page 2 here: http://www.tcl.tk/doc/styleGuide.pdf While we might get away with ignoring that convention for this specific use-case, it carries a risk of breaking things, eg for people who have multiple Tcl installations. I'm disinclined to change it, especially not if the reason is rpmdiff is too dumb to recognize tcl scripts. Somebody's gonna need to fix that anyway. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel Seq Scan
On Thu, Mar 12, 2015 at 8:33 PM, Thom Brown t...@linux.com wrote: On 12 March 2015 at 14:46, Amit Kapila amit.kapil...@gmail.com wrote: One additional change (we need to SetLatch() in HandleParallelMessageInterrupt) is done to handle the hang issue reported on parallel-mode thread. Without this change it is difficult to verify the patch (will remove this change once new version of parallel-mode patch containing this change will be posted). Applied parallel-mode-v7.patch and parallel_seqscan_v10.patch, but getting this error when building: gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -O2 -I../../../../src/include -D_GNU_SOURCE -c -o brin.o brin.c -MMD -MP -MF .deps/brin.Po In file included from ../../../../src/include/nodes/execnodes.h:18:0, from ../../../../src/include/access/brin.h:14, from brin.c:18: ../../../../src/include/access/heapam.h:119:34: error: unknown type name ‘ParallelHeapScanDesc’ extern void heap_parallel_rescan(ParallelHeapScanDesc pscan, HeapScanDesc scan); ^ Am I missing another patch here? Yes, the below parallel-heap-scan patch. http://www.postgresql.org/message-id/ca+tgmoyjetgeaxuszrona7bdtwzptqexpjntv1gkcavmgsd...@mail.gmail.com Please note that parallel_setup_cost and parallel_startup_cost are still set to zero by default, so you need to set it to higher values if you don't want the parallel plans once parallel_seqscan_degree is set. I have yet to comeup with default values for them, needs some tests. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: [HACKERS] assessing parallel-safety
[ deferring responses to some points until a later time ]
On Thu, Feb 19, 2015 at 1:19 AM, Noah Misch n...@leadboat.com wrote:
This seems backwards to me. If some hypothetical selectivity
estimator were PROPARALLEL_UNSAFE, then any operator that uses that
function would also need to be PROPARALLEL_UNSAFE.
It's fair to have a PROPARALLEL_UNSAFE estimator for a PROPARALLEL_SAFE
function, because the planning of a parallel query is often not itself done in
parallel mode. In that case, SELECT * FROM tablename WHERE colname OP 0
might use a parallel seqscan but fail completely if called from inside a
function running in parallel mode. That is to say, an affected query can
itself use parallelism, but placing the query in a function makes the function
PROPARALLEL_UNSAFE. Surprising, but not wrong.
Rereading my previous message, I failed to make the bottom line clear: I
recommend marking eqsel etc. PROPARALLEL_UNSAFE but *not* checking an
estimator's proparallel before calling it in the planner.
But what do these functions do that is actually unsafe?
- Assuming you don't want to propagate XactLastRecEnd from the slave back
to
the master, restrict XLogInsert() during parallel mode. Restrict
functions
that call it, including pg_create_restore_point, pg_switch_xlog and
pg_stop_backup.
Hmm. Altogether prohibiting XLogInsert() in parallel mode seems
unwise, because it would foreclose heap_page_prune_opt() in workers.
I realize there's separate conversation about whether pruning during
SELECT queries is good policy, but in the interested of separating
mechanism from policy, and in the sure knowledge that allowing at
least some writes in parallel mode is certainly going to be something
people will want, it seems better to look into propagating
XactLastRecEnd.
Good points; that works for me.
The key design decision here seems to be this: How eagerly do we need
to synchronize XactLastRecEnd? What exactly goes wrong if it isn't
synchronized? For example, if the value were absolutely critical in
all circumstances, one could imagine storing a shared XactLastRecEnd
in shared memory. This doesn't appear to be the case: the main
requirement is that we definitely need an up-to-date value at commit
time. Also, at abort time, we don't really the value for anything
critical, but it's worth kicking the WAL writer so that any
accumulated WAL gets flushed.
Here's an incremental patch - which I will incorporate into the
parallel mode patch if it seems about right to you - that tries to
tackle all this.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
commit 26bd8f366415906a67abd49824fe3a980d5d4555
Author: Robert Haas rh...@postgresql.org
Date: Thu Mar 12 11:09:19 2015 -0400
Synchronize XactLastRecEnd.
diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index 225ec89..a97c899 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -15,6 +15,7 @@
#include postgres.h
#include access/xact.h
+#include access/xlog.h
#include access/parallel.h
#include commands/async.h
#include libpq/libpq.h
@@ -73,10 +74,15 @@ typedef struct FixedParallelState
/* Entrypoint for parallel workers. */
parallel_worker_main_type entrypoint;
- /* Track whether workers have attached. */
+ /* Mutex protects remaining fiedlds. */
slock_t mutex;
+
+ /* Track whether workers have attached. */
int workers_expected;
int workers_attached;
+
+ /* Maximum XactLastRecEnd of any worker. */
+ XLogRecPtr last_xlog_end;
} FixedParallelState;
/*
@@ -90,6 +96,9 @@ int ParallelWorkerNumber = -1;
/* Is there a parallel message pending which we need to receive? */
bool ParallelMessagePending = false;
+/* Pointer to our fixed parallel state. */
+static FixedParallelState *MyFixedParallelState;
+
/* List of active parallel contexts. */
static dlist_head pcxt_list = DLIST_STATIC_INIT(pcxt_list);
@@ -257,6 +266,7 @@ InitializeParallelDSM(ParallelContext *pcxt)
SpinLockInit(fps-mutex);
fps-workers_expected = pcxt-nworkers;
fps-workers_attached = 0;
+ fps-last_xlog_end = 0;
shm_toc_insert(pcxt-toc, PARALLEL_KEY_FIXED, fps);
/* Serialize GUC state to dynamic shared memory. */
@@ -398,6 +408,9 @@ LaunchParallelWorkers(ParallelContext *pcxt)
* important to call this function afterwards. We must not miss any errors
* the workers may have thrown during the parallel operation, or any that they
* may yet throw while shutting down.
+ *
+ * Also, we want to update our notion of XactLastRecEnd based on worker
+ * feedback.
*/
void
WaitForParallelWorkersToFinish(ParallelContext *pcxt)
@@ -431,6 +444,15 @@ WaitForParallelWorkersToFinish(ParallelContext *pcxt)
WaitLatch(MyProc-procLatch, WL_LATCH_SET, -1);
ResetLatch(MyProc-procLatch);
}
+
+ if (pcxt-toc != NULL)
+ {
+ FixedParallelState *fps;
+
+ fps = shm_toc_lookup(pcxt-toc,
Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns
Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes: On 2015/03/12 13:35, Tom Lane wrote: I don't like the execMain.c changes much at all. They look somewhat like they're intended to allow foreign tables to adopt a different locking strategy, I didn't intend such a thing. My intention is, foreign tables have markType = ROW_MARK_COPY as ever, but I might not have correctly understood what you pointed out. Could you elaborate on that? I think the real fix as far as postgres_fdw is concerned is in fact to let it adopt a different ROW_MARK strategy, since it has meaningful ctid values. However, that is not a one-size-fits-all answer. The fundamental problem I've got with this patch is that it's trying to impose some one-size-fits-all assumptions on all FDWs about whether ctids are meaningful; which is wrong, not to mention not backwards compatible. I don't see the need for the change in nodeForeignscan.c. If the FDW has returned a physical tuple, it can fix that for itself, while if it has returned a virtual tuple, the ctid will be garbage in any case. If you leave nodeForeignscan.c unchanged, file_fdw would cause the problem that I pointed out upthread. Here is an example. But that's self-inflicted damage, because in fact ctid correctly shows as invalid in this case in HEAD, without this patch. The tableoid problem can be fixed much less invasively as per the attached patch. I think that we should continue to assume that ctid is not meaningful (and hence should read as (4294967295,0)) in FDWs that use ROW_MARK_COPY, and press forward on fixing the locking issues for postgres_fdw by letting it use ROW_MARK_REFERENCE or something close to that. That would also cause ctid to read properly for rows from postgres_fdw. regards, tom lane diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 33b172b..5a1c3b3 100644 *** a/src/backend/executor/execMain.c --- b/src/backend/executor/execMain.c *** EvalPlanQualFetchRowMarks(EPQState *epqs *** 2438,2444 /* build a temporary HeapTuple control structure */ tuple.t_len = HeapTupleHeaderGetDatumLength(td); ItemPointerSetInvalid((tuple.t_self)); ! tuple.t_tableOid = InvalidOid; tuple.t_data = td; /* copy and store tuple */ --- 2438,2445 /* build a temporary HeapTuple control structure */ tuple.t_len = HeapTupleHeaderGetDatumLength(td); ItemPointerSetInvalid((tuple.t_self)); ! tuple.t_tableOid = getrelid(erm-rti, ! epqstate-estate-es_range_table); tuple.t_data = td; /* copy and store tuple */ -- 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] Parallel Seq Scan
On 12 March 2015 at 14:46, Amit Kapila amit.kapil...@gmail.com wrote: One additional change (we need to SetLatch() in HandleParallelMessageInterrupt) is done to handle the hang issue reported on parallel-mode thread. Without this change it is difficult to verify the patch (will remove this change once new version of parallel-mode patch containing this change will be posted). Applied parallel-mode-v7.patch and parallel_seqscan_v10.patch, but getting this error when building: gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -O2 -I../../../../src/include -D_GNU_SOURCE -c -o brin.o brin.c -MMD -MP -MF .deps/brin.Po In file included from ../../../../src/include/nodes/execnodes.h:18:0, from ../../../../src/include/access/brin.h:14, from brin.c:18: ../../../../src/include/access/heapam.h:119:34: error: unknown type name ‘ParallelHeapScanDesc’ extern void heap_parallel_rescan(ParallelHeapScanDesc pscan, HeapScanDesc scan); ^ Am I missing another patch here? -- Thom -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_dump: CREATE TABLE + CREATE RULE vs. relreplident
Andrew Gierth wrote: Tom == Tom Lane t...@sss.pgh.pa.us writes: Tom Yeah, possibly. The existing pg_dump coding dates from before we Tom had CREATE OR REPLACE VIEW. As it happens it does not; the issue came up originally because of a hack I came up with, and I've never used any pg version so old it didn't have CREATE OR REPLACE VIEW. Nor does it look like the change was ever backpatched (or at least not that far). http://www.postgresql.org/message-id/20986.1102296...@sss.pgh.pa.us Wow --- We've had CREATE OR REPLACE VIEW since 2002: commit 248c67d7ed505d98d3a94cd3954835255317ff16 Author: Tom Lane t...@sss.pgh.pa.us Date: Mon Sep 2 02:13:02 2002 + CREATE OR REPLACE VIEW, CREATE OR REPLACE RULE. Gavin Sherry, Neil Conway, and Tom Lane all got their hands dirty on this one ... What is newer is the ability to add columns: commit ff1ea2173a92dea975d399a4ca25723f83762e55 Author: Bruce Momjian br...@momjian.us Date: Sat Dec 6 23:22:46 2008 + Allow CREATE OR REPLACE VIEW to add columns to the _end_ of the view. Robert Haas (Andrew's original post is from 2004) -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pg_xlog_replay_resume() considered armed and dangerous
Hi, I think it's quite confusing that a function named pg_xlog_replay_resume() can cause a node to be promoted. That this is happened is kind of documented in the recovery.conf section of the manual: The intended use of the pause setting is to allow queries to be executed against the database to check if this recovery target is the most desirable point for recovery. The paused state can be resumed by using pg_xlog_replay_resume() (see Table 9-69), which then causes recovery to end. If this recovery target is not the desired stopping point, then shut down the server, change the recovery target settings to a later target and restart to continue recovery. But it's not mentioned at all in the section about the functions: http://www.postgresql.org/docs/devel/static/functions-admin.html#FUNCTIONS-RECOVERY-CONTROL-TABLE Promotion only happens when a node is paused due to a recovery target setting, and not when it's stopped due to pg_xlog_replay_pause(). I think this, at the very least, needs a big caveat in the documentation of the resume function. But a different API would probably be better. I'd actually argue that for now pg_xlog_replay_resume() should refuse to work if paused due to a recovery target. Promotion should be done via the normal promotion methods. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_dump: CREATE TABLE + CREATE RULE vs. relreplident
Tom == Tom Lane t...@sss.pgh.pa.us writes: I've wondered for a while whether this wouldn't have been better handled as: create view qwr(colnames...) as select null::type1, null::type2, ...; /* ... */ create or replace view qwr as ...; Tom Yeah, possibly. The existing pg_dump coding dates from before we Tom had CREATE OR REPLACE VIEW. As it happens it does not; the issue came up originally because of a hack I came up with, and I've never used any pg version so old it didn't have CREATE OR REPLACE VIEW. Nor does it look like the change was ever backpatched (or at least not that far). http://www.postgresql.org/message-id/20986.1102296...@sss.pgh.pa.us Of course, at the time I myself didn't think of using a view rather than a table for the initial creation; I was focused on rules because that was the only way to get updateable views then. So arguably it is at least partly my fault. Tom But we'll have to live with pg_dump files that do this for the Tom indefinite future, so I agree some fix is needed on the backend Tom side. Certainly. -- Andrew (irc:RhodiumToad) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel Seq Scan
On 12 March 2015 at 15:29, Amit Kapila amit.kapil...@gmail.com wrote:
On Thu, Mar 12, 2015 at 8:33 PM, Thom Brown t...@linux.com wrote:
On 12 March 2015 at 14:46, Amit Kapila amit.kapil...@gmail.com wrote:
One additional change (we need to SetLatch() in
HandleParallelMessageInterrupt)
is done to handle the hang issue reported on parallel-mode thread.
Without this change it is difficult to verify the patch (will remove
this
change
once new version of parallel-mode patch containing this change will be
posted).
Applied parallel-mode-v7.patch and parallel_seqscan_v10.patch, but
getting this error when building:
gcc -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Wendif-labels
-Wmissing-format-attribute -Wformat-security -fno-strict-aliasing
-fwrapv -fexcess-precision=standard -O2 -I../../../../src/include
-D_GNU_SOURCE -c -o brin.o brin.c -MMD -MP -MF .deps/brin.Po
In file included from ../../../../src/include/nodes/execnodes.h:18:0,
from ../../../../src/include/access/brin.h:14,
from brin.c:18:
../../../../src/include/access/heapam.h:119:34: error: unknown type
name ‘ParallelHeapScanDesc’
extern void heap_parallel_rescan(ParallelHeapScanDesc pscan,
HeapScanDesc scan);
^
Am I missing another patch here?
Yes, the below parallel-heap-scan patch.
http://www.postgresql.org/message-id/ca+tgmoyjetgeaxuszrona7bdtwzptqexpjntv1gkcavmgsd...@mail.gmail.com
Please note that parallel_setup_cost and parallel_startup_cost are
still set to zero by default, so you need to set it to higher values
if you don't want the parallel plans once parallel_seqscan_degree
is set. I have yet to comeup with default values for them, needs
some tests.
Thanks. Getting a problem:
createdb pgbench
pgbench -i -s 200 pgbench
CREATE TABLE pgbench_accounts_1 (CHECK (bid = 1)) INHERITS (pgbench_accounts);
...
CREATE TABLE pgbench_accounts_200 (CHECK (bid = 200)) INHERITS
(pgbench_accounts);
WITH del AS (DELETE FROM pgbench_accounts WHERE bid = 1 RETURNING *)
INSERT INTO pgbench_accounts_1 SELECT * FROM del;
...
WITH del AS (DELETE FROM pgbench_accounts WHERE bid = 200 RETURNING *)
INSERT INTO pgbench_accounts_200 SELECT * FROM del;
VACUUM ANALYSE;
# SELECT name, setting FROM pg_settings WHERE name IN
('parallel_seqscan_degree','max_worker_processes','seq_page_cost');
name | setting
-+-
max_worker_processes| 20
parallel_seqscan_degree | 8
seq_page_cost | 1000
(3 rows)
# EXPLAIN SELECT DISTINCT bid FROM pgbench_accounts;
ERROR: too many dynamic shared memory segments
And separately, I've seen this in the logs:
2015-03-12 16:09:30 GMT [7880]: [4-1] user=,db=,client= LOG:
registering background worker parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [5-1] user=,db=,client= LOG:
registering background worker parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [6-1] user=,db=,client= LOG:
registering background worker parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [7-1] user=,db=,client= LOG:
registering background worker parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [8-1] user=,db=,client= LOG:
registering background worker parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [9-1] user=,db=,client= LOG:
registering background worker parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [10-1] user=,db=,client= LOG:
registering background worker parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [11-1] user=,db=,client= LOG:
registering background worker parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [12-1] user=,db=,client= LOG:
starting background worker process parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [13-1] user=,db=,client= LOG:
starting background worker process parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [14-1] user=,db=,client= LOG:
starting background worker process parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [15-1] user=,db=,client= LOG:
starting background worker process parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [16-1] user=,db=,client= LOG:
starting background worker process parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [17-1] user=,db=,client= LOG:
starting background worker process parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [18-1] user=,db=,client= LOG:
starting background worker process parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [19-1] user=,db=,client= LOG:
starting background worker process parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [20-1] user=,db=,client= LOG: worker
process: parallel worker for PID 7889 (PID 7913) exited with exit code
0
2015-03-12 16:09:30 GMT [7880]: [21-1] user=,db=,client= LOG:
unregistering background worker parallel worker for PID 7889
2015-03-12 16:09:30 GMT
Re: [HACKERS] pg_dump: CREATE TABLE + CREATE RULE vs. relreplident
On Thu, Mar 12, 2015 at 12:46 PM, Tom Lane t...@sss.pgh.pa.us wrote: As it happens it does not; the issue came up originally because of a hack I came up with, and I've never used any pg version so old it didn't have CREATE OR REPLACE VIEW. Nor does it look like the change was ever backpatched (or at least not that far). Sorry, that's mere historical revisionism. Can we please keep this a little more civil? Saying it that way implies bad faith. You could instead write I think you are mistaken. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_dump: CREATE TABLE + CREATE RULE vs. relreplident
Andrew Gierth and...@tao11.riddles.org.uk writes: Tom == Tom Lane t...@sss.pgh.pa.us writes: Tom Yeah, possibly. The existing pg_dump coding dates from before we Tom had CREATE OR REPLACE VIEW. As it happens it does not; the issue came up originally because of a hack I came up with, and I've never used any pg version so old it didn't have CREATE OR REPLACE VIEW. Nor does it look like the change was ever backpatched (or at least not that far). Sorry, that's mere historical revisionism. The oldest PG version I still have in captivity is 7.0, and in it pg_dump does this: $ createdb db1 CREATE DATABASE $ psql db1 Welcome to psql, the PostgreSQL interactive terminal. Type: \copyright for distribution terms \h for help with SQL commands \? for help on internal slash commands \g or terminate with semicolon to execute query \q to quit db1=# select version(); version -- PostgreSQL 7.0.3 on hppa2.0-hp-hpux10.20, compiled by gcc 2.95.3 (1 row) db1=# create table t1 (f1 int, f2 text); CREATE db1=# create view v1 as select * from t1; CREATE 148340 1 db1=# \q $ pg_dump db1 \connect - postgres CREATE TABLE t1 ( f1 int4, f2 text ); CREATE TABLE v1 ( f1 int4, f2 text ); COPY t1 FROM stdin; \. CREATE RULE _RETv1 AS ON SELECT TO v1 DO INSTEAD SELECT t1.f1, t1.f2 FROM t1; $ Later (in 7.1, looks like) we improved the pg_dump code to dump views as views, but the underlying ability to dump the ON SELECT rule separately was still there. I think what you are remembering is commit 86a069bbed9264daaa85270ece0a2d5959017336, but that just re-enabled the aboriginal behavior when we discover a circularity involving a view rule. If I'd had to write actual new dumping code, I probably would not have done it like that, and might have hit on the CREATE OR REPLACE VIEW solution instead. OTOH, some experimenting shows that 7.3 is the oldest version that accepts the syntax CREATE OR REPLACE VIEW, so at the time we might not have wanted to use that solution in pg_dump anyway. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_dump: CREATE TABLE + CREATE RULE vs. relreplident
Robert Haas robertmh...@gmail.com writes: On Thu, Mar 12, 2015 at 12:46 PM, Tom Lane t...@sss.pgh.pa.us wrote: Sorry, that's mere historical revisionism. Can we please keep this a little more civil? Saying it that way implies bad faith. You could instead write I think you are mistaken. Hmm, I take that phrase as being a bit of a joke. I didn't mean it to be uncivil, and if Andrew perceives it as such, I apologize. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cube extension kNN support
Documentation along with style fix. distances2r3.patch Description: Binary data On 08 Feb 2015, at 00:32, Alexander Korotkov aekorot...@gmail.com wrote: Hi! On Sat, Feb 7, 2015 at 12:45 PM, Stas Kelvich stas.kelv...@gmail.com wrote: I had updated old patch with kNN operators for cube data structures. Copying description from old message: Following distance operators introduced: # taxicab distance - euclidean distance = chebyshev distance For example: SELECT * FROM objects ORDER BY objects.coord - '(137,42,314)'::cube LIMIT 10; Also there is operator - for selecting ordered rows directly from index. This request selects rows ordered ascending by 3rd coordinate: SELECT * FROM objects ORDER BY objects.coord-3 LIMIT 10; For descendent ordering suggested syntax with minus before coordinate. This request selects rows ordered descending by 4th coordinate: SELECT * FROM objects ORDER BY objects.coord--4 LIMIT 10; I've checked the patch. The first notes are so: 1) Check coding style, in particular braces. Postgres coding style require using it for multiline statements. 2) Update documentation according to new features. -- With best regards, Alexander Korotkov. -- 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] Parallel Seq Scan
On 12 March 2015 at 16:20, Thom Brown t...@linux.com wrote:
On 12 March 2015 at 15:29, Amit Kapila amit.kapil...@gmail.com wrote:
On Thu, Mar 12, 2015 at 8:33 PM, Thom Brown t...@linux.com wrote:
On 12 March 2015 at 14:46, Amit Kapila amit.kapil...@gmail.com wrote:
One additional change (we need to SetLatch() in
HandleParallelMessageInterrupt)
is done to handle the hang issue reported on parallel-mode thread.
Without this change it is difficult to verify the patch (will remove
this
change
once new version of parallel-mode patch containing this change will be
posted).
Applied parallel-mode-v7.patch and parallel_seqscan_v10.patch, but
getting this error when building:
gcc -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Wendif-labels
-Wmissing-format-attribute -Wformat-security -fno-strict-aliasing
-fwrapv -fexcess-precision=standard -O2 -I../../../../src/include
-D_GNU_SOURCE -c -o brin.o brin.c -MMD -MP -MF .deps/brin.Po
In file included from ../../../../src/include/nodes/execnodes.h:18:0,
from ../../../../src/include/access/brin.h:14,
from brin.c:18:
../../../../src/include/access/heapam.h:119:34: error: unknown type
name ‘ParallelHeapScanDesc’
extern void heap_parallel_rescan(ParallelHeapScanDesc pscan,
HeapScanDesc scan);
^
Am I missing another patch here?
Yes, the below parallel-heap-scan patch.
http://www.postgresql.org/message-id/ca+tgmoyjetgeaxuszrona7bdtwzptqexpjntv1gkcavmgsd...@mail.gmail.com
Please note that parallel_setup_cost and parallel_startup_cost are
still set to zero by default, so you need to set it to higher values
if you don't want the parallel plans once parallel_seqscan_degree
is set. I have yet to comeup with default values for them, needs
some tests.
Thanks. Getting a problem:
createdb pgbench
pgbench -i -s 200 pgbench
CREATE TABLE pgbench_accounts_1 (CHECK (bid = 1)) INHERITS (pgbench_accounts);
...
CREATE TABLE pgbench_accounts_200 (CHECK (bid = 200)) INHERITS
(pgbench_accounts);
WITH del AS (DELETE FROM pgbench_accounts WHERE bid = 1 RETURNING *)
INSERT INTO pgbench_accounts_1 SELECT * FROM del;
...
WITH del AS (DELETE FROM pgbench_accounts WHERE bid = 200 RETURNING *)
INSERT INTO pgbench_accounts_200 SELECT * FROM del;
VACUUM ANALYSE;
# SELECT name, setting FROM pg_settings WHERE name IN
('parallel_seqscan_degree','max_worker_processes','seq_page_cost');
name | setting
-+-
max_worker_processes| 20
parallel_seqscan_degree | 8
seq_page_cost | 1000
(3 rows)
# EXPLAIN SELECT DISTINCT bid FROM pgbench_accounts;
ERROR: too many dynamic shared memory segments
And separately, I've seen this in the logs:
2015-03-12 16:09:30 GMT [7880]: [4-1] user=,db=,client= LOG:
registering background worker parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [5-1] user=,db=,client= LOG:
registering background worker parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [6-1] user=,db=,client= LOG:
registering background worker parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [7-1] user=,db=,client= LOG:
registering background worker parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [8-1] user=,db=,client= LOG:
registering background worker parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [9-1] user=,db=,client= LOG:
registering background worker parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [10-1] user=,db=,client= LOG:
registering background worker parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [11-1] user=,db=,client= LOG:
registering background worker parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [12-1] user=,db=,client= LOG:
starting background worker process parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [13-1] user=,db=,client= LOG:
starting background worker process parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [14-1] user=,db=,client= LOG:
starting background worker process parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [15-1] user=,db=,client= LOG:
starting background worker process parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [16-1] user=,db=,client= LOG:
starting background worker process parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [17-1] user=,db=,client= LOG:
starting background worker process parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [18-1] user=,db=,client= LOG:
starting background worker process parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [19-1] user=,db=,client= LOG:
starting background worker process parallel worker for PID 7889
2015-03-12 16:09:30 GMT [7880]: [20-1] user=,db=,client= LOG: worker
process: parallel worker for PID 7889 (PID 7913) exited with exit code
0
2015-03-12 16:09:30 GMT
Re: [HACKERS] Documentation of bt_page_items()'s ctid field
Jeff Janes jeff.ja...@gmail.com writes: I think we do want this. It was asked how much we want to include internals of the btree code into pageinspect documentation, but I think the nature of pageinspect makes that unavoidable. Yeah. Committed with a little bit of additional wordsmithing. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] get_object_address support for additional object types
Stephen Frost wrote:
I thought the rest of it looked alright. I agree it's a bit odd how the
opfamily is handled but I agree with your assessment that there's not
much better we can do with this object representation.
Actually, on second thought I revisited this and changed the
representation for opfamilies and opclasses: instead of putting the AM
name in objargs, we can put it as the first element of objname instead.
That way, objargs is unused for opfamilies and opclasses, and we're free
to use it for the type arguments in amops and amprocs. This makes the
lists consistent for the four cases: in objname, amname first, then
qualified opclass/opfamily name. For amop/amproc, the member number
follows. Objargs is unused in opclass/opfamily, and it's a two-element
list of types in amop/amproc.
The attached patch changes the grammar to comply with the above, and
adds the necessary get_object_address and getObjectIdentityParts support
code for amop/amproc objects.
The only thing a bit unusual is that in does_not_exist_skipping() we
need to do an list_copy_tail() to strip out the amname before reporting
the skipping message, for DROP OPERATOR CLASS/FAMILY IF NOT EXISTS.
I don't think this is a problem. In return, the code to deconstruct
amop and amproc addresses is more sensible.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
From 35336c997b8be18d890a3a8bdf2822f757f70faf Mon Sep 17 00:00:00 2001
From: Alvaro Herrera alvhe...@alvh.no-ip.org
Date: Fri, 6 Mar 2015 12:39:50 -0300
Subject: [PATCH] Support opfamily members in get_object_address
In the spirit of 890192e99af and 4464303405f, have get_object_address
understand pg_amop and pg_amproc objects. There is no way to refer to
such objects directly in the grammar, but in event triggers and
pg_get_object_address it becomes possible to become involved with them.
Reviewed by: Stephen Frost
---
src/backend/catalog/objectaddress.c | 234 +--
src/backend/commands/dropcmds.c | 24 ++-
src/backend/commands/event_trigger.c | 2 +
src/backend/parser/gram.y| 43 ++---
src/include/nodes/parsenodes.h | 2 +
src/test/regress/expected/object_address.out | 60 ---
src/test/regress/sql/object_address.sql | 16 +-
7 files changed, 264 insertions(+), 117 deletions(-)
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 142bc68..46ea09a 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -492,9 +492,9 @@ ObjectTypeMap[] =
/* OCLASS_OPFAMILY */
{ operator family, OBJECT_OPFAMILY },
/* OCLASS_AMOP */
- { operator of access method, -1 }, /* unmapped */
+ { operator of access method, OBJECT_AMOP },
/* OCLASS_AMPROC */
- { function of access method, -1 }, /* unmapped */
+ { function of access method, OBJECT_AMPROC },
/* OCLASS_REWRITE */
{ rule, OBJECT_RULE },
/* OCLASS_TRIGGER */
@@ -552,9 +552,12 @@ static ObjectAddress get_object_address_attrdef(ObjectType objtype,
List *objname, Relation *relp, LOCKMODE lockmode,
bool missing_ok);
static ObjectAddress get_object_address_type(ObjectType objtype,
- List *objname, bool missing_ok);
+ ListCell *typecell, bool missing_ok);
static ObjectAddress get_object_address_opcf(ObjectType objtype, List *objname,
- List *objargs, bool missing_ok);
+ bool missing_ok);
+static ObjectAddress get_object_address_opf_member(ObjectType objtype,
+ List *objname, List *objargs, bool missing_ok);
+
static ObjectAddress get_object_address_usermapping(List *objname,
List *objargs, bool missing_ok);
static ObjectAddress get_object_address_defacl(List *objname, List *objargs,
@@ -567,8 +570,7 @@ static void getRelationTypeDescription(StringInfo buffer, Oid relid,
int32 objectSubId);
static void getProcedureTypeDescription(StringInfo buffer, Oid procid);
static void getConstraintTypeDescription(StringInfo buffer, Oid constroid);
-static void getOpFamilyIdentity(StringInfo buffer, Oid opfid, List **objname,
- List **objargs);
+static void getOpFamilyIdentity(StringInfo buffer, Oid opfid, List **objname);
static void getRelationIdentity(StringInfo buffer, Oid relid, List **objname);
/*
@@ -661,7 +663,8 @@ get_object_address(ObjectType objtype, List *objname, List *objargs,
ObjectAddress domaddr;
char *constrname;
- domaddr = get_object_address_type(OBJECT_DOMAIN, objname, missing_ok);
+ domaddr = get_object_address_type(OBJECT_DOMAIN,
+ list_head(objname), missing_ok);
constrname = strVal(linitial(objargs));
address.classId = ConstraintRelationId;
@@ -685,7 +688,7 @@ get_object_address(ObjectType objtype, List *objname, List *objargs,
break;
case OBJECT_TYPE:
case OBJECT_DOMAIN:
-address =
Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)
On Mon, Mar 9, 2015 at 11:18 PM, Kouhei Kaigai kai...@ak.jp.nec.com wrote:
The attached patch integrates a suggestion from Ashutosh Bapat.
It allows to track set of relations involved in a join, but
replaced by foreign-/custom-scan. It enables to make correct
EXPLAIN output, if FDW/CSP driver makes human readable symbols
using deparse_expression() or others.
Differences from v7 is identical with what I posted on the
join push-down support thread.
I took a look at this patch today and noticed that it incorporates not
only documentation for the new functionality it adds, but also for the
custom-scan functionality whose documentation I previously excluded
from commit on the grounds that it needed more work, especially to
improve the English. That decision was not popular at the time, and I
think we need to remedy it before going further with this. I had
hoped that someone else would care about this work enough to help with
the documentation, but it seems not, so today I went through the
documentation in this patch, excluded all of the stuff specific to
custom joins, and heavily edited the rest. The result is attached.
If there are no objections, I'll commit this; then, someone can rebase
this patch over these changes and we can proceed from there.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/doc/src/sgml/custom-scan.sgml b/doc/src/sgml/custom-scan.sgml
new file mode 100644
index 000..fa6f457
--- /dev/null
+++ b/doc/src/sgml/custom-scan.sgml
@@ -0,0 +1,284 @@
+!-- doc/src/sgml/custom-scan.sgml --
+
+chapter id=custom-scan
+ titleWriting A Custom Scan Provider/title
+
+ indexterm zone=custom-scan
+ primarycustom scan provider/primary
+ secondaryhandler for/secondary
+ /indexterm
+
+ para
+ productnamePostgreSQL/ supports a set of experimental facilities which
+ are intended to allow extension modules to add new scan types to the system.
+ Unlike a link linkend=fdwhandlerforeign data wrapper/, which is only
+ responsible for knowing how to scan its own foreign tables, a custom scan
+ provider can provide an alternative method of scanning any relation in the
+ system. Typically, the motivation for writing a custom scan provider will
+ be to allow the use of some optimization not supported by the core
+ system, such as caching or some form of hardware acceleration. This chapter
+ outlines how to write a new custom scan provider.
+ /para
+
+ para
+ Implementing a new type of custom scan is a three-step process. First,
+ during planning, it is necessary to generate access paths representing a
+ scan using the proposed strategy. Second, if one of those access paths
+ is selected by the planner as the optimal strategy for scanning a
+ particular relation, the access path must be converted to a plan.
+ Finally, it must be possible to execute the plan and generate the same
+ results that would have been generated for any other access path targeting
+ the same relation.
+ /para
+
+ sect1 id=custom-scan-path
+ titleImplementing Custom Paths/title
+
+ para
+A custom scan provider will typically add paths by setting the following
+hook, which is called after the core code has generated what it believes
+to be the complete and correct set of access paths for the relation.
+programlisting
+typedef void (*set_rel_pathlist_hook_type) (PlannerInfo *root,
+RelOptInfo *rel,
+Index rti,
+RangeTblEntry *rte);
+extern PGDLLIMPORT set_rel_pathlist_hook_type set_rel_pathlist_hook;
+/programlisting
+ /para
+
+ para
+Although this hook function can be used to examine, modify, or remove
+paths generated by the core system, a custom scan provider will typically
+confine itself to generating structnameCustomPath/ objects and adding
+them to literalrel/ using functionadd_path/. The custom scan
+provider is responsible for initializing the structnameCustomPath/
+object, which is declared like this:
+programlisting
+typedef struct CustomPath
+{
+Path path;
+uint32flags;
+List *custom_private;
+const CustomPathMethods *methods;
+} CustomPath;
+/programlisting
+ /para
+
+ para
+structfieldpath/ must be initialized as for any other path, including
+the row-count estimate, start and total cost, and sort ordering provided
+by this path. structfieldflags/ is a bitmask, which should include
+literalCUSTOMPATH_SUPPORT_BACKWARD_SCAN/ if the custom path can support
+a backward scan and literalCUSTOMPATH_SUPPORT_MARK_RESTORE/ if it
+can support mark and restore. Both capabilities are optional.
+structfieldcustom_private/ can be used to store the custom path's
+private data. Private data should be stored in a form that can be handled
+by literalnodeToString/, so that debugging routines which attempt to
+print the
Re: [HACKERS] forward vs backward slashes in msvc build code
Peter Eisentraut wrote: On 3/4/15 11:00 PM, Andrew Dunstan wrote: On 03/04/2015 10:37 PM, Peter Eisentraut wrote: I can't tell from just looking at the code how chkpass would normally find crypt. The msvc tools neither parse SHLIB_LINK nor have hardcoded knowledge. Any idea? Which library is it in? There are sections at the top of Mkvcbuild.pm for including various libraries in contrib modules that need them. This is contrib/chkpass not finding the crypt symbol, which is presumably in some library. But I can't see how it would normally find it, without my patch. It seems crypt is provided by libpgport. So chkpass should be mentioned in @contrib_uselibpgport, but isn't. Maybe the fix is just to add it there? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Turning off HOT/Cleanup sometimes
On Thu, Mar 12, 2015 at 3:48 PM, Peter Eisentraut pete...@gmx.net wrote: On 3/12/15 5:41 AM, Andres Freund wrote: On 2015-03-11 20:55:18 -0400, Peter Eisentraut wrote: I don't think so. Andres basically wanted a nontrival algorithm to determine how much pruning to do during a read-only scan. And Robert basically said, that's not really possible. I don't think either of us made really strong statements. I didn't mean to put words in your mouth. I just wanted to summarize the thread as, Andres wanted more fine-tuning on the behavior, Robert expressed serious doubts that that will lead to an acceptable result. Or to put that another way, I'm not sure there's one behavior here that will please everybody. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reduce pinning in btree indexes
Kevin Grittner kgri...@ymail.com wrote: Because these changes are just to a comment and a README file, I'm posting a patch-on-patch v3a (to be applied on top of v3). Here is some additional comment work that I hope will make things easier to understand for whoever next visits this code. There is also a minor reorganization of some code that should not have any impact except to skip the restore memcpy() calls where they are not needed in a corner case that I'm not sure we can currently even reach. That reorganization is intended more for code clarity than for the marginal performance it could provide. I'm posting this as v3b to be applied on top of v3 and v3a, so that these changes can be reviewed and commented on separately. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company bt-nopin-v3b.patch Description: invalid/octet-stream -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)
Robert Haas robertmh...@gmail.com writes: I took a look at this patch today and noticed that it incorporates not only documentation for the new functionality it adds, but also for the custom-scan functionality whose documentation I previously excluded from commit on the grounds that it needed more work, especially to improve the English. That decision was not popular at the time, and I think we need to remedy it before going further with this. I had hoped that someone else would care about this work enough to help with the documentation, but it seems not, so today I went through the documentation in this patch, excluded all of the stuff specific to custom joins, and heavily edited the rest. The result is attached. Looks good; I noticed one trivial typo --- please s/returns/return/ under ExecCustomScan. Also, perhaps instead of just set ps_ResultTupleSlot that should say fill ps_ResultTupleSlot with the next tuple in the current scan direction. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] OOM-killer issue when updating a inheritance table which has large number of child tables
* Tom Lane (t...@sss.pgh.pa.us) wrote: chenhj chjis...@163.com writes: In my test(PG9.3.4), i found when update a parent table which has a large number of child tables, the execute plan will consume lots of memory. And possibly cause OOM. See file:///net/sss1/home/postgres/pgsql/doc/src/sgml/html/ddl-partitioning.html#DDL-PARTITIONING-CAVEATS particularly the last paragraph. Or perhaps, if you're on the Internet, this instead: http://www.postgresql.org/docs/9.4/static/ddl-partitioning.html#DDL-PARTITIONING-CAVEATS :) Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] pg_rewind in contrib
On 03/12/2015 08:49 AM, Amit Kapila wrote: With attached modified script, I am able to reproduce the error (I have used the latest pg_rewind patch (pg_rewind-bin-8)) Thanks! That reproduced the error for me too. Not sure why I couldn't reproduce it earlier. The cause was a silly typo in truncate_target_file: @@ -397,7 +397,7 @@ truncate_target_file(const char *path, off_t newsize) snprintf(dstpath, sizeof(dstpath), %s/%s, datadir_target, path); - fd = open(path, O_WRONLY, 0); + fd = open(dstpath, O_WRONLY, 0); if (fd 0) pg_fatal(could not open file \%s\ for truncation: %s\n, dstpath, strerror(errno)); Attached is a new version of the patch, including that fix, and rebased over current git master. - Heikki pg_rewind-bin-9.patch.gz Description: application/gzip -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical column ordering
On 3/12/15 10:07 AM, Andres Freund wrote: I actually wonder if it'd not make more sense to define it as the physical column number. That'd reduce the invasiveness and risk of the patch considerably. Clearly, the number of places where attnum has to be changed to something else is not zero, and so it doesn't matter if a lot or a few have to be changed. They all have to be looked at and considered. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical column ordering
On 3/11/15 10:16 PM, Tom Lane wrote: I think using an OID would break more stuff than it fixes in dependency tracking; in particular you would now need an explicit dependency link from each column to its table, because the sub-object knowledge would no longer work. That might not be a bad thing, but ... In any case this patch is going to be plenty big enough already without saddling it with a major rewrite of the dependency system. ... is true. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] forward vs backward slashes in msvc build code
On 3/4/15 11:00 PM, Andrew Dunstan wrote: On 03/04/2015 10:37 PM, Peter Eisentraut wrote: On 2/15/15 6:55 AM, Michael Paquier wrote: I tested quickly the second patch with MS 2010 and I am getting a build failure: chkpass cannot complete because of crypt missing. On master build passes. More details here: C:\Users\mpaquier\git\postgres\pgsql.sln (default target) (1) - C:\Users\mpaquier\git\postgres\chkpass.vcxproj (default target) (36) - (Link target) - chkpass.obj : error LNK2019: unresolved external symbol crypt referenced in function chkpass_in [C:\Users\ioltas\git\postgres\chkpass.vcxproj] .\Release\chkpass\chkpass.dll : fatal error LNK1120: 1 unresolved externals [C:\Users\mpaquier\git\postgres\chkpass.vcxproj] I can't tell from just looking at the code how chkpass would normally find crypt. The msvc tools neither parse SHLIB_LINK nor have hardcoded knowledge. Any idea? Which library is it in? There are sections at the top of Mkvcbuild.pm for including various libraries in contrib modules that need them. This is contrib/chkpass not finding the crypt symbol, which is presumably in some library. But I can't see how it would normally find it, without my patch. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] OOM-killer issue when updating a inheritance table which has large number of child tables
Stephen Frost sfr...@snowman.net writes: * Tom Lane (t...@sss.pgh.pa.us) wrote: See file:///net/sss1/home/postgres/pgsql/doc/src/sgml/html/ddl-partitioning.html#DDL-PARTITIONING-CAVEATS Or perhaps, if you're on the Internet, this instead: http://www.postgresql.org/docs/9.4/static/ddl-partitioning.html#DDL-PARTITIONING-CAVEATS Ooops, pasted a link to my local copy. Sorry bout that ... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Turning off HOT/Cleanup sometimes
On 3/12/15 5:41 AM, Andres Freund wrote: On 2015-03-11 20:55:18 -0400, Peter Eisentraut wrote: I don't think so. Andres basically wanted a nontrival algorithm to determine how much pruning to do during a read-only scan. And Robert basically said, that's not really possible. I don't think either of us made really strong statements. I didn't mean to put words in your mouth. I just wanted to summarize the thread as, Andres wanted more fine-tuning on the behavior, Robert expressed serious doubts that that will lead to an acceptable result. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical column ordering
On 12.3.2015 03:16, Tom Lane wrote: Peter Eisentraut pete...@gmx.net writes: Side idea: Let attnum be the logical number, introduce attphysnum as the storage position, and add an oid to pg_attribute as the eternal identifier. That way you avoid breaking pretty much all user code that looks at pg_attribute, which will probably do something like ORDER BY attnum. Also, one could get rid of all sorts of ugly code that works around the lack of an oid in pg_attribute, such as in the dependency tracking. I think using an OID would break more stuff than it fixes in dependency tracking; in particular you would now need an explicit dependency link from each column to its table, because the sub-object knowledge would no longer work. In any case this patch is going to be plenty big enough already without saddling it with a major rewrite of the dependency system. Exactly. I believe Alvaro considered that option in the past. I agree though that it's worth considering defining pg_attribute.attnum as the logical column position so as to minimize the effects on client-side code. I doubt there is much stuff client-side that cares about column creation order, but there is plenty that cares about logical column order. OTOH this would introduce confusion into the backend code, since Alvaro's definition of attnum is what most of the backend should care about. IMHO reusing attnum for logical column order would actually make it more complex, especially if we allow users to modify the logical order using ALTER TABLE. Because if you change it, you have to walk through all the places where it might be referenced and update those too (say, columns referenced in indexes and such). Keeping attnum immutable makes this much easier and simpler. regards -- Tomas Vondrahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)
On 12 March 2015 at 21:28, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: I took a look at this patch today and noticed that it incorporates not only documentation for the new functionality it adds, but also for the custom-scan functionality whose documentation I previously excluded from commit on the grounds that it needed more work, especially to improve the English. That decision was not popular at the time, and I think we need to remedy it before going further with this. I had hoped that someone else would care about this work enough to help with the documentation, but it seems not, so today I went through the documentation in this patch, excluded all of the stuff specific to custom joins, and heavily edited the rest. The result is attached. Looks good; I noticed one trivial typo --- please s/returns/return/ under ExecCustomScan. Also, perhaps instead of just set ps_ResultTupleSlot that should say fill ps_ResultTupleSlot with the next tuple in the current scan direction. Also: s/initalization/initialization/ -- Thom -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bug in point releases 9.3.6 and 9.2.10?
Hi,
On 2015-03-12 16:42:24 -0700, Peter Geoghegan wrote:
We want to create a new role when this happens, for various reasons.
This occurs after recovery ends, but before the database has been
unfenced. The template code that generates various ALTER ROLE
statements in our internal provisioning system - which has apparently
worked just fine for a long time - is:
Is this all the code that's exececuted after recovery? How are these
forks brought up? Promoted how? Is it a common 'source' database?
db.execute(ALTER ROLE #{old_database_user} RENAME TO #{database_user})
db.execute(ALTER ROLE #{database_user} PASSWORD '#{database_password}'
LOGIN)
db.execute(CREATE ROLE \#{old_database_user}\ PASSWORD
'#{old_database_password}' IN ROLE \#{database_user}\ LOGIN)
I've seen multiple reports of apparent corruption, appearing as the
resulting ALTER ROLE statements are executed:
PG::DataCorrupted: ERROR: could not read block 0 in file
global/12811: read only 0 of 8192 bytes
or:
PG::DataCorrupted: ERROR: could not read block 0 in file
global/12785: read only 0 of 8192 bytes
or:
PG::DataCorrupted: ERROR: could not read block 0 in file
global/12811: read only 0 of 8192 bytes
Have you looked at these files? Are they indeed zero bytes when this
error occurs?
Do you still have a base backup from the relevant time, so you could
repeat the whole thing?
The only common factor is that this occurs on the latest point
releases (either 9.3.6 and 9.2.10, at least so far). In all cases I've
seen so far, the relation in question is the pg_auth_members heap
relation. For example:
Any chance that the new nodes also use a different kernel version or
such?
redacteddb=# select pg_relation_filenode(oid), oid, relname, relkind
from pg_class where pg_relation_filenode(oid) = 12785;
pg_relation_filenode | oid | relname | relkind
--+--+-+-
12785 | 1261 | pg_auth_members | r
(1 row)
This filenode got to be pg_auth_member's original one, given it's below
FirstNormalObjectId. I get a lower value, but that's probably caused by
having fewer collations and other data generated during initdb. That
implies that the table hasn't ever been rewritten.
What's 12811?
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bug in point releases 9.3.6 and 9.2.10?
In a hurry right now, so unfortunately I'll need to be brief for now. On Thu, Mar 12, 2015 at 5:21 PM, Andres Freund and...@2ndquadrant.com wrote: On 2015-03-12 16:42:24 -0700, Peter Geoghegan wrote: We want to create a new role when this happens, for various reasons. This occurs after recovery ends, but before the database has been unfenced. The template code that generates various ALTER ROLE statements in our internal provisioning system - which has apparently worked just fine for a long time - is: Is this all the code that's exececuted after recovery? How are these forks brought up? Promoted how? Is it a common 'source' database? We do PITR up to a recovery target. We're talking about the same issue occurring on entirely distinct customer databases, with entirely distinct major PG versions. I'm not sure what other code might have already been run at this point, but it won't have been much. As I said, the only common factor that I know of is all affected databases being on the latest point release. Have you looked at these files? Are they indeed zero bytes when this error occurs? I think that they are indeed zero. I looked at that last week, when I didn't consider that this might be a more widespread issue. I'll check again later. Do you still have a base backup from the relevant time, so you could repeat the whole thing? Yes. The only common factor is that this occurs on the latest point releases (either 9.3.6 and 9.2.10, at least so far). In all cases I've seen so far, the relation in question is the pg_auth_members heap relation. For example: Any chance that the new nodes also use a different kernel version or such? They may differ, but that doesn't seem likely to be relevant, at least to me. This has happened something like 6 or 7 times already, starting late last week. I am unfamiliar with this provisioning code, so, as I mentioned, offhand I cannot be entirely sure that there isn't some other code run when the problem originally arises (that I should have included in my report). What I can tell you is that I saw the same error messages when I manually ran the statements generated by the above code within a transaction...until I ran VACUUM FULL pg_auth_members;. This filenode got to be pg_auth_member's original one, given it's below FirstNormalObjectId. I get a lower value, but that's probably caused by having fewer collations and other data generated during initdb. That implies that the table hasn't ever been rewritten. What's 12811? It's the same catalog, pg_auth_member. As I said, the messages you saw are on entirely different customer databases, servers and (sometimes) PG version. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bug in point releases 9.3.6 and 9.2.10?
On 2015-03-12 17:42:33 -0700, Peter Geoghegan wrote: On Thu, Mar 12, 2015 at 5:21 PM, Andres Freund and...@2ndquadrant.com wrote: On 2015-03-12 16:42:24 -0700, Peter Geoghegan wrote: We want to create a new role when this happens, for various reasons. This occurs after recovery ends, but before the database has been unfenced. The template code that generates various ALTER ROLE statements in our internal provisioning system - which has apparently worked just fine for a long time - is: Is this all the code that's exececuted after recovery? How are these forks brought up? Promoted how? Is it a common 'source' database? We do PITR up to a recovery target. So, no hot standby enabled? I'm not sure what other code might have already been run at this point, but it won't have been much. Have you looked at these files? Are they indeed zero bytes when this error occurs? I think that they are indeed zero. I looked at that last week, when I didn't consider that this might be a more widespread issue. I'll check again later. Any chance that the new nodes also use a different kernel version or such? They may differ, but that doesn't seem likely to be relevant, at least to me. There've been some issues with seek(END) sometimes returning the wrong length in the past. And I've seen a report that might indicate a similar bug has been reintroduced. That could certainly cause such anerror. I am unfamiliar with this provisioning code, so, as I mentioned, offhand I cannot be entirely sure that there isn't some other code run when the problem originally arises (that I should have included in my report). It's probably worthwhile to dig out what's happening. What I can tell you is that I saw the same error messages when I manually ran the statements generated by the above code within a transaction...until I ran VACUUM FULL pg_auth_members;. You can reproduce that problem? How easily? If you can, please produce a backtrace. It'll certainly be interesting to see whether that access is through an index or whatnot. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bug in point releases 9.3.6 and 9.2.10?
On Thu, Mar 12, 2015 at 5:56 PM, Andres Freund and...@2ndquadrant.com wrote: So, no hot standby enabled? Right. There've been some issues with seek(END) sometimes returning the wrong length in the past. And I've seen a report that might indicate a similar bug has been reintroduced. That could certainly cause such anerror. Perhaps. I am unfamiliar with this provisioning code, so, as I mentioned, offhand I cannot be entirely sure that there isn't some other code run when the problem originally arises (that I should have included in my report). It's probably worthwhile to dig out what's happening. I'll get to that after the backtrace. What I can tell you is that I saw the same error messages when I manually ran the statements generated by the above code within a transaction...until I ran VACUUM FULL pg_auth_members;. You can reproduce that problem? How easily? If you can, please produce a backtrace. It'll certainly be interesting to see whether that access is through an index or whatnot. I suspect I can reproduce it quite easily. The next step should be to do a backtrace. I'll look at that tomorrow. -- Peter Geoghegan -- 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] Reduce pinning in btree indexes
On Sat, Feb 14, 2015 at 4:19 PM, Kevin Grittner kgri...@ymail.com wrote: At some point we could consider building on this patch to recheck index conditions for heap access when a non-MVCC snapshot is used, check the visibility map for referenced heap pages when the TIDs are read for an index-only scan, and/or skip LP_DEAD hinting for non-WAL-logged indexes. But all those are speculative future work; this is a conservative implementation that just didn't modify pinning where there were any confounding factors. I don't have the bandwidth for a full review, but I took a quick look at this. I think you should call out those confounding factors in the README. It's not hard to piece them together from _bt_drop_lock_and_maybe_pin(), but I think you should anyway. Don't use BUFFER_LOCK_SHARE -- use BT_READ, as the existing nbtree LockBuffer() callers do. You're inconsistent about that in V3. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Moving Pivotal's Greenplum work upstream
Hi all,There has been some press regarding Pivotal's intent to release Greenplum source as part of an Open Development Platform (along with some of their Hadoop projects). Can anyone speak on whether any of Greenplum might find its way upstream? For example, if(!) the work is being released under an appropriate license, are people at Pivotal planning to push patches for the parallel architecture and associated query planner upstream? Thanks,Ewan
Re: [HACKERS] Moving Pivotal's Greenplum work upstream
On 03/12/2015 03:24 PM, Ewan Higgs wrote: Hi all, There has been some press regarding Pivotal's intent to release Greenplum source as part of an Open Development Platform (along with some of their Hadoop projects). Can anyone speak on whether any of Greenplum might find its way upstream? For example, if(!) the work is being released under an appropriate license, are people at Pivotal planning to push patches for the parallel architecture and associated query planner upstream? At this point, we have no idea. We'll know better when the code is actually available. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: searching in array function - array_position
On 3/11/15 1:19 AM, Pavel Stehule wrote: 2015-03-11 2:57 GMT+01:00 Robert Haas robertmh...@gmail.com mailto:robertmh...@gmail.com: On Tue, Mar 10, 2015 at 5:53 PM, Jim Nasby jim.na...@bluetreble.com mailto:jim.na...@bluetreble.com wrote: I don't think we need both array_offset and array_offset_start; can't both SQL functions just call one C function? Not if you want the opr_sanity tests to pass. (But I'm seriously starting to wonder if that's actually a smart rule for us to be enforcing. It seems to be something of a pain in the neck, and I'm unclear as to whether it is preventing any real problem.) It is simple protection against some oversights. I am not against this check - this rule cleans a interface between C and SQL. More, the additional C code is usually very short and trivial. But it should be commented well. Ahh, ok, makes more sense now. If the separate C functions are serving a purpose that's fine. I think the comment should mention it though, as it's not exactly the most obvious thing. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
Hi I am looking to code. Some small issues: 1. fix missing semicolon pg_proc.h Oid protrftypes[1]; /* types for which to apply transforms */ 2. strange load lib by in sql scripts: DO '' LANGUAGE plperl; SELECT NULL::hstore; use load plperl; load hstore; instead 3. missing documentation for new contrib modules, 4. pg_dump - wrong comment +---/* +--- * protrftypes was added at v9.4 +--- */ 4. Why guc-use-transforms? Is there some possible negative side effect of transformations, so we have to disable it? If somebody don't would to use some transformations, then he should not to install some specific transformation. 5. I don't understand to motivation for introduction of protrftypes in pg_proc and TRANSFORM clause for CREATE FUNCTION - it is not clean from documentation, and examples in contribs works without it. Is it this functionality really necessary? Missing tests, missing examples. Regards Pavel 2015-03-08 16:34 GMT+01:00 Peter Eisentraut pete...@gmx.net: On 3/6/15 9:56 AM, Pavel Stehule wrote: Hi I am checking this patch, but it is broken still Here is an updated patch. (Note that because of the large pg_proc.h changes, it is likely to get outdated again soon. But for reviewing, you can always apply it against an older version of master.)
Re: [HACKERS] Proposal : REINDEX xxx VERBOSE
On Thu, Mar 12, 2015 at 6:36 AM, Jim Nasby jim.na...@bluetreble.com wrote:
On 3/11/15 6:33 AM, Sawada Masahiko wrote:
As a refresher, current commands are:
VACUUM (ANALYZE, VERBOSE) table1 (col1);
REINDEX INDEX index1 FORCE;
COPY table1 FROM 'file.txt' WITH (FORMAT csv);
CREATE MATERIALIZED VIEW mv1 WITH (storageparam, ...) AS qry
WITH
DATA;
CREATE EXTENSION ext1 WITH SCHEMA s1 VERSION v1 FROM over;
CREATE ROLE role WITH LOGIN;
GRANT WITH GRANT OPTION;
CREATE VIEW v1 AS qry WITH CASCADED CHECK OPTION;
ALTER DATABASE db1 WITH CONNECTION LIMIT 50;
DECLARE c1 INSENSITIVE SCROLL CURSOR WITH HOLD;
BTW, I'm fine with Tom's bare-word with WITH idea. That seems to be
the
most
consistent with everything else. Is there a problem with doing that?
I
know
getting syntax is one of the hard parts of new features, but it
seems
like
we reached consensus here...
Attached is latest version patch based on Tom's idea as follows.
REINDEX { INDEX | ... } name WITH ( options [, ...] )
Are the parenthesis necessary? No other WITH option requires them, other
than create table/matview (COPY doesn't actually require them).
I was imagining EXPLAIN syntax.
Is there some possibility of supporting multiple options for REINDEX
command in future?
If there is, syntax will be as follows, REINDEX { INDEX | ... } name
WITH VERBOSE, XXX, XXX;
I thought style with parenthesis is better than above style.
The thing is, ()s are actually an odd-duck. Very little supports it, and
while COPY allows it they're not required. EXPLAIN is a different story,
because that's not WITH; we're actually using () *instead of* WITH.
So because almost all commands that use WITH doen't even accept (), I don't
think this should either. It certainly shouldn't require them, because
unlike EXPLAIN, there's no need to require them.
I understood what your point is.
Attached patch is changed syntax, it does not have parenthesis.
Regards,
---
Sawada Masahiko
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index 0a4c7d4..3cea35f 100644
--- a/doc/src/sgml/ref/reindex.sgml
+++ b/doc/src/sgml/ref/reindex.sgml
@@ -22,6 +22,11 @@ PostgreSQL documentation
refsynopsisdiv
synopsis
REINDEX { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } replaceable class=PARAMETERname/replaceable [ FORCE ]
+REINDEX { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } replaceable class=PARAMETERname/replaceable WITH replaceable class=PARAMETERoptions/replaceable [, ...]
+
+phrasewhere replaceable class=PARAMETERoption/replaceable can be one of:/phrase
+
+VERBOSE
/synopsis
/refsynopsisdiv
@@ -159,6 +164,15 @@ REINDEX { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } replaceable class=PARAM
/para
/listitem
/varlistentry
+
+ varlistentry
+termliteralVERBOSE/literal/term
+listitem
+ para
+ Prints a progress report as each index is reindexed.
+ /para
+/listitem
+ /varlistentry
/variablelist
/refsect1
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index f85ed93..786f173 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -63,6 +63,7 @@
#include utils/inval.h
#include utils/lsyscache.h
#include utils/memutils.h
+#include utils/pg_rusage.h
#include utils/syscache.h
#include utils/tuplesort.h
#include utils/snapmgr.h
@@ -3130,13 +3131,18 @@ IndexGetRelation(Oid indexId, bool missing_ok)
* reindex_index - This routine is used to recreate a single index
*/
void
-reindex_index(Oid indexId, bool skip_constraint_checks, char persistence)
+reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
+bool verbose)
{
Relation iRel,
heapRelation;
Oid heapId;
IndexInfo *indexInfo;
volatile bool skipped_constraint = false;
+ int elevel = verbose ? INFO : DEBUG2;
+ PGRUsage ru0;
+
+ pg_rusage_init(ru0);
/*
* Open and lock the parent heap relation. ShareLock is sufficient since
@@ -3280,6 +3286,13 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence)
heap_close(pg_index, RowExclusiveLock);
}
+ /* Log what we did */
+ ereport(elevel,
+ (errmsg(index \%s\ was reindexed.,
+ get_rel_name(indexId)),
+ errdetail(%s.,
+ pg_rusage_show(ru0;
+
/* Close rels, but keep locks */
index_close(iRel, NoLock);
heap_close(heapRelation, NoLock);
@@ -3321,7 +3334,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence)
* index rebuild.
*/
bool
-reindex_relation(Oid relid, int flags)
+reindex_relation(Oid relid, int flags, bool verbose)
{
Relation rel;
Oid toast_relid;
@@ -3412,7 +3425,7 @@ reindex_relation(Oid relid, int flags)
RelationSetIndexList(rel, doneIndexes, InvalidOid);
reindex_index(indexOid, !(flags REINDEX_REL_CHECK_CONSTRAINTS),
- persistence);
+ persistence, verbose);
CommandCounterIncrement();
@@
Re: [HACKERS] Precedence of standard comparison operators
On 3/10/15 4:34 PM, Tom Lane wrote: There's one more reason, too: the code I have is designed to give correct warnings within the context of a parser that parses according to the spec-compliant rules. It's possible that a similar approach could be used to generate correct warnings within a parsetree built according to the old rules, but it would be entirely different in detail and would need a lot of additional work to develop. I don't particularly want to do that additional work. So you want to change the precedence behavior in the next release and have a warning about this code would have worked differently before. My understanding was that we would keep the precedence behavior for a while but warn about this code would work differently in the future. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Bug in point releases 9.3.6 and 9.2.10?
Heroku Postgres runs provisioning code that performs certain actions
on roles when creating a new fork of an existing database. This
often causes the new fork to be on the latest point release, where the
database being forked was not.
We want to create a new role when this happens, for various reasons.
This occurs after recovery ends, but before the database has been
unfenced. The template code that generates various ALTER ROLE
statements in our internal provisioning system - which has apparently
worked just fine for a long time - is:
db.execute(ALTER ROLE #{old_database_user} RENAME TO #{database_user})
db.execute(ALTER ROLE #{database_user} PASSWORD '#{database_password}' LOGIN)
db.execute(CREATE ROLE \#{old_database_user}\ PASSWORD
'#{old_database_password}' IN ROLE \#{database_user}\ LOGIN)
I've seen multiple reports of apparent corruption, appearing as the
resulting ALTER ROLE statements are executed:
PG::DataCorrupted: ERROR: could not read block 0 in file
global/12811: read only 0 of 8192 bytes
or:
PG::DataCorrupted: ERROR: could not read block 0 in file
global/12785: read only 0 of 8192 bytes
or:
PG::DataCorrupted: ERROR: could not read block 0 in file
global/12811: read only 0 of 8192 bytes
The only common factor is that this occurs on the latest point
releases (either 9.3.6 and 9.2.10, at least so far). In all cases I've
seen so far, the relation in question is the pg_auth_members heap
relation. For example:
redacteddb=# select pg_relation_filenode(oid), oid, relname, relkind
from pg_class where pg_relation_filenode(oid) = 12785;
pg_relation_filenode | oid | relname | relkind
--+--+-+-
12785 | 1261 | pg_auth_members | r
(1 row)
Running VACUUM FULL pg_auth_members; has made the problem go away
(to the extent that the above code doesn't trip up and everything is
at least superficially okay) on at least one occasion. I'm currently
investigating how consistently that works as a short term remediation.
Theories?
--
Peter Geoghegan
--
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_rewind in contrib
On Wed, Mar 11, 2015 at 2:23 PM, Heikki Linnakangas hlinn...@iki.fi wrote: On 03/11/2015 05:01 AM, Amit Kapila wrote: I have tried without backslash as well, but still it returns same error. pg_rewind.exe -D ..\..\Data --source-pgdata=..\..\Database1 The servers diverged at WAL position 0/1769BD8 on timeline 5. Rewinding from last common checkpoint at 0/1769B30 on timeline 5 could not open file ..\..\Data/base/12706/16394 for truncation: No such file or directory Failure, exiting I tried to reproduce this, but it tripped the Assert(entry-isrelfile) assertion in process_block_change. However, that seems to be an unrelated issue - pg_rewind was not handling FSM blocks correctly. It's supposed to ignore them but extactPageInfo didn't get the memo. I think I broke that when doing the changes for the new WAL record format. After fixing that (new patch attached), your test case works fine for me. I'm using the attached bash script to test it. Can you test if the attached script works for you, and if it does, see if you can fix the script so that it reproduces the error you're seeing? With attached modified script, I am able to reproduce the error (I have used the latest pg_rewind patch (pg_rewind-bin-8)) The servers diverged at WAL position 0/1693400 on timeline 1. Rewinding from last common checkpoint at 0/1693390 on timeline 1 could not open file data-master/base/12706/16384 for truncation: No such file or directory Failure, exiting I am able to reproduce it on Windows (haven't tried it on linux). With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com amits-test-modify.sh Description: Bourne shell script -- 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: searching in array function - array_position
2015-03-11 22:50 GMT+01:00 Jim Nasby jim.na...@bluetreble.com:
On 3/11/15 4:37 PM, Pavel Stehule wrote:
+ /*
+ * array_offset - returns the offset of a value in an array
(array_offset and
+ * array_offset_start are wrappers for safe call (look on opr_sanity
test) a
+ * array_offset_common function.
+ *
+ * Returns NULL when value is not found. It uses a NOT DISTINCT FROM
operator
+ * for comparation to be safe against NULL.
+ */
would be better as...
+ /*
+ * array_offset - returns the offset of a value in an array.
array_offset and
+ * array_offset_start are wrappers for the sake of the opr_sanity test.
+ *
+ * Returns NULL when value is not found. It uses a NOT DISTINCT FROM
operator
+ * for comparation to be safe against NULL.
+ */
fixed
Regards
Pavel
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com
diff --git a/doc/src/sgml/array.sgml b/doc/src/sgml/array.sgml
new file mode 100644
index 9ea1068..d90266f
*** a/doc/src/sgml/array.sgml
--- b/doc/src/sgml/array.sgml
*** SELECT * FROM sal_emp WHERE pay_by_quart
*** 600,605
--- 600,614
index, as described in xref linkend=indexes-types.
/para
+ para
+ You can also search for a value in an array using the functionarray_offset/
+ function. It returns the position of the first occurrence of a value in an array:
+
+ programlisting
+ SELECT array_offset(ARRAY['sun','mon','tue','wen','thu','fri','sat'], 'mon');
+ /programlisting
+ /para
+
tip
para
Arrays are not sets; searching for specific array elements
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
new file mode 100644
index c198bea..311f2fe
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
*** SELECT NULLIF(value, '(none)') ...
*** 11480,11485
--- 11480,11488
primaryarray_lower/primary
/indexterm
indexterm
+ primaryarray_offset/primary
+ /indexterm
+ indexterm
primaryarray_prepend/primary
/indexterm
indexterm
*** SELECT NULLIF(value, '(none)') ...
*** 11598,11603
--- 11601,11637
/row
row
entry
+ literal
+ functionarray_offset/function(typeanyarray/type, typeanyelement/type optional, typeint/type/optional)
+ /literal
+ /entry
+ entrytypeint/type/entry
+ entryreturns the offset of the first occurrence of a value in an
+ array. It uses the literalIS NOT DISTINCT FROM/ operator for
+ comparation. The optional third argument specifies an initial offset to
+ begin the search at. Returns NULL when the value is not found. Note:
+ multi-dimensional arrays are squashed to one dimension before
+ searching./entry
+ entryliteralarray_offset(ARRAY['sun','mon','tue','wen','thu','fri','sat'], 'mon')/literal/entry
+ entryliteral2/literal/entry
+/row
+row
+ entry
+ literal
+ functionarray_offsets/function(typeanyarray/type, typeanyelement/type)
+ /literal
+ /entry
+ entrytypeint[]/type/entry
+ entryreturns an array of offsets of all occurrences of a value in a array. It uses
+ the literalIS NOT DISTINCT FROM/ operator for comparation. Returns an empty array
+ when there are no occurences of the value in the array. Note:
+ multi-dimensional input arrays are squashed to one dimension before
+ searching./entry
+ entryliteralarray_offsets(ARRAY['A','A','B','A'], 'A')/literal/entry
+ entryliteral{1,2,4}/literal/entry
+/row
+row
+ entry
literal
functionarray_prepend/function(typeanyelement/type, typeanyarray/type)
/literal
diff --git a/src/backend/utils/adt/array_userfuncs.c b/src/backend/utils/adt/array_userfuncs.c
new file mode 100644
index 6679333..0f9ea48
*** a/src/backend/utils/adt/array_userfuncs.c
--- b/src/backend/utils/adt/array_userfuncs.c
***
*** 12,20
--- 12,24
*/
#include postgres.h
+ #include catalog/pg_type.h
#include utils/array.h
#include utils/builtins.h
#include utils/lsyscache.h
+ #include utils/typcache.h
+
+ static Datum array_offset_common(FunctionCallInfo fcinfo);
/*
*** array_agg_array_finalfn(PG_FUNCTION_ARGS
*** 652,654
--- 656,905
PG_RETURN_DATUM(result);
}
+
+
+ /*
+ * array_offset - returns the offset of a value in an array. array_offset and
+ * array_offset_start are wrappers for the sake of the opr_sanity test.
+ *
+ * Returns NULL when value is not found. It uses a NOT DISTINCT FROM operator
+ * for comparation to be safe against NULL.
+ */
+ Datum
+ array_offset(PG_FUNCTION_ARGS)
+ {
+ return array_offset_common(fcinfo);
+ }
+
+ Datum
+ array_offset_start(PG_FUNCTION_ARGS)
+ {
+ return array_offset_common(fcinfo);
+ }
+
+ /*
+ * Common part
Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace
On 10/03/2015 13:27, Alvaro Herrera wrote: Robert Haas wrote: On Mon, Mar 9, 2015 at 7:26 PM, Andreas Karlsson andr...@proxel.se wrote: I think we should allow moving the indexes for consistency. With this patch we can move everything except for TOAST indexes. It might make sense to always put the TOAST index with the TOAST table, but it seems strange to put the TOAST index with the heap and the TOAST table someplace else. Or at least, that's how it seems to me. Agreed. It doesn't seem necessary to allow moving the toast index to a tablespace other than the one containing the toast table. In other words, if you move the toast table, the index always follows it, and there's no option to move it independently. This behaviour is already implemented, the TOAST index always follows the TOAST table. I'll add a couple of regression tests about it. -- Julien -- 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: searching in array function - array_position
On 3/10/15 9:30 AM, Robert Haas wrote: On Sat, Mar 7, 2015 at 1:06 PM, Petr Jelinek p...@2ndquadrant.com wrote: You still duplicate the type cache code, but many other array functions do that too so I will not hold that against you. (Maybe somebody should write separate patch that would put all that duplicate code into common function?) I think this patch is ready for committer so I am going to mark it as such. The documentation in this patch needs some improvements to the English. Can anyone help with that? I'll take a look at it. The documentation should discuss what happens if the array is multi-dimensional. The code for array_offset and for array_offset_start appear to be byte-for-byte identical. There's no comment explaining why, but I bet it's to make the opr_sanity test pass. How about adding a comment? The comment for array_offset_common refers to array_offset_start as array_offset_startpos. I am sure in agreement with the idea that it would be good to factor out the common typecache code (for setting up my_extra). Any chance we get a preliminary patch that does that refactoring, and then rebase the main patch on top of it? I agree that it's not really this patch's job to solve that problem, but it would be nice. Since this patch is here and ready to go I would prefer that we commit it and refactor later. I can tackle that unless Pavel specifically wants to. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Stateful C-language function with state managed by third-party library
Denys Rtveliashvili r...@icloud.com writes: My function neeeds to call a third-party library which would create a state and then that state should be kept for the duration of the current query. The library can deallocate that state in a correct way. I understand that fn_extra is normally used for this and usually the state is created in a memory context which is deallocated at the end of the query. So normally it is not an issue. However, I cannot make that library use PostgreSQL utilities for memory management. I am afraid that for long-running sessions it may cause serious memory leaks if they do not deallocate state correctly and in a timely manner. Is there a mechanism for adding a finalizer hook which would be called and passed that pointer after the query is complete? Or perhaps there is another mechanism? I looked in the documentation and in the source but I do not see it mentioned. In HEAD, you could use a memory context reset callback for this purpose. I don't believe there's any fully satisfactory solution in the released branches; the closest you could get is an ExprContext callback, which has the fatal-for-this-purpose defect that it's only called on successful query completion, not if an error occurs. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: searching in array function - array_position
2015-03-10 16:53 GMT+01:00 Jim Nasby jim.na...@bluetreble.com: On 3/10/15 9:30 AM, Robert Haas wrote: On Sat, Mar 7, 2015 at 1:06 PM, Petr Jelinek p...@2ndquadrant.com wrote: You still duplicate the type cache code, but many other array functions do that too so I will not hold that against you. (Maybe somebody should write separate patch that would put all that duplicate code into common function?) I think this patch is ready for committer so I am going to mark it as such. The documentation in this patch needs some improvements to the English. Can anyone help with that? I'll take a look at it. The documentation should discuss what happens if the array is multi-dimensional. The code for array_offset and for array_offset_start appear to be byte-for-byte identical. There's no comment explaining why, but I bet it's to make the opr_sanity test pass. How about adding a comment? The comment for array_offset_common refers to array_offset_start as array_offset_startpos. I am sure in agreement with the idea that it would be good to factor out the common typecache code (for setting up my_extra). Any chance we get a preliminary patch that does that refactoring, and then rebase the main patch on top of it? I agree that it's not really this patch's job to solve that problem, but it would be nice. Since this patch is here and ready to go I would prefer that we commit it and refactor later. I can tackle that unless Pavel specifically wants to. I'll look on this part this evening - but I don't have any idea how to find some common pattern yet. So I am with Jim - we can do it later. Regards Pavel -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com
Re: [HACKERS] proposal: disallow operator = and use it for named parameters
2015-03-10 17:07 GMT+01:00 Petr Jelinek p...@2ndquadrant.com: On 10/03/15 17:01, Pavel Stehule wrote: 2015-03-10 16:50 GMT+01:00 Tom Lane t...@sss.pgh.pa.us mailto:t...@sss.pgh.pa.us: Robert Haas robertmh...@gmail.com mailto:robertmh...@gmail.com writes: Committed with a few documentation tweaks. Was there any consideration given to whether ruleutils should start printing NamedArgExprs with =? Or do we think that needs to wait? I didn't think about it? I don't see any reason why it have to use deprecated syntax. There is one, loading the output into older version of Postgres. Don't know if that's important one though. I don't think so it is a hard issue. We doesn't support downgrades - and if somebody needs it, it can fix it with some regexp. We should to use preferred syntax everywhere - and preferred syntax should be ANSI. I forgot it :( Pavel -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services
