Re: [HACKERS] confusing checkpoint_flush_after / bgwriter_flush_after

[Fabien COELHO]

Maybe something like the following, or maybe it should include "bufmgr.h",
not sure.


As-is this patch seems like a maintenance time bomb; it really needs to
use the #defines rather than have the values hard-wired in.  However, just
including bufmgr.h in frontend code doesn't work, so I moved the #defines
to pg_config_manual.h, which seems like a more reasonable place for them
anyway. Pushed with that and some other polishing.


Indeed, that's much cleaner and easier to maintain. Thanks.

--
Fabien.


--
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] References to arbitrary database objects that are suitable for pg_dump

[Jim Nasby]

On 11/25/16 6:00 PM, Tom Lane wrote:

OIDs?  Then use pg_describe_object() to turn that into text when dumping.


How would I get pg_dump to do that?

I could certainly make this work if there was some way to customize how 
pg_dump dumps things, but AFAIK there's no way to do that, even with 
extensions.

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)


--
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] macaddr 64 bit (EUI-64) datatype support

[Tom Lane]

Haribabu Kommi  writes:
> Currently the casting is supported from macaddr to macaddr8, but not the
> other way. This is because, not all 8 byte MAC addresses can be converted
> into 6 byte addresses.

Well, yeah, so you'd throw an error if it can't be converted.  This is
no different from casting int8 to int4, for example.  We don't refuse
to provide that cast just because it will fail for some values.

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] Skipping PgStat_FunctionCallUsage for many expressions

[Andres Freund]

Hi,

while working on my faster expression evaluation stuff I noticed that a
lot of expression types that call functions don't call the necessary
functions to make track_functions work.

ExecEvalFunc/ExecEvalOper (via ExecMakeFunctionResultNoSets) call
pgstat_init_function_usage/pgstat_end_function_usage, but others like
ExecEvalRowCompare, ExecEvalMinMax, ExecEvalNullIf, ExecEvalDistinct,
ExecEvalScalarArrayOp (and indirectly ExecEvalArrayCoerceExpr) don't.

Similarly InvokeFunctionExecuteHook isn't used very thoroughly.

Are these worth fixing? I suspect yes. If so, do we want to backpatch?

- Andres


-- 
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] make default TABLESPACE belong to target table.

[Michael Paquier]

On Sat, Nov 26, 2016 at 11:25 AM, Amos Bird  wrote:
> How about making a storage parameter "default_tablespace" that also
> covers CREATE INDEX and other stuff?

That's exactly the idea, the one at relation-level gets priority on
the global one defined in postgresql.conf.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] make default TABLESPACE belong to target table.

[Amos Bird]

> The only scenario where this would be useful is when using ALTER TABLE
> ADD CONSTRAINT in which case a fresh index is built (not USING INDEX).
> That's a bit narrow, because it would mean that you would either
> append a TABLESPACE clause to this existing clause, or create a
> storage parameter to enforce all indexes created for a relation on a
> wanted tablespace... For the other cases you could just do something
> like that, and that's what the large majority of people would care
> about:
> SET default_tablespace TO 'foo';
> CREATE TABLE foobar (id int PRIMARY KEY);
> But that's not the one you are interesting in, so likely a storage
> parameter is what pops up in my mind, with parameter defined at table
> creation: CREATE TABLE foo (id primary key) WITH
> (constraint_default_tablespace = foo) TABLESPACE bar;
> In this case the parent relation gets created in tablespace bar, but
> its primary key gets in tablespace foo.

How about making a storage parameter "default_tablespace" that also
covers CREATE INDEX and other stuff?



-- 
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] make default TABLESPACE belong to target table.

[Amos Bird]

> I had a similar problem in writing the range_partitioning extension: CREATE
> TABLE y (LIKE x INCLUDING ALL) didn't set the tablespace of y to match x.
> I don't have a solution, I'm just indicating a similar need in userland.

Cool, I didn't think of that. It seems this feature is at least useful
for extension devs like us. I'll start coding a POC patch. What do you
think of making default tablespace derived from parent table?



-- 
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] User-defined Operator Pushdown and Collations

[Paul Ramsey]

On Fri, Nov 25, 2016 at 11:30 AM, Paul Ramsey 
wrote:

> I've been trying to figure out an issue with operators not being pushed
> down for user defined types, in this case "hstore". TL;DR:
>
> hstore=# explain (verbose) select * from hs_fdw where h -> 'a' = '1';
>   QUERY PLAN
> --
>  Foreign Scan on public.hs_fdw  (cost=100.00..157.78 rows=7 width=36)
>Output: id, h
>Filter: ((hs_fdw.h -> 'a'::text) = '1'::text)
>Remote SQL: SELECT id, h FROM public.hs
> (4 rows)
>
> In terms of "shippability" the "->" operator passes fine. It ends up not
> being shipped because its collation bubbles up as FDW_COLLATE_NONE, and
> gets kicked back as not deparseable around here:
>
> https://github.com/postgres/postgres/blob/4e026b32d4024b03856b4981b26c74
> 7b7fef7afb/contrib/postgres_fdw/deparse.c#L499
>
>
I'm finding this piece of code a little suspect, but that may just be my
not fully understanding why/what determines when a collation is shippable.

In the case of my example above, the OpExpr '->' has an input collation of
100 (DEFAULT_COLLATION_ID). The Var below has a collation of 0 (InvalidOid)
and state of FDW_COLLATE_NONE, and the Const has collation of 100
(DEFAULT_COLLATION_ID ) and state of FDW_COLLATE_NONE.  In other parts of
the code, the default collation and collection 0 are treated as both
leading to a state of FDW_COLLATE_NONE. But the OpExpr instead of bubbling
FDW_COLLATE_NONE up the chain, returns false and ends the shippability of
the Node.

What are the issues around shipping nodes of different collations to the
remote? All the nodes in my example are foreign Var, or local Const, and
all either are collation 0 or DEFAULT_COLLATION_ID. Surely it should be
shippable?

P



> I'm still working at wrapping my head around why this is good or not, but
> if there's an obvious explanation and/or workaround, I'd love to know.
>
> Thanks!
>
> P
>
>

Re: [HACKERS] References to arbitrary database objects that are suitable for pg_dump

[Tom Lane]

Jim Nasby  writes:
> The challenge I'm running into is finding a way to store a reference to 
> an arbitrary object that will survive a rename as well as working with 
> pg_dump.

Can't you do it like pg_depend does, that is store the class and object
OIDs?  Then use pg_describe_object() to turn that into text when dumping.

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] Fixed pg_class refcache leak when the meta tuple in pg_class in invalid.

[Tom Lane]

Ming Li  writes:
> In some cases the meta tuple in pg_class for a specific relation is
> invalid, which will cause relcache leak, and then report warning:
> WARNING: relcache reference leak: relation "pg_class" not closed.

> The diff file in the attachment can fix this problem.

I'm confused.  RelationBuildDesc doesn't open pg_class and shouldn't
be responsible for closing it either; both of those things happen in
ScanPgRelation, leaving no apparent scope for a leak such as you suggest.
Moreover, there's no variable named pg_class_relation in this whole file,
so your patch wouldn't even compile.

Could you show us a test case that provokes the warning you see?

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] confusing checkpoint_flush_after / bgwriter_flush_after

[Tom Lane]

Fabien COELHO  writes:
>>> What we do in some similar cases is put the burden on initdb to fill in
>>> the correct value by modifying postgresql.conf.sample appropriately.
>>> It seems like that could be done easily here too.  And it'd be a
>>> back-patchable fix.

>> I haven't realized initdb can do that. I agree that would be the best 
>> solution.

> Indeed.

> Maybe something like the following, or maybe it should include "bufmgr.h", 
> not sure.

As-is this patch seems like a maintenance time bomb; it really needs to
use the #defines rather than have the values hard-wired in.  However, just
including bufmgr.h in frontend code doesn't work, so I moved the #defines
to pg_config_manual.h, which seems like a more reasonable place for them
anyway.

Pushed with that and some other polishing.

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] References to arbitrary database objects that are suitable for pg_dump

[Jim Nasby]

I'm working on an extension that is meant to enable metaprogramming 
within Postgres, namely around the creation of objects. For example, a 
"lookup table" class might create a table, grant specific permissions on 
that table, and create several functions to support getting data from 
the table. You could think of this as similar to creating an extension, 
except you get to control aspects of the objects being created (notably 
their names), and you can create as many as you want (baring name 
collisions).


As part of doing this, I need a way to identify each of these classes. 
Since this is geared towards creating database objects, a natural choice 
is to pick a particular object as the "unique object" for the class. In 
this example, the table would serve that purpose.


The challenge I'm running into is finding a way to store a reference to 
an arbitrary object that will survive a rename as well as working with 
pg_dump.


One option would be to store the output of pg_get_object_address(); 
AFAICT that's guaranteed never to change unless the object is dropped. 
But, that would become useless after restoring from a pg_dump; the oid's 
could now be pointing at any random object.


Another option would be to store what you would pass into 
pg_get_object_address(). That would work great with pg_dump, but a 
rename, a move to another schema, or possibly other operations would 
render the stored names incorrect.


The last option I see is to have a table that contains a number of reg* 
fields. Those would remain accurate through any renames or other 
operations (other than a DROP, which would be easy enough to handle), 
and because they would dump as text a reload would work as well. The 
downside is not every object has a reg* pseudotype, but I can live with 
that for now.


Have I missed any other possibilities?
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)


--
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] confusing checkpoint_flush_after / bgwriter_flush_after

[Fabien COELHO]

What we do in some similar cases is put the burden on initdb to fill in
the correct value by modifying postgresql.conf.sample appropriately.
It seems like that could be done easily here too.  And it'd be a
back-patchable fix.


I haven't realized initdb can do that. I agree that would be the best 
solution.


Indeed.

Maybe something like the following, or maybe it should include "bufmgr.h", 
not sure.


--
Fabien.diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index c8a8c52..e014336 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -1107,6 +1107,23 @@ setup_config(void)
 			  "#update_process_title = off");
 #endif
 
+#ifdef HAVE_SYNC_FILE_RANGE
+	/*
+	 * see default in server/storage/bufmgr.h:
+	 * DEFAULT_CHECKPOINT_FLUSH_AFTER 32
+	 * DEFAULT_BGWRITER_FLUSH_AFTER 64
+	 */
+	snprintf(repltok, sizeof(repltok), "#checkpoint_flush_after = %dkb",
+			 32 * (BLCKSZ / 1024));
+	conflines = replace_token(conflines, "#checkpoint_flush_after = 0",
+			  repltok);
+
+	snprintf(repltok, sizeof(repltok), "#bgwriter_flush_after = %dkb",
+			 64 * (BLCKSZ / 1024));
+	conflines = replace_token(conflines, "#bgwriter_flush_after = 0",
+			  repltok);
+#endif   /* HAVE_SYNC_FILE_RANGE */
+
 	snprintf(path, sizeof(path), "%s/postgresql.conf", pg_data);
 
 	writefile(path, conflines);

-- 
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] [sqlsmith] Failed assertion in parallel worker in ExecInitSubPlan

[Tom Lane]

Amit Kapila  writes:
> On Fri, Nov 25, 2016 at 10:33 AM, Tom Lane  wrote:
>> The impression I got in poking at this for a few minutes, before
>> going off to stuff myself with turkey, was that we were allowing
>> a SubqueryScanPath to mark itself as parallel-safe even though the
>> resulting plan node would have an InitPlan attached.  So my thought
>> about fixing it was along the lines of if-subroot-contains-initplans-
>> then-dont-let-SubqueryScanPath-be-parallel-safe.

> I think this will work for the reported case, see the patch attached.

After further thought, it seems to me that the fundamental error here
is that we're not accounting for initPlans while marking paths as
parallel safe or not.  They should be correctly marked when they come
out of subquery_planner, rather than expecting callers to know that
they can't trust that marking.  That way, we don't need to do anything
special in create_subqueryscan_path, and we can also get rid of that
kluge in the force_parallel_mode logic.

> However, don't we need to worry about if there is a subplan
> (non-initplan) instead of initplan.

I don't think so.  References to subplans are already known to be parallel
restricted.  The issue that we're fixing here is that if a plan node has
initPlans attached, ExecutorStart will try to access those subplans,
whether or not they actually get used while running.  That's why the plan
node itself has to be marked parallel-unsafe so it won't get shipped to
parallel workers.

>> There's another issue here, which is that the InitPlan is derived from an
>> outer join qual whose outer join seems to have been deleted entirely by
>> analyzejoins.c.  Up to now it was possible to avert our eyes from the fact
>> that join removal is lazy about getting rid of unused InitPlans, but if
>> they're going to disable parallelization of the surrounding query, maybe
>> we need to work harder on that.

> Yeah, that makes sense, but not sure whether we should try it along
> with this patch.

I'm not certain that sublinks in deletable outer join quals is a case
important enough to sweat over.  But this is the first time I've realized
that failure to remove those initPlans is capable of making a plan worse.
So it's something to keep an eye on.  (I still wish that we could make
join removal happen during join tree preprocessing; doing it where it is
is nothing but a kluge, with unpleasant side effects like this one.)

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] UNDO and in-place update

[Greg Stark]

On 24 November 2016 at 23:03, Robert Haas  wrote:
>> For snapshot isolation Oracle has yet a *third* copy of the data in a
>> space called the "rollback segment(s)".
>
> My understanding is that this isn't correct.  I think the rollback
> segments are what they call the thing that stores UNDO.  See e.g.
> http://ss64.com/ora/syntax-redo.html

It looks like you're right. Rollback segments and Undo segments are
two different pieces of code but one is just the old way and the other
the new way of managing the same data. You can't have both active in
the same database at the same time. I'm a bit confused because I
distinctly remembered an UNDO log back in the 8i days as well but
apparently that's just me imagining things. UNDO segments were
introduced in 9i.

This explained a bunch
http://satya-dba.blogspot.ie/2009/09/undo-tablespace-undo-management.html


-- 
greg


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] confusing checkpoint_flush_after / bgwriter_flush_after

[Tomas Vondra]

On 11/25/2016 06:10 PM, Tom Lane wrote:

Fabien COELHO  writes:

#checkpoint_flush_after = 0   # 0 disables,
  # default is 256kB on linux, 0 otherwise



I find this pretty confusing, because for all other GUCs in the file, the
commented-out value is the default one. In this case that would mean "0",
disabling the flushing.



Although I understand the issue, I'm not sure about -1 as a special value
to mean the default.


Agreed --- I think that's making things more confusing not less.

What we do in some similar cases is put the burden on initdb to fill in
the correct value by modifying postgresql.conf.sample appropriately.
It seems like that could be done easily here too.  And it'd be a
back-patchable fix.



I haven't realized initdb can do that. I agree that would be the best 
solution.



--
Tomas Vondra  http://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] User-defined Operator Pushdown and Collations

[Paul Ramsey]

I've been trying to figure out an issue with operators not being pushed
down for user defined types, in this case "hstore". TL;DR:

hstore=# explain (verbose) select * from hs_fdw where h -> 'a' = '1';
  QUERY PLAN
--
 Foreign Scan on public.hs_fdw  (cost=100.00..157.78 rows=7 width=36)
   Output: id, h
   Filter: ((hs_fdw.h -> 'a'::text) = '1'::text)
   Remote SQL: SELECT id, h FROM public.hs
(4 rows)

In terms of "shippability" the "->" operator passes fine. It ends up not
being shipped because its collation bubbles up as FDW_COLLATE_NONE, and
gets kicked back as not deparseable around here:

https://github.com/postgres/postgres/blob/4e026b32d4024b03856b4981b26c747b7fef7afb/contrib/postgres_fdw/deparse.c#L499

I'm still working at wrapping my head around why this is good or not, but
if there's an obvious explanation and/or workaround, I'd love to know.

Thanks!

P

Re: [HACKERS] UNDO and in-place update

[Bruce Momjian]

On Thu, Nov 24, 2016 at 12:23:28PM -0500, Robert Haas wrote:
> I agree up to a point.  I think we need to design our own system as
> well as we can, not just copy what others have done.  For example, the
> design I sketched will work with all of PostgreSQL's existing index
> types.  You need to modify each AM in order to support in-place
> updates when a column indexed by that AM has been modified, and that's
> probably highly desirable, but it's not a hard requirement.

I feel you are going to get into the problem of finding the index entry
for the old row --- the same thing that is holding back more aggressive
WARM updates.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +


-- 
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] make default TABLESPACE belong to target table.

[Corey Huinker]

>
> I'd like to implement this small feature --- making table's auxiliary
> structures store their data to the target table's tablespace by default.
> I've done a thorough search over the mailing list and there is nothing
> relevant. Well I may miss some corners :-)
>

I had a similar problem in writing the range_partitioning extension: CREATE
TABLE y (LIKE x INCLUDING ALL) didn't set the tablespace of y to match x.
I don't have a solution, I'm just indicating a similar need in userland.

Re: [HACKERS] confusing checkpoint_flush_after / bgwriter_flush_after

[Tom Lane]

Fabien COELHO  writes:
>> #checkpoint_flush_after = 0   # 0 disables,
>>   # default is 256kB on linux, 0 otherwise

>> I find this pretty confusing, because for all other GUCs in the file, the 
>> commented-out value is the default one. In this case that would mean "0", 
>> disabling the flushing.

> Although I understand the issue, I'm not sure about -1 as a special value 
> to mean the default.

Agreed --- I think that's making things more confusing not less.

What we do in some similar cases is put the burden on initdb to fill in
the correct value by modifying postgresql.conf.sample appropriately.
It seems like that could be done easily here too.  And it'd be a
back-patchable fix.

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: session server side variables

[Craig Ringer]

On 14 October 2016 at 23:09, Pavel Stehule  wrote:

>> but only within the session, right? You're not proposing some kind of
>> inter-backend IPC where one backend sets a session var and another
>> backend accesses it and sees the value set by the first session?
>
> In this moment I propose only local (not shared variables). I hope so access
> can be safe with IMMUTABLE access function.

OK, good. Though I suspect you'll have a hard time with IMMUTABLE
functions and need STABLE.

I don't think it's correct to claim that these vars are immutable,
since that'd allow users to do silly things like build them into index
expressions. Splat.

> Default access function should VOLATILE PARALLEL UNSAFE - but immutable sets
> can be defined and used (and I see a sense of these function, because with
> these function the variables are accessed in query planning time).

I don't really understand the purpose of an immutable variable. It
seems inherently contradictory.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] confusing checkpoint_flush_after / bgwriter_flush_after

[Tomas Vondra]

On 11/25/2016 04:40 PM, Fabien COELHO wrote:


Hello Tomas,


While the 9.6 cat is out of the bag, I think we can fix this quite
easily - use "-1" to specify the default value should be used, and use
that in the sample file. This won't break any user configuration.


Although I understand the issue, I'm not sure about -1 as a special
value to mean the default.


Why? We use wal_buffers=-1 to use the default (depending on the size
of shared_buffers), for example.


Indeed. Just my 0.02€:

ISTM that the use of -1 is not very consistent, as it may mean:

 - default: autovacuum_work_mem, wal_buffers

 - disable: temp_file_limit, old_snapshot_limit,
 max_standby_*_delay, log_min_duration_statement

And sometimes disable is the default, but not always:-) Basically I'm
not sure that adding some more confusion around that helps much...



Well, the inconsistency is already there. Some GUCs use -1 as "use 
default value" and others using it as "disable". Picking one of those 
does not really increase the confusion, and it fixes the issue of having 
a default mismatching the commented-out example.


regards

--
Tomas Vondra  http://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] Fixed pg_class refcache leak when the meta tuple in pg_class in invalid.

[Ming Li]

Hi all,

In some cases the meta tuple in pg_class for a specific relation is
invalid, which will cause relcache leak, and then report warning:
WARNING: relcache reference leak: relation "pg_class" not closed.

The diff file in the attachment can fix this problem.
diff --git a/src/backend/utils/cache/relcache.c 
b/src/backend/utils/cache/relcache.c
index 79e0b1f..6485fc4 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -951,7 +951,11 @@ RelationBuildDesc(Oid targetRelId, bool insertIt)
 * if no such tuple exists, return NULL
 */
if (!HeapTupleIsValid(pg_class_tuple))
+   {
+   if(RelationIsValid(pg_class_relation))
+   heap_close(pg_class_relation, AccessShareLock);
return NULL;
+   }
 
/*
 * get information from the pg_class_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] confusing checkpoint_flush_after / bgwriter_flush_after

[Fabien COELHO]

Hello Tomas,


While the 9.6 cat is out of the bag, I think we can fix this quite
easily - use "-1" to specify the default value should be used, and use
that in the sample file. This won't break any user configuration.


Although I understand the issue, I'm not sure about -1 as a special
value to mean the default.


Why? We use wal_buffers=-1 to use the default (depending on the size of 
shared_buffers), for example.


Indeed. Just my 0.02€:

ISTM that the use of -1 is not very consistent, as it may mean:

 - default: autovacuum_work_mem, wal_buffers

 - disable: temp_file_limit, old_snapshot_limit,
 max_standby_*_delay, log_min_duration_statement

And sometimes disable is the default, but not always:-) Basically I'm not 
sure that adding some more confusion around that helps much...


--
Fabien.
--
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] UNDO and in-place update

[Amit Kapila]

On Fri, Nov 25, 2016 at 8:03 PM, Amit Kapila  wrote:
>>
>
> Another way to do is to write UNDO log for split operation (with exact
> record locations on pages that are split) so that you don't need to
> perform this expensive search operation.  I can understand writing
> UNDO for split could be costly but so is writing WAL for it.  I think
> for UNDO, we should follow a generic rule as for heap
>

typo.
/heap/WAL

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: session server side variables

[Pavel Stehule]

Hi

2016-10-14 9:56 GMT+02:00 Craig Ringer :

> On 14 October 2016 at 13:30, Pavel Stehule 
> wrote:
> > Hi,
> >
> > long time I working on this topic. Session server side variables are one
> > major missing feature in PLpgSQL. Now I hope, I can summarize requests
> for
> > implementation in Postgres:
>
> +1
>
> > 2. accessed with respecting access rights:
> >
> > GRANT SELECT|UPDATE|ALL ON VARIABLE variable TO role
> > REVOKE SELECT|UPDATE|ALL ON VARIABLE variable FROM role
>
> This bit is important.
>
> For those wondering "why the hell would you want these, just (ab)use
> GUCs"... this is why.
>
> Think RLS. Especially when we eventually have session start / at login
> triggers, but even before then, you can initialise some expensive
> state once at the start of the session, transfer it from the app, or
> whatever. You initialise it via a SECURITY DEFINER procedure so the
> session user does not have the rights to write to the variable, and it
> can only be set via arbitration from the database security logic. From
> then on your RLS policies, your triggers, etc, can all simply inspect
> the session variable.
>
> People use package variables in another major database with a feature
> called virtual private database for something similar. So this will
> interest anyone who wants to make porting those users easier, too.
>
> > 4. non transactional  - the metadata are transactional, but the content
> is
> > not.
>
> but only within the session, right? You're not proposing some kind of
> inter-backend IPC where one backend sets a session var and another
> backend accesses it and sees the value set by the first session?
>
> Speaking of which: parallel query. How do you envision this working in
> parallel query, where the workers are different backends? Especially
> since things like RLS are where it'd be quite desirable.
>

In first stage the session variables should be marked as parallel unsafe -
but in future - there can be used similar technique  like shared hashjoin.

I am sending proof concept - it doesn't support access to fields of
composite variables, but any other functionality is done.

Most important features:

1. the values are stored in native types
2. access to content is protected by ACL - like the content of tables
3. the content is not MVCC based - no any cost of UPDATE
4. simple API allows access to content of variables from any supported
environment.

Regards

Pavel


> --
>  Craig Ringer   http://www.2ndQuadrant.com/
>  PostgreSQL Development, 24x7 Support, Training & Services
>
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index c0df671..a4361f2 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -274,6 +274,9 @@ restrict_and_check_grant(bool is_grant, AclMode avail_goptions, bool all_privs,
 		case ACL_KIND_TYPE:
 			whole_mask = ACL_ALL_RIGHTS_TYPE;
 			break;
+		case ACL_KIND_VARIABLE:
+			whole_mask = ACL_ALL_RIGHTS_VARIABLE;
+			break;
 		default:
 			elog(ERROR, "unrecognized object kind: %d", objkind);
 			/* not reached, but keep compiler quiet */
@@ -488,6 +491,10 @@ ExecuteGrantStmt(GrantStmt *stmt)
 			all_privileges = ACL_ALL_RIGHTS_FOREIGN_SERVER;
 			errormsg = gettext_noop("invalid privilege type %s for foreign server");
 			break;
+		case ACL_OBJECT_VARIABLE:
+			all_privileges = ACL_ALL_RIGHTS_VARIABLE;
+			errormsg = gettext_noop("invalid privilege type %s for variable");
+			break;
 		default:
 			elog(ERROR, "unrecognized GrantStmt.objtype: %d",
  (int) stmt->objtype);
@@ -558,6 +565,7 @@ ExecGrantStmt_oids(InternalGrant *istmt)
 	{
 		case ACL_OBJECT_RELATION:
 		case ACL_OBJECT_SEQUENCE:
+		case ACL_OBJECT_VARIABLE:
 			ExecGrant_Relation(istmt);
 			break;
 		case ACL_OBJECT_DATABASE:
@@ -625,6 +633,7 @@ objectNamesToOids(GrantObjectType objtype, List *objnames)
 	{
 		case ACL_OBJECT_RELATION:
 		case ACL_OBJECT_SEQUENCE:
+		case ACL_OBJECT_VARIABLE:
 			foreach(cell, objnames)
 			{
 RangeVar   *relvar = (RangeVar *) lfirst(cell);
@@ -773,6 +782,10 @@ objectsInSchemaToOids(GrantObjectType objtype, List *nspnames)
 objs = getRelationsInNamespace(namespaceId, RELKIND_SEQUENCE);
 objects = list_concat(objects, objs);
 break;
+			case ACL_OBJECT_VARIABLE:
+objs = getRelationsInNamespace(namespaceId, RELKIND_VARIABLE);
+objects = list_concat(objects, objs);
+break;
 			case ACL_OBJECT_FUNCTION:
 {
 	ScanKeyData key[1];
@@ -948,6 +961,10 @@ ExecAlterDefaultPrivilegesStmt(ParseState *pstate, AlterDefaultPrivilegesStmt *s
 			all_privileges = ACL_ALL_RIGHTS_TYPE;
 			errormsg = gettext_noop("invalid privilege type %s for type");
 			break;
+		case ACL_OBJECT_VARIABLE:
+			all_privileges = ACL_ALL_RIGHTS_VARIABLE;
+			errormsg = gettext_noop("invalid privilege type %s for variable");
+			break;
 		default:
 			elog(ERROR, "unrecognized GrantStmt.objtype: %d",
  (int) action->objtype);
@@ -1135,6 +1152,12 @@ 

Re: [HACKERS] UNDO and in-place update

[Amit Kapila]

On Thu, Nov 24, 2016 at 10:09 PM, Robert Haas  wrote:
>
> I don't really understand how a system of this type copes with page
> splits.  Suppose there are entries 1000, 2000, ..., 1e6 in the btree.
> I now start two overlapping transactions, one inserting all the
> positive odd values < 1e6 and the other inserting all the positive
> even values < 1e6 that are not already present.  The insertions are
> interleaved with each other.  After they get done inserting, what does
> the 3D time traveling btree look like at this point?
>
> The problem I'm getting at here is that the heap, unlike an index, is
> unordered.  So when I've got all values 1..1e6, they've got to be in a
> certain order.  Necessarily, the values from the in-flight
> transactions are interleaved with each other and with the old data.
> If they're not, I can no longer support efficient searches by the
> in-flight transactions, plus I have an unbounded amount of cleanup
> work to do at commit time.  But if I *do* have all of those values
> interleaved in the proper order, then an abort leaves fragmented free
> space.
>

Yeah, that's right, but at next insert, on the same page, we can
defragment the page and claim all the space.  We might want to perform
such a cleanup only in certain cases like when the remaining space in
the page is below a certain threshold.

>  I can go and kill all of the inserted tuples during UNDO of an
> aborted transactions, but because page splits have happened, the index
> tuples I need to kill may not be on the same pages where I inserted
> them, so I might need to just search from the top of the tree, so it's
> expensive.
>

Another way to do is to write UNDO log for split operation (with exact
record locations on pages that are split) so that you don't need to
perform this expensive search operation.  I can understand writing
UNDO for split could be costly but so is writing WAL for it.  I think
for UNDO, we should follow a generic rule as for heap which is that
any change in the page should have it's corresponding UNDO.

>  And even if I do it anyway, the pages are left half-full.
> The point is that the splits are the joint result of the two
> concurrent transactions taken together - you can't blame individual
> splits on individual transactions.
>

Sure, but you are talking about an extreme case, so it might be okay
to leave pages half-full in such cases, next inserts will consume this
space.  Also aborts are less frequent operations, so the impact should
be minimal.

>
> Do you see some trick here that I'm missing?
>

I think we should be able to find some way to tackle the problems you
have listed above as there are databases (including Oracle) which
write UNDO for indexes (btree's) as well.  However, I think here the
crucial question is whether the only-heap-based-undo design can give
us consistent data for all possible cases if so, then it is better to
do that in the first phase and then later we can implement UNDO for
indexes as well.   As of now, nobody has reported any such problem, so
maybe we can stick with the only-heap-based-undo design.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] make default TABLESPACE belong to target table.

[Michael Paquier]

On Fri, Nov 25, 2016 at 10:47 PM, Amos Bird  wrote:
>> So you would like locate those index tablespaces into the same
>> tablespace as its parent relation when the index is created for a
>> unique index or as a primary key?
>
> Yes, and I'd like this behavior take effect when default_tablespace is
> set to something like "parent".

The only scenario where this would be useful is when using ALTER TABLE
ADD CONSTRAINT in which case a fresh index is built (not USING INDEX).
That's a bit narrow, because it would mean that you would either
append a TABLESPACE clause to this existing clause, or create a
storage parameter to enforce all indexes created for a relation on a
wanted tablespace... For the other cases you could just do something
like that, and that's what the large majority of people would care
about:
SET default_tablespace TO 'foo';
CREATE TABLE foobar (id int PRIMARY KEY);
But that's not the one you are interesting in, so likely a storage
parameter is what pops up in my mind, with parameter defined at table
creation: CREATE TABLE foo (id primary key) WITH
(constraint_default_tablespace = foo) TABLESPACE bar;
In this case the parent relation gets created in tablespace bar, but
its primary key gets in tablespace foo.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] make default TABLESPACE belong to target table.

[Amos Bird]

> So you would like locate those index tablespaces into the same
> tablespace as its parent relation when the index is created for a
> unique index or as a primary key?

Yes, and I'd like this behavior take effect when default_tablespace is
set to something like "parent".

> But what would be the difference with default_tablespace?

What do you mean? AFAIK, default_tablespace option cannot tell which
tablespace the parent table is in.

> I think that you are looking for a replacement for something that is
> already doable.

Hmm, I've done my research and asked around IRC channels. There is
little info come to my mind. could you give me some hint?

Michael Paquier writes:

> On Fri, Nov 25, 2016 at 4:48 PM, Amos Bird  wrote:
>> I've been using postgres for a long time. Recently I'm doing table
>> sharding over a bunch of pgsql instances. I'm using multiple tablespaces
>> one per disk to utilize all the IO bandwidth. Things went on pretty
>> well, however there is a troublesome problem I have when adding
>> auxiliary structures to sharded tables, such as Indexes. These objects
>> have their storage default to the database's tablespace, and it's
>> difficult to shard them by hand.
>>
>> I'd like to implement this small feature --- making table's auxiliary
>> structures store their data to the target table's tablespace by default.
>> I've done a thorough search over the mailing list and there is nothing
>> relevant. Well I may miss some corners :-)
>
> So you would like locate those index tablespaces into the same
> tablespace as its parent relation when the index is created for a
> unique index or as a primary key? Perhaps we could have a
> session-level parameter that enforces the creation of such indexes on
> the same tablespace as the table... But what would be the difference
> with default_tablespace? I think that you are looking for a
> replacement for something that is already doable.


-- 
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] [sqlsmith] Failed assertion in parallel worker in ExecInitSubPlan

[Amit Kapila]

On Fri, Nov 25, 2016 at 10:33 AM, Tom Lane  wrote:
> Amit Kapila  writes:
>> On Fri, Nov 25, 2016 at 3:26 AM, Andreas Seltenreich  
>> wrote:
>>> just caught another InitPlan below Gather with the recent patches in
>>> (master as of 4cc6a3f).  Recipe below.
>
>> I think this problem exists since commit
>> 110a6dbdebebac9401b43a8fc223e6ec43cd4d10 where we have allowed
>> subqueries to be pushed to parallel workers.
>
> The impression I got in poking at this for a few minutes, before
> going off to stuff myself with turkey, was that we were allowing
> a SubqueryScanPath to mark itself as parallel-safe even though the
> resulting plan node would have an InitPlan attached.  So my thought
> about fixing it was along the lines of if-subroot-contains-initplans-
> then-dont-let-SubqueryScanPath-be-parallel-safe.
>

I think this will work for the reported case, see the patch attached.
However, don't we need to worry about if there is a subplan
(non-initplan) instead of initplan.  I have tried by tweaking the
above query such that it will generate a subplan and for such a case
it will make SubqueryScanPath as parallel-safe.

explain select 1 from public.quad_point_tbl as ref_0, lateral (select
ref_0.p as c3, sample_0.d as c5 from public.nv_child_2010 as sample_0
left join public.mvtest_tvv as ref_1 on ('x' = ANY (select contype
from pg_catalog.pg_constraint limit 1)) limit 82) as subq_0;

>  What you propose
> here seems like it would shut off parallel query in more cases than
> that.  But I'm still full of turkey and may be missing something.
>
> There's another issue here, which is that the InitPlan is derived from an
> outer join qual whose outer join seems to have been deleted entirely by
> analyzejoins.c.  Up to now it was possible to avert our eyes from the fact
> that join removal is lazy about getting rid of unused InitPlans, but if
> they're going to disable parallelization of the surrounding query, maybe
> we need to work harder on that.
>

Yeah, that makes sense, but not sure whether we should try it along
with this patch.

> Another question worth asking is whether it was okay for the subquery to
> decide to use a Gather.  Are we OK with multiple Gathers per plan tree,
> independently of the points above?
>

As of now, we don't support nested Gather case.  Example:

Not Okay Plan
-
-> Gather
 -> Nested Loop
 -> Parallel Seq Scan
 -> Gather
  -> Parallel Seq Scan

Okay Plan
-
-> Nested Loop
 -> Gather
  -> Parallel Seq Scan
 -> Gather
  -> Parallel Seq Scan



-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


allow_safe_subquery_parallel_worker_v2.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] make default TABLESPACE belong to target table.

[Michael Paquier]

On Fri, Nov 25, 2016 at 4:48 PM, Amos Bird  wrote:
> I've been using postgres for a long time. Recently I'm doing table
> sharding over a bunch of pgsql instances. I'm using multiple tablespaces
> one per disk to utilize all the IO bandwidth. Things went on pretty
> well, however there is a troublesome problem I have when adding
> auxiliary structures to sharded tables, such as Indexes. These objects
> have their storage default to the database's tablespace, and it's
> difficult to shard them by hand.
>
> I'd like to implement this small feature --- making table's auxiliary
> structures store their data to the target table's tablespace by default.
> I've done a thorough search over the mailing list and there is nothing
> relevant. Well I may miss some corners :-)

So you would like locate those index tablespaces into the same
tablespace as its parent relation when the index is created for a
unique index or as a primary key? Perhaps we could have a
session-level parameter that enforces the creation of such indexes on
the same tablespace as the table... But what would be the difference
with default_tablespace? I think that you are looking for a
replacement for something that is already doable.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Creating a DSA area to provide work space for parallel execution

[Thomas Munro]

On Fri, Nov 25, 2016 at 4:32 AM, Dilip Kumar  wrote:
> I have one more question,
>
> In V1 we were calling dsa_detach in ExecParallelCleanup and in
> ParallelQueryMain, but it's removed in v2.
>
> Any specific reason ?
> Does this need to be used differently ?
>
>  ExecParallelCleanup(ParallelExecutorInfo *pei)
>  {
> + if (pei->area != NULL)
> + {
> + dsa_detach(pei->area);
> + pei->area = NULL;
> + }
>
> After this changes, I am getting DSM segment leak warning.

Thanks!  I had some chicken-vs-egg problems dealing with cleanup of
DSM segments belonging to DSA areas created inside DSM segments.
Here's a new version to apply on top of dsa-v7.patch.

-- 
Thomas Munro
http://www.enterprisedb.com


dsa-area-for-executor-v3.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] confusing checkpoint_flush_after / bgwriter_flush_after

[Tomas Vondra]

On 11/25/2016 01:20 PM, Fabien COELHO wrote:


Hello Tomas,


#checkpoint_flush_after = 0   # 0 disables,
 # default is 256kB on linux, 0 otherwise



I find this pretty confusing, because for all other GUCs in the file,
the commented-out value is the default one. In this case that would
mean "0", disabling the flushing.

But in practice we use platform-dependent defaults - 256/512K on
Linux, 0 otherwise. There are other GUCs where the default is
platform-specific, but none of them suggests "disabled" is the default
state.

While the 9.6 cat is out of the bag, I think we can fix this quite
easily - use "-1" to specify the default value should be used, and use
that in the sample file. This won't break any user configuration.


Although I understand the issue, I'm not sure about -1 as a special
value to mean the default.



Why? We use wal_buffers=-1 to use the default (depending on the size of 
shared_buffers), for example.



If that's considered not acceptable, perhaps we should at least
improve the comments, so make this clearer.


Yep, what about not putting a value and inverting/adapting the comments,
maybe something like:

 #checkpoint_flush_after = ...  # default is 256kB on linux, 0 otherwise
# where 0 disables flushing



Yeah, something like that.

regards

--
Tomas Vondra  http://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] confusing checkpoint_flush_after / bgwriter_flush_after

[Fabien COELHO]

Hello Tomas,


#checkpoint_flush_after = 0   # 0 disables,
 # default is 256kB on linux, 0 otherwise


I find this pretty confusing, because for all other GUCs in the file, the 
commented-out value is the default one. In this case that would mean "0", 
disabling the flushing.


But in practice we use platform-dependent defaults - 256/512K on Linux, 0 
otherwise. There are other GUCs where the default is platform-specific, but 
none of them suggests "disabled" is the default state.


While the 9.6 cat is out of the bag, I think we can fix this quite easily - 
use "-1" to specify the default value should be used, and use that in the 
sample file. This won't break any user configuration.


Although I understand the issue, I'm not sure about -1 as a special value 
to mean the default.


If that's considered not acceptable, perhaps we should at least improve the 
comments, so make this clearer.


Yep, what about not putting a value and inverting/adapting the comments, 
maybe something like:


 #checkpoint_flush_after = ...  # default is 256kB on linux, 0 otherwise
# where 0 disables flushing


--
Fabien.


--
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] Typo in comment

[Magnus Hagander]

On Fri, Nov 25, 2016 at 4:10 AM, Thomas Munro  wrote:

> Hi
>
> Here is a tiny patch to fix a typo in execParallel.c.
>

Applied, thanks.


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Re: [HACKERS] Declarative partitioning - another take

[Amit Langote]

On 2016/11/25 11:44, Robert Haas wrote:
> On Thu, Nov 24, 2016 at 6:13 AM, Amit Langote wrote:
>> Also, it does nothing to help the undesirable situation that one can
>> insert a row with a null partition key (expression) into any of the range
>> partitions if targeted directly, because of how ExecQual() handles
>> nullable constraint expressions (treats null value as satisfying the
>> partition constraint).
> 
> That's going to have to be fixed somehow.  How bad would it be if we
> passed ExecQual's third argument as false for partition constraints?
> Or else you could generate the actual constraint as expr IS NOT NULL
> AND expr >= lb AND expr < ub.

About the former, I think that might work.  If a column is NULL, it would
be caught in ExecConstraints() even before ExecQual() is called, because
of the NOT NULL constraint.  If an expression is NULL, or for some reason,
the partitioning operator (=, >=, or <) returned NULL even for a non-NULL
column or expression, then ExecQual() would fail if we passed false for
resultForNull.  Not sure if that would be violating the SQL specification
though.

The latter would work too.  But I guess we would only emit expr IS NOT
NULL, not column IS NOT NULL, because columns are covered by NOT NULL
constraints.

>> An alternative possibly worth considering might be to somehow handle the
>> null range partition keys within the logic to compare against range bound
>> datums.  It looks like other databases will map the rows containing nulls
>> to the unbounded partition.  One database allows specifying NULLS
>> FIRST/LAST and maps a row containing null key to the partition with
>> -infinity as the lower bound or +infinity as the upper bound, respectively
>> with  NULLS LAST the default behavior.
> 
> It seems more future-proof not to allow NULLs at all for now, and
> figure out what if anything we want to do about that later.  I mean,
> with the syntax we've got here, anything else is basically deciding
> whether NULL is the lowest value or the highest value.  It would be
> convenient for my employer if we made the same decision that Oracle
> did, here, but it doesn't really seem like the PostgreSQL way - or to
> put that another way, it's really ugly and unprincipled.  So I
> recommend we decide for now that a partitioning column can't be null
> and a partitioning expression can't evaluate to NULL.  If it does,
> ERROR.

OK, we can decide later if we want to handle NULLs somehow.

Thanks,
Amit




-- 
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] Broken SSL tests in master

[Mithun Cy]

On Fri, Nov 25, 2016 at 12:03 PM, Andreas Karlsson 
wrote:
> Another thought about this code: should we not check if it is a unix
socket first before splitting the host? While I doubt that it is common to
have a unix >socket in a directory with comma in the path it is a bit
annoying that we no longer support this.

I think it is a bug.

*Before this feature:*

./psql postgres://%2fhome%2fmithun%2f%2c
psql: could not connect to server: No such file or directory
Is the server running locally and accepting
connections on Unix domain socket "*/home/mithun/,/.*s.PGSQL.5444"?

*After this feature:*
./psql postgres://%2fhome%2fmithun%2f%2c
psql: could not connect to server: No such file or directory
Is the server running locally and accepting
connections on Unix domain socket "*/home/mithun//.*s.PGSQL.5432"?
*could not connect to server: Connection refused*
* Is the server running on host "" (::1) and accepting*
* TCP/IP connections on port 5432?*
*could not connect to server: Connection refused*
* Is the server running on host "" (127.0.0.1) and accepting*
* TCP/IP connections on port 5432?*

So comma (%2c) is misinterpreted as separator not as part of UDS path.

Reason is we first decode the URI(percent encoded character) then try to
split the string into multiple host assuming they are separated by *','*. I
think we need to change the order here. Otherwise difficult the say whether
*','* is part of USD path or a separator.

-- 
Thanks and Regards
Mithun C Y
EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] Declarative partitioning - another take

[Amit Langote]

On 2016/11/25 13:51, Ashutosh Bapat wrote:
>>
>> I assume you meant "...right after the column name"?
>>
>> I will modify the grammar to allow that way then, so that the following
>> will work:
>>
>> create table p1 partition of p (
>>  a primary key
>> ) for values in (1);
>>
> 
> That seems to be non-intuitive as well. The way it's written it looks
> like "a primary key" is associated with p rather than p1.

It kind of does, but it's still a "create table p1" statement, so it's not
that ambiguous, IMHO.  Although I'm not attached to this syntax, there may
not be other options, such as moving the parenthesized list right before
"partition of", due to resulting grammar conflicts.

> Is there any column constraint that can not be a table constraint?

NOT NULL, DEFAULT, and COLLATE (although only the first of these is really
a constraint, albeit without a pg_constraint entry)

> If no, then we can think of dropping column constraint syntax all
> together and let the user specify column constraints through table
> constraint syntax. OR we may drop constraints all-together from the
> "CREATE TABLE .. PARTITION OF" syntax and let user handle it through
> ALTER TABLE commands. In a later version, we will introduce constraint
> syntax in that DDL.

Hmm, it was like that in the past versions of the patch, but I thought
it'd be better to follow the CREATE TABLE OF style to allow specifying the
table and column constraints during table creation time.  If many think
that it is not required (or should have some other syntax), I will modify
the patch accordingly.

Thanks,
Amit




-- 
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] Random PGDLLIMPORTing

[Craig Ringer]

On 25 November 2016 at 14:16, Tom Lane  wrote:
> Craig Ringer  writes:
>> PGDLLIMPORT is free, so the question should be "is there a reason not
>> to add it here?".
>
> TBH, my basic complaint about it is that I do not like Microsoft's tool
> chain assuming that it's entitled to demand that people sprinkle
> Microsoft-specific droppings throughout what would otherwise be platform
> independent source code.

Yeah, I know how you feel. I'm not a fan myself, I just tolerate it as
one of those annoying things we must put up with, like Perl 5.8 for
ancient systems, not using C99, and !Linux/FBSD support. Which,
actually, we _do_ actively work for - take for example the atomics
work, which went to considerable lengths not to break weird niche
platforms.

> However, Victor Wagner's observation upthread is quite troubling:
>
>>> It worth checking actual variable definitions, not just declarations.
>>> I've found recently, that at least in MSVC build system, only
>>> initialized variables are included into postgres.def file, and so are
>>> actually exported from the backend binary.
>
> If this is correct (don't ask me, I don't do Windows) then the issue is
> not whether "PGDLLIMPORT is free".  This puts two separate source-code
> demands on variables that we want to make available to extensions, neither
> of which is practically checkable on non-Windows platforms.

I agree that, if true, that's a real concern and something that needs
addressing to avoid a growing maintenance burden from Windows.

> I think that basically it's going to be on the heads of people who
> want to work on Windows to make sure that things work on that platform.
> That is the contract that every other platform under the sun understands,
> but it seems like Windows people think it's on the rest of us to make
> their platform work.  I'm done with that.

Well, I'm trying to be one of those "Windows people" to the degree of
spotting and addressing issues. Like this one. But it's not worth
arguing this one if it's more than a trivial "meh, done" fix, since
it's likely to only upset code that's testing assertions (like BDR or
pglogical).

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers